Add API allowing developers and admins to reply to add-on reviews

docs/topics/api/reviews.rst

@@ -55,7 +55,7 @@ This endpoint allows you to fetch a review by its id.
     :>json string|null body: The text of the review.
     :>json string|null: 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`` and ``reply`` are omitted).
+    :>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.
     :>json object user: Object holding information about the user who posted the review.
     :>json string user.url: The user profile URL.
@@ -69,9 +69,31 @@ Post
 
 This endpoint allows you to post a new review for a given add-on and version.
 
+ .. note::
+     Requires authentication.
+
+
 .. 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 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.
+
+
+-----
+Reply
+-----
+
+.. review-list-addon:
+
+This endpoint allows you to reply to an existing user review.
+
+ .. note::
+     Requires authentication and either Addons:Edit permission or a user account
+     listed as a developer of the add-on.
+
+.. 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.

src/olympia/api/permissions.py

@@ -183,8 +183,17 @@ class ByHttpMethod(BasePermission):
 
 
 class AllowRelatedObjectPermissions(BasePermission):
+    """
+    Permission class that tests given permissions against a related object.
+
+    The first argument, `related_property`, is the property that will be used
+    to find the related object to test the permissions against.
+
+    The second argument, `related_permissions`, is the list of permissions
+    (behaving like DRF default implementation: all need to pass to be allowed).
+    """
     def __init__(self, related_property, related_permissions):
-        self.perms = related_permissions
+        self.perms = [p() for p in related_permissions]
         self.related_property = related_property
 
     def has_permission(self, request, view):

src/olympia/api/tests/test_permissions.py

@@ -11,8 +11,9 @@ from olympia.access.models import Group, GroupUser
 from olympia.addons.models import Addon
 from olympia.api.permissions import (
     AllowAddonAuthor, AllowNone, AllowOwner, AllowReadOnlyIfReviewedAndListed,
-    AllowReviewer, AllowReviewerUnlisted, AnyOf, ByHttpMethod, GroupPermission)
-from olympia.amo.tests import TestCase, WithDynamicEndpoints
+    AllowRelatedObjectPermissions, AllowReviewer, AllowReviewerUnlisted, AnyOf,
+    ByHttpMethod, GroupPermission)
+from olympia.amo.tests import TestCase, user_factory, WithDynamicEndpoints
 from olympia.users.models import UserProfile
 
 
@@ -431,3 +432,30 @@ class TestByHttpMethod(TestCase):
 
         self.request = RequestFactory().options('/')
         assert self.permission.has_permission(self.request, 'myview') is False
+
+
+class TestAllowRelatedObjectPermissions(TestCase):
+    def setUp(self):
+        self.permission = AllowRelatedObjectPermissions(
+            'test_property', [AllowOwner, AllowAny])
+        self.allowed_user = user_factory()
+        self.related_obj = Mock(user=self.allowed_user)
+        self.obj = Mock(test_property=self.related_obj)
+        self.request = RequestFactory().post('/')
+        self.request.user = self.allowed_user
+
+    def test_all_must_pass(self):
+        assert self.permission.has_permission(
+            self.request, 'myview') is True
+
+        self.request.user = AnonymousUser()
+        assert self.permission.has_permission(
+            self.request, 'myview') is False
+
+    def test_all_must_pass_object(self):
+        assert self.permission.has_object_permission(
+            self.request, 'myview', self.obj) is True
+
+        self.request.user = AnonymousUser()
+        assert self.permission.has_permission(
+            self.request, 'myview') is False

src/olympia/reviews/serializers.py

@@ -15,16 +15,38 @@ class BaseReviewSerializer(serializers.ModelSerializer):
     title = serializers.CharField(allow_null=True, required=False)
     user = BaseUserSerializer(read_only=True)
 
+    class Meta:
+        model = Review
+        fields = ('id', 'body', 'title', 'user')
+
+    def validate(self, data):
+        data = super(BaseReviewSerializer, self).validate(data)
+        # Get the add-on pk from the URL, no need to pass it as POST data since
+        # the URL is always going to have it.
+        data['addon'] = self.context['view'].get_addon_object()
+
+        # Get the user from the request, don't allow clients to pick one
+        # themselves.
+        data['user'] = self.context['request'].user
+
+        return data
+
+
+class ReviewSerializer(BaseReviewSerializer):
+    reply = BaseReviewSerializer(read_only=True)
+    rating = serializers.IntegerField(min_value=1, max_value=5)
+
     # Version queryset is unfiltered, the version is checked more thoroughly
     # in the validate() method.
     version = PrimaryKeyRelatedField(queryset=Version.unfiltered)
 
     class Meta:
         model = Review
-        fields = ('id', 'body', 'title', 'version', 'user')
+        fields = BaseReviewSerializer.Meta.fields + (
+            'rating', 'reply', 'version')
 
     def to_representation(self, obj):
