Merge pull request #3094 from diox/add-abuse-website-api

API for "Report an issue" with a Website (bug 1167171)

docs/api/topics/abuse.rst

@@ -1,10 +1,11 @@
 .. _abuse:
 
-=====
-Abuse
-=====
+===================
+Abuse and reporting
+===================
 
-Abusive apps and users may be reported to Marketplace staff.
+Abusive apps, users and websites may be reported to Marketplace staff. It can
+also be used to signal issues about the corresponding listing on Marketplace.
 
     .. note:: Authentication is optional for abuse reports.
 
@@ -98,3 +99,48 @@ Report An Abusive User
     :status 201: successfully submitted.
     :status 400: submission error.
     :status 429: exceeded rate limit.
+
+
+Report A Website
+================
+
+.. http:post:: /api/v2/abuse/website/
+
+    Report an issue with a website to Marketplace staff.
+
+    **Request**
+
+    :param text: a textual description of the issue
+    :type text: string
+    :param app: the id of the website being reported
+    :type app: int
+
+    .. code-block:: json
+
+        {
+            "sprout": "potato",
+            "text": "There is a problem with this site.",
+            "website": 42
+        }
+
+    This endpoint uses `PotatoCaptcha`, so there must be a field named `sprout`
+    with the value `potato` and cannot be a field named `tuber` with a truthy
+    value.
+
+    **Response**
+
+    .. code-block:: json
+
+        {
+            "reporter": null,
+            "text": "There is a problem with this app.",
+            "website": {
+                "id": 42,
+                "name": "cvan's site",
+                "...": "more info"
+            }
+        }
+
+    :status 201: successfully submitted.
+    :status 400: submission error.
+    :status 429: exceeded rate limit.

mkt/abuse/models.py

@@ -19,7 +19,7 @@ class AbuseReport(ModelBase):
                                  blank=True, related_name='abuse_reported')
     ip_address = models.CharField(max_length=255, default='0.0.0.0')
     # An abuse report can be for an addon, a user, or a website. Only one of
-    # these should be null.
+    # these should be set.
     addon = models.ForeignKey(Webapp, null=True, related_name='abuse_reports')
     user = models.ForeignKey(UserProfile, null=True,
                              related_name='abuse_reports')
@@ -42,17 +42,25 @@ def send(self):
         else:
             user_name = 'An anonymous coward'
 
-        if self.addon:
-            type_ = 'App'
-        elif self.user:
-            type_ = 'User'
+        if self.website:
+            # For Websites, it's not just abuse, the scope is broader, it could
+            # be any issue about the website listing itself, so use a different
+            # wording and recipient list.
+            type_ = u'Website'
+            subject = u'[%s] Issue Report for %s' % (type_, obj.name)
+            recipient_list = (settings.MKT_FEEDBACK_EMAIL,)
         else:
