From cae436ffb0cd9697fc75ca309756095e4933565b Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Tue, 15 Oct 2013 14:40:51 +0200 Subject: [PATCH] Allow rocketfuel add/remove_curator API to accept email addresses (bug 924020) --- docs/api/topics/rocketfuel.rst | 10 +++-- mkt/collections/tests/test_views.py | 57 +++++++++++++++++++++++++---- mkt/collections/views.py | 24 +++++++++--- 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/docs/api/topics/rocketfuel.rst b/docs/api/topics/rocketfuel.rst index 98a5ced66c..e3e22aded2 100644 --- a/docs/api/topics/rocketfuel.rst +++ b/docs/api/topics/rocketfuel.rst @@ -397,8 +397,9 @@ Add Curator **Request**: - :param user: the ID of the user to add as a curator of this collection. - :type user: int + :param user: the ID or email of the user to add as a curator of this + collection. + :type user: int|string **Response**: @@ -423,8 +424,9 @@ Remove Curator **Request**: - :param user: the ID of the user to add as a curator of this collection. - :type user: int + :param user: the ID or email of the user to remove as a curator of this + collection. + :type user: int|string **Response**: diff --git a/mkt/collections/tests/test_views.py b/mkt/collections/tests/test_views.py index 7e5613eae8..8eb277dceb 100644 --- a/mkt/collections/tests/test_views.py +++ b/mkt/collections/tests/test_views.py @@ -56,6 +56,7 @@ class BaseCollectionViewSetTest(RestOAuth): self.collection = Collection.objects.create(**self.collection_data) self.apps = [] self.list_url = reverse('collections-list') + self.user = UserProfile.objects.get(pk=2519) self.user2 = UserProfile.objects.get(pk=999) def setup_unique(self): @@ -205,7 +206,7 @@ class TestCollectionViewSetListing(BaseCollectionViewSetTest): self.create_apps() self.add_apps_to_collection(*self.apps) self.collection.update(is_public=False) - self.collection.curators.add(self.user.get_profile()) + self.collection.curators.add(self.user) res = self.client.get(self.list_url) data = json.loads(res.content) eq_(res.status_code, 200) @@ -806,8 +807,8 @@ class TestCollectionViewSetDuplicate(BaseCollectionViewSetTest): class CollectionViewSetChangeAppsMixin(BaseCollectionViewSetTest): """ - Mixin containing common methods to actions that modify the apps belonging to - a collection. + Mixin containing common methods to actions that modify the apps belonging + to a collection. """ def add_app(self, client, app_id=None): if app_id is None: @@ -1330,7 +1331,7 @@ class TestCollectionViewSetAddCurator(BaseCollectionViewSetTest): """ def add_curator(self, client, user_id=None): if user_id is None: - user_id = 2519 + user_id = self.user.pk form_data = {'user': user_id} if user_id else {} url = self.collection_url('add-curator', self.collection.pk) res = client.post(url, json.dumps(form_data)) @@ -1372,12 +1373,32 @@ class TestCollectionViewSetAddCurator(BaseCollectionViewSetTest): eq_(res.status_code, 400) eq_(CollectionViewSet.exceptions['user_doesnt_exist'], data['detail']) + res, data = self.add_curator(self.client, user_id='doesnt@exi.st') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['user_doesnt_exist'], data['detail']) + def test_add_curator_empty(self): self.make_publisher() res, data = self.add_curator(self.client, user_id=False) eq_(res.status_code, 400) eq_(CollectionViewSet.exceptions['user_not_provided'], data['detail']) + def test_add_curator_email(self): + self.make_curator() + res, data = self.add_curator(self.client, user_id=self.user.email) + eq_(res.status_code, 200) + eq_(data[0]['id'], self.user.pk) + + def test_add_curator_garbage(self): + self.make_publisher() + res, data = self.add_curator(self.client, user_id='garbage') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['wrong_user_format'], data['detail']) + + res, data = self.add_curator(self.client, user_id='garbage@') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['wrong_user_format'], data['detail']) + class TestCollectionViewSetRemoveCurator(BaseCollectionViewSetTest): """ @@ -1385,7 +1406,7 @@ class TestCollectionViewSetRemoveCurator(BaseCollectionViewSetTest): """ def remove_curator(self, client, user_id=None): if user_id is None: - user_id = 2519 + user_id = self.user.pk form_data = {'user': user_id} if user_id else {} url = self.collection_url('remove-curator', self.collection.pk) res = client.post(url, json.dumps(form_data)) @@ -1412,18 +1433,37 @@ class TestCollectionViewSetRemoveCurator(BaseCollectionViewSetTest): res, data = self.remove_curator(self.client) eq_(res.status_code, 205) + def test_remove_curator_email(self): + self.make_curator() + res, data = self.remove_curator(self.client, user_id=self.user.email) + eq_(res.status_code, 205) + def test_remove_curator_nonexistent(self): self.make_publisher() res, data = self.remove_curator(self.client, user_id=100000) eq_(res.status_code, 400) eq_(CollectionViewSet.exceptions['user_doesnt_exist'], data['detail']) + res, data = self.remove_curator(self.client, user_id='doesnt@exi.st') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['user_doesnt_exist'], data['detail']) + def test_remove_curator_empty(self): self.make_publisher() res, data = self.remove_curator(self.client, user_id=False) eq_(res.status_code, 400) eq_(CollectionViewSet.exceptions['user_not_provided'], data['detail']) + def test_remove_curator_garbage(self): + self.make_publisher() + res, data = self.remove_curator(self.client, user_id='garbage') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['wrong_user_format'], data['detail']) + + res, data = self.remove_curator(self.client, user_id='garbage@') + eq_(res.status_code, 400) + eq_(CollectionViewSet.exceptions['wrong_user_format'], data['detail']) + class TestCollectionImageViewSet(RestOAuth): def setUp(self): @@ -1433,9 +1473,10 @@ class TestCollectionImageViewSet(RestOAuth): **CollectionDataMixin.collection_data) self.url = reverse('collection-image-detail', kwargs={'pk': self.collection.pk}) - self.img = ('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAAA1BMVEUAAA' - 'Cnej3aAAAAAXRSTlMAQObYZgAAAApJREFUCNdjYAAAAAIAAeIhvDMAAAAA' - 'SUVORK5CYII=').decode('base64') + self.img = ( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAAA1BMVEUAAA' + 'Cnej3aAAAAAXRSTlMAQObYZgAAAApJREFUCNdjYAAAAAIAAeIhvDMAAAAA' + 'SUVORK5CYII=').decode('base64') def add_img(self): path = self.collection.image_path() diff --git a/mkt/collections/views.py b/mkt/collections/views.py index 884967ba70..44bc21f79d 100644 --- a/mkt/collections/views.py +++ b/mkt/collections/views.py @@ -1,5 +1,6 @@ from django.core.exceptions import ValidationError from django.core.files.storage import default_storage as storage +from django.core.validators import validate_email from django.db import IntegrityError from django.db.models import Q from django.http import Http404 @@ -44,6 +45,7 @@ class CollectionViewSet(CORSMixin, SlugOrIdMixin, viewsets.ModelViewSet): exceptions = { 'not_provided': '`app` was not provided.', 'user_not_provided': '`user` was not provided.', + 'wrong_user_format': '`user` must be an ID or email.', 'doesnt_exist': '`app` does not exist.', 'user_doesnt_exist': '`user` does not exist.', 'not_in': '`app` not in collection.', @@ -202,17 +204,27 @@ class CollectionViewSet(CORSMixin, SlugOrIdMixin, viewsets.ModelViewSet): }, status=status.HTTP_400_BAD_REQUEST, exception=True) return self.return_updated(status.HTTP_200_OK) - def serialized_curators(self): - return Response([CuratorSerializer(instance=c).data for c in - self.get_object().curators.no_cache().all()]) + def serialized_curators(self, no_cache=False): + queryset = self.get_object().curators.all() + if no_cache: + queryset = queryset.no_cache() + return Response([CuratorSerializer(instance=c).data for c in queryset]) def get_curator(self, request): try: - return UserProfile.objects.get(pk=request.DATA['user']) + userdata = request.DATA['user'] + if (isinstance(userdata, int) or isinstance(userdata, basestring) + and userdata.isdigit()): + return UserProfile.objects.get(pk=userdata) + else: + validate_email(userdata) + return UserProfile.objects.get(email=userdata) except (KeyError, MultiValueDictKeyError): raise ParseError(detail=self.exceptions['user_not_provided']) except UserProfile.DoesNotExist: raise ParseError(detail=self.exceptions['user_doesnt_exist']) + except ValidationError: + raise ParseError(detail=self.exceptions['wrong_user_format']) @link(permission_classes=[StrictCuratorAuthorization]) def curators(self, request, pk=None): @@ -221,14 +233,14 @@ class CollectionViewSet(CORSMixin, SlugOrIdMixin, viewsets.ModelViewSet): @action(methods=['POST']) def add_curator(self, request, pk=None): self.get_object().add_curator(self.get_curator(request)) - return self.serialized_curators() + return self.serialized_curators(no_cache=True) @action(methods=['POST']) def remove_curator(self, request, pk=None): removed = self.get_object().remove_curator(self.get_curator(request)) if not removed: return Response(status=status.HTTP_205_RESET_CONTENT) - return self.serialized_curators() + return self.serialized_curators(no_cache=True) class CollectionImageViewSet(CORSMixin, viewsets.ViewSet,