918035 - Custom routes do not accept slugs

Status: RESOLVED FIXED

(Marketplace Graveyard :: API, defect, P3)

Description

[Matt Basta [:basta]]

https://marketplace-dev.allizom.org/api/v1/rocketfuel/collections/sssss/curators/

The API 500s both with and without a token.

Comment 1

[Matt Basta [:basta]]

Updating the summary with new information

Comment 2

[Chuck Harmston [:chuck]]

Traceback: http://sentry.dmz.phx1.mozilla.com/addons/marketplace-dev/group/16752/

Comment 3

[Chuck Harmston [:chuck]]

So, there are two problems here:

1) In the CollectionViewSet().curators() route, we don't accept slug as a keyword argument. This is a problem, as SlugOrIDMixin works by converting the `pk` keyword argument to a slug before it's handed off to a router. This is an easy fix.

2) We can't write a fix to verify this fix, as it would require us to reverse the slugged URL. Since :cvan nuked SlugRouter in favor of the vastly simpler SlugOrIDMixin, there are no URL patterns with the name slug, so it's not reversible.

My prescription:

1) For the interim, rocketfuel can use IDs instead of slugs.
2) We reinstate SlugRouter.
3) We ensure that all custom routes (e.g. those using @link or @action) accept the `slug` keyword, and write tests allowing slugs to be accepted.

This is a lower priority than all the basic collection-related work.

Comment 4

[Matt Basta [:basta]]

This won't mean changing *all* of the slugs to ids, just the curator ones...right?

Comment 5

[Chuck Harmston [:chuck]]

Correct. That's the only place in Zamboni where we use custom routes.

Comment 6

[Mathieu Pillard [:mat]]

Testing using cURL locally, I can't reproduce this. Did we fix it by accident ? :)

Basta: was rocketfuel ever changed to use IDs like chuck suggested? Does it work for you with slugs now ?

Comment 7

[Matt Basta [:basta]]

When this bug was filed, I just switched over to IDs for everything that might have needed them.

Comment 8

[Mathieu Pillard [:mat]]

Fixed in https://github.com/mozilla/zamboni/commit/9bd177fcbb3d8c4d647247b88d49090f529c5bad

STR:
- This affects all the collections APIs, so make sure curation tool still work correctly, including adding and removing apps, adding and removing curators.

Comment 9

[Chuck Harmston [:chuck]]

Note that bug 947210 may interfere with testing this bug.

Comment 10

[Victor Carciu]

I will verify when bug 947210 will be solved .