-        data = super(BaseReviewSerializer, self).to_representation(obj)
+        data = super(ReviewSerializer, self).to_representation(obj)
         # For the version, we want to accept PKs for input, but use the version
         # string for output.
         data['version'] = unicode(obj.version.version) if obj.version else None
@@ -39,15 +61,7 @@ class BaseReviewSerializer(serializers.ModelSerializer):
         return version
 
     def validate(self, data):
-        data = super(BaseReviewSerializer, self).validate(data)
-        # Get the add-on pk from the URL, no need to pass it as POST data since
-        # the URL is always going to have it.
-        data['addon'] = self.context['view'].get_addon_object()
-
-        # Get the user from the request, don't allow clients to pick one
-        # themselves.
-        data['user'] = self.context['request'].user
-
+        data = super(ReviewSerializer, self).validate(data)
         if data['addon'].authors.filter(pk=data['user'].pk).exists():
             raise serializers.ValidationError(
                 'An add-on author can not leave a review on its own add-on.')
@@ -58,14 +72,23 @@ class BaseReviewSerializer(serializers.ModelSerializer):
             raise serializers.ValidationError(
                 'The same user can not leave a review on the same version more'
                 ' than once.')
-
         return data
 
 
-class ReviewSerializer(BaseReviewSerializer):
-    reply = BaseReviewSerializer(read_only=True)
-    rating = serializers.IntegerField(min_value=1, max_value=5)
+class ReviewSerializerReply(BaseReviewSerializer):
+    """Serializer used when posting a developer reply (write-only)."""
+    body = serializers.CharField(
+        allow_null=False, required=True, allow_blank=False)
 
-    class Meta:
-        model = Review
-        fields = BaseReviewSerializer.Meta.fields + ('rating', 'reply')
+    def validate(self, data):
+        # review_object is set on the view by the reply() method.
+        data['reply_to'] = self.context['view'].review_object
+
+        if data['reply_to'].reply_to:
+            # Only one level of replying is allowed, so if it's already a
+            # reply, we shouldn't allow that.
+            raise serializers.ValidationError(
+                'Can not reply to a review that is already a reply.')
+
+        data = super(ReviewSerializerReply, self).validate(data)
+        return data

src/olympia/reviews/tests/test_serializers.py

@@ -61,4 +61,3 @@ class TestBaseReviewSerializer(TestCase):
             'name': unicode(reply_user.name),
             'url': absolutify(reply_user.get_url_path()),
         }
-        assert result['reply']['version'] == reply.version.version

src/olympia/reviews/tests/test_views.py

@@ -1010,3 +1010,117 @@ class TestReviewViewSetPost(TestCase):
         assert response.data['non_field_errors'] == [
             'The same user can not leave a review on the same version more'
             ' than once.']
+
+
+class TestReviewViewSetReply(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-reply',
+            kwargs={'addon_pk': self.addon.pk, 'pk': self.review.pk})
+
+    def test_url(self):
+        expected_url = '/api/v3/addons/addon/%d/reviews/%d/reply/' % (
+            self.addon.pk, self.review.pk)
+        assert self.url == expected_url
+
+    def test_get_method_not_allowed(self):
+        self.addon_author = user_factory()
+        self.addon.addonuser_set.create(user=self.addon_author)
+        self.client.login_api(self.addon_author)
+        response = self.client.get(self.url)
+        assert response.status_code == 405
+
+    def test_reply_anonymous(self):
+        response = self.client.post(self.url, data={})
+        assert response.status_code == 401
+
+    def test_reply_non_addon_author(self):
+        self.client.login_api(self.review_user)
+        response = self.client.post(self.url, data={})
+        assert response.status_code == 403
+
+    def test_reply_no_such_review(self):
+        self.addon_author = user_factory()
+        self.addon.addonuser_set.create(user=self.addon_author)
+        self.client.login_api(self.addon_author)
+        self.url = reverse(
+            'addon-review-reply',
+            kwargs={'addon_pk': self.addon.pk, 'pk': self.review.pk + 42})
+        response = self.client.post(self.url, data={})
+        assert response.status_code == 404
+
+    def test_reply_admin(self):
+        self.admin_user = user_factory()
+        self.grant_permission(self.admin_user, 'Addons:Edit')
+        self.client.login_api(self.admin_user)
+        response = self.client.post(self.url, data={
+            'body': u'My âdmin réply...',
+        })
+        assert response.status_code == 201
+        review = Review.objects.latest('pk')
+        assert review.pk == response.data['id']
+        assert review.body == response.data['body'] == u'My âdmin réply...'
+        assert review.rating is None
+        assert 'rating' not in response.data
+        assert review.user == self.admin_user
+        assert review.title is None
+        assert response.data['title'] is None
+        assert review.reply_to == self.review
+        assert 'reply_to' not in response.data  # It's already in the URL...
+        assert review.addon == self.addon
+        assert review.version is None
+        assert 'version' not in response.data
+
+    def test_reply(self):
+        self.addon_author = user_factory()
+        self.addon.addonuser_set.create(user=self.addon_author)
+        self.client.login_api(self.addon_author)
+        response = self.client.post(self.url, data={
+            'body': u'My réply...',
+        })
+        assert response.status_code == 201
+        review = Review.objects.latest('pk')
+        assert review.pk == response.data['id']
+        assert review.body == response.data['body'] == u'My réply...'
+        assert review.rating is None
+        assert 'rating' not in response.data
+        assert review.user == self.addon_author
+        assert review.title is None
+        assert response.data['title'] is None
+        assert review.reply_to == self.review
+        assert 'reply_to' not in response.data  # It's already in the URL...
+        assert review.addon == self.addon
+        assert review.version is None
+        assert 'version' not in response.data
+
+    def test_reply_disabled_addon(self):
+        self.addon_author = user_factory()
+        self.addon.addonuser_set.create(user=self.addon_author)
+        self.client.login_api(self.addon_author)
+        self.addon.update(disabled_by_user=True)
+        response = self.client.post(self.url, data={})
+        assert response.status_code == 403
+
+    def test_replying_to_a_reply_is_not_possible(self):
+        self.addon_author = user_factory()
+        self.addon.addonuser_set.create(user=self.addon_author)
+        self.client.login_api(self.addon_author)
+        self.original_review = Review.objects.create(
+            addon=self.addon, version=self.addon.current_version, rating=1,
+            body='My review', user=self.review_user)
+        self.review.update(
+            user=self.addon_author, rating=None, reply_to=self.original_review)
+        response = self.client.post(self.url, data={
+            'body': u'LOL øø!'
+        })
+        assert response.status_code == 400
+        assert response.data['non_field_errors'] == [
+            'Can not reply to a review that is already a reply.']

