Closed Bug 863775 Opened 11 years ago Closed 11 years ago

[fireplace] Create API middleware for geodude

Categories

(Marketplace Graveyard :: API, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
2013-04-25

People

(Reporter: basta, Assigned: chuck)

References

Details

(Whiteboard: [fireplace] p=1)

If a client makes a GET request to any API endpoint which serves app listings (at minimum the homepage, search, and featured APIs) and the query parameter `region` is present but empty or `null` (e.g.: /api/v1/home/page/?region=&_user=... or /api/v1/home/page/?region=null&_user=...), geodude should be polled before the request is processed.

Geodude's response should be used as the region for the request. An HTTP header should be set (e.g.: `X-User-Region: br`).

The client should respect the header and use it for all future requests (because of this, geodude will only be queried for the first hit to the API).

This conceivably doesn't necessarily need to be middleware, but if I were doing it, that's probably how I'd do it. Feel free to update the bug.
Blocks: 859511
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [fireplace] p=1
Note - we were planning on adding in a header for filters here: bug 861229

Are you expecting the client to always pass the ?region query param, or a header? It seems perfectly reasonable for client to pass this a header, in which case we should that to CORS (and will we need to add to CORS for fireplace to access it?)
I think the actual implementation doesn't really matter (query string, headers, etc.), so long as there's a reasonable way to do it.

Fireplace will always pass in a region, null or otherwise.

The browser automatically passes which headers the XHR is trying to use on the CORS preflight request, so there aren't any client-side changes. On the API side, though, the new header will likely need to be added to one of the Access-Control headers.
I don't think this bug is really about a geodude lookup, but rather a region lookup. We already place requests into a REGION as defined in mkt/constants/regions.py, we should be doing the same lookup and assigning to request.REGION. This bug is just about defining when.

If request.REGION is defined, then the results will be filtered based on the region. Which I think is the intent of the bug.
Final comment, honest. Not sure how we can get around this, but this essentially allows the client to use any region they would like? There a legal issue there?
(In reply to Andy McKay [:andym] from comment #4)
> Final comment, honest. Not sure how we can get around this, but this
> essentially allows the client to use any region they would like? There a
> legal issue there?

As of today, that's not a problem.  We let people choose their region from their settings and that was the original concept for the marketplace.  That said, we've discovered more and more legal issues, as you mention.  So far I haven't heard of it being a problem, but I'm CCing David and Jishnu for confirmation.

Jishnu:  The question is, from a legal perspective, are we still ok letting people choose which region they'd like to browse the marketplace with?  This includes any rules or special cases we have for those regions, eg. If I'm in the US and change my region to Spain I'll be able to access apps which may not be available in the US (because of COPPA or whatever).
Flags: needinfo?(jmenon)
I think when a developer submits an app for a region, we are saying your app is in that region store, not that users are from that region.  And I fundamentally believe that restrictions, when necessary, should not user restrictions, but developer choices.

In the future, we should have a checkbox for developer submission that says something like 'restrictive' (or whatever).  This will mean that developers are choosing to only have their apps available to users within that region, probably as determined by IP (for desktop) or SIM.  This is a better route, to have developers restrict their content by choice, rather than restrict users.

That is, only apps are displayed to an end user where we can verify their location selected country and user location.  We know this isn't foolproof, nor is any system, but works for mere mortal users and is a best effort.
(In reply to David Bialer [:dbialer] from comment #6)
> I think when a developer submits an app for a region, we are saying your app
> is in that region store, not that users are from that region.  And I
> fundamentally believe that restrictions, when necessary, should not user
> restrictions, but developer choices.
> 
> In the future, we should have a checkbox for developer submission that says
> something like 'restrictive' (or whatever).  This will mean that developers
> are choosing to only have their apps available to users within that region,
> probably as determined by IP (for desktop) or SIM.  This is a better route,
> to have developers restrict their content by choice, rather than restrict
> users.
> 
> That is, only apps are displayed to an end user where we can verify their
> location selected country and user location.  We know this isn't foolproof,
> nor is any system, but works for mere mortal users and is a best effort.

Agreed. The scope of this bug is to enable a sensible default region for users when a default region has not been specified by the user's carrier.
Assignee: nobody → wraithan
Can we get traction on this bug? This is pretty important as Fireplace is the future.

I'd like to fix bug 865132 by getting this one fixed.
Flags: needinfo?(jmenon)
Assignee: wraithan → charmston
Landed: https://github.com/mozilla/zamboni/commit/332d43f72a5d269e372ea70395ae8b8c30c4fe8a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2013-04-25
What HTTP header does this set to pass the region back to the client?
It's the same header introduced in #861229: X-API-Filter, which reads from request.REGION. This commit introduces logic to more intelligently grab a default when a region hasn't been specified, per your note in comment 7.
You need to log in before you can comment on or make changes to this bug.