Closed
Bug 867793
Opened 11 years ago
Closed 11 years ago
Use fancy new SIM API to determine location
Categories
(Marketplace Graveyard :: Integration, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
2013-05-23
People
(Reporter: clouserw, Assigned: cvan)
References
Details
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•11 years ago
|
Whiteboard: [tef-triage]
Comment 1•11 years ago
|
||
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•11 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.
Comment 3•11 years ago
|
||
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.
Comment 4•11 years ago
|
||
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•11 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)
Comment 6•11 years ago
|
||
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•11 years ago
|
Assignee | ||
Comment 7•11 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•11 years ago
|
Target Milestone: --- → 2013-05-16
Assignee | ||
Comment 8•11 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•11 years ago
|
||
MCC is already there, just need MNC: https://github.com/mozilla/zamboni/blob/master/mkt/constants/regions.py
Assignee | ||
Comment 10•11 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•11 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.
Reporter | ||
Updated•11 years ago
|
Target Milestone: 2013-05-16 → ---
Reporter | ||
Updated•11 years ago
|
Target Milestone: --- → 2013-05-23
Assignee | ||
Comment 13•11 years ago
|
||
https://github.com/mozilla/fireplace/commit/7c68e0462
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•