Custom routes do not accept slugs

RESOLVED FIXED in 2013-12-10

Status

P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: basta, Assigned: mat)

Tracking

2013-12-10
Points:
---

Details

(Reporter)

Description

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

The API 500s both with and without a token.
(Reporter)

Comment 1

5 years ago
Updating the summary with new information
Summary: Collection curators API returning 500 on -dev → Collection curators API returning 500 on -dev when a session cookie exists
Traceback: http://sentry.dmz.phx1.mozilla.com/addons/marketplace-dev/group/16752/
Summary: Collection curators API returning 500 on -dev when a session cookie exists → Collection curators API returning 500 on -dev
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.
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Summary: Collection curators API returning 500 on -dev → Custom routes do not accept slugs
(Reporter)

Comment 4

5 years ago
This won't mean changing *all* of the slugs to ids, just the curator ones...right?
Correct. That's the only place in Zamboni where we use custom routes.
(Assignee)

Comment 6

5 years ago
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 ?
Flags: needinfo?(mattbasta)
(Reporter)

Comment 7

5 years ago
When this bug was filed, I just switched over to IDs for everything that might have needed them.
Flags: needinfo?(mattbasta)
(Assignee)

Updated

5 years ago
Assignee: nobody → mpillard
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Target Milestone: --- → 2013-12-10
Note that bug 947210 may interfere with testing this bug.

Comment 10

5 years ago
I will verify when bug 947210 will be solved .
You need to log in before you can comment on or make changes to this bug.