1062595 - No confirm window pops up to inform users that deleted numbers are removed from an ICE contact.

Status: VERIFIED FIXED

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

Description

[Maria Angeles Oteo (:oteo)]

According to the ICE spec (https://bug1026682.bugzilla.mozilla.org/attachment.cgi?id=8459474 on page 15) when number/s is/are deleted from an ICE contact, user should be informed with a confirm window pop-up.

Flame
2.1
Gecko-6be7031
Gaia-e1f6d88

Pre-requisites:
1. ICE Contact saved

Steps to reproduce:
1.Go to contacts app and select an ICE contact from the list
2.Tap to edit contact
3.Remove a number or several numbers from the Contact

Expected results:
A confirm window should inform to the user that the deleted number are removed from the ICE list as well

Actual results:
no Confirm window implemented

Comment 1

[Maria Angeles Oteo (:oteo)]

[Blocking Requested - why for this release]: It's really important for the user be aware that deleting those numbers can make impossible to call to those preferred contacts in case of emergency

Comment 2

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: major issue to new feature

Comment 3

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

Carrie, what should we say if we remove all the phones for a contact? Same wording?

Comment 4

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

Hi, 


If the ICE contact has more than one number and we only delete one of them, we show the strings on p.15 Screen 3 and the contact will not be removed from ICE list.

If all the numbers of the ICE contact are deleted the contact will be removed from ICE contacts and we show the strings as below,

----------------------------------------------
All the numbers are deleted. This contact is 
removed from the ICE Contacts.

I'll add this case to the spec to keep it updated.

Thanks!

Comment 6

[Dietrich Ayala (:dietrich)]

Triage group discussed and this doesn't meet the blocking criteria for stopping a whole release. Please review the criteria at https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines.

This is a new feature, and the lack of this confirmation prompt doesn't prevent the use of the feature. If this is fixed in 2.1 timeframe, ask for approval and land! Otherwise, it seems like it can ship in the next version.

Wesley and Maria, if there's some reason this should actually stop the release from shipping, please discuss!

Comment 7

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

This will have an easy fix and the improve to the user experience will be significant, so let's fix this and ask for approval

Comment 8

[Jose Manuel Cantera]

Attachment: 24000.html

Francisco,

Please could you do a quick review? Then, I can polish it up and create a test

Comment 9

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

Comment on attachment 8488563 [details]
24000.html

Definitely looking pretty good

Comment 10

[Jose Manuel Cantera]

Comment on attachment 8488563 [details]
24000.html

Francisco,

I believe this is ready to be reviewed. There are no tests in the PR because we are going to create an integration test for this (bug 1067234).

Comment 11

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

Comment on attachment 8488563 [details]
24000.html

We are not removing from ICE the contact if we remove all the phone numbers, we are just displaying the message (which looks good).

Comment 12

[Jose Manuel Cantera]

Francisco,

The purpose of the bug is to show the alert to the user but not to actually remove the ICE contact. That should be another bug, and I believed you were implementing that. 

If that is not the case I can open a new one and solve it.

thanks

Comment 13

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

(In reply to Jose Manuel Cantera from comment #12)
> Francisco,
> 
> The purpose of the bug is to show the alert to the user but not to actually
> remove the ICE contact. That should be another bug, and I believed you were
> implementing that. 
> 
> If that is not the case I can open a new one and solve it.
> 
> thanks

The previous work done was about delete contacts (full contacts), and removing the ICE group. If we perform a check about removing just the phone, if we remove all the phones, we should remove that contact from the ICE group as well.

Comment 14

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

Sorry, when I mean the ice group, I mean removing them as ICE contact.

Then the group will dissapear if it was the only ICE contact.

Comment 15

[Jose Manuel Cantera]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #14)
> Sorry, when I mean the ice group, I mean removing them as ICE contact.
> 
> Then the group will dissapear if it was the only ICE contact.

Yes, but that should be solved in another bug. Can we land this and then open a new bug for that functionality?

Comment 16

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

Fair enough, we do have bug 1061420 to actually perform that last bit.

Please, go ahead and merge once everything is green.

Comment 17

[Jose Manuel Cantera]

landed in master:

https://github.com/mozilla-b2g/gaia/commit/0d11eae32af28284ab9c4520d45af4e12dcd624f

Comment 18

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

Comment on attachment 8488563 [details]
24000.html

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Ice contacts feature
[User impact] if declined:
User won't know that could be delete information that later could be use in case of an emergency
[Testing completed]:
Smoke test passed on flame
[Risk to taking this patch] (and alternatives if risky):
Pretty low risk
[String changes made]:
ICEContactDelTel
ICEContactDelTelAll

Comment 19

[Loli (:lolimartinezcr)]

Tested and *not* working, because if i set an ICE contact with 3 phones numbers only I can see warning when I delete only first phone number. If i delete a phone number, I can see warning, but if i close contact application and after i open app and delete other phone number to ICE contact I can't see warning.

Flame
2.2
Gecko: cce3dff
Gaia: 897bd62

Comment 20

[Loli (:lolimartinezcr)]

(In reply to Loli (:lolimartinezcr) from comment #19)
> Tested and *not* working, because if i set an ICE contact with 3 phones
> numbers only I can see warning when I delete only first phone number. If i
> delete a phone number, I can see warning, but if i close contact application
> and after i open app and delete other phone number to ICE contact I can't
> see warning.
> 
> Flame
> 2.2
> Gecko: cce3dff
> Gaia: 897bd62

Sorry but I have confused because when i tap in "X" button in phone number doesn't appear warning, it only appears when user presses "update" button.

Tested and working
 Flame
 2.2
 Gecko: cce3dff
 Gaia: 897bd62

Comment 21

[[:fabrice] Fabrice Desré]

Comment on attachment 8488563 [details]
24000.html

Too late for new strings.

Comment 22

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

Attachment: [2.1 Contacts] Support for ICE Contacts v0.3.pdf

Update the spec as a reference. Thanks!

Comment 23

[Duane Dixon [:ddixon]]

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

Actual Results: Assigning an ICE contact (Person A), then deleting their number(s) prompts the notification "Deleted phone numbers are removed from ICE Contact". 

Device: Flame Master
Build ID: 20141015040201
Gaia: 5f1f0960ae9d22acf2a324ad37a48174d6df87f6
Gecko: 62f0b771583c
Version: 36.0a1 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0