Refactor abuse API internals to make adding new fields easier

src/olympia/abuse/models.py

@@ -40,7 +40,7 @@ class AbuseReport(ModelBase):
         if self.reporter:
             user_name = '%s (%s)' % (self.reporter.name, self.reporter.email)
         else:
-            user_name = 'An anonymous coward'
+            user_name = 'An anonymous user'
 
         target_url = ('%s%s' % (settings.SITE_URL, self.target.get_url_path())
                       if self.target else 'GUID not in database')
@@ -50,6 +50,12 @@ class AbuseReport(ModelBase):
         send_mail(
             six.text_type(self), msg, recipient_list=(settings.ABUSE_EMAIL,))
 
+    def save(self, *args, **kwargs):
+        creation = not self.pk
+        super(AbuseReport, self).save(*args, **kwargs)
+        if creation:
+            self.send()
+
     @property
     def target(self):
         return self.addon or self.user
@@ -77,4 +83,3 @@ def send_abuse_report(request, obj, message):
     elif isinstance(obj, UserProfile):
         report.user = obj
     report.save()
-    report.send()

src/olympia/abuse/serializers.py

@@ -4,13 +4,55 @@ from olympia.abuse.models import AbuseReport
 from olympia.accounts.serializers import BaseUserSerializer
 
 
-class AddonAbuseReportSerializer(serializers.ModelSerializer):
-    addon = serializers.SerializerMethodField()
+class BaseAbuseReportSerializer(serializers.ModelSerializer):
     reporter = BaseUserSerializer(read_only=True)
 
     class Meta:
         model = AbuseReport
-        fields = ('reporter', 'addon', 'message')
+        fields = ('message', 'reporter')
+
+    def validate_target(self, data, target_name):
+        if target_name not in data:
+            msg = serializers.Field.default_error_messages['required']
+            raise serializers.ValidationError({
+                target_name: [msg]
+            })
+
+    def to_internal_value(self, data):
+        output = super(BaseAbuseReportSerializer, self).to_internal_value(data)
+        request = self.context['request']
+        output['ip_address'] = request.META.get('REMOTE_ADDR')
+        if request.user.is_authenticated:
+            output['reporter'] = request.user
+        return output
+
+
+class AddonAbuseReportSerializer(BaseAbuseReportSerializer):
+    addon = serializers.SerializerMethodField()
+
+    class Meta:
+        model = AbuseReport
+        fields = BaseAbuseReportSerializer.Meta.fields + (
+            'addon',
+        )
+
+    def to_internal_value(self, data):
+        self.validate_target(data, 'addon')
+        view = self.context.get('view')
+        output = {
+            # get_guid() needs to be called first because get_addon_object()
+            # would otherwise 404 on add-ons that don't match an existing
+            # add-on in our database.
+            'guid': view.get_guid(),
+            'addon': view.get_addon_object(),
+        }
+        # Pop 'addon' before passing it to super(), we already have the
+        # output value.
+        data.pop('addon')
+        output.update(
+            super(AddonAbuseReportSerializer, self).to_internal_value(data)
+        )
+        return output
 
     def get_addon(self, obj):
         addon = obj.addon
@@ -23,10 +65,25 @@ class AddonAbuseReportSerializer(serializers.ModelSerializer):
         }
 
 
-class UserAbuseReportSerializer(serializers.ModelSerializer):
-    reporter = BaseUserSerializer(read_only=True)
-    user = BaseUserSerializer()
+class UserAbuseReportSerializer(BaseAbuseReportSerializer):
+    user = BaseUserSerializer(required=False)  # We validate it ourselves.
 
     class Meta:
         model = AbuseReport
-        fields = ('reporter', 'user', 'message')
+        fields = BaseAbuseReportSerializer.Meta.fields + (
+            'user',
+        )
+
+    def to_internal_value(self, data):
+        view = self.context.get('view')
+        self.validate_target(data, 'user')
+        output = {
+            'user': view.get_user_object()
+        }
+        # Pop 'user' before passing it to super(), we already have the
+        # output value and did the validation above.
+        data.pop('user')
+        output.update(
+            super(UserAbuseReportSerializer, self).to_internal_value(data)
+        )
+        return output

src/olympia/abuse/tests/test_views.py

@@ -102,7 +102,7 @@ class AddonAbuseViewSetTestBase(object):
             data={'message': 'abuse!'})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Need an addon parameter'}
