Store add-on guid when saving an Abuse Report against an add-on in database

src/olympia/abuse/serializers.py

@@ -76,15 +76,9 @@ class AddonAbuseReportSerializer(BaseAbuseReportSerializer):
     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.
+        output = view.get_guid_and_addon()
+        # Pop 'addon' from data before passing that data to super(), we already
+        # have it in the output value.
         data.pop('addon')
         output.update(
             super(AddonAbuseReportSerializer, self).to_internal_value(data)

src/olympia/abuse/tests/test_serializers.py

@@ -80,7 +80,10 @@ class TestAddonAbuseReportSerializer(TestCase):
         request = RequestFactory().get('/')
         request.user = AnonymousUser()
         view = Mock()
-        view.get_guid.return_value = '@someguid'
+        view.get_guid_and_addon.return_value = {
+            'guid': '@someguid',
+            'addon': None,
+        }
         view.get_addon_object.return_value = None
         extra_context = {
             'request': request,

src/olympia/abuse/tests/test_views.py

@@ -42,6 +42,7 @@ class AddonAbuseViewSetTestBase(object):
 
         assert AbuseReport.objects.filter(addon_id=addon.id).exists()
         report = AbuseReport.objects.get(addon_id=addon.id)
+        assert report.guid == addon.guid
         self.check_report(report,
                           u'[Extension] Abuse Report for %s' % addon.name)
 
@@ -55,6 +56,7 @@ class AddonAbuseViewSetTestBase(object):
 
         assert AbuseReport.objects.filter(addon_id=addon.id).exists()
         report = AbuseReport.objects.get(addon_id=addon.id)
+        assert report.guid == addon.guid
         self.check_report(report,
                           u'[Extension] Abuse Report for %s' % addon.name)
 
@@ -68,6 +70,7 @@ class AddonAbuseViewSetTestBase(object):
 
         assert AbuseReport.objects.filter(addon_id=addon.id).exists()
         report = AbuseReport.objects.get(addon_id=addon.id)
+        assert report.guid == addon.guid
         self.check_report(report,
                           u'[Extension] Abuse Report for %s' % addon.name)
 
@@ -81,6 +84,7 @@ class AddonAbuseViewSetTestBase(object):
 
         assert AbuseReport.objects.filter(guid=guid).exists()
         report = AbuseReport.objects.get(guid=guid)
+        assert not report.addon
         self.check_report(report,
                           u'[Addon] Abuse Report for %s' % guid)
 

src/olympia/abuse/views.py

@@ -40,11 +40,16 @@ class AddonAbuseViewSet(CreateModelMixin, GenericViewSet):
         self.addon_object = self.get_addon_viewset().get_object()
         return self.addon_object
 
-    def get_guid(self):
-        # See if the addon input is guid-like, if so set guid.
+    def get_guid_and_addon(self):
+        data = {
+            'guid': None,
+            'addon': None,
+        }
+        # See if the addon input is guid-like first. It doesn't have to exist
+        # in our database.
         if self.get_addon_viewset().get_lookup_field(
                 self.kwargs['addon_pk']) == 'guid':
-            guid = self.kwargs['addon_pk']
+            data['guid'] = self.kwargs['addon_pk']
             try:
                 # But see if it's also in our database.
                 self.get_addon_object()
@@ -52,8 +57,15 @@ class AddonAbuseViewSet(CreateModelMixin, GenericViewSet):
                 # If it isn't, that's okay, we have a guid.  Setting
                 # addon_object=None here means get_addon_object won't raise 404
                 self.addon_object = None
-            return guid
-        return None
+        # At this point get_addon_object() will either return None because we
+        # set self.addon_object earlier, or find an add-on with its pk/slug,
+        # or raise a 404.
+        data['addon'] = self.get_addon_object()
+        if data['addon']:
+            # If we did find an add-on in database, regardless of how, make
+            # sure we always store the guid as well.
+            data['guid'] = data['addon'].guid
+        return data
 
 
 class UserAbuseViewSet(CreateModelMixin, GenericViewSet):

src/olympia/migrations/1087-abuse-add-guid-to-existing-addon-reports.sql

@@ -0,0 +1,5 @@
+UPDATE `abuse_reports`
+    INNER JOIN `addons` ON (`abuse_reports`.`addon_id` = `addons`.`id`)
+    SET `abuse_reports`.`guid` = `addons`.`guid`
+    WHERE `abuse_reports`.`addon_id` IS NOT NULL
+    AND `abuse_reports`.`guid` IS NULL;