Use fancy new SIM API to determine location

RESOLVED FIXED in 2013-05-23

Status

Marketplace
Integration
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: clouserw, Assigned: cvan)

Tracking

2013-05-23
Points:
---
Dependency tree / graph

Details

(Reporter)

Description

4 years ago
Bug 866272 is giving access to an API which the packaged marketplace can use to query the SIM and determine what marketplace to show the user.  Our current detection path, defined over in bug 865132 comment 6, is:

> URL parameter -> localStorage -> cookie -> geoip

We should prepend this list with SIM, if the device has one:

> SIM -> URL parameter -> localStorage -> cookie -> geoip

Setting tef? because if this bug finds a problem with the API it'll need to be fixed, so suggesting we have this as a blocker until verified.

Updated

4 years ago
Whiteboard: [tef-triage]
If Fireplace side could pass the region from SIM as a GET param that would make life a lot easier for me. I assume that is what you were planning but wanted to make it explicit.

Comment 2

4 years ago
We already do!

https://github.com/mozilla/fireplace/blob/master/hearth/media/js/urls.js#L71

That parameter will always contain what we believe the user's region to be, or `null` or an empty string if we don't know the user's region.
Triage results - triage is concerned about the following comment implying significant 1.01 risk:

"Setting tef? because if this bug finds a problem with the API it'll need to be fixed, so suggesting we have this as a blocker until verified"

The original argument to my understanding for why bug 866272 was needed was to allow marketplace to figure out what country it was in to serve country-specific content.

However, there's open questions here to resolve whether to block or not. I'm following up with clouserw in IRC to talk more about this.
Talked this over with a few people. I understand now that there isn't a different option to meet the requirements needed here. However, there's still a large concern about risk here that needs to be addressed here after talking with krupa.

Such questions that we talked about include:

1. What bugs may we find from these changes?
2. What fallback behavior are we implementing in case the underlying SIM API fails? Gives incorrect data?
3. Are the certification testing processes covering workflows involving the underlying SIM API?

Asking for a risk assessment from Wil on what he can answer from the above questions.
Flags: needinfo?(clouserw)
(Reporter)

Comment 5

4 years ago
(In reply to Jason Smith [:jsmith] from comment #4)
> Talked this over with a few people. I understand now that there isn't a
> different option to meet the requirements needed here. However, there's
> still a large concern about risk here that needs to be addressed here after
> talking with krupa.
> 
> Such questions that we talked about include:
> 
> 1. What bugs may we find from these changes?

You're changing an API.  We're the first customers of that API.  Bugs may range from complete failure to minor changes we work around.

> 2. What fallback behavior are we implementing in case the underlying SIM API
> fails? Gives incorrect data?

See comment 0 for the fallback pattern.  If it fails or doesn't exist, we follow that pattern.  If it gives incorrect data I'd file a new TEF+ bug.

> 3. Are the certification testing processes covering workflows involving the
> underlying SIM API?

A question for Krupa.  I don't know the status of what SIMs we currently can test with.

> Asking for a risk assessment from Wil on what he can answer from the above
> questions.
Flags: needinfo?(clouserw)
The blocking request here is updating the app on device, not the hosted work per talking with Wil. Clearing the flag. I filed bug 868571 for the Gaia-specific work here.
blocking-b2g: tef? → ---
Whiteboard: [tef-triage]

Updated

4 years ago
Depends on: 868571
No longer depends on: 866272
(Assignee)

Comment 7

4 years ago
https://github.com/mozilla/zamboni/commit/2f07d90

Unsigned package:
https://marketplace-dev.allizom.org/media/packaged-apps/yulelog_prod_2013.05.10_01.16.03.zip

To sign the package:
manage.py sign_marketplace --settings=settings_local_mkt

Region/carrier detection code:
https://github.com/mozilla/fireplace/blob/master/yulelog/index.html#L27

I'll keep this bug open as we still need code in Zamboni to map MCC/MNC to country/carrier, respectively.
(Assignee)

Updated

4 years ago
Target Milestone: --- → 2013-05-16
(Assignee)

Comment 8

4 years ago
Not really looking forward to adding these to Zamboni, but here are the MCC/MNC mappings:

https://en.wikipedia.org/wiki/Mobile_country_code
(Reporter)

Comment 9

4 years ago
MCC is already there, just need MNC: https://github.com/mozilla/zamboni/blob/master/mkt/constants/regions.py
(Assignee)

Comment 10

4 years ago
(In reply to Wil Clouser [:clouserw] from comment #9)
> MCC is already there, just need MNC:
> https://github.com/mozilla/zamboni/blob/master/mkt/constants/regions.py

That's correct, yeah - we just need middleware to parse the region's MCC instead of by region's slug.

Comment 11

4 years ago
If we have the MCC/MNC in Fireplace, I don't see why we couldn't just have a map in settings.js with something like this:

{
732:'co',
734:{1:'ve'}
}

The MCC is probably all that we'll need for now. Each carrier can already force their own carrier code in settings.js.

No sense in making a request to Zamboni to get a mapping that could easily fit in a few dozen (someday a few hundred) bytes of code.
(Assignee)

Updated

4 years ago
Duplicate of this bug: 871680
(Assignee)

Updated

4 years ago
Blocks: 872902
(Reporter)

Updated

4 years ago
Target Milestone: 2013-05-16 → ---
(Reporter)

Updated

4 years ago
Target Milestone: --- → 2013-05-23
(Assignee)

Comment 13

4 years ago
https://github.com/mozilla/fireplace/commit/7c68e0462
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.