1068651 - [Contacts] [ICE] In occasions the ICE Group and Contact Settings info is not correctly updated

Status: VERIFIED FIXED

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

Description

[Jose Manuel Cantera]

I have not been able to reproduce this bug 100%. 

STR: 


Set an ICE Contact. Then remove the ICE Contact. 

Actual: The ICE Group does not disappear from the Contacts List. The ICE Contact remains in Contacts Settings.

Comment 1

[Jose Manuel Cantera]

Loli,

Are you able to reproduce?

thanks

Comment 2

[Jose Manuel Cantera]

Latest news from this bug: I've been to reproduce 100% by following these steps:

1. Contacts --> Settings --> ICE Contacts --> Check ICE Contact 1 but not select any contact. --> Back to Contact List. 
2. Now go to Settings --> ICE Contacts and Select a Contact
3. Remove the selected ICE Contact

Actual: The removed ICE Contact Remains setted in Contact Settings

Comment 3

[Jose Manuel Cantera]

We need at least a patch that guarantees that the ICE Contact Check is not actually checked if no Contact is finally selected.

Comment 5

[Jose Manuel Cantera]

I've been able to reproduce the bug as per the steps described in comment #2. Loli, please verify

Comment 6

[Jose Manuel Cantera]

(In reply to Jose Manuel Cantera from comment #2)
> Latest news from this bug: I've been to reproduce 100% by following these
> steps:
> 
> 1. Contacts --> Settings --> ICE Contacts --> Check ICE Contact 1 but not
> select any contact. --> Back to Contact List. 
> 2. Now go to Settings --> ICE Contacts and Select a Contact. Edit such a Contact. 
> 3. Remove all telephone phones of the edited ICE Contact
> 
> Actual: The removed ICE Contact Remains setted in Contact Settings

Comment 7

[Jose Manuel Cantera]

Attachment: 24224.html

Basically the patch, guarentees that everytime we enter the ICE Settings screen the information is refreshed. As a result we will no longer have issues with showing outdated information due to race conditions in event handling.

Furthermore, The ice_settings_tests have been refactored as thery should be asynchronous as we are calling getContactById which resolves asynchronously.

Comment 8

[Loli (:lolimartinezcr)]

(In reply to Jose Manuel Cantera from comment #7)
> Created attachment 8492997 [details]
> 24224.html
> 
> Basically the patch, guarentees that everytime we enter the ICE Settings
> screen the information is refreshed. As a result we will no longer have
> issues with showing outdated information due to race conditions in event
> handling.
> 
> Furthermore, The ice_settings_tests have been refactored as thery should be
> asynchronous as we are calling getContactById which resolves asynchronously.

I can't reproduce this bug. Because when contact is deleted, ICE Group disappear and in settings "Set ICE contact" is disabled

Comment 9

[Loli (:lolimartinezcr)]

(In reply to Loli (:lolimartinezcr) from comment #8)
> (In reply to Jose Manuel Cantera from comment #7)
> > Created attachment 8492997 [details]
> > 24224.html
> > 
> > Basically the patch, guarentees that everytime we enter the ICE Settings
> > screen the information is refreshed. As a result we will no longer have
> > issues with showing outdated information due to race conditions in event
> > handling.
> > 
> > Furthermore, The ice_settings_tests have been refactored as thery should be
> > asynchronous as we are calling getContactById which resolves asynchronously.
> 
 I can't reproduce this bug. Because when contact is deleted, ICE Group
 disappear and in settings "Set ICE contact" is disabled

Flame
2.2
Gecko-93e8d33
Gaia-dbc8904
Gecko-93e8d33
Gaia-dbc8904

Comment 10

[Jose Manuel Cantera]

(In reply to Loli (:lolimartinezcr) from comment #9)
> (In reply to Loli (:lolimartinezcr) from comment #8)
> > (In reply to Jose Manuel Cantera from comment #7)
> > > Created attachment 8492997 [details]
> > > 24224.html
> > > 
> > > Basically the patch, guarentees that everytime we enter the ICE Settings
> > > screen the information is refreshed. As a result we will no longer have
> > > issues with showing outdated information due to race conditions in event
> > > handling.
> > > 
> > > Furthermore, The ice_settings_tests have been refactored as thery should be
> > > asynchronous as we are calling getContactById which resolves asynchronously.
> > 
>  I can't reproduce this bug. Because when contact is deleted, ICE Group
>  disappear and in settings "Set ICE contact" is disabled
> 
> Flame
> 2.2
> Gecko-93e8d33
> Gaia-dbc8904
> Gecko-93e8d33
> Gaia-dbc8904

Obviously you have to have more than one contact on your list

Comment 11

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8492997 [details]
24224.html

Stunning job JM!

Looks fantastic and will solve lots of possible race conditions. Left some notes on the PR, nothing important, just mainly nits.

Comment 12

[Loli (:lolimartinezcr)]

(In reply to Jose Manuel Cantera from comment #10)
> (In reply to Loli (:lolimartinezcr) from comment #9)
> > (In reply to Loli (:lolimartinezcr) from comment #8)
> > > (In reply to Jose Manuel Cantera from comment #7)
> > > > Created attachment 8492997 [details]
> > > > 24224.html
> > > > 
> > > > Basically the patch, guarentees that everytime we enter the ICE Settings
> > > > screen the information is refreshed. As a result we will no longer have
> > > > issues with showing outdated information due to race conditions in event
> > > > handling.
> > > > 
> > > > Furthermore, The ice_settings_tests have been refactored as thery should be
> > > > asynchronous as we are calling getContactById which resolves asynchronously.
> > > 
> >  I can't reproduce this bug. Because when contact is deleted, ICE Group
> >  disappear and in settings "Set ICE contact" is disabled
> > 
> > Flame
> > 2.2
> > Gecko-93e8d33
> > Gaia-dbc8904
> > Gecko-93e8d33
> > Gaia-dbc8904
> 
> Obviously you have to have more than one contact on your list

Retest and *reproducible* bug with steps JM has indicated.

Comment 13

[Jose Manuel Cantera]

Comment on attachment 8492997 [details]
24224.html

ready for a new review round with comments addressed.

thanks

Comment 14

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8492997 [details]
24224.html

Perfect thanks!

Comment 15

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/a91dc5fc442bc99aa1f38fdf067ad78978f86f1a

Comment 16

[Loli (:lolimartinezcr)]

Tested and working
2.2
Flame
User
Gecko-27b927d
Gaia-83d7245

Pending 2.1

Comment 17

[Loli (:lolimartinezcr)]

Tested and working
2.2
Flame
User
Gecko-27b927d
Gaia-83d7245

Comment 18

[Shally Li（leave from Mozilla）]

This bug has been successfully verified on latest Flame v2.1.
See attachment: verified_v2.1.mp4.
Reproduce rate: 0/5

STR:
1. Go to Contacts ->Settings ->ICE Contacts,turn on "ICE Contact 1" but not select any contact,then back to Contact list. 
2. Now go to Settings ->ICE Contacts ->select a Contact as "ICE Contact 1".
**The contact name shows under "ICE Contact 1".And show "ICE Contacts" at the top of Contact list page.
3. Turn off "ICE Contact 1".
**The contact name still shows and the box is grey under "ICE Contact 1" as expected.
4. Back to contact list.
**It will not show "ICE Contacts" icon as expected.
5.Delete the contact in Contact list,then go to Settings->ICE Contacts.
**The contact name doesn't show under "ICE Contact 1" as expected.

Flame 2.1 build:
Build ID               20150203001532
Gaia Revision          17bf14f12e43043654498330d610d469b8b55e64
Gaia Date              2015-02-03 05:19:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/014f57033b8a
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150203.041012
Firmware Date          Tue Feb  3 04:10:23 EST 2015
Bootloader             L1TC000118D0

Comment 19

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4