Tickets :: [#12180] Unification of instant messenger attributes

6.0.0-beta1

7/5/25

Summary	Unification of instant messenger attributes
Queue	Turba
Queue Version	Git master
Type	Bug
State	Resolved
Priority	1. Low
Owners	mrubinsk (at) horde (dot) org
Requester	thomas.jarosch (at) intra2net (dot) com
Created	04/15/2013 (4464 days ago)
Due
Updated	04/18/2013 (4461 days ago)
Assigned	04/17/2013 (4462 days ago)
Resolved	04/18/2013 (4461 days ago)
Github Issue Link
Github Pull Request
Milestone
Patch	Yes

04/18/2013 05:26:41 PM	Michael Rubinsky	State ⇒ Resolved

04/18/2013 05:07:33 PM	Git Commit	Comment #13	Reply to this comment
Changes have been made in Git (master): commit 2b9e0896111eb40b02324a827fc6177e2d6fa660 Author: Thomas Jarosch <thomas.jarosch@intra2net.com> Date: Thu Apr 18 13:06:58 2013 -0400 ~~Bug: 12180~~ Standardize on imaddress fields. Signed-off-by: Michael J Rubinsky <mrubinsk@horde.org> .../turba/config/backends.d/20-kolab.php \| 4 +- ...Add-support-to-sync-the-country-fields-bu.patch \| 36 ++++++++++++++++++++ turba/config/attributes.php \| 12 ++----- turba/config/backends.php \| 4 +- turba/data.php \| 2 +- turba/docs/UPGRADING \| 9 +++++ turba/lib/Driver.php \| 7 +++- turba/test/Turba/TestCase.php \| 2 +- 8 files changed, 59 insertions(+), 17 deletions(-) http://git.horde.org/horde-git/-/commit/2b9e0896111eb40b02324a827fc6177e2d6fa660

04/18/2013 07:17:31 AM	Thomas Jarosch	Comment #12	Reply to this comment
Sounds with me, it's a rather cosmetic issue anyway. Sounds _fine_ with me ;) Need more tea.

04/18/2013 07:16:19 AM	Thomas Jarosch	Comment #11	Reply to this comment
[Show Quoted Text - 16 lines][Hide Quoted Text] At the very least, this will also need to be documented in UPGRADING. Exactly. That's why I mentioned it :) Let's just drop the hunk from the patch that removes the attribute for the FRAMEWORK_5_0 branch. The rename inside the Kolab section is safe. Well, none of this will be in FRAMEWORK_5_0, and dropping just the deletion of the attribute isn't enough to avoid sort-of-bc-issues because we are changing the name of the attribute when it is used in code. I.e., keeping it attributes might avoid undefined index errors (or whatever), but the attribute won't be used for anything in the code. In other words, we should do all of it in master, and document the need to change the attribute in any custom backend configurations in UPGRADING. Sounds with me, it's a rather cosmetic issue anyway. For my users, I'll keep it in my local FRAMEWORK_5_0 branch since I have full control of all the packages / configs involved.

04/18/2013 07:14:08 AM	Thomas Jarosch	Comment #10	Reply to this comment
Side note: The name "imaddress" is fixed in the ActiveSync code, changing it to something else would create a BC mess :) We avoided it anyway... But that is the backend specific name, and is what the protocol calls it. It is mapped, in Turba, to the turba field name in the protected static member Turba_Driver::_asMap. I hate it when you are right :) I missed that map before. Then of course there would be no BC issue.

04/17/2013 04:28:34 PM	Michael Rubinsky	Comment #9 State ⇒ Assigned Assigned to Michael Rubinsky	Reply to this comment
At the very least, this will also need to be documented in UPGRADING. Exactly. That's why I mentioned it :) Let's just drop the hunk from the patch that removes the attribute for the FRAMEWORK_5_0 branch. The rename inside the Kolab section is safe. Well, none of this will be in FRAMEWORK_5_0, and dropping just the deletion of the attribute isn't enough to avoid sort-of-bc-issues because we are changing the name of the attribute when it is used in code. I.e., keeping it attributes might avoid undefined index errors (or whatever), but the attribute won't be used for anything in the code. In other words, we should do all of it in master, and document the need to change the attribute in any custom backend configurations in UPGRADING.

