Merge pull request #3480 from diox/api-reviews-flag

Add API to flag a review

docs/topics/api/reviews.rst

@@ -61,7 +61,7 @@ This endpoint allows you to fetch a review by its id.
 
     :>json int id: The review id.
     :>json string|null body: The text of the review.
-    :>json string|null: The title of the review.
+    :>json string|null title: The title of the review.
     :>json int rating: The rating the user gave as part of the review.
     :>json object|null reply: The review object containing the developer reply to this review, if any (The fields ``rating``, ``reply`` and ``version`` are omitted).
     :>json string version: The add-on version string the review applies to.
@@ -73,7 +73,7 @@ This endpoint allows you to fetch a review by its id.
 Post
 ----
 
-.. review-list-addon:
+.. review-post:
 
 This endpoint allows you to post a new review for a given add-on and version.
 
@@ -84,7 +84,7 @@ This endpoint allows you to post a new review for a given add-on and version.
 .. http:post:: /api/v3/addons/addon/(int:id|string:slug|string:guid)/reviews/
 
     :<json string|null body: The text of the review.
-    :<json string|null: The title of the review.
+    :<json string|null title: The title of the review.
     :<json int rating: The rating the user wants to give as part of the review (required).
     :<json int version: The add-on version id the review applies to.
 
@@ -93,7 +93,7 @@ This endpoint allows you to post a new review for a given add-on and version.
 Edit
 ----
 
-.. review-edit-addon:
+.. review-edit:
 
 This endpoint allows you to edit an existing review by its id.
 
@@ -106,7 +106,7 @@ This endpoint allows you to edit an existing review by its id.
 .. http:patch:: /api/v3/addons/addon/(int:id|string:slug|string:guid)/reviews/(int:id)/
 
     :<json string|null body: The text of the review.
-    :<json string|null: The title of the review.
+    :<json string|null title: The title of the review.
     :<json int rating: The rating the user wants to give as part of the review.
 
 
@@ -114,7 +114,7 @@ This endpoint allows you to edit an existing review by its id.
 Delete
 ------
 
-.. review-delete-addon:
+.. review-delete:
 
 This endpoint allows you to delete an existing review by its id.
 
@@ -131,7 +131,7 @@ This endpoint allows you to delete an existing review by its id.
 Reply
 -----
 
-.. review-list-addon:
+.. review-reply:
 
 This endpoint allows you to reply to an existing user review.
 
@@ -142,4 +142,37 @@ This endpoint allows you to reply to an existing user review.
 .. http:post:: /api/v3/addons/addon/(int:id|string:slug|string:guid)/reviews/(int:id)/reply/
 
     :<json string body: The text of the reply (required).
-    :<json string|null: The title of the reply.
+    :<json string|null title: The title of the reply.
+
+
+----
+Flag
+----
+
+.. review-flag:
+
+This endpoint allows you to flag an existing user review, to let an editor know
+that something may be wrong with it.
+
+ .. note::
+     Requires authentication and a user account different from the one that
+     posted the review.
+
+.. http:post:: /api/v3/addons/addon/(int:id|string:slug|string:guid)/reviews/(int:id)/flag/
+
+    :<json string flag: A :ref:`constant<review-flag-constants>` describing the reason behind the flagging.
+    :<json string|null note: A note to explain further the reason behind the flagging.
+        This field is required if the flag is ``review_flag_reason_other``, and passing it will automatically change the flag to that value.
+
+.. _review-flag-constants:
+
+    Available constants for the ``flag`` property:
+
+    ===============================  ==========================================
+                          Constant    Description
+    ===============================  ==========================================
+            review_flag_reason_spam  Spam or otherwise non-review content
+        review_flag_reason_language  Inappropriate language/dialog
+     review_flag_reason_bug_support  Misplaced bug report or support request
+           review_flag_reason_other  Other (please specify)
+    ===============================  ==========================================

src/olympia/reviews/forms.py

@@ -90,6 +90,9 @@ class ReviewFlagForm(forms.ModelForm):
         data = super(ReviewFlagForm, self).clean()
         if 'note' in data and data['note'].strip():
             data['flag'] = ReviewFlag.OTHER
+        elif data.get('flag') == ReviewFlag.OTHER:
+            raise forms.ValidationError(
+                'A short explanation must be provided when selecting "Other".')
         return data
 
 

src/olympia/reviews/tests/test_views.py