+            'addon': ['This field is required.']}
 
     def test_message_required_empty(self):
         addon = addon_factory()
@@ -112,7 +112,7 @@ class AddonAbuseViewSetTestBase(object):
                   'message': ''})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Abuse reports need a message'}
+            'message': ['This field may not be blank.']}
 
     def test_message_required_missing(self):
         addon = addon_factory()
@@ -121,7 +121,7 @@ class AddonAbuseViewSetTestBase(object):
             data={'addon': six.text_type(addon.id)})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Abuse reports need a message'}
+            'message': ['This field is required.']}
 
     def test_throttle(self):
         addon = addon_factory()
@@ -201,7 +201,7 @@ class UserAbuseViewSetTestBase(object):
             data={'message': 'abuse!'})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Need a user parameter'}
+            'user': ['This field is required.']}
 
     def test_message_required_empty(self):
         user = user_factory()
@@ -210,7 +210,7 @@ class UserAbuseViewSetTestBase(object):
             data={'user': six.text_type(user.username), 'message': ''})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Abuse reports need a message'}
+            'message': ['This field may not be blank.']}
 
     def test_message_required_missing(self):
         user = user_factory()
@@ -219,7 +219,7 @@ class UserAbuseViewSetTestBase(object):
             data={'user': six.text_type(user.username)})
         assert response.status_code == 400
         assert json.loads(response.content) == {
-            'detail': 'Abuse reports need a message'}
+            'message': ['This field is required.']}
 
     def test_throttle(self):
         user = user_factory()

src/olympia/abuse/views.py

@@ -59,37 +59,13 @@ class AddonAbuseViewSet(CreateModelMixin, GenericViewSet):
             return guid
         return None
 
-    def create(self, request, *args, **kwargs):
-        addon_id = self.request.data.get('addon')
-        if not addon_id:
-            raise ParseError('Need an addon parameter')
-
-        message = self.request.data.get('message')
-        if not message:
-            raise ParseError('Abuse reports need a message')
-
-        abuse_kwargs = {
-            'ip_address': request.META.get('REMOTE_ADDR'),
-            'message': message,
-            # get_guid() must be called first or addons not in our DB will 404.
-            'guid': self.get_guid(),
-            'addon': self.get_addon_object()}
-        if request.user.is_authenticated:
-            abuse_kwargs['reporter'] = request.user
-
-        report = AbuseReport.objects.create(**abuse_kwargs)
-        report.send()
-
-        serializer = self.get_serializer(report)
-        return Response(serializer.data, status=status.HTTP_201_CREATED)
-
 
 class UserAbuseViewSet(CreateModelMixin, GenericViewSet):
     permission_classes = []
     serializer_class = UserAbuseReportSerializer
     throttle_classes = (AbuseThrottle,)
 
-    def get_user_object(self, user_id):
+    def get_user_object(self):
         if hasattr(self, 'user_object'):
             return self.user_object
 
@@ -101,25 +77,3 @@ class UserAbuseViewSet(CreateModelMixin, GenericViewSet):
         return AccountViewSet(
             request=self.request, permission_classes=[],
             kwargs={'pk': self.kwargs['user_pk']}).get_object()
-
-    def create(self, request, *args, **kwargs):
-        user_id = self.request.data.get('user')
-        if not user_id:
-            raise ParseError('Need a user parameter')
-
-        message = self.request.data.get('message')
-        if not message:
-            raise ParseError('Abuse reports need a message')
-
-        abuse_kwargs = {
-            'ip_address': request.META.get('REMOTE_ADDR'),
-            'message': message,
-            'user': self.get_user_object(user_id)}
-        if request.user.is_authenticated:
-            abuse_kwargs['reporter'] = request.user
-
-        report = AbuseReport.objects.create(**abuse_kwargs)
-        report.send()
-
-        serializer = self.get_serializer(report)
-        return Response(serializer.data, status=status.HTTP_201_CREATED)

src/olympia/reviewers/tests/test_views.py

@@ -4708,7 +4708,7 @@ class TestAbuseReports(TestCase):
         AbuseReport.objects.create(user=someone, message=u'hey nöw')
         # Make a user abuse report for one of the add-on developers: it should
         # show up.
-        AbuseReport.objects.create(user=addon_developer, message='bü!')
+        AbuseReport.objects.create(user=addon_developer, message=u'bü!')
 
     def test_abuse_reports_list(self):
         assert self.client.login(email='admin@mozilla.com')