diff --git a/docs/topics/api/collections.rst b/docs/topics/api/collections.rst index 14d7fde79f..b3ff82f685 100644 --- a/docs/topics/api/collections.rst +++ b/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`. -.. 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 ` 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 ` to apply. :query string sort: The sort parameter. The available parameters are documented in the :ref:`table below `. @@ -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/ :`). @@ -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)/ :`). @@ -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)/ diff --git a/src/olympia/bandwagon/tests/test_views.py b/src/olympia/bandwagon/tests/test_views.py index 5b1f303af7..4b72a2b1fd 100644 --- a/src/olympia/bandwagon/tests/test_views.py +++ b/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 diff --git a/src/olympia/bandwagon/views.py b/src/olympia/bandwagon/views.py index a4fca6a9da..977c38adf5 100644 --- a/src/olympia/bandwagon/views.py +++ b/src/olympia/bandwagon/views.py @@ -667,17 +667,7 @@ class CollectionViewSet(ModelViewSet): AllOf(AllowReadOnlyIfPublic, PreventActionPermission('list'))), ] - lookup_url_kwarg = '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 + lookup_field = 'slug' def get_account_viewset(self): if not hasattr(self, 'account_viewset'):