918499 - Collections API should be able to return all possible fallbacks for a given filter combination

Status: RESOLVED WONTFIX

(Marketplace Graveyard :: API, defect, P3)

Description

[Matt Basta [:basta]]

https://marketplace-dev.allizom.org/api/v1/rocketfuel/collections/cache-rewriting-test/?carrier&cat&region=worldwide
https://marketplace-dev.allizom.org/api/v1/rocketfuel/collections/cache-rewriting-test/

Since DRF is being silly and applying filtering to lone objects, I'll use that to demonstrate my point.

The first URL should not 404, conceivably, since the collection's region is NULL. The fallback order should allow that to be matched:

region+carrier+category
NULL+carrier+category
region+NULL+category
NULL+NULL+category

The collection has NULL for its region, so the first URL is matching at the third line, which should then fall back to the fourth.

Is there something I'm missing?

Comment 1

[Mathieu Pillard [:mat]]

It looks like a problem with region=worldwide specifically : passing any other region works. Not sure what's going on yet, investigating.

Comment 2

[Mathieu Pillard [:mat]]

Spoke too soon, /some/ regions works and some don't. Definitely something to do with the fallback, but I'm not sure what yet.

Comment 3

[Mathieu Pillard [:mat]]

Ok what happens is, the fallback filtering is done *before* DRF tries to filter on pk/slug. So, in your case:
- The filtering code tries carrier=NULL, cat=NULL, region=worldwide per the querystring.
- It finds some results, so it never applies the fallback
- Then, later, when it uses that queryset to find your slug, it fails, and returns a 404

IMHO we should just fix 915801 to prevent DRF from applying filters to pk/slug lookups and be done with it.

Comment 4

[Matt Basta [:basta]]

This does also happen if you don't request a specific collection. If you request /rocketfuel/collections/ with the query string parameters of the original URL in comment #0, you'll notice that cache-rewriting-test doesn't show up.

Comment 5

[Mathieu Pillard [:mat]]

It makes sense that it doesn't show up, since other collections are found, the fallback doesn't need to be used and never kicks in.

Comment 6

[Matt Basta [:basta]]

Is there any way--for the /rocketfuel APIs--to have it return everything?

Comment 7

[Matt Basta [:basta]]

(In reply to Matt Basta [:basta] from comment #6)
> Is there any way--for the /rocketfuel APIs--to have it return everything?

To provide more details, we want the curators to be able to see that there are fallbacks available beyond the ones that we're displaying now. So right now we might show a single collection that exactly matches the filters, but the user should be able to see that the one filter "overrides" maybe three or four other collections that are fallbacks.

Comment 8

[Mathieu Pillard [:mat]]

mmm, I'm not sure what the best way to do this. We could have a special parameter allowing you to show *all* results from your main query + all possible fallbacks (using Q() objects and | operator), but it'd be tricky to limit it in clever way. Would that work for you ? Alternatively, I guess you could do multiple API queries to do this manually from the client.

Comment 9

[Chuck Harmston [:chuck]]

What if, instead of returning lots of results, rocketfuel told the user why the current collection/s are chosen?

"These collections are shown because they match <Carrier> in <Country>."

Comment 10

[Matt Basta [:basta]]

(In reply to Chuck Harmston [:chuck] from comment #9)
> What if, instead of returning lots of results, rocketfuel told the user why
> the current collection/s are chosen?
> 
> "These collections are shown because they match <Carrier> in <Country>."

That's along the lines of what we're doing, but we want to be able to say, "Here's the collections that match exactly, and here's the collections that would match if the exactly matching ones weren't there."

(In reply to Mathieu Pillard [:mat] from comment #8)
> mmm, I'm not sure what the best way to do this. We could have a special
> parameter allowing you to show *all* results from your main query + all
> possible fallbacks (using Q() objects and | operator), but it'd be tricky to
> limit it in clever way. Would that work for you ? Alternatively, I guess you
> could do multiple API queries to do this manually from the client.

My plan A was to have multiple requests, but it gets complicated if the collections are already fallbacks.

A query parameter to return all would be great, though.

Comment 11

[krupa raj[:krupa]]

Adding this note since the current weird behavior I'm seeing has been attributed to this bug.

I featured this collection for region = US:  https://marketplace.allizom.org/curation/collection/krupas-ultimate-featured-colle

However, I see apps from https://marketplace.allizom.org/curation/collection/featured-apps (region = --) being listed in the homepage

Expected behavior would be that apps featured in that region get precedence.

Comment 12

[Christopher Van Wiemeersch [:cvan]]

Turns out the US collection wasn't public. This is working as expected.

Comment 13

[Mathieu Pillard [:mat]]

I have code for this but no tests yet : https://github.com/diox/zamboni/commit/467f1a45573970921125f240725a4787152a007f

I'm caught up in other stuff atm so I'm downgrading priority but I'll revisit this when I have time.

Comment 14

[Mathieu Pillard [:mat]]

Wontfixing this.

- Feed will make this obsolete.
- The rocketfuel endpoints are already pretty slow because they bring ton of data.
- I coded it, but it's pain to test and would likely be a pain to maintain as well.
- With this idea we'd return tons of collections the user doesn't need / it'd be annoying to show the user which collections come from the possible fallbacks and which come from the real filter combination.