[Contacts] ICE can be set to a contact without number

VERIFIED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia::Contacts
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: ericcc, Assigned: arcturus)

Tracking

(Blocks: 1 bug)

unspecified
2.1 S5 (26sep)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(tracking-b2g:backlog, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [p=4])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
### STR
1. Set ICE to some contact A(without number), make sure it appears on top of contact list.

### Actual 
ICE can be set 

### Expected
Warning should pop up, ICE should not be set
Graph 3, Page 16 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
(Reporter)

Updated

3 years ago
QA Whiteboard: [COM=Gaia::Contacts]
(Reporter)

Updated

3 years ago
Blocks: 1026682
(Reporter)

Comment 1

3 years ago
[Blocking Requested - why for this release]: Spec
blocking-b2g: --- → 2.1?
Assignee: nobody → francisco
Whiteboard: [p=4]
Target Milestone: --- → 2.1 S4 (12sep)
triage: major issue on new feature.
blocking-b2g: 2.1? → 2.1+
Triage group reviewed, this doesn't meet blocker criteria, defined at https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines. There's no crash, dataloss or feature breakage.

This issue is slightly odd, but doesn't block the usage of ICE features at all.

2.1 is open for approved landings, so request approval from Fabrice or Bhavana in order to land this if it's fixed in time!
blocking-b2g: 2.1+ → 2.1?
triage: we need this so please ask approval for uplift
blocking-b2g: 2.1? → backlog
Let's fix this and ask for approval.

The fix should be pretty simple and it will enhance the user experience a lot.
Depends on: 1061411
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
Duplicate of this bug: 1068402
Created attachment 8491573 [details] [review]
Pointer to PR 14186
Attachment #8491573 - Flags: review?(jmcf)
Status: NEW → ASSIGNED
status-b2g-v2.2: --- → affected

Comment 8

3 years ago
Comment on attachment 8491573 [details] [review]
Pointer to PR 14186

I believe a new round is needed

thanks Francisco
Attachment #8491573 - Flags: review?(jmcf)
Comment on attachment 8491573 [details] [review]
Pointer to PR 14186

PR updated, thanks!
Attachment #8491573 - Flags: review?(jmcf)

Comment 10

3 years ago
Comment on attachment 8491573 [details] [review]
Pointer to PR 14186

working perfectly

thanks Francisco
Attachment #8491573 - Flags: review?(jmcf) → review+
Landed:

https://github.com/mozilla-b2g/gaia/commit/12d634c63ec8a41553a9f150c38260c992dc0020
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 12

3 years ago
We need to uplift this bug to v2.1. It is not acceptable to allow the user to make the mistake of choosing as ICE Contact a Contact which does not have a tel number. Fortunately, thanks to Cristian We have found we can reuse an existing L10N string

https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/communications/contacts/locales/contacts.en-US.properties#L21

Comment 13

3 years ago
[Blocking Requested - why for this release]:

see comment #12
blocking-b2g: backlog → 2.1?
[Blocking Requested - why for this release]:(In reply to Jose Manuel Cantera from comment #12)
> We need to uplift this bug to v2.1. It is not acceptable to allow the user
> to make the mistake of choosing as ICE Contact a Contact which does not have
> a tel number. Fortunately, thanks to Cristian We have found we can reuse an
> existing L10N string
> 
> https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/communications/contacts/
> locales/contacts.en-US.properties#L21

Jose, can you describe the level of risk in this patch?
Flags: needinfo?(jmcf)

Comment 15

3 years ago
(In reply to Dietrich Ayala (:dietrich) from comment #14)
> [Blocking Requested - why for this release]:(In reply to Jose Manuel Cantera
> from comment #12)
> > We need to uplift this bug to v2.1. It is not acceptable to allow the user
> > to make the mistake of choosing as ICE Contact a Contact which does not have
> > a tel number. Fortunately, thanks to Cristian We have found we can reuse an
> > existing L10N string
> > 
> > https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/communications/contacts/
> > locales/contacts.en-US.properties#L21
> 
> Jose, can you describe the level of risk in this patch?

the patch will be the same as the one for 2.2 

https://github.com/mozilla-b2g/gaia/pull/24186/files 

excepting the L10N stuff that will not be added. The patch contains an integration test as well. for those reasons, I would say the risk is low

thanks
Flags: needinfo?(jmcf)
Keywords: verifyme
Not blocking according to comment 3. (Though, this actually creates significant user impact)
We should still get the patch uplifted to 2.1.
blocking-b2g: 2.1? → backlog
Flags: needinfo?(francisco)
Created attachment 8498786 [details]
Github pull request for 2.1

Basically the same code but reusing a string already existing in 2.1
Attachment #8498786 - Flags: review?(jmcf)
Flags: needinfo?(francisco)

Comment 18

3 years ago
Comment on attachment 8498786 [details]
Github pull request for 2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): ICE Contacts new feature
[User impact] if declined: High. A user can choose as ICE Contact a Contact which actually does not have a phone number. As a result in an emergency situation there will be nobody to phone. 
[Testing completed]: Yes, integration test included and manual testing done by the reviewer. 
[Risk to taking this patch] (and alternatives if risky): Low risk patch. Alternative is a useless app. 
[String changes made]:
Attachment #8498786 - Flags: review?(jmcf)
Attachment #8498786 - Flags: review+
Attachment #8498786 - Flags: approval-gaia-v2.1?

Updated

3 years ago
status-b2g-v2.2: affected → fixed

Updated

3 years ago
Attachment #8498786 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
The Gaia-Try for the v2.1 PR appears to have real-looking test failures. Please take a look.
Flags: needinfo?(francisco)
Redirecting the needinfo to Cristian.

Cristian can you check the errors that Ryan is commenting in 2.1 branch?

Thx!
Flags: needinfo?(francisco) → needinfo?(crdlc)
Yes, I fixed the problems at least related to this patch because there are more fails regarding to system, sms, etc...

https://tbpl.mozilla.org/?rev=18159d9e9ddda1f835f13225d8c66b231a7e6082&tree=Gaia-Try

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #20)
> Redirecting the needinfo to Cristian.
> 
> Cristian can you check the errors that Ryan is commenting in 2.1 branch?
> 
> Thx!
Flags: needinfo?(crdlc)
Issue is verified fixed in Flame 2.2, 2.1 (Full Flash, nightly) 

Actual Results: Contacts without a phone number cannot be set as ICE Contacts. 

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

Device: Flame 2.1
Build ID: 20141015001201
Gaia: 379ea4c9dd6d3f8ca2f79ce59c15f6afe6e557c3
Gecko: 4853208cb48a
Version: 34.0 (2.1)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Gaia::Contacts] → [COM=Gaia::Contacts] [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [COM=Gaia::Contacts] [QAnalyst-Triage?] → [COM=Gaia::Contacts] [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.