[Dialer] Contact name is not shown in the call log after having two contacts with same number and the first one is removed

VERIFIED FIXED in 2.0 S6 (18july)

Status

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: isabelrios, Assigned: rik)

Tracking

unspecified
2.0 S6 (18july)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)

Details

(Whiteboard: [planned-sprint][in-sprint=v2.0-S5])

Attachments

(5 attachments, 1 obsolete attachment)

Reporter

Description

5 years ago
This bug has been seen on 1.3 build during cert process. It has also been reproducible with today's (24/06) master buri build:
Gecko-fda21c3
Gaia-ed5231f

STR
1) Enter a new contact in the phonebook C1 & "NUMBER 1"
2) Make a call to C1, and check that the call log shows "C1"     
3) Enter a new contact in the phonebook C2 with "NUMBER 1" (same number)
4) Do not merge both contacts 
5) From call logs, go to the phonebook (second tab) and check that both contacts are created
6) Delete C1, and check the call log
7) Call to "NUMBER 1" from call log

ACTUAL
When C1 is removed, call log shows the number but when tapping on it, the attention screen displays C2. After finishing the call, the call log is not updated with C2, instead it still shows the phone number.

EXPECTED
Call log should identify the number as a number of contact 2, C2.
Reporter

Updated

5 years ago
blocking-b2g: --- → 2.0?
cert waiver
blocking-b2g: 2.0? → 2.0+
Requesting feedback on the fix.

Ignore the test, not ready yet - mock contacts module doesn't support what's needed here yet.
Assignee: nobody → dietrich
Status: NEW → ASSIGNED
Attachment #8445525 - Flags: feedback?(etienne)
Comment on attachment 8445525 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20944

Bah, I had STR wrong, not ready for review yet.
Attachment #8445525 - Flags: feedback?(etienne)
Found another issue:

STR:

1. create contact A with number N1
2. call it - hang up without waiting for ring
3. check call log - says A
4. create contact B with number N1 (do not merge)
5. open call log

Expected: shows call to A

Actual: shows call to B
Hrm, comment #5 is not correct - I cannot reproduce now.

Would help to have a full set of testcases. QA, do you have testcases for "two contacts, one number" scenario besides this bug?

Eg, some scenarios and actual/expected outcomes:

* call a number, call log shows the number. (Correct)

* create contact B with same number. call log shows B. (Correct)

* create contact C with same number. call log shows B. (Correct)

* create contact A with same number. call log now shows A. (<- new bug?)

* delete contact A. call log now shows number. (<- this bug)
Keywords: qawanted
With my current PR, this bug is addressed, but only for the first contact. Eg, assuming the final step above works, and then:

* delete contact B. call log still shows B. (<- new and different issue!)

Sounds like the policy for this functionality should be:

* For calls without a contact, show the number.

* For calls to a contact, show the contact associated with the user action.

* For calls whose contact was deleted, show the next matching contact [1] by some predictable order. Alphabetical? Contact creation time?

* Names of deleted contacts should never be shown, only a number or another contact with the same number.

* Numbers should never be shown if there's a contact with that number.
Requesting UX input on above. Let's figure out the expected behavior before sinking engineering time into this.
Flags: needinfo?(firefoxos-ux-bugzilla)

Comment 9

5 years ago
Flagging Carrie.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(cawang)
Hi, 

The rule should be like this:

1. Create a new contact C1 with number 1
2. Make a call to C1, and check that the call log shows "C1"     
3. Enter a new contact C2 with number 1 (same number)
4. Do not merge both contacts 
5. Delete C1, and check the call log
6. It should display C2 for the previous C1's call log

***If there are more than two contacts sharing the same number and one of them is deleted by user, show the next matching contact in call log according to the contact creation time.

***If the contact is deleted and there is no existing contact with the same number, then only show the number in call log.

Thanks!
Flags: needinfo?(cawang)
Ok, thanks Carrie. That validates what I wrote above.

The only new bug then is that creating a new contact with name starting with letter prior in alphabetical order will "steal" the call log entry.

Eg: with a single call log entry by B, create contact A with same number, and call log now says A.

Will fix both issues here.

Note to self: test should include fallback through multiple contact deletions. Eg, delete C1 -> fallback to C2 -> delete C2 -> fallback to C3.
Latest PR fixes original STR. Still need to fix alphabetical log stealing and multi delete fallback.
Any update on this ?
Dietrich, would it help for someone else to take this and finish it?
Flags: needinfo?(dietrich)
Target Milestone: --- → 2.0 S5 (4july)
Assignee

Comment 15

5 years ago
I don't think we should fix more things than needed here. This is a patch that has to be uplifted to 2.0.

