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)

defect

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.
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.
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)
(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]
Depends on: 868571
No longer depends on: 866272
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.
Target Milestone: --- → 2013-05-16
Not really looking forward to adding these to Zamboni, but here are the MCC/MNC mappings:

https://en.wikipedia.org/wiki/Mobile_country_code
(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.
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.
Blocks: 872902
Target Milestone: 2013-05-16 → ---
Target Milestone: --- → 2013-05-23
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.