@@ -176,6 +176,11 @@ class TestFlag(ReviewTest):
     def test_new_flag_mine(self):
         self.make_it_my_review()
         response = self.client.post(self.url, {'flag': ReviewFlag.SPAM})
+        assert response.status_code == 403
+
+    def test_flag_review_deleted(self):
+        Review.objects.get(pk=218468).delete()
+        response = self.client.post(self.url, {'flag': ReviewFlag.SPAM})
         assert response.status_code == 404
 
     def test_update_flag(self):
@@ -417,6 +422,13 @@ class TestEdit(ReviewTest):
                              X_REQUESTED_WITH='XMLHttpRequest')
         assert r.status_code == 403
 
+    def test_edit_deleted(self):
+        Review.objects.get(pk=218207).delete()
+        url = helpers.url('addons.reviews.edit', self.addon.slug, 218207)
+        response = self.client.post(url, {'rating': 2, 'body': 'woo woo'},
+                                    X_REQUESTED_WITH='XMLHttpRequest')
+        assert response.status_code == 404
+
     def test_edit_reply(self):
         self.login_dev()
         url = helpers.url('addons.reviews.edit', self.addon.slug, 218468)
@@ -443,13 +455,19 @@ class TestTranslate(ReviewTest):
                                             title='or', body='yes')
 
     def test_regular_call(self):
-        review = self.review
-        url = helpers.url('addons.reviews.translate', review.addon.slug,
-                          review.id, 'fr')
+        url = helpers.url('addons.reviews.translate', self.review.addon.slug,
+                          self.review.id, 'fr')
         r = self.client.get(url)
         assert r.status_code == 302
         assert r.get('Location') == 'https://translate.google.com/#auto/fr/yes'
 
+    def test_translate_deleted(self):
+        self.review.delete()
+        url = helpers.url('addons.reviews.translate', self.review.addon.slug,
+                          self.review.id, 'fr')
+        response = self.client.get(url)
+        assert response.status_code == 404
+
     def test_supports_dsb_hsb(self):
         # Make sure 3 character long locale codes resolve properly.
         for code in ('dsb', 'hsb'):
@@ -1346,6 +1364,138 @@ class TestReviewViewSetPost(TestCase):
             ' than once.']
 
 
