essentially revert #7728 - allow collection lookup by slug only (#8138)

docs/topics/api/collections.rst

@@ -33,13 +33,13 @@ Detail
 
 .. _collection-detail:
 
-This endpoint allows you to fetch a single collection by its ``id`` or ``slug``.
+This endpoint allows you to fetch a single collection by its ``slug``.
 It returns any ``public`` collection by the specified user. You can access
 a non-``public`` collection only if it was authored by you, the authenticated user.
 If your account has the `Collections:Edit` permission then you can access any collection.
 
 
-.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/
+.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/
 
     .. _collection-detail-object:
 
@@ -65,7 +65,7 @@ Filtering is as per :ref:`collection addon list endpoint<collection-addon-filter
 Additional add-ons can be returned from the :ref:`Collection Add-on list endpoint<collection-addon-list>`.
 
 
-.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/?with_addons
+.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/?with_addons
 
     .. _collection-detail-object-with-addons:
 
@@ -113,7 +113,7 @@ Edit
 This endpoint allows some of the details for a collection to be updated.  Any fields
 in the :ref:`collection <collection-detail-object>` but not listed below are not editable and will be ignored in the patch request.
 
-.. http:patch:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/
+.. http:patch:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/
 
     .. _collection-edit-request:
 
@@ -136,7 +136,7 @@ Delete
 
 This endpoint allows the collection to be deleted.
 
-.. http:delete:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/
+.. http:delete:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/
 
 
 
@@ -148,7 +148,7 @@ Collection Add-ons List
 
 This endpoint lists the add-ons in a collection, together with collector's notes.
 
-.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/addons/
+.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/addons/
 
     :query string filter: The :ref:`filter <collection-addon-filtering-param>` to apply.
     :query string sort: The sort parameter. The available parameters are documented in the :ref:`table below <collection-addon-list-sort>`.
@@ -199,7 +199,7 @@ Collection Add-ons Detail
 
 This endpoint gets details of a single add-on in a collection, together with collector's notes.
 
-.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/addons/(int:addon_id|string:slug)/
+.. http:get:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/addons/(int:addon_id|string:slug)/
 
     .. _collection-addon-detail-object:
 
@@ -220,7 +220,7 @@ Collection Add-ons Create
 
 This endpoint allows a single add-on to be added to a collection, optionally with collector's notes.
 
-.. http:post:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/addons/
+.. http:post:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/addons/
 
     :<json string addon: The add-on id or slug to be added (required).
     :<json string|object|null notes: The collectors notes for this item. (See :ref:`translated fields <api-overview-translations>`).
@@ -238,7 +238,7 @@ Collection Add-ons Edit
 
 This endpoint allows the collector's notes for single add-on to be updated.
 
-.. http:patch:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/addons/(int:addon_id|string:slug)/
+.. http:patch:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/addons/(int:addon_id|string:slug)/
 
     :<json string|object|null notes: The collectors notes for this item. (See :ref:`translated fields <api-overview-translations>`).
 
@@ -255,4 +255,4 @@ Collection Add-ons Delete
 
 This endpoint allows a single add-on to be removed from a collection.
 
-.. http:delete:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(int:id|string:collection_slug)/addons/(int:addon_id|string:slug)/
+.. http:delete:: /api/v3/accounts/account/(int:user_id|string:username)/collections/(string:collection_slug)/addons/(int:addon_id|string:slug)/

src/olympia/bandwagon/tests/test_views.py

@@ -1410,6 +1410,20 @@ class TestCollectionViewSetDetail(TestCase):
         assert response.status_code == 200
         assert response.data['id'] == self.collection.id
 
+    def test_no_id_lookup(self):
+        collection = collection_factory(author=self.user, slug='999')
+        id_url = reverse(
+            'collection-detail', kwargs={
+                'user_pk': self.user.pk, 'slug': collection.id})
+        response = self.client.get(id_url)
+        assert response.status_code == 404
+        slug_url = reverse(
+            'collection-detail', kwargs={
+                'user_pk': self.user.pk, 'slug': collection.slug})
+        response = self.client.get(slug_url)
+        assert response.status_code == 200
+        assert response.data['id'] == collection.id
+
     def test_not_listed(self):
         self.collection.update(listed=False)
 
@@ -1513,25 +1527,6 @@ class TestCollectionViewSetDetail(TestCase):
             unicode(addon.support_url))
 
 
-class TestCollectionViewSetDetailWithId(TestCollectionViewSetDetail):
-    def _get_url(self, user, collection):
-        return reverse(
-            'collection-detail', kwargs={
-                'user_pk': user.pk, 'slug': collection.id})
-
-    def test_404(self):
-        # Invalid user.
-        response = self.client.get(reverse(
-            'collection-detail', kwargs={
-                'user_pk': self.user.pk + 66, 'slug': self.collection.id}))
-        assert response.status_code == 404
-        # Invalid collection.
-        response = self.client.get(reverse(
-            'collection-detail', kwargs={
-                'user_pk': self.user.pk, 'slug': '123456'}))
-        assert response.status_code == 404
-
-
 class CollectionViewSetDataMixin(object):
     client_class = APITestClient
     data = {
@@ -1685,6 +1680,18 @@ class TestCollectionViewSetCreate(CollectionViewSetDataMixin, TestCase):
         response = self.send(url=url)
         assert response.status_code == 403
 
+    def test_create_numeric_slug(self):
+        self.client.login_api(self.user)
+        data = {
+            'name': u'this',
+            'slug': u'1',
+        }
+        response = self.send(data=data)
+        assert response.status_code == 201, response.content
+        collection = Collection.objects.get()
+        assert collection.name == data['name']
+        assert collection.slug == data['slug']
+
 
 class TestCollectionViewSetPatch(CollectionViewSetDataMixin, TestCase):
 
@@ -1749,13 +1756,6 @@ class TestCollectionViewSetPatch(CollectionViewSetDataMixin, TestCase):
         assert response.status_code == 403
 
 
-class TestCollectionViewSetPatchWithId(TestCollectionViewSetPatch):
-    def get_url(self, user):
-        return reverse(
-            'collection-detail', kwargs={
-                'user_pk': user.pk, 'slug': self.collection.id})
-
-
 class TestCollectionViewSetDelete(TestCase):
     client_class = APITestClient
 
@@ -1813,13 +1813,6 @@ class TestCollectionViewSetDelete(TestCase):
         assert response.status_code == 403
 
 
-class TestCollectionViewSetDeleteWithId(TestCollectionViewSetDelete):
-    def get_url(self, user):
-        return reverse(
-            'collection-detail', kwargs={
-                'user_pk': user.pk, 'slug': self.collection.id})
-
-
 class CollectionAddonViewSetMixin(object):
     def check_response(self, response):
         raise NotImplementedError

src/olympia/bandwagon/views.py

@@ -667,17 +667,7 @@ class CollectionViewSet(ModelViewSet):
             AllOf(AllowReadOnlyIfPublic,
                   PreventActionPermission('list'))),
     ]
-    lookup_url_kwarg = 'slug'
+    lookup_field = 'slug'
-
-    @property
-    def lookup_field(self):
-        identifier = self.kwargs.get(self.lookup_url_kwarg)
-        if identifier and identifier.isdigit():
-            lookup_field = 'pk'
-        else:
-            # If the identifier is anything other than a digit, it's the slug.
-            lookup_field = 'slug'
-        return lookup_field
 
     def get_account_viewset(self):
         if not hasattr(self, 'account_viewset'):