04/17/2013 04:25:44 PM	Michael Rubinsky	Comment #8	Reply to this comment
Side note: The name "imaddress" is fixed in the ActiveSync code, changing it to something else would create a BC mess :) We avoided it anyway... But that is the backend specific name, and is what the protocol calls it. It is mapped, in Turba, to the turba field name in the protected static member Turba_Driver::_asMap.

04/17/2013 04:18:19 PM	Thomas Jarosch	Comment #7	Reply to this comment
Unifying everything to "imaddress" is IMHO the only way to maintain BC when Horde_ActiveSync gets updated and turba is hold back. The mapping is done completely in Turba, so it really doesn't matter as far as BC goes. However, "imaddress" seems a good choice to for consistency. Side note: The name "imaddress" is fixed in the ActiveSync code, changing it to something else would create a BC mess :) We avoided it anyway...

04/17/2013 02:55:25 PM	Thomas Jarosch	Comment #6	Reply to this comment
At the very least, this will also need to be documented in UPGRADING. Exactly. That's why I mentioned it :) Let's just drop the hunk from the patch that removes the attribute for the FRAMEWORK_5_0 branch. The rename inside the Kolab section is safe.

04/17/2013 02:44:18 PM	Michael Rubinsky	Comment #5 State ⇒ Feedback	Reply to this comment
Thinking this through a bit though, flat out removing an attribute from attributes.php as well as changing the name of the attribute that is used in code could break people's existing custom configurations/backends. At the very least, this will also need to be documented in UPGRADING.

04/17/2013 02:38:36 PM	Michael Rubinsky	Comment #4	Reply to this comment
Unifying everything to "imaddress" is IMHO the only way to maintain BC when Horde_ActiveSync gets updated and turba is hold back. The mapping is done completely in Turba, so it really doesn't matter as far as BC goes. However, "imaddress" seems a good choice to for consistency.

04/17/2013 01:36:28 PM	Thomas Jarosch	Comment #3 New Attachment: v2-0001-Unify-instant-messenger-field-to-new-imaddress-field.patch	Reply to this comment
New version of the patch. Forgot the updated kolab bundle in the commit.

04/17/2013 01:33:21 PM	Thomas Jarosch	Comment #2 New Attachment: 0001-Unify-instant-messenger-field-to-new-imaddress-field.patch Patch ⇒ Yes	Reply to this comment
Patch to unify the instant messenger fields. I've tested the vcard export and syncing contacts via ActiveSync <-> Kolab. Should we keep the "instantMessenger" field in attributes.php till Horde 5.1? It might break otherwise if the user customized his backends.php. May be we should just keep it for the FRAMEWORK_5_0 backport.

04/15/2013 03:20:26 PM	Thomas Jarosch	Comment #1 Priority ⇒ 1. Low State ⇒ Unconfirmed Patch ⇒ No Milestone ⇒ Summary ⇒ Unification of instant messenger attributes Type ⇒ Bug Queue ⇒ Turba	Reply to this comment
Hi, while testing the transfer of every contact attribute via ActiveSync + Kolab, I realized the instant messenger field is not synced to/from my device. Turns out Kolab uses an attribute named "instantMessenger" while the attributes used for ActiveSync were added as "imaddress", "imaddress2" and "imaddress3". At first I thought replacing "instantMessenger" with "imaddress" would be a no-brainer. Grepping through the code showed that other places like vcard import/export use that field, too. So vcard import/export is currently not playing nice with ActiveSync regarding the instant messenger field. My current plan looks like this - Adapt Kolab attribute mapping to use "imaddress" - Fix up code to use "imaddress" - Use _("Instant messenger") label for the "imaddress" UI - Remove "instantMessenger" from attributes.php Unifying everything to "imaddress" is IMHO the only way to maintain BC when Horde_ActiveSync gets updated and turba is hold back. Objections?