+class TestReviewViewSetFlag(TestCase):
+    client_class = APITestClient
+
+    def setUp(self):
+        self.addon = addon_factory(
+            guid=generate_addon_guid(), name=u'My Addôn', slug='my-addon')
+        self.review_user = user_factory()
+        self.review = Review.objects.create(
+            addon=self.addon, version=self.addon.current_version, rating=1,
+            body='My review', user=self.review_user)
+        self.url = reverse(
+            'addon-review-flag',
+            kwargs={'addon_pk': self.addon.pk, 'pk': self.review.pk})
+
+    def test_flag_anonymous(self):
+        response = self.client.post(self.url)
+        assert response.status_code == 401
+        assert self.review.reload().editorreview is False
+
+    def test_flag_logged_in_no_flag_field(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(self.url)
+        assert response.status_code == 400
+        data = json.loads(response.content)
+        assert data['flag'] == [u'This field is required.']
+        assert self.review.reload().editorreview is False
+
+    def test_flag_logged_in(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam'})
+        assert response.status_code == 202
+        assert ReviewFlag.objects.count() == 1
+        flag = ReviewFlag.objects.latest('pk')
+        assert flag.flag == 'review_flag_reason_spam'
+        assert flag.user == self.user
+        assert flag.review == self.review
+        assert self.review.reload().editorreview is True
+
+    def test_flag_logged_in_with_note(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam',
+                            'note': u'This is my nøte.'})
+        assert response.status_code == 202
+        assert ReviewFlag.objects.count() == 1
+        flag = ReviewFlag.objects.latest('pk')
+        # Flag was changed automatically since a note is being posted.
+        assert flag.flag == 'review_flag_reason_other'
+        assert flag.user == self.user
+        assert flag.review == self.review
+        assert flag.note == u'This is my nøte.'
+        assert self.review.reload().editorreview is True
+
+    def test_flag_reason_other_without_notes_is_forbidden(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_other'})
+        assert response.status_code == 400
+        data = json.loads(response.content)
+        assert data['__all__'] == [
+            'A short explanation must be provided when selecting "Other".']
+
+    def test_flag_logged_in_unknown_flag_type(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'lol'})
+        assert response.status_code == 400
+        data = json.loads(response.content)
+        assert data['flag'] == [
+            'Select a valid choice. lol is not one of the available choices.']
+        assert self.review.reload().editorreview is False
+
+    def test_flag_logged_in_flag_already_exists(self):
+        other_user = user_factory()
+        other_flag = ReviewFlag.objects.create(
+            user=other_user, review=self.review,
+            flag='review_flag_reason_language')
+        self.user = user_factory()
+        flag = ReviewFlag.objects.create(
+            user=self.user, review=self.review,
+            flag='review_flag_reason_other')
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam'})
+        assert response.status_code == 202
+        # We should have re-used the existing flag posted by self.user, so the
+        # count should still be 2.
+        assert ReviewFlag.objects.count() == 2
+        flag.reload()
+        # Flag was changed from other to spam.
+        assert flag.flag == 'review_flag_reason_spam'
+        assert flag.user == self.user
+        assert flag.review == self.review
+        # Other flag was untouched.
+        other_flag.reload()
+        assert other_flag.user == other_user
+        assert other_flag.flag == 'review_flag_reason_language'
+        assert other_flag.review == self.review
+        assert self.review.reload().editorreview is True
+
+    def test_flag_logged_in_addon_denied(self):
+        self.addon.update(is_listed=False)
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam'})
+        assert response.status_code == 403
+        assert self.review.reload().editorreview is False
+
+    def test_flag_logged_in_no_such_review(self):
+        self.review.delete()
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam'})
+        assert response.status_code == 404
+        assert Review.unfiltered.get(pk=self.review.pk).editorreview is False
+
+    def test_flag_logged_in_review_author(self):
+        self.client.login_api(self.review_user)
+        response = self.client.post(
+            self.url, data={'flag': 'review_flag_reason_spam'})
+        assert response.status_code == 403
+        assert self.review.reload().editorreview is False
+
+
 class TestReviewViewSetReply(TestCase):
     client_class = APITestClient
 

src/olympia/reviews/views.py

@@ -125,7 +125,7 @@ def translate(request, addon, review_id, language):
     Use the Google Translate API for ajax, redirect to Google Translate for
     non ajax calls.
     """
-    review = get_object_or_404(Review, pk=review_id, addon=addon)
+    review = get_object_or_404(Review.objects, pk=review_id, addon=addon)
     if '-' in language:
         language = language.split('-')[0]
 
@@ -144,9 +144,9 @@ def translate(request, addon, review_id, language):
 @login_required(redirect=False)
 @json_view
 def flag(request, addon, review_id):
-    review = get_object_or_404(Review, pk=review_id, addon=addon)
+    review = get_object_or_404(Review.objects, pk=review_id, addon=addon)
     if review.user_id == request.user.id:
-        raise http.Http404()
+        raise PermissionDenied
     d = dict(review=review_id, user=request.user.id)
     try:
         instance = ReviewFlag.objects.get(**d)
@@ -160,7 +160,7 @@ def flag(request, addon, review_id):
         return {'msg': _('Thanks; this review has been flagged '
                          'for editor approval.')}
     else:
-        return json_view.error(unicode(form.errors))
+        return json_view.error(form.errors)
 
 
 @addon_view
@@ -268,7 +268,7 @@ def add(request, addon, template=None):
 @login_required(redirect=False)
 @post_required
 def edit(request, addon, review_id):
-    review = get_object_or_404(Review, pk=review_id, addon=addon)
+    review = get_object_or_404(Review.objects, pk=review_id, addon=addon)
     is_admin = acl.action_allowed(request, 'Addons', 'Edit')
     if not (request.user.id == review.user.id or is_admin):
         raise PermissionDenied
@@ -438,3 +438,17 @@ class ReviewViewSet(AddonChildMixin, ModelViewSet):
         # Call get_object() to trigger 404 if it does not exist.
         self.review_object = self.get_object()
         return self.create(*args, **kwargs)
+
+    @detail_route(methods=['post'])
+    def flag(self, request, *args, **kwargs):
+        # Re-use flag view since it's already returning json. We just need to
+        # pass it the addon slug (passing it the PK would result in a redirect)
+        # and make sure request.POST is set with whatever data was sent to the
+        # DRF view.
+        addon = self.get_addon_object()
+        request._request.POST = request.data
+        request = request._request
+        response = flag(request, addon.slug, kwargs.get('pk'))
+        if response.status_code == 200:
+            response.status_code = 202
+        return response