Closed Bug 981674 Opened 6 years ago Closed 5 years ago

[Contacts] Contact's name merged to another one previously then exported to sd card and imported again, is not shown fine

Categories

(Firefox OS Graveyard :: Gaia::Contacts, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:-, b2g-v1.2 wontfix, b2g-v1.3 wontfix, b2g-v1.3T wontfix, b2g-v1.4 wontfix, b2g-v2.0 verified, b2g-v2.1 verified)

VERIFIED FIXED
2.0 S4 (20june)
blocking-b2g -
Tracking Status
b2g-v1.2 --- wontfix
b2g-v1.3 --- wontfix
b2g-v1.3T --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: isabelrios, Assigned: fcampo)

References

Details

(Whiteboard: [p=1])

Attachments

(3 files)

Buri master build (03/10)
Gecko-3f542fd
Gaia-e91db4c

STR
1. Create a contact ie: Name1, LN1 and a valid phone number: 611223344
2. Create another contact: Name2, LN2 and the same phone number: 611223344
3. Merge both contact
4. Export that contact to SD Card
5. Remove the contact from the addressbook
6. Import the contact from the SD Card

EXPECTED
The contact is correctly imported from the sd card

ACTUAL
After importing from the sd card, the contact appears in the contact list as:
Name: Name2, Name1
Last Name: LN2, LN1
Phone Number: 611223344
Fyi, v1.2 and v1.3 are also affected
triage: minus, not blocking. please fix this on trunk .thanks
blocking-b2g: 1.4? → -
Assignee: nobody → mbudzynski
It's probably affected by the same part of the contacts merges as Bug 935636, since I probably know how to fix the previous one, I would like to work on this one as well.
Assignee: mbudzynski → fernando.campo
Quick patch (1 line!).

Problem is that when we merge we keep all the data (previous and current) on an array. Then we export to vcard, which doesn't accept arrays (we use comma separated), and when we import again from vcard, we didn't have this situation in mind.
So fix is easy, split the strings for the NAME PARTS from the vcard when importing.

Which I'm not sure of is if we should use this same approach for the rest of the fields. Advice?

P.S. I'm getting some tests for this specific situation, update if confirm that the approach is correct.
Attachment #8436995 - Flags: feedback?(jmcf)
Can I haz unit test?
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #5)
> Can I haz unit test?

(In reply to Fernando Campo (:fcampo) from comment #4)
> Created attachment 8436995 [details] [review]
> Link to PR - https://github.com/mozilla-b2g/gaia/pull/20238
> P.S. I'm getting some tests for this specific situation, update if confirm
> that the approach is correct.

:D
Target Milestone: --- → 2.0 S4 (20june)
Fernando,

Looks good. We should do the same with the givenName and familyName fields. 

And please be careful with rebasing you were breaking the recently landed duplicates contacts notification

thanks
Attachment #8436995 - Flags: feedback?(jmcf)
Whiteboard: [p=1]
Comment on attachment 8436995 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/20238

PR updated, test added
Attachment #8436995 - Flags: review?(jmcf)
Duplicate of this bug: 974563
Comment on attachment 8436995 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/20238

thanks Fernando, good work

Please land once you get a Green Travis
Attachment #8436995 - Flags: review?(jmcf) → review+
green green, merged merged - b85f072304d7b3eead596100182fd1b1f1adc421
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Tested and working
Hamachi
2.1
Gecko-42ab217
Gaia-56812bb

is this bug going to resolve in 2.0?
Attached file request for approval
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no regression, always've been like this
[User impact] if declined: import previously merged contact looks bad
[Testing completed]: unit tests plus testing on device
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]:
Attachment #8439070 - Flags: approval-gaia-v2.0?
Comment on attachment 8439070 [details]
request for approval

Thanks for the unit tests and master verification on this bug, looks low risk to get uplifted to 2.0 given where we are in the cycle, so approving this change.
Attachment #8439070 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
QA Contact: lolimartinezcr
Tested and working
Hamachi
2.1
Gecko-a3c4c53
Gaia-5f9f64a

2.0
Gecko-3bfc06b
Gaia-5e14503
Status: RESOLVED → VERIFIED
This issue has been successfully verified on Flame 2.1&2.0.
See attachment: verified_v2.1.mp4.
Reproducing rate: 0/5

Flame 2.0 build:
Gaia-Rev        f9d6e3d83c3922e9399a6c27f5ce4cdd27bdfd05
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/45112935086f
Build-ID        20141126000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141126.032754
FW-Date         Wed Nov 26 03:28:05 EST 2014
Bootloader      L1TC00011880

Flame 2.1 build:
Gaia-Rev        db2e84860f5a7cc334464618c6ea9e92ff82e9dd
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119
Build-ID        20141126001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141126.033519
FW-Date         Wed Nov 26 03:35:30 EST 2014
Bootloader      L1TC00011880
You need to log in before you can comment on or make changes to this bug.