Here's my take on it: https://github.com/Rik/gaia/tree/find-contacts-when-removing-1029581 .

I'm asking qawanted to run the call log tests that we have against this branch. It is not well unit tested on our end so I'm not sure if this fix doesn't have side effects.
Assignee: dietrich → anthony
Flags: needinfo?(dietrich)
Whiteboard: [dialer-in-sprint=v2.0-S5]
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
Whiteboard: [dialer-in-sprint=v2.0-S5] → [in-sprint=v2.0-S5]
Whiteboard: [in-sprint=v2.0-S5] → [planned-sprint][in-sprint=v2.0-S5]
Assignee

Comment 16

5 years ago
Eric: Could you look at comment 15, please ?
Flags: needinfo?(echang)
(In reply to Anthony Ricaud (:rik) from comment #16)
> Eric: Could you look at comment 15, please ?

Current v2.0 build has this problem, let me run some test cases to see if we have any more issues, will update tomorrow.

(In reply to Dietrich Ayala (:dietrich) from comment #11)
> The only new bug then is that creating a new contact with name starting with
> letter prior in alphabetical order will "steal" the call log entry.
Flags: needinfo?(echang)
Assignee

Comment 18

5 years ago
Eric: Of course 2.0 has this problem, this is why this bug is opened. I'm asking to test the branch https://github.com/Rik/gaia/tree/find-contacts-when-removing-1029581 that fixes this bug but may introduce other problems. I'm interested to know if my tentative patch breaks any other call log related test cases that you have.
Flags: needinfo?(echang)
I do some exploratory testing on the patch, it works okay, now a log entry sticks to a contact as long as it exists, I saw the contactId is added as a parameter there.
Flags: needinfo?(echang)
Assignee

Comment 20

5 years ago
Attachment #8445525 - Attachment is obsolete: true
Attachment #8454413 - Flags: review?(drs+bugzilla)
Comment on attachment 8454413 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/21624

This patch is beautiful! It took me several hours to understand it, but it's very well done and the comments I left on the PR are minimal. I learned some stuff just by reading it.
Attachment #8454413 - Flags: review?(drs+bugzilla) → review+
Assignee

Comment 22

5 years ago
Hey thanks :) Maybe ignoring whitespace changes, it would have been easier to review: https://github.com/mozilla-b2g/gaia/pull/21624/files?w=1 .

Merged in https://github.com/mozilla-b2g/gaia/commit/534d16f011922ab0b06355887dbb3484e044be50
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Comment 25

5 years ago
I haver tested using: 
 - Hamachi 2.0, gecko 5b0fd39, Gaia db0358b
 - Unagi master Gecko 73bd274, Gaia b3ed124

In both cases, following the steps to reproduce the bug, the contacts are properly identified in the call log. (see screenshot)
Per comment 25 bug marked as Verified. Thanks for the effort to fix this.
Status: RESOLVED → VERIFIED
Depends on: 1038749
No longer depends on: 1038749
I was out for the last three weeks. Thanks for finishing this up Rik!

(In reply to Anthony Ricaud (:rik) from comment #15)
> I don't think we should fix more things than needed here. This is a patch
> that has to be uplifted to 2.0.

I'll see if the other bugs I found still exist after your patch and file new bugs for them if so.

(In reply to Anthony Ricaud (:rik) from comment #15)
> I'm asking qawanted to run the call log tests that we have against this
> branch. It is not well unit tested on our end so I'm not sure if this fix
> doesn't have side effects.

Since the call log is a touchy issue for certification, we should improve the regression tests if they're not covering enough. Rik, do you have more information about where the tests are lacking? Or who would I ask?
Flags: needinfo?(anthony)
Assignee

Comment 29

5 years ago
I meant we don't have enough tests in our codebase, aka automated tests.
Flags: needinfo?(anthony)
Alphabetical call log entry theft still occurs. Filed bug 1043684.
Flags: in-testsuite?
Flags: in-testsuite? → in-qa-testsuite?(jlorenzo)
QA Whiteboard: [fxosqa-auto-backlog?]
Flags: in-qa-testsuite?(jlorenzo)
QA Whiteboard: [fxosqa-auto-backlog?] → [fxosqa-auto-backlog+]

Comment 31

5 years ago
This issue has been verified successfully on Flame2.1&2.0.
Reproducing rate: 0/5
See attachment:Verify_Flame_Callog.mp4

Flame2.0 build version:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1
Build-ID        20141208000206
Version         32.0

Flame2.1 build version:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0

Comment 32

5 years ago
You need to log in before you can comment on or make changes to this bug.