1069895 - [Contacts] User can't set ICE contact, when this contact has been selected previously and switch of ICE contact is disabled

Status: VERIFIED FIXED

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

Description

[Loli (:lolimartinezcr)]

Flame
User
2.2
Gecko: 45f4500
Gaia: d170091

Prerequisited: 
Contacts saved.
ICE contact 1 is setted with a contact 1 and after user changes switch to OFF

STRs:
1. Tap contacts application.
2. Tap Settings.
3. Tap Set ICE contact.
4. Enable ICE contact 2.
5. Tap in textbox of ICE Contacts 2 and select contact 1

Actual result:
Contact 1 can't be selected.

Expected result:
Contact 1 should be selected.


3. Tap "Set ICE contats".
4.

Comment 1

[Loli (:lolimartinezcr)]

User can se(In reply to Loli (:lolimartinezcr) from comment #0)
> Flame
> User
> 2.2
> Gecko: 45f4500
> Gaia: d170091
> 
> Prerequisited: 
> Contacts saved.
> ICE contact 1 is setted with a contact 1 and after user changes switch to OFF
> 
> STRs:
> 1. Tap contacts application.
> 2. Tap Settings.
> 3. Tap Set ICE contact.
> 4. Enable ICE contact 2.
> 5. Tap in textbox of ICE Contacts 2 and select contact 1
> 
> Actual result:
> Contact 1 can't be selected.
> 
> Expected result:
> Contact 1 should be selected.
> 
 User can't select "contact 1" because sees a warning "This contact is already an existing ICE contact". But really "contact 1" was selected but switch ICE Contact 1 is OFF

Comment 2

[Jose Manuel Cantera]

I think here we need a UX decision. Because if the user decides to later activate ICE Contact 1 he will end up having two times the same contact. Probably we would need to reset ICEContact 1 and set such a Contact as ICE Contact 2. But anyway I think here we need UX input. Ni Carrie

Comment 3

[Carrie Wang [New account: [e-mail]]]

Hi, 

I just checked the behavior of "Switch" from building block and found it doesn't prohibit resetting the value in the field. Hence, I'd suggest let's reset the contact name when users switch off the ICE contact. So we will not face the issue here and the warning will only pop up when both ICE are enabled and user is selecting the same contact for the second one. I'll change it in the spec and update it later. Thanks!

Comment 4

[Jose Manuel Cantera]

(In reply to Carrie Wang [:carrie] from comment #3)
> Hi, 
> 
> I just checked the behavior of "Switch" from building block and found it
> doesn't prohibit resetting the value in the field. Hence, I'd suggest let's
> reset the contact name when users switch off the ICE contact. So we will not
> face the issue here and the warning will only pop up when both ICE are
> enabled and user is selecting the same contact for the second one. I'll
> change it in the spec and update it later. Thanks!

Carrie,

Do you mean that we no longer need the state 'active' or 'inactive' for an ICE Contact ? 

please confirm, thanks

Comment 5

[Carrie Wang [New account: [e-mail]]]

No, we will still have the "active"/ "inactive" switch, but if user disables ICE 1, the contact in the name field (let's say, Catherine) will be removed (it will be reset to none ). So if the user wants to select Catherine as ICE 2, they can still do it. What do you think? Thanks!

Comment 6

[Jose Manuel Cantera]

(In reply to Carrie Wang [:carrie] from comment #5)
> No, we will still have the "active"/ "inactive" switch, but if user disables
> ICE 1, the contact in the name field (let's say, Catherine) will be removed
> (it will be reset to none ). So if the user wants to select Catherine as ICE
> 2, they can still do it. What do you think? Thanks!

Yes, but in the end the ICE Contact will be set or not set. until now we could have an ICE Contact set but not active (if the check was unchecked). 

This makes perfectly sense and will simplify the code a lot. 

thanks Carrie

Comment 8

[Jose Manuel Cantera]

Attachment: 24345.html

Comment 9

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

Comment on attachment 8495277 [details]
24345.html

LGTM, very good actually.

Thanks Jose!

Comment 10

[Jose Manuel Cantera]

landed in master:

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

Comment 11

[Loli (:lolimartinezcr)]

Tested and working
Flame
Eng
Gecko-a310d15
Gaia-931d547

Pending 2.1

Comment 12

[Jose Manuel Cantera]

Comment on attachment 8495277 [details]
24345.html

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): ICE Contacts New Feature
[User impact] if declined: application will be considered as inconsistent 
[Testing completed]: Yes, includes unit tests
[Risk to taking this patch] (and alternatives if risky): Low-risk patch. alternative is keep an inconsistent app functionality
[String changes made]:

Comment 13

[[:fabrice] Fabrice Desré]

Comment on attachment 8495277 [details]
24345.html

Not bad enough to justify uplift at this point.

Comment 14

[Duane Dixon [:ddixon]]

Issue is verified fixed in Flame 2.2 (Full Flash, nightly).

Actual Results: ICE contacts feature functions correctly when:  user selects a duplicate ICE contact, interchanges them, and/or deactivates them.  

Device: Flame Master
Build ID: 20141014040203
Gaia: 4f86c631e0465c0e56ccebeb1324fd28be9ea32f
Gecko: 54217864bae9
Version: 36.0a1 (Master)
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Removing 'verifyme' tag based on Comment 13--patch uplift to 2.1 deemed unnecessary.