expose rating flags in detail response when`?include_flags_for=<user>` (#9988)

docs/topics/api/overview.rst

@@ -289,3 +289,4 @@ v4 API changelog
 * 2018-10-25: added ``fxa_edit_email_url`` parameter on accounts API to return the full URL for editing the user's email on FxA. https://github.com/mozilla/addons-server/issues/8674
 * 2018-10-31: added ``id`` to discovery API ``addons.current_version`` object. This change was also backported to the `v3` API. https://github.com/mozilla/addons-server/issues/9855
 * 2018-11-15: added ``is_custom`` to the license object in version detail output in the addons API.
+* 2018-11-22: added ``flags`` to the rating object in the ratings API when ``show_flags_for`` parameter supplied.

docs/topics/api/ratings.rst

@@ -27,6 +27,7 @@ user has already posted a rating for the current version of an add-on.
 .. http:get:: /api/v4/ratings/rating/
 
     :query string addon: The :ref:`add-on <addon-detail>` id, slug, or guid to fetch ratings from. When passed, the ratings shown will always be the latest posted by each user on this particular add-on (which means there should only be one rating per user in the results), unless the ``version`` parameter is also passed.
+    :query string show_flags_for: The user id to show flags for. If the request is made with an authenticated user matching this parameter value, a ``flags`` property will be added to the response as described below in :ref:`ratings <rating-detail-object>`.
     :query string show_permissions_for: The user id to show permissions for. If the request is made with an authenticated user matching this parameter value, and the ``addon`` parameter is also present, a ``can_reply`` property will be added to the response as described below.
     :query string filter: The :ref:`filter(s) <rating-filtering-param>` to apply.
     :query int user: The user id to fetch ratings from.
@@ -78,12 +79,16 @@ This endpoint allows you to fetch a rating by its id.
 
     .. _rating-detail-object:
 
+    :query string show_flags_for: The user id to show flags for. If the request is made with an authenticated user matching this parameter value, a ``flags`` property will be added to the response as described below.
     :>json int id: The rating id.
     :>json object addon: A simplified :ref:`add-on <addon-detail-object>` object that contains only a few properties: ``id``, ``name``, ``icon_url`` and ``slug``.
     :>json string|null body: The text of the rating.
     :>json boolean is_deleted: Boolean indicating whether the rating has been deleted or not.
     :>json boolean is_latest: Boolean indicating whether the rating is the latest posted by the user on the same add-on.
     :>json int previous_count: The number of ratings posted by the user on the same add-on before this one.
+    :>json object flags[]: A list of flags the user requesting has previously applied to this rating (that haven't been processed by moderators already). Only present if ``show_flags_for`` parameter sent.
+    :>json string flags.flag: A :ref:`constant<rating-flag-constants>` describing the reason behind the flagging.
+    :>json string|null flags.note: A note to explain further the reason behind the flagging if ``flag`` was ``rating_flag_reason_other``; null otherwise.
     :>json int score: The score the user gave as part of the rating.
     :>json object|null reply: The rating object containing the developer reply to this rating, if any (The fields ``rating``, ``reply`` and ``version`` are omitted).
     :>json int version.id: The add-on version id the rating applies to.

src/olympia/lib/settings_base.py

@@ -1775,6 +1775,7 @@ DRF_API_GATES = {
         'del-addons-created-field',
         'del-accounts-fxa-edit-email-url',
         'del-version-license-is-custom',
+        'del-ratings-flags',
     ),
     'v4': (
         'l10n_flat_input_output',

src/olympia/ratings/serializers.py

@@ -33,10 +33,11 @@ class BaseRatingSerializer(serializers.ModelSerializer):
     is_latest = serializers.BooleanField(read_only=True)
     previous_count = serializers.IntegerField(read_only=True)
     user = BaseUserSerializer(read_only=True)
+    flags = serializers.SerializerMethodField()
 
     class Meta:
         model = Rating
-        fields = ('id', 'addon', 'body', 'created', 'is_deleted',
+        fields = ('id', 'addon', 'body', 'created', 'flags', 'is_deleted',
                   'is_developer_reply', 'is_latest', 'previous_count', 'user')
 
     def __init__(self, *args, **kwargs):
@@ -90,10 +91,22 @@ class BaseRatingSerializer(serializers.ModelSerializer):
 
         return data
 
+    def get_flags(self, obj):
+        if self.context['view'].should_include_flags():
+            # should be maximum one RatingFlag per rating+user anyway.
+            rating_flags = obj.ratingflag_set.filter(
+                user=self.context['request'].user)
+            return [
+                {'flag': flag.flag, 'note': flag.note or None}
+                for flag in rating_flags]
+        return None
+
     def to_representation(self, instance):
         out = super(BaseRatingSerializer, self).to_representation(instance)
         if self.request and is_gate_active(self.request, 'ratings-title-shim'):
             out['title'] = None
+        if not self.context['view'].should_include_flags():
+            out.pop('flags', None)
         return out
 
 
@@ -160,7 +173,7 @@ class RatingSerializer(BaseRatingSerializer):
             is_gate_active(self.request, 'ratings-rating-shim'))
         if score_to_rating:
             score_field = self.fields.pop('score')
-            score_field.source = None  # drf complains if we specifiy source.
+            score_field.source = None  # drf complains if we specify source.
             self.fields['rating'] = score_field
 
     def validate_version(self, version):

src/olympia/ratings/tests/test_serializers.py

@@ -6,15 +6,16 @@ from rest_framework.test import APIRequestFactory
 
 from olympia.amo.templatetags.jinja_helpers import absolutify
 from olympia.amo.tests import TestCase, addon_factory, user_factory
-from olympia.ratings.models import Rating
+from olympia.ratings.models import Rating, RatingFlag
 from olympia.ratings.serializers import RatingSerializer
 
 
 class TestBaseRatingSerializer(TestCase):
     def setUp(self):
         self.request = APIRequestFactory().get('/')
-        self.view = Mock(spec=['get_addon_object'])
+        self.view = Mock(spec=['get_addon_object', 'should_include_flags'])
         self.view.get_addon_object.return_value = None
+        self.view.should_include_flags.return_value = False
         self.user = user_factory()
 
     def serialize(self, **extra_context):
@@ -67,6 +68,8 @@ class TestBaseRatingSerializer(TestCase):
         assert result['version'] is None
         # Check the default, when DRF_API_GATES['ratings-title-shim'] isn't set
         assert 'title' not in result
+        # Check the default, when `show_flags_for=...` isn't sent.
+        assert 'flags' not in result
 
     def test_deleted_rating_but_view_allowing_it_to_be_shown(self):
         # We don't need to change self.view.should_access_deleted_ratings
@@ -293,3 +296,23 @@ class TestBaseRatingSerializer(TestCase):
         assert serializer.fields['id'].read_only is True
         assert serializer.fields['reply'].read_only is True
         assert serializer.fields['user'].read_only is True
+
+    def test_include_flags(self):
+        addon = addon_factory()
+        self.request.user = user_factory()
+        self.view.get_addon_object.return_value = addon
+        self.view.should_include_flags.return_value = True
+        self.rating = Rating.objects.create(
+            addon=addon, user=self.user, rating=4,
+            version=addon.current_version, body=u'This is my rëview. Like ît?')
+
+        result = self.serialize()
+        assert 'flags' in result
+        assert result['flags'] == []
+
+        RatingFlag.objects.create(
+            rating=self.rating, user=self.request.user, flag=RatingFlag.OTHER,
+            note=u'foo')
+        result = self.serialize()
+        assert 'flags' in result
+        assert result['flags'] == [{'flag': RatingFlag.OTHER, 'note': 'foo'}]

src/olympia/ratings/tests/test_views.py

@@ -823,6 +823,11 @@ class TestRatingViewSetGet(TestCase):
 
         if 'show_grouped_ratings' not in kwargs:
             assert 'grouped_ratings' not in data
+
+        if 'show_for' not in kwargs:
+            assert 'flags' not in data['results'][0]
+            assert 'flags' not in data['results'][1]
+
         return data
 
     def test_list_show_permission_for_anonymous(self):
@@ -1301,6 +1306,92 @@ class TestRatingViewSetGet(TestCase):
         data = json.loads(response.content)
         assert data['detail'] == 'Need an addon or user parameter'
 
+    def test_list_show_flags_for_anonymous(self):
+        response = self.client.get(
+            self.url, {'addon': self.addon.pk, 'show_flags_for': 666})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_list_show_flags_for_not_int(self):
+        response = self.client.get(
+            self.url, {'addon': self.addon.pk, 'show_flags_for': 'nope'})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_list_show_flags_for_not_right_user(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        response = self.client.get(
+            self.url, {'addon': self.addon.pk,
+                       'show_flags_for': self.user.pk + 42})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_list_rating_flags(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        rating1 = Rating.objects.create(
+            addon=self.addon, body='review 1', user=user_factory(),
+            rating=2)
+        rating0 = Rating.objects.create(
+            addon=self.addon, body='review 0', user=user_factory(),
+            rating=1)
+        reply_to_0 = Rating.objects.create(
+            addon=self.addon, body='reply to review 0', reply_to=rating0,
+            user=user_factory())
+        params = {'addon': self.addon.pk, 'show_flags_for': self.user.pk}
+
+        # First, not flagged
+        response = self.client.get(self.url, params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        assert data['results'][0]['flags'] == []
+        assert data['results'][0]['reply']['flags'] == []
+        assert data['results'][1]['flags'] == []
+
+        # then add some RatingFlag - one for a rating, the other a reply
+        RatingFlag.objects.create(
+            rating=rating1, user=self.user, flag=RatingFlag.LANGUAGE)
+        RatingFlag.objects.create(
+            rating=reply_to_0, user=self.user, flag=RatingFlag.OTHER,
+            note=u'foo')
+
+        response = self.client.get(self.url, params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        rating0 = data['results'][0]
+        rating1 = data['results'][1]
+        assert 'flags' in rating0
+        assert 'flags' in rating1
+        assert 'flags' in rating0['reply']
+        assert rating0['flags'] == []
+        assert rating0['reply']['flags'] == [
+            {'flag': RatingFlag.OTHER, 'note': 'foo'}]
+        assert rating1['flags'] == [
+            {'flag': RatingFlag.LANGUAGE, 'note': None}]
+
+    def test_list_rating_flags_absent_in_v3(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        rating = Rating.objects.create(
+            addon=self.addon, body='review', user=user_factory(),
+            rating=1)
+        RatingFlag.objects.create(
+            rating=rating, user=self.user, flag=RatingFlag.OTHER,
+            note=u'foo')
+        params = {'addon': self.addon.pk, 'show_flags_for': self.user.pk}
+        response = self.client.get(
+            reverse_ns('rating-list', api_version='v3'), params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        assert 'flags' not in data['results'][0]
+
     def test_detail(self):
         review = Rating.objects.create(
             addon=self.addon, body='review 1', user=user_factory())
@@ -1368,6 +1459,83 @@ class TestRatingViewSetGet(TestCase):
         assert data['reply']
         assert data['reply']['id'] == reply.pk
 
+    def test_detail_show_flags_for_anonymous(self):
+        rating = Rating.objects.create(
+            addon=self.addon, body='review', user=user_factory())
+        detail_url = reverse_ns(self.detail_url_name, kwargs={'pk': rating.pk})
+        response = self.client.get(detail_url, {'show_flags_for': 666})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_detail_show_flags_for_not_int(self):
+        rating = Rating.objects.create(
+            addon=self.addon, body='review', user=user_factory())
+        detail_url = reverse_ns(self.detail_url_name, kwargs={'pk': rating.pk})
+        response = self.client.get(detail_url, {'show_flags_for': 'nope'})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_detail_show_flags_for_not_right_user(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        rating = Rating.objects.create(
+            addon=self.addon, body='review', user=user_factory())
+        detail_url = reverse_ns(self.detail_url_name, kwargs={'pk': rating.pk})
+        response = self.client.get(
+            detail_url, {'show_flags_for': self.user.pk + 42})
+        assert response.status_code == 400
+        assert response.data['detail'] == (
+            'show_flags_for parameter value should be equal to the user '
+            'id of the authenticated user')
+
+    def test_detail_rating_flags(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        rating = Rating.objects.create(
+            addon=self.addon, body='review 1', user=user_factory(),
+            rating=2)
+
+        detail_url = reverse_ns(self.detail_url_name, kwargs={'pk': rating.pk})
+        params = {'show_flags_for': self.user.pk}
+
+        # First, not flagged
+        response = self.client.get(detail_url, params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        assert data['flags'] == []
+
+        # then add some RatingFlag - one for a rating, the other a reply
+        RatingFlag.objects.create(
+            rating=rating, user=self.user, flag=RatingFlag.LANGUAGE)
+
+        response = self.client.get(detail_url, params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        assert 'flags' in data
+        assert data['flags'] == [
+            {'flag': RatingFlag.LANGUAGE, 'note': None}]
+
+    def test_detail_rating_flags_absent_in_v3(self):
+        self.user = user_factory()
+        self.client.login_api(self.user)
+        rating = Rating.objects.create(
+            addon=self.addon, body='review', user=user_factory(),
+            rating=1)
+        RatingFlag.objects.create(
+            rating=rating, user=self.user, flag=RatingFlag.OTHER,
+            note=u'foo')
+        detail_url = reverse_ns(
+            self.detail_url_name, kwargs={'pk': rating.pk}, api_version='v3')
+        params = {'show_flags_for': self.user.pk}
+        response = self.client.get(detail_url, params)
+        assert response.status_code == 200
+        data = json.loads(response.content)
+        assert 'flags' not in data
+
     def test_list_by_admin_does_not_show_deleted_by_default(self):
         self.user = user_factory()
         self.grant_permission(self.user, 'Addons:Edit')

src/olympia/ratings/views.py

@@ -313,6 +313,27 @@ class RatingViewSet(AddonChildMixin, ModelViewSet):
         return super(RatingViewSet, self).get_addon_object(
             permission_classes=[AllowIfPublic])
 
+    def should_include_flags(self):
+        if not hasattr(self, '_should_include_flags'):
+            request = self.request
+            self._should_include_flags = (
+                'show_flags_for' in request.GET and
+                not is_gate_active(request, 'del-ratings-flags')
+            )
+            if self._should_include_flags:
+                # Check the parameter was sent correctly
+                try:
+                    show_flags_for = (
+                        serializers.IntegerField().to_internal_value(
+                            request.GET['show_flags_for']))
+                    if show_flags_for != request.user.pk:
+                        raise serializers.ValidationError
+                except serializers.ValidationError:
+                    raise ParseError(
+                        'show_flags_for parameter value should be equal to '
+                        'the user id of the authenticated user')
+        return self._should_include_flags
+
     def check_permissions(self, request):
         """Perform permission checks.
 
@@ -412,6 +433,9 @@ class RatingViewSet(AddonChildMixin, ModelViewSet):
                     'the user id of the authenticated user')
             extra_data['can_reply'] = (
                 self.check_can_reply_permission_for_ratings_list())
+        # Call this here so the validation checks on the `show_flags_for` are
+        # carried out even when there are no results to serialize.
+        self.should_include_flags()
         response = super(RatingViewSet, self).get_paginated_response(data)
         if extra_data:
             response.data.update(extra_data)