Last Comment Bug 863775 - [fireplace] Create API middleware for geodude
: [fireplace] Create API middleware for geodude
Status: RESOLVED FIXED
[fireplace] p=1
:
Product: Marketplace
Classification: Server Software
Component: API (show other bugs)
: 1.0
: All All
: P1 normal (vote)
: 2013-04-25
Assigned To: Chuck Harmston [:chuck]
:
Mentors:
Depends on:
Blocks: 859511
  Show dependency treegraph
 
Reported: 2013-04-19 10:02 PDT by Matt Basta [:basta]
Modified: 2013-04-24 16:41 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Matt Basta [:basta] 2013-04-19 10:02:13 PDT
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.
Comment 1 Andy McKay [:andym] 2013-04-19 10:45:09 PDT
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?)
Comment 2 Matt Basta [:basta] 2013-04-19 10:48:01 PDT
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.
Comment 3 Andy McKay [:andym] 2013-04-19 10:50:54 PDT
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.
Comment 4 Andy McKay [:andym] 2013-04-19 10:54:02 PDT
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?
Comment 5 Wil Clouser [:clouserw] 2013-04-19 11:12:50 PDT
(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).
Comment 6 David Bialer [:dbialer] 2013-04-19 12:30:47 PDT
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.
Comment 7 Matt Basta [:basta] 2013-04-19 12:39:50 PDT
(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.
Comment 8 Christopher Van Wiemeersch [:cvan] 2013-04-24 10:34:42 PDT
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.
Comment 10 Matt Basta [:basta] 2013-04-24 16:39:29 PDT
What HTTP header does this set to pass the region back to the client?
Comment 11 Chuck Harmston [:chuck] 2013-04-24 16:41:26 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.