Closed Bug 921971 Opened 6 years ago Closed 6 years ago

[DSDS][FDN] Manage FDN list for both SIMs

Categories

(Firefox OS Graveyard :: RIL, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
1.4 S3 (14mar)

People

(Reporter: khu, Assigned: sergi)

References

Details

(Whiteboard: [ucid:DSDS24, 1.4:p1, ft:RIL, ft:comms])

User Story

As a user I expect to manage FDN list for both SIMs. 

AC1: I expect to have the same functionality as in the single SIM implementation for each of the SIMs.
AC2: I expect to have the FDN list active only for the current SIM for Voice
AC2.1. If a short tap the call button then SIM FDN setting is active
AC2.2. when i long tap and change the outgoing call SIM I want the new changed SIM FDN list to be active
AC3: I expect SIM2 to still remember the data (though disabled) when SIM1 is the default for Voice, and vice versa.

Attachments

(2 files, 2 obsolete files)

As I user, I should be able to edit FDN entries per subscription.
I would like to suggestion moving this feature to 1.4.
This user story isn't in 1.3.
No longer blocks: b2g-dsds-1.3
Update whiteboard tag to follow format [ucid:{id}, {release}:p{1,2}, ft:{team-id}]
Whiteboard: [FT:RIL, 1.4:p1, ucid:DSDS24] → [ucid:DSDS24, 1.4:p1, ft:RIL]
blocking-b2g: --- → 1.4+
Updating user story and some acceptance criteria for v1.4

As a user I expect to manage FDN list for both SIMs. 

Acceptance Criteria:

AC1: I expect to have the same functionality as in the single SIM implementation for each of the SIMs.
AC2: I expect to have the FDN list active only for the current default SIM for Voice
AC3: I expect SIM2 to still remember the data (though disabled) when SIM1 is the default for Voice, and vice versa.
Summary: [B2G][DSDS][User Story] Edit FDN → [DSDS] Manage FDN list for both SIMs
Summary: [DSDS] Manage FDN list for both SIMs → [DSDS][FDN] Manage FDN list for both SIMs
(In reply to Wilfred Mathanaraj [:WDM] from comment #4)
> Updating user story and some acceptance criteria for v1.4
> 
> As a user I expect to manage FDN list for both SIMs. 
> 
> Acceptance Criteria:
> 
> AC1: I expect to have the same functionality as in the single SIM
> implementation for each of the SIMs.
> AC2: I expect to have the FDN list active only for the current default SIM
> for Voice

Hi Wilfred,

For AC2, will this conflict with bug 946866? Or even user select different outgoing voice for one call only, FDN for that SIM should be active, correct?

Thanks.
Flags: needinfo?(wmathanaraj)
Agreed and let me extend this acceptance criteria:

1. If a short tap the call button then default SIM FDN setting is active
2. when i long tap and change the outgoing call SIM I want the new changed SIM FDN list to be active
Flags: needinfo?(wmathanaraj)
Assignee: nobody → nhsieh
Hi All, I try to use FDN in FFOS 1.3. It's not work.  
Because we don't have any options for edit it. Only enable or disable and list, reset PIN2. 

As far as I know, this feature is very old function and this function need to access SIM card's contacts information. 
But our FFOS or any new O.S all doesn't encourage users to save or read contacts information from SIM card. To save or read contacts from phone storage and backup/restore from cloud is a trend. My question is why we need this feature ?
Flags: needinfo?(wmathanaraj)
(In reply to Neo Hsieh from comment #7)
> storage and backup/restore from cloud is a trend. My question is why we need
> this feature ?
Agree with what you said. However, it may be a partner's requirement.
(In reply to Neo Hsieh from comment #7)
> Hi All, I try to use FDN in FFOS 1.3. It's not work.  
> Because we don't have any options for edit it. Only enable or disable and
> list, reset PIN2. 
> 
> As far as I know, this feature is very old function and this function need
> to access SIM card's contacts information. 
> But our FFOS or any new O.S all doesn't encourage users to save or read
> contacts information from SIM card. To save or read contacts from phone
> storage and backup/restore from cloud is a trend. My question is why we need
> this feature ?

This sounds like a bug  - the FDN functionality was done for v1.3.  
Enpei is checking out on v1.3 if this works.
Kaze is on NI as he was the original implementer of the feature - if this is bug then we need to raise a bug against v1.3 as this sounds like a regression
Flags: needinfo?(wmathanaraj)
Flags: needinfo?(kaze)
Flags: needinfo?(echu)
Just a comment, we didn't add multi ICC card support to the FDN features while working on bug 928294 so the FDN features only work for the first ICC card in case of multi ICC card devices.
There is a "Authorized numbers" item where user can add numbers this phone is allowed to dial out. It works although there are other bugs I found during the test, but it won't break FDN function, I will file them later.
Flags: needinfo?(echu)
Attached file [DSDS]FDN_140109.pdf
Please check attached UX document file. Thanks
(In reply to Neo Hsieh from comment #7)
> Hi All, I try to use FDN in FFOS 1.3. It's not work.  
> Because we don't have any options for edit it. Only enable or disable and
> list, reset PIN2. 

My bet is that you haven’t seen the [+] icon in the upper-right corner of the FDN List panel. This button has a very low contrast since the visual refresh we had in the 1.3 branch, and it’s been fixed by my patch for bug 956942.

Neo, please double-check and let me know if it works for your now.
Flags: needinfo?(kaze)
Assignee: nhsieh → nobody
we should not mark feature work with blocking-b2g flag.
blocking-b2g: 1.4+ → backlog
features should not block release, remove blocking flag
blocking-b2g: backlog → ---
Whiteboard: [ucid:DSDS24, 1.4:p1, ft:RIL] → [ucid:DSDS24, 1.4:p1, ft:RIL, ft:comms]
Flags: in-moztrap?(echu)
Flags: in-moztrap?(echu) → in-moztrap?(atsai)
QA Contact: atsai
Assignee: nobody → kaze
Flags: in-moztrap?(atsai) → in-moztrap?(echu)
I’m afraid I have to unassign myself for now, as I still don’t have any DSDS device right now.

Feel free to take this bug, I’ll be happy to review patches or do some mentoring.
Assignee: kaze → nobody
hi david, let's find someone to take FDN work. Thanks
Flags: needinfo?(dscravaglieri)
Assignee: nobody → sergi.mansilla
clear ni?
Flags: needinfo?(dscravaglieri)
User Story: (updated)
Just a heads up to Neo that the UX PDF uses 'Call Settings' and 'Messaging Settings' for the same screen.
Flags: needinfo?(neo.hsieh)
Hi Sergi, My job all transferred to another UX designers. 
So please ni Stephany. Thanks.
Flags: needinfo?(neo.hsieh) → needinfo?(swilkes)
I am temporarily blocked due to the lack of having 2 SIMs with PIN2 enabled. Hopefully I will receive them tomorrow.
Flagging Carrie on DSDS interaction design and Fang on the visual design, since an icon issue was mentioned earlier. Please note that Taipei Friday, Feb. 28 is a national holiday so they may not be able to get to this before Monday.
Flags: needinfo?(swilkes)
Flags: needinfo?(fshih)
Flags: needinfo?(cawang)
Welcome back Neo :) you are the One
Hi Sergi,

FDN only appears in call settings. So you just need to implement the call settings part.
Thanks!
Flags: needinfo?(fshih)
Flags: needinfo?(cawang)
Target Milestone: --- → 1.4 S3 (14mar)
Just a heads up that I am continuing with development since I got my compatible PIN2 sim cards.

About the long tap/short tap point in the user story, are we blocked by https://bugzilla.mozilla.org/show_bug.cgi?id=946866?
Flags: needinfo?(wmathanaraj)
i believe if we can make sure the FDN list is maintained separately for each SIM and the FDN list is obeyed for each of the respective SIM when making calls, the long/short press Acceptance criteria should just work when bug 946866 lands. So the verification of the short/long press Acceptance critria is blocked but i assume development of this feature isn't blocked. please let us know if you need anything to unblock your developement. thanks
Attached file Github Pull Request (obsolete) —
Attachment #8385388 - Flags: review?(kaze)
Since there are a bunch of indentation changes, I recommend reviewing the patch with whitespace differences disabled (https://github.com/mozilla-b2g/gaia/pull/16831/files?w=1).
Comment on attachment 8385388 [details] [review]
Github Pull Request

Looks very good. Nice work Sergi!

I left a few nitpicks on your PR, mostly:
 • a better description of the new `getIccByIndex' function is needed;
 • noise reduction: unnecessary changes should be avoided. 

I’m clearing the r? flag because of the unwanted l10n change (fdnAction-edit-header → fdnAction-edit) — see bug 944749 for some background.

Please r? me again when you have addressed my nits. I’m testing this on the DSDS device I finally received (yay!).
Attachment #8385388 - Flags: review?(kaze)
More generally: this patch introduces a new `getIccByIndex' function in js/utils.js and should get us rid of all the direct calls to the old mozMobileConnection API (which wasn’t DSDS-proof).

A nice follow-up would be to stop using IccHelper in the Settings app: this shared helper was introduced to provide a backward compatibility with the old mozMobileConnection API, and we don’t need it any more. See bug 979808.
Attached file Corrected Github PR (obsolete) —
Attachment #8386054 - Flags: review?(kaze)
Flags: needinfo?(wmathanaraj)
Added unit tests for getIccByIndex.
Comment on attachment 8386054 [details] [review]
Corrected Github PR

Very good work.
Attachment #8386054 - Flags: review?(kaze) → review+
Thanks for the review!
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
1) Please post the hash in the bug after landing.
2) Reverted for Gaia unit test failures.
https://github.com/mozilla-b2g/gaia/commit/777dbb042e9662bfe23cdd047c4d1488778e2509

https://tbpl.mozilla.org/php/getParsedLog.php?id=35797980&tree=B2g-Inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ugh, I thought Travis was OK for these unit tests but failed for another reason. Sorry for missing that. :-(

Sergi, please either copy apps/communications/contacts/test/unit/mock_iccmanager.js to Settings or move it to shared/test/unit/mocks (and update the related tests in Contacts). While you’re at it, please use `require' rather than `requireApp' as we’re going to deprecate the latter. Thanks!
Attachment #8385388 - Attachment is obsolete: true
Attachment #8386054 - Attachment is obsolete: true
Instead of introducing a new mock for icc_manager, please merge the two already existing ones into shared.
Fixed and merged with Anthony's r+
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Verified this user story on :
Fugu
Gaia      b193de26878dae53a4b20b78e453ed07bfbf4d09
Gecko     85f09a609c5d96f23988df41c58932e57d1e398e
BuildID   20140311053055
Version   30.0a1

Result: Some AC are not passsed, so reopen this user story and link those bug
* Bug 981989 failed AC1: I expect to have the same functionality as in the single SIM implementation for each of the SIMs.
* Bug 947140 and 946866 blocked AC 2.1 verification.
Status: RESOLVED → REOPENED
Depends on: 947140, 946866, 981989
Resolution: FIXED → ---
(In reply to Enpei from comment #41)
> Verified this user story on :
> Fugu
> Gaia      b193de26878dae53a4b20b78e453ed07bfbf4d09
> Gecko     85f09a609c5d96f23988df41c58932e57d1e398e
> BuildID   20140311053055
> Version   30.0a1
> 
> Result: Some AC are not passsed, so reopen this user story and link those bugs
> * Bug 981989 failed AC1: I expect to have the same functionality as in the
> single SIM implementation for each of the SIMs.
Correct last comment:
* Bug 947140, 946866 blocked AC2.2. when i long tap and change the outgoing call SIM I want the new changed SIM FDN list to be active
blocking-b2g: --- → 1.4?
Enpei - Can you open followup bugs for the issues you found? This bug has a patch already landed at this point.
Status: REOPENED → RESOLVED
blocking-b2g: 1.4? → ---
Closed: 6 years ago6 years ago
Flags: needinfo?(echu)
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #43)
> Enpei - Can you open followup bugs for the issues you found? This bug has a
> patch already landed at this point.

Hi Jason,

When there are bugs failed acceptance criteria, QA will reopen the bug and link to those new bugs and will wait till all bugs fixed before verify it again. So please keep the bug in reopened status, not resolved fixed or QA will keep being asked to verify the user story while we actually can't. I've comment which bugs failed which AC in comment 41 and 42. 

This is a user story bug, not ordinary engineering bugs. The new rule should already be discussed in https://etherpad.mozilla.org/CommsAcceptance.
Status: RESOLVED → REOPENED
Flags: needinfo?(echu)
Resolution: FIXED → ---
(In reply to Enpei from comment #44)
> (In reply to Jason Smith [:jsmith] from comment #43)
> > Enpei - Can you open followup bugs for the issues you found? This bug has a
> > patch already landed at this point.
> 
> Hi Jason,
> 
> When there are bugs failed acceptance criteria, QA will reopen the bug and
> link to those new bugs and will wait till all bugs fixed before verify it
> again. So please keep the bug in reopened status, not resolved fixed or QA
> will keep being asked to verify the user story while we actually can't. I've
> comment which bugs failed which AC in comment 41 and 42. 
> 
> This is a user story bug, not ordinary engineering bugs. The new rule should
> already be discussed in https://etherpad.mozilla.org/CommsAcceptance.

The problem is that this bug has been morphed into an engineering bug, so this can't be treated as a user story bug anymore, given that this bug contains an associated landing with it. I do know of that acceptance criteria requirement, but a tracking process problem happened here - the implementation work happened in the user story bug, which is incorrect. The user story bug is supposed to act as a meta bug separate from engineering work.

Triage analyzed this situation & decided the best solution was to track the followup work in separate bugs. This isn't an optimal situation to be in, but I don't think we have a choice since a landing occurred on the bug.

Are the two bugs you've linked here the followups needed here?
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(echu)
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #45)
> (In reply to Enpei from comment #44)
> > (In reply to Jason Smith [:jsmith] from comment #43)
> > > Enpei - Can you open followup bugs for the issues you found? This bug has a
> > > patch already landed at this point.
> > 
> > Hi Jason,
> > 
> > When there are bugs failed acceptance criteria, QA will reopen the bug and
> > link to those new bugs and will wait till all bugs fixed before verify it
> > again. So please keep the bug in reopened status, not resolved fixed or QA
> > will keep being asked to verify the user story while we actually can't. I've
> > comment which bugs failed which AC in comment 41 and 42. 
> > 
> > This is a user story bug, not ordinary engineering bugs. The new rule should
> > already be discussed in https://etherpad.mozilla.org/CommsAcceptance.
> 
> The problem is that this bug has been morphed into an engineering bug, so
> this can't be treated as a user story bug anymore, given that this bug
> contains an associated landing with it. I do know of that acceptance
> criteria requirement, but a tracking process problem happened here - the
> implementation work happened in the user story bug, which is incorrect. The
> user story bug is supposed to act as a meta bug separate from engineering
> work.
> 
> Triage analyzed this situation & decided the best solution was to track the
> followup work in separate bugs. This isn't an optimal situation to be in,
> but I don't think we have a choice since a landing occurred on the bug.
> 
> Are the two bugs you've linked here the followups needed here?

Yes.

But if you use resolved fixed here, it will always be treated as QA's action item, which is not a good choice as well. What's best for all of us?
Flags: needinfo?(echu)
(In reply to Enpei from comment #46)
> But if you use resolved fixed here, it will always be treated as QA's action
> item, which is not a good choice as well. What's best for all of us?

A workaround you could do here is file a tracking bug to track "all required followups for this X user story" and use that as a reference against acceptance criteria. That will allow you to maintain an understanding of what's completed in the user story vs. not.
(In reply to Jason Smith [:jsmith] from comment #47)
> A workaround you could do here is file a tracking bug to track "all required
> followups for this X user story" and use that as a reference against
> acceptance criteria. That will allow you to maintain an understanding of
> what's completed in the user story vs. not.

That's really troublesome for QA to do so I can imagine. Bottom line this user story is not valid to verify. I just hope any rule should have a space for adjustment for everyone to work more easily. 

I'll verify the bug again when all related bugs are fixed but I won't do another bug work which is not strait thinking at all.
See Also: → 980399
Verified okay with v1.4 on Fugu.

GAIA_REV=441c4bcd8ac4f8c01a9bc5a2f8d64eaa87844803
GECKO_REV=0c18beacb37a7e1e80bcf6d46cc8ff9a18c0c653
GAIA_BRANCH=mozillaorg/v1.4
GECKO_BRANCH=mozillaorg/v1.4
BUILD_TAG=jenkins-B2G.v1.4.0.fugu-25
BuildID=20140416041552

Moztrap case
http://mzl.la/1l068FE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.