1061420 - [Contacts] Delete all numbers which belong to an ICE contact, that ICE should be removed from ICE list

Status: VERIFIED FIXED

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

Description

[Eric Chang [:ericcc] [:echang]]

### STR
1. Set ICE to some contact A(with number), make sure it appears on top of contact list.
2. Remove all phone numbers of contact A, but still keep A in contact.

### Actual 
ICE list still has the contact A.

### Expected
Contact A should be removed from ICE list.
Graph 5, Page 15 of https://bugzilla.mozilla.org/attachment.cgi?id=8459474

### Version
Gaia      e7d31f0e9b6b19d9b484eeec8fb980718bc40d79
Gecko     https://hg.mozilla.org/mozilla-central/rev/532b5fb77ba1
BuildID   20140901160203
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

Comment 1

[Eric Chang [:ericcc] [:echang]]

[Blocking Requested - why for this release]: Spec

Comment 2

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

triage: align to spec of essential feature

Comment 3

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

This is a case of bug 1062595

Comment 4

[Jose Manuel Cantera]

Attachment: 24132.html

Comment 5

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

Comment on attachment 8490631 [details]
24132.html

Working perfect and minimun impact in code. Awesome!

Comment 6

[Jose Manuel Cantera]

landed in master:

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

Comment 7

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

Comment on attachment 8490631 [details]
24132.html

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Ice contacts feature
[User impact] if declined:
Wrong information displayed on to the user from a feature dedicated to security.
[Testing completed]:
Smoke test passed on flame
[Risk to taking this patch] (and alternatives if risky):
Pretty low, patch is just 2 lines of code executing the action of removing a conatct without numbers
[String changes made]:


It requires bug 1062595 to be uplifted first. That one also has been asked for approval-gaia-v2.1

Comment 8

[[:fabrice] Fabrice Desré]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> 
> It requires bug 1062595 to be uplifted first. That one also has been asked
> for approval-gaia-v2.1

Unfortunately bug 1062595 has really-too-late-l10n. What is it blocking this one? It's not obvious from reading this patche's code.

Comment 9

[Jose Manuel Cantera]

It is feasible to provide a patch for this functionality that does not depend on the L10N one.

Francisco, what do you think?

Comment 10

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

I think we will need Carrie input here to decide, IMHO, a message is necessary.

Carrie what do you think about Jose's proposal?

Comment 11

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

Yes, I think the message would be very important. Agree with you guys on this one. Thanks!

Comment 13

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

Please request approval to uplift to 2.1.
This is major issue on new feature ICE contact.

Comment 14

[Jose Manuel Cantera]

(In reply to Wesley Huang [:wesley_huang] from comment #13)
> Please request approval to uplift to 2.1.
> This is major issue on new feature ICE contact.

Wesley,

I totally agree with you but unfortunately this bug depends on bug 1062595 which was not approved for uplifting due to L10N issues.

Please let us know on next steps

best

Comment 15

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

Hi Jose.

Comment 9 seems to be the best approach we can take for 2.1.
I spoke with Carrie and she had no objection since bug 1062595 as late l10n couldn't uplift.

(here I also ni? Carrie in case she has more to input)

Comment 16

[Jose Manuel Cantera]

Attachment: 24611.html

patch for v2.1 as per request from Wesley. this patch does not contain any L10n change or addition

Comment 17

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8498178 [details]
24611.html

Good job thx

Comment 18

[Jose Manuel Cantera]

as per comment #15, landed on v2.1 

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

Comment 19

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

I've checked it on today's build. Looks good! Thanks!

Comment 20

[[:RJ]]

Issue fixed and verified on Flame 2.2 Master KK (319mb) (Full Flash), Flame 2.1 KK (319mb) (Full Flash)

when all numbers are removed ICE contact, Contact is removed the ICE contact list on the top of the contacts list. This works with either ICE contact slot with varying numbers and number type variations.  

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2 Master
BuildID: 20141011040204
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

2.1 KK (319mb) (Full Flash)

Environmental Variables:
Device: Flame 2.1 KK (319mb) (Full Flash)
Build ID: 20141010000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Version: 34.0a2 Flame 2.1 KK (319mb)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0