-            type_ = 'Website'
-        subject = u'[%s] Abuse Report for %s' % (type_, obj.name)
-        msg = u'%s reported abuse for %s (%s%s).\n\n%s' % (
+            if self.addon:
+                type_ = 'App'
+            elif self.user:
+                type_ = 'User'
+            subject = u'[%s] Abuse Report for %s' % (type_, obj.name)
+            recipient_list = (settings.ABUSE_EMAIL,)
+
+        msg = u'%s reported an issue for %s (%s%s).\n\n%s' % (
             user_name, obj.name, settings.SITE_URL, obj.get_url_path(),
             self.message)
-        send_mail(subject, msg, recipient_list=(settings.ABUSE_EMAIL,))
+        send_mail(subject, msg, recipient_list=recipient_list)
 
     @classmethod
     def recent_high_abuse_reports(cls, threshold, period, addon_id=None):

mkt/abuse/serializers.py

@@ -3,28 +3,37 @@
 from mkt.abuse.models import AbuseReport
 from mkt.account.serializers import UserSerializer
 from mkt.api.fields import SlugOrPrimaryKeyRelatedField, SplitField
+from mkt.api.serializers import PotatoCaptchaSerializer
 from mkt.webapps.models import Webapp
 from mkt.webapps.serializers import SimpleAppSerializer
+from mkt.websites.serializers import WebsiteSerializer
 
 
-class BaseAbuseSerializer(serializers.ModelSerializer):
+class BaseAbuseSerializer(PotatoCaptchaSerializer,
+                          serializers.ModelSerializer):
     text = serializers.CharField(source='message')
-    ip_address = serializers.CharField(required=False)
     reporter = SplitField(serializers.PrimaryKeyRelatedField(required=False),
                           UserSerializer())
 
-    def save(self, force_insert=False):
-        serializers.ModelSerializer.save(self)
-        del self.data['ip_address']
-        return self.object
+    class Meta:
+        model = AbuseReport
+        fields = ('text', 'reporter')
+
+    def validate(self, attrs):
+        request = self.context['request']
+        if request.user.is_authenticated():
+            attrs['reporter'] = request.user
+        else:
+            attrs['reporter'] = None
+        attrs['ip_address'] = request.META.get('REMOTE_ADDR', '')
+        return super(BaseAbuseSerializer, self).validate(attrs)
 
 
 class UserAbuseSerializer(BaseAbuseSerializer):
     user = SplitField(serializers.PrimaryKeyRelatedField(), UserSerializer())
 
-    class Meta:
-        model = AbuseReport
-        fields = ('text', 'ip_address', 'reporter', 'user')
+    class Meta(BaseAbuseSerializer.Meta):
+        fields = BaseAbuseSerializer.Meta.fields + ('user',)
 
 
 class AppAbuseSerializer(BaseAbuseSerializer):
@@ -33,6 +42,13 @@ class AppAbuseSerializer(BaseAbuseSerializer):
                                      queryset=Webapp.objects.all()),
         SimpleAppSerializer(source='addon'))
 
-    class Meta:
-        model = AbuseReport
-        fields = ('text', 'ip_address', 'reporter', 'app')
+    class Meta(BaseAbuseSerializer.Meta):
+        fields = BaseAbuseSerializer.Meta.fields + ('app',)
+
+
+class WebsiteAbuseSerializer(BaseAbuseSerializer):
+    website = SplitField(serializers.PrimaryKeyRelatedField(),
+                         WebsiteSerializer())
+
+    class Meta(BaseAbuseSerializer.Meta):
+        fields = BaseAbuseSerializer.Meta.fields + ('website',)

mkt/abuse/tests/test_models.py

@@ -8,6 +8,7 @@
 from mkt.site.fixtures import fixture
 from mkt.webapps.models import Webapp
 from mkt.users.models import UserProfile
+from mkt.websites.utils import website_factory
 
 
 class TestAbuse(mkt.site.tests.TestCase):
@@ -25,8 +26,16 @@ def test_user(self):
     def test_addon(self):
         AbuseReport(addon=self.app).send()
         assert mail.outbox[0].subject.startswith('[App]')
+        eq_(mail.outbox[0].to, [settings.ABUSE_EMAIL])
 
     def test_addon_fr(self):
         with self.activate(locale='fr'):
             AbuseReport(addon=self.app).send()
         assert mail.outbox[0].subject.startswith('[App]')
+        eq_(mail.outbox[0].to, [settings.ABUSE_EMAIL])
+
+    def test_website(self):
+        website = website_factory()
+        AbuseReport(website=website).send()
+        assert mail.outbox[0].subject.startswith('[Website]')
+        eq_(mail.outbox[0].to, [settings.MKT_FEEDBACK_EMAIL])

mkt/abuse/tests/test_views.py

@@ -11,6 +11,7 @@
 from mkt.site.fixtures import fixture
 from mkt.webapps.models import Webapp
 from mkt.users.models import UserProfile
+from mkt.websites.utils import website_factory
 
 
 class BaseTestAbuseResource(object):
