1135044 - Call information gets never updated after creating a contact

Status: VERIFIED FIXED

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

Description

[Johan Lorenzo [:jlorenzo]]

STR
1. Make a call to an unknown contact
2. Create a new contact from the Call info page.
3. Once the contact created, go back and then go to call info again.

Expected result
In bug 1062799, we used to update the call info page at least when you went back to the call info page a second time. See this video: http://mzl.la/17zJ5fk

Actual result
Going back and forth multiple times doesn't update the call info page. We need to close the app now.

Additional info
I tested against the current 2.1 and master just before bug 1112577 landed[1], the behavior hasn't changed since bug 1062799.


[1] Build ID               20150214010227
Gaia Revision          f0b93e0668ef9565bd6f050b15b4f794d59feb65
Gaia Date              2015-02-13 13:13:27
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/a7c177546ca0
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150214.043959
Firmware Date          Sat Feb 14 04:40:10 EST 2015
Bootloader             L1TC000118D0

Comment 1

[Johan Lorenzo [:jlorenzo]]

[Blocking Requested - why for this release]: Regression.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Triage: Regression from a non-working feature. Not a blocker, but definitely something we need to get fixed after all the blockers are handled. Doug, do you think adopting this strategy is possible?

Comment 3

[Noemí Freire (:noemi)]

I'd like to add that under the current scenario users could create a contact or add to an existing one once and again - > "duplicate contact window" will appear (a really weird scenario). Notice that closing and opening the dialer again DOES NOT fix the issue, because all of this renoming to 2.2? again. 


STR
1. Make a call to an unknown contact
2. Create a new contact from the Call info page
3. Once the contact created, go back and then go to call info again

Expected result
The call info page is properly updated showing the Contact name and "Create a new contact" / "Add to an existing contact" options are not available

Actual result
Going back and forth multiple times doesn't update the call info page, the number (instead of Contact name) and "Create a new contact" / "Add to an existing contact" options are shown. Notice that closing and opening again the Dialer app does not fix the issue.


Environmental variables:
*checked with today's (2/23) master build:
flame 3.0 device
BuildId:20150223081049
Gecko: 6dcfd64
Gaia: a688120
Platform version: 38.0a1

*checked with today's (2/23) 2.2 branch build:
flame 2.2 device
BuildId: 20150223060029
Gecko: 84e31b2
Gaia: 389542b
Platform version: 37.0a2

Comment 4

[Noemí Freire (:noemi)]

Beacuse of comment 3, updating the title accordingly.

Comment 5

[Noemí Freire (:noemi)]

adding the video gathering the scenario raised in comment 3. Thanks!

Comment 6

[Johan Lorenzo [:jlorenzo]]

Triage: Per comment 3, this is a broken functionality.

Comment 7

[Johan Lorenzo [:jlorenzo]]

(In reply to Brogan Zumwalt [:BroganZ] from bug 1136646 comment #4)
> NOTE: Contact details button does not appear at step 9 in comment 0 unless
> user has at least 2 contacts (including newly created one from STR.) I do
> not know if this is a new bug or a side-effect of one of the existing bugs
> mentioned in comment 1. Advice on this would be appreciated.

This explains why we didn't see the same thing between comment 0 and comment 3. Before shooting the video, I tried multiple times on my phone and then I removed all the call log entries and all my contacts. I guess Noemí did the STR right after flashing so the DB was absolutely untouched. Thanks for noticing this difference Brogan!

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

Oh, the horror. Here's a quick summary of my findings while fixing this; the code that applies contact changes to the call log database is currently doing the following for each change (irrespective of its type or content):

1. Gather information from the Facebook contact database
2. Gather all call log DOM nodes
3. For each node find the contact corresponding to the node's number
4. Try to update the corresponding entry in the call log database
5. If we did find a matching entry in the database, update it and reflect the change in the UI

There's three things that are horribly wrong with this:

- First of all since we're iterating over all the DOM nodes we're hammering the database for *every single entry*, not just the ones that are potentially affected by the change, there's no pre-filtering whatsoever based on the contact change information.

- Second, since we don't care about the type of an update (e.g. it's a change to an existing contact) we triple-check every entry in the database to decide if we need to remove the contact, add it or update it even though we already know it from the event.

- Third, if we do update an entry the database we don't send an event to inform listeners that it has changed. This is what's causing this bug and this is the only part that my patch addresses plus removing some code that was a half-hearted attempt at pre-filtering the changes but had become useless and finally bit-rotted.

I'll file a follow up to clean up this mess for v3.

Comment 11

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

Redirecting to Doug so we split the review load.

Comment 12

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

This needs at least 1 test for the functional change, and preferably another for the cleanup.

Comment 13

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

Pushed another patch with added tests. This also fixes the case where we don't update the contact information but we remove it instead. The fix is conceptually the same so I've added a similar test.

Note that we could further trim down oncontactchange() but I don't want to do it right now because the code there served a purpose which was disabled at some point and I'd like to have it back so I'm leaving it as a memento.

Comment 14

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

Looks good. I left one comment on the PR.

Comment 15

[Gabriele Svelto [:gsvelto] (PTO)]

Addressed the last review comment and squashed. I'll open a follow up for the filter-by-contact-before-updating part.

Comment 16

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/1bd1a1bd950e1d5c2dea30a84d9ce42cfd50802b

Comment 17

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8578077 [details] [review]
[gaia] gabrielesvelto:bug-1135044-update-call-information-when-contact-changes > mozilla-b2g:master

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 #): Long standing issue
User impact if declined: If the user updates the contacts associated with the call information that's currently open then the update won't be reflected in the call information page
Testing completed: Tested on a device and covered with unit-tests
Risk to taking this patch (and alternatives if risky): Not taking the patch, risks are rather limited as the patch is small and mostly removes unused code
String or UUID changes made by this patch: None

Comment 18

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/4e0633463571377ad4badc680b666771684e862d

Comment 20

[SandKing]

Attachment: Verified video: verify_1135044.MP4

This issue has been verified successfully on Flame2.2
Verify video:"verify_1135044.mp4".
Repro Rate: 0/10

STR:
1. Have an item in the call log with a phone number without a contact.
2. Long press on the phone number, choose "call information"
3. Choose "create new contact"
4. Add a name, press save
**When you back to call information page and call log page, you can see the contact infomation has been updated.

Flame 2.2:
Build ID               20150323162503
Gaia Revision          e54c4ed1cc188f70ddf1156534d364005dc45490
Gaia Date              2015-03-23 19:09:26
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7ba1778d237b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150323.200543
Firmware Date          Mon Mar 23 20:05:54 EDT 2015
Bootloader             L1TC000118D0

Comment 21

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Flame 3.0. After an unknown number gets created as a contact and returning to call log view, re-accessing call information on the contact is then updated and does not show add contact related options.

Device: Flame (full flashed, 319MB, KK)
BuildID: 20150608010204
Gaia: 1d62b32408567f9f7cf1c71c1e5a0c6593be757b
Gecko: 7d4ab4a9febd
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0