998914 - [Settings] Cannot preselect the APN correctly if there are several APNs with the same carrier name (follow-up bug 991628)

Status: RESOLVED FIXED

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

Description

[isabelrios]

Seen with 1.4 (04-21) buri build:
Gecko-3f64d65
Gaia-961f7d4

STR
Receive via OMA CP and store the same apn more than once (for example, three times)
Go to Settings-> Cellular & Data -> Data settings.
Select the third apn with the same name
Tap on Ok
Go back to Data settings

EXPECTED
The apn selected previously is kept as selected

ACTUAL
The apn selected previously is deselected and the first one with the same name is selected instead.

Comment 1

[Beatriz Rodríguez [:brg]]

This is a cert blocker in 1.3.

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/18528

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v1

Arthur, bug 991628 was filed because the user wasn't able to select an APN in case there were some of them with the same name in the list (the first APN in the list was always selected). We fixed that by assigning a hash code for each APN in the list so we were to able to switch between APNs. There was still an issue I didn't notice (my fault sorry) and this is what I fix here. Let me explain what is going on. We allowed the user to switch between APNs by using the hash code mention before but we didn't store the hash code for the APN selected. It causes we cannot (pre)select the APN selected by the user when she/he opens the APN list panel again. This patch stores the hash code so we can (pre)select the APN correctly.

I've added some unit test for the change.

Would you mind to review this work please? Thanks!

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8410361 [details] [diff] [review]
v1

The patch looks good to me. I've added some comments regarding the tests, please have a look, thanks!

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v2

Arthur, thanks for the previous review. This version of the patch addresses the comment you made. Could you take a look a it again please? Thanks.

Comment 7

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8411642 [details] [diff] [review]
v2

r=me with the nits addressed, thanks!

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Thanks for the reviews Arthur.

Travis goes green. Tests pass and everything seems to work correctly. Landed in Gaia master branch at https://github.com/mozilla-b2g/gaia/commit/88452273be2f912d250e661c29b5a7c1af7aa012

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v3 r=arthurcc

This is the version we landed. Carrying out r=arthurcc.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

This is the follow-up patch from bug 991628 we requested approval and merged in 1.3 branch a few days ago. Please, ping me if you need further information about why this follow-up is needed.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): DSDS stuff / bug 962453
[User impact] if declined: If there are some APNs with the same carrier name the user is not able to switch correctly between them. 
[Testing completed]: Yes, unit test added too.
[Risk to taking this patch] (and alternatives if risky): Low, in fact this is a corner case. I mean all the APN in the apn.json data base have different names. Only APNs set through OMA client provisioning (CP) could have the same carrier name for all of them and the carrier rarely sends this APNs to the device.
[String changes made]: No changes.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/82e16ae1e46735715150b527247aeda159e0442f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v1.3 uplift.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR v1.3 https://github.com/mozilla-b2g/gaia/pull/18732

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Thanks!

v1.3: https://github.com/mozilla-b2g/gaia/commit/caab7a78f0c04913f48a3051259663ee85625906

Comment 14

[isabelrios]

Already checked on 1.3, this is working fine now.
Buri 1.3 04-30 build:
Gecko-a8664b5
Gaia-a8f5ca7

Thanks!