@@ -59,7 +60,7 @@ def _test_success(self, res, data):
         """
         Tests common when looking to ensure complete successful responses.
         """
-        eq_(201, res.status_code)
+        eq_(201, res.status_code, res.content)
         fields = self.default_data.copy()
 
         del fields['sprout']
@@ -70,12 +71,16 @@ def _test_success(self, res, data):
         if 'app' in fields:
             eq_(int(data.pop('app')['id']), self.app.pk)
             del fields['app']
+        if 'website' in fields:
+            eq_(int(data.pop('website')['id']), self.website.pk)
+            del fields['website']
 
         for name in fields.keys():
             eq_(fields[name], data[name])
 
         newest_report = AbuseReport.objects.order_by('-id')[0]
         eq_(newest_report.message, data['text'])
+        eq_(newest_report.ip_address, self.headers['REMOTE_ADDR'])
 
         eq_(len(mail.outbox), 1)
         assert self.default_data['text'] in mail.outbox[0].body
@@ -88,11 +93,13 @@ def test_send(self):
         res, data = self._call()
         self._test_success(res, data)
         assert 'display_name' in data['reporter']
+        assert 'ip_address' not in data
 
     def test_send_anonymous(self):
         res, data = self._call(anonymous=True)
         self._test_success(res, data)
         eq_(data['reporter'], None)
