Closed
Bug 1029581
Opened 10 years ago
Closed 10 years ago
[Dialer] Contact name is not shown in the call log after having two contacts with same number and the first one is removed
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)
People
(Reporter: isabelrios, Assigned: rik)
Details
(Whiteboard: [planned-sprint][in-sprint=v2.0-S5])
Attachments
(5 files, 1 obsolete file)
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 | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 2.0?
Comment 3•10 years ago
|
||
Requesting feedback on the fix. Ignore the test, not ready yet - mock contacts module doesn't support what's needed here yet.
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
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
Comment 6•10 years ago
|
||
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
Comment 7•10 years ago
|
||
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.
Comment 8•10 years ago
|
||
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•10 years ago
|
||
Flagging Carrie.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(cawang)
Comment 10•10 years ago
|
||
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)
Comment 11•10 years ago
|
||
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.
Comment 12•10 years ago
|
||
Latest PR fixes original STR. Still need to fix alphabetical log stealing and multi delete fallback.
Comment 13•10 years ago
|
||
Any update on this ?
Comment 14•10 years ago
|
||
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•10 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)
Updated•10 years ago
|
Whiteboard: [dialer-in-sprint=v2.0-S5]
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
Updated•10 years ago
|
Whiteboard: [dialer-in-sprint=v2.0-S5] → [in-sprint=v2.0-S5]
Updated•10 years ago
|
Whiteboard: [in-sprint=v2.0-S5] → [planned-sprint][in-sprint=v2.0-S5]
Assignee | ||
Comment 16•10 years ago
|
||
Eric: Could you look at comment 15, please ?
Flags: needinfo?(echang)
Comment 17•10 years ago
|
||
(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•10 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)
Comment 19•10 years ago
|
||
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•10 years ago
|
||
Attachment #8445525 -
Attachment is obsolete: true
Attachment #8454413 -
Flags: review?(drs+bugzilla)
Comment 21•10 years ago
|
||
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•10 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: 10 years ago
Resolution: --- → FIXED
Comment 23•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/e5617480547edadf5674becee560fc09f62a3aa8
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Comment 24•10 years ago
|
||
I had to re-add value_selector.js and wallpaper.js to the jshint whitelist due to Linter failures. v2.0: https://github.com/mozilla-b2g/gaia/commit/8d89ca60aafcc0bb0ddc779c430b4d87205b6590 https://tbpl.mozilla.org/php/getParsedLog.php?id=43755595&tree=Mozilla-Aurora
Comment 25•10 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)
Comment 26•10 years ago
|
||
Comment 27•10 years ago
|
||
Per comment 25 bug marked as Verified. Thanks for the effort to fix this.
Status: RESOLVED → VERIFIED
Comment 28•10 years ago
|
||
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•10 years ago
|
||
I meant we don't have enough tests in our codebase, aka automated tests.
Flags: needinfo?(anthony)
Comment 30•10 years ago
|
||
Alphabetical call log entry theft still occurs. Filed bug 1043684.
Updated•9 years ago
|
Flags: in-testsuite?
Updated•9 years ago
|
Flags: in-testsuite? → in-qa-testsuite?(jlorenzo)
Updated•9 years ago
|
QA Whiteboard: [fxosqa-auto-backlog?]
Flags: in-qa-testsuite?(jlorenzo)
Updated•9 years ago
|
QA Whiteboard: [fxosqa-auto-backlog?] → [fxosqa-auto-backlog+]
Comment 31•9 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•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•