src/olympia/reviews/views.py

@@ -13,6 +13,7 @@ from django.utils.translation import ugettext as _
 
 import commonware.log
 from mobility.decorators import mobile_template
+from rest_framework.decorators import detail_route
 from rest_framework.exceptions import ParseError
 from rest_framework.mixins import (
     CreateModelMixin, ListModelMixin, RetrieveModelMixin)
@@ -31,13 +32,13 @@ from olympia.addons.decorators import addon_view_factory
 from olympia.addons.models import Addon
 from olympia.addons.views import AddonChildMixin
 from olympia.api.permissions import (
-    AllowIfReviewedAndListed, AllowOwner, AnyOf, ByHttpMethod,
-    GroupPermission)
+    AllowAddonAuthor, AllowIfReviewedAndListed, AllowOwner,
+    AllowRelatedObjectPermissions, AnyOf, ByHttpMethod, GroupPermission)
 
 from .helpers import user_can_delete_review
 from .models import Review, ReviewFlag, GroupedRating, Spam
 from .permissions import CanDeleteReviewPermission
-from .serializers import ReviewSerializer
+from .serializers import ReviewSerializer, ReviewSerializerReply
 from . import forms
 
 
@@ -327,6 +328,7 @@ def spam(request):
 
 class ReviewViewSet(AddonChildMixin, CreateModelMixin, ListModelMixin,
                     RetrieveModelMixin, GenericViewSet):
+    serializer_class = ReviewSerializer
     permission_classes = [
         ByHttpMethod({
             'get': AllowAny,
@@ -344,8 +346,12 @@ class ReviewViewSet(AddonChildMixin, CreateModelMixin, ListModelMixin,
             'put': AnyOf(AllowOwner, GroupPermission('Addons', 'Edit')),
         }),
     ]
+    reply_permission_classes = [AnyOf(
+        GroupPermission('Addons', 'Edit'),
+        AllowRelatedObjectPermissions('addon', [AllowAddonAuthor]),
+    )]
+    reply_serializer_class = ReviewSerializerReply
 
-    serializer_class = ReviewSerializer
     queryset = Review.objects.all()
 
     def get_addon_object(self):
@@ -393,3 +399,13 @@ class ReviewViewSet(AddonChildMixin, CreateModelMixin, ListModelMixin,
         # need it, we already loaded it for permission checks through the pk
         # specified in the URL.
         return qs.defer('addon').prefetch_related('reply', 'user', 'version')
+
+    @detail_route(
+        methods=['post'], permission_classes=reply_permission_classes,
+        serializer_class=reply_serializer_class)
+    def reply(self, *args, **kwargs):
+        # A reply is just like a regular post, except that we set the reply
+        # FK to the current review object and only allow add-on authors/admins.
+        # Call get_object() to trigger 404 if it does not exist.
+        self.review_object = self.get_object()
+        return self.create(*args, **kwargs)