+        assert 'ip_address' not in data
 
     def test_send_potato(self):
         tuber_res, tuber_data = self._call(data={'tuber': 'potat-toh'},
@@ -144,3 +151,22 @@ def test_invalid_app(self):
     def test_slug_app(self):
         res, data = self._call(data={'app': self.app.app_slug})
         eq_(201, res.status_code)
+
+
+class TestWebsiteAbuseResource(AbuseResourceTests, BaseTestAbuseResource,
+                               RestOAuth):
+    resource_name = 'website'
+
+    def setUp(self):
+        super(TestWebsiteAbuseResource, self).setUp()
+        self.website = website_factory()
+        self.default_data = {
+            'text': 'This website is weird.',
+            'sprout': 'potato',
+            'website': self.website.pk
+        }
+
+    def test_invalid_website(self):
+        res, data = self._call(data={'website': self.website.pk + 42})
+        eq_(400, res.status_code)
+        assert 'does not exist' in data['website'][0]

mkt/abuse/urls.py

@@ -2,11 +2,14 @@
 
 from rest_framework.routers import SimpleRouter
 
-from mkt.abuse.views import AppAbuseViewSet, UserAbuseViewSet
+from mkt.abuse.views import (AppAbuseViewSet, UserAbuseViewSet,
+                             WebsiteAbuseViewSet)
 
 abuse = SimpleRouter()
 abuse.register('user', UserAbuseViewSet, base_name='user-abuse')
 abuse.register('app', AppAbuseViewSet, base_name='app-abuse')
+abuse.register('website', WebsiteAbuseViewSet, base_name='website-abuse')
+
 
 api_patterns = patterns(
     '',

mkt/abuse/views.py

@@ -2,11 +2,12 @@
 from rest_framework.permissions import AllowAny
 from rest_framework.throttling import UserRateThrottle
 
-from mkt.abuse.serializers import AppAbuseSerializer, UserAbuseSerializer
+from mkt.abuse.serializers import (AppAbuseSerializer, UserAbuseSerializer,
+                                   WebsiteAbuseSerializer)
 from mkt.api.authentication import (RestOAuthAuthentication,
                                     RestAnonymousAuthentication,
                                     RestSharedSecretAuthentication)
-from mkt.api.base import check_potatocaptcha, CORSMixin
+from mkt.api.base import CORSMixin
 
 
 class AbuseThrottle(UserRateThrottle):
@@ -25,19 +26,6 @@ class BaseAbuseViewSet(CORSMixin, generics.CreateAPIView,
                               RestAnonymousAuthentication]
     permission_classes = (AllowAny,)
 
-    def create(self, request, *args, **kwargs):
-        fail = check_potatocaptcha(request.DATA)
-        if fail:
-            return fail
-        # Immutable? *this* *is* PYYYYTHONNNNNNNNNN!
-        request.DATA._mutable = True
-        if request.user.is_authenticated():
-            request.DATA['reporter'] = request.user.pk
-        else:
-            request.DATA['reporter'] = None
-        request.DATA['ip_address'] = request.META.get('REMOTE_ADDR', '')
-        return super(BaseAbuseViewSet, self).create(request, *args, **kwargs)
-
     def post_save(self, obj, created=False):
         obj.send()
 
@@ -48,3 +36,7 @@ class AppAbuseViewSet(BaseAbuseViewSet):
 
 class UserAbuseViewSet(BaseAbuseViewSet):
     serializer_class = UserAbuseSerializer
+
+
+class WebsiteAbuseViewSet(BaseAbuseViewSet):
+    serializer_class = WebsiteAbuseSerializer

mkt/account/views.py

@@ -161,7 +161,7 @@ def create_action(self, request, serializer):
         sender = getattr(request.user, 'email', settings.NOBODY_EMAIL)
         send_mail_jinja(u'Marketplace Feedback', 'account/email/feedback.txt',
                         context_data, headers={'Reply-To': sender},
-                        recipient_list=[settings.MKT_FEEDBACK_EMAIL])
+                        recipient_list=[settings.MKT_APPS_FEEDBACK_EMAIL])
 
     def get_context_data(self, request, serializer):
         context_data = {

mkt/api/base.py

@@ -1,10 +1,8 @@
 import functools
-import json
 
 from django.db.models.sql import EmptyResultSet
 
 import commonware.log
-from rest_framework import status
 from rest_framework.decorators import api_view
 from rest_framework.exceptions import ParseError
 from rest_framework.mixins import ListModelMixin
@@ -48,15 +46,6 @@ def form_errors(forms):
     raise ParseError(errors)
 
 
-def check_potatocaptcha(data):
-    if data.get('tuber', False):
-        return Response(json.dumps({'tuber': 'Invalid value'}),
-                        status=status.HTTP_400_BAD_REQUEST)
-    if data.get('sprout', None) != 'potato':
-        return Response(json.dumps({'sprout': 'Invalid value'}),
-                        status=status.HTTP_400_BAD_REQUEST)
-
-
 def get_region_from_request(request):
     region = request.GET.get('region')
     if region and region == 'None':

mkt/api/serializers.py

@@ -31,6 +31,20 @@ class PotatoCaptchaSerializer(serializers.Serializer):
     # This field's value should always be 'potato' (set by JS).
     sprout = serializers.CharField()
 
+    def get_fields(self):
+        fields = super(PotatoCaptchaSerializer, self).get_fields()
+        if self.request.user.is_authenticated():
+            # If the user is authenticated, we don't need PotatoCaptcha (tm).
+            fields.pop('tuber', None)
+            fields.pop('sprout', None)
+            self.has_potato_recaptcha = False
+        elif 'tuber' not in fields or 'sprout' not in fields:
+            # If 'tuber' and 'sprout' were not included, for instance because
+            # the serializer explicitely set 'fields', add them back.
+            fields['tuber'] = self.base_fields['tuber']
+            fields['sprout'] = self.base_fields['sprout']
+        return fields
+
     def __init__(self, *args, **kwargs):
         super(PotatoCaptchaSerializer, self).__init__(*args, **kwargs)
         if hasattr(self, 'context') and 'request' in self.context:
@@ -39,10 +53,6 @@ def __init__(self, *args, **kwargs):
             raise serializers.ValidationError('Need request in context')
 
         self.has_potato_recaptcha = True
-        if self.request.user.is_authenticated():
-            self.fields.pop('tuber')
-            self.fields.pop('sprout')
-            self.has_potato_recaptcha = False
 
     def validate(self, attrs):
         attrs = super(PotatoCaptchaSerializer, self).validate(attrs)