875495 - [fireplace] Desktop-only apps can be installed on mobile

Status: VERIFIED FIXED

(Marketplace Graveyard :: Consumer Pages, defect, P1)

Description

[Lisa Brewster [:adora]]

I spot checked the following desktop only apps and found them all via search on Firefox OS:

https://marketplace.firefox.com/reviewers/apps/review/evernote-web
https://marketplace.firefox.com/reviewers/apps/review/wger
https://marketplace.firefox.com/reviewers/apps/review/darf

Comment 1

[Thomas Elin [:relaas]]

We need action on this asap. Will cause partner frustration and could promote negative app reviews from users.

Comment 2

[Christopher Van Wiemeersch [:cvan]]

This looks like a bug in our API:

https://marketplace-dev.allizom.org/api/v1/apps/search/?dev=firefoxos&lang=en-US&pro=3040fe.32.1&q=evernote&region=es

Comment 3

[Christopher Van Wiemeersch [:cvan]]

So, interestingly, `Webapp.from_search()` does not filter by device: https://github.com/mozilla/zamboni/blob/master/mkt/webapps/models.py#L635

(Which is bug 869573.)

The API calls `_filter_search` (https://github.com/mozilla/zamboni/blob/master/mkt/search/views.py#L102) but then we use the `device` querystring paramater (not `dev`): https://github.com/mozilla/zamboni/blob/master/mkt/search/forms.py#L151

So that when `DeviceDetectionMiddleware` sets `request.GAIA` (https://github.com/mozilla/zamboni/blob/master/mkt/site/middleware.py#L187) the API literally does not care. That's the problem.

We could fix this in Fireplace or in the Zamboni API - what do people think?

Comment 4

[Rob Hudson [:robhudson]]

Awesome breakdown, cvan.

I think what's happening is this:
Fireplace includes the dev= on all API calls, not just search and that's picked up in the middleware to set the various attributes on the request object that's passed around.

In other words, ?dev= isn't intended to filter the search.

If we only want to see apps that work on the device in use when not on desktop we need to add that to the query string as `?device=` for whatever the device is currently. For the search API this means we'll have 2 devices in the query string: ?dev={1}&device={2} where {1} is the device as used in middleware and {2} is the device as documented in the search API already:
http://firefox-marketplace-api.readthedocs.org/en/latest/topics/search.html

Changing one will break the other and I don't think they should be tied together as they are used in different ways.

Moving to the fireplace component to add this when querying the API and not on desktop.

Comment 5

[Rob Hudson [:robhudson]]

I'm happy to change the API to change the value from ?device=gaia to ?device=firefoxos if it helps Fireplace and consistency.

Comment 6

[Andrew Williamson [:eviljeff]]

Btw, its not just that incompatible apps are discoverable via search (which they shouldn't).  Its also that they aren't identified as being incompatible on the listing page like they used to be.

And its not just Desktop on Mobile, its Mobile on Desktop too.

Comment 7

[Andy McKay]

Other places we use the word device:

* app submission- http://firefox-marketplace-api.readthedocs.org/en/latest/topics/apps.html#put--api-v1-apps-app-%28int-id%29- (device-types)
* app reading: http://firefox-marketplace-api.readthedocs.org/en/latest/topics/apps.html#get--api-v1-apps-app-%28int-id%29|%28string-slug%29- (device-types)

I know all this is going away but being consistentish would be nice.

Comment 8

[Rob Hudson [:robhudson]]

Ok. I'll definitely change the search API to use firefoxos instead of gaia then. Thanks Andy.

Comment 9

[Rob Hudson [:robhudson]]

(In reply to Rob Hudson [:robhudson] from comment #8)
> Ok. I'll definitely change the search API to use firefoxos instead of gaia
> then. Thanks Andy.

r? for renaming gaia to firefox in API: https://github.com/robhudson/zamboni/commit/f20e0e0

Comment 10

[Wil Clouser [:clouserw]]

Let's fix this in fireplace by sending tablet/mobile for now.  Feel free to file another bug to back this out which is dependent on the buchet stuff, but we should fix this for now.

Comment 11

[Rob Hudson [:robhudson]]

https://github.com/mozilla/zamboni/commit/72cb6f5 for consistently naming "firefoxos" on the api side.

Comment 12

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/fireplace/commit/c760e1b

Thanks, Rob!

P.S. This is an interim solution until buchets land.

Comment 13

[krupa raj[:krupa]]

desktop-only apps don't show up in Firefox OS anymore. Verified with apps listed in comment 0 and a newly submitted app.

Note that mobile-only apps should still show up in desktop since desktop is mainly for discovery purpose. However, the install buttons should be disabled (bug 877494)