abuse/signals.py

@@ -1,7 +1,7 @@
 import logging
 
 from actstream import action
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, pre_delete
 from django.dispatch import receiver
 
 from abuse.models import AbuseReport
@@ -45,3 +45,8 @@ def _create_action_from_report(
         target=instance.extension,
         action_object=instance,
     )
+
+
+@receiver(pre_delete, sender=AbuseReport)
+def _log_deletion(sender: object, instance: AbuseReport, **kwargs: object) -> None:
+    instance.record_deletion()

common/model_mixins.py

@@ -2,7 +2,9 @@ from typing import Set, Tuple, Mapping, Any
 import copy
 import logging
 
+from django.contrib.admin.models import DELETION
 from django.contrib.contenttypes.models import ContentType
+from django.core import serializers
 from django.db import models
 from django.shortcuts import reverse
 from django.utils import timezone
@@ -45,7 +47,36 @@ class CreatedModifiedMixin(models.Model):
         return f'{path}?{query}'
 
 
-class TrackChangesMixin(models.Model):
+class RecordDeletionMixin:
+    def serialise(self) -> dict:
+        data = serializers.serialize('python', [self])[0]
+        data['fields']['pk'] = data['pk']
+        return data['fields']
+
+    def record_deletion(self):
+        """Create a LogEntry describing a deletion of this object."""
+        msg_args = {'type': type(self), 'pk': self.pk}
+        logger.info('Deleting %(type)s pk=%(pk)s', msg_args)
+        if hasattr(self, 'cannot_be_deleted_reasons'):
+            cannot_be_deleted_reasons = self.cannot_be_deleted_reasons
+            if len(cannot_be_deleted_reasons) > 0:
+                # This shouldn't happen: prior validation steps should have taken care of this.
+                msg_args['reasons'] = cannot_be_deleted_reasons
+                logger.error("%(type)s pk=%(pk)s is being deleted but it %(reasons)s", msg_args)
+        state = self.serialise()
+        message = [
+            {
+                'deleted': {
+                    'name': str(self._meta.verbose_name),
+                    'object': repr(self),
+                    'old_state': state,
+                },
+            }
+        ]
+        attach_log_entry(self, message, action_flag=DELETION)
+
+
+class TrackChangesMixin(RecordDeletionMixin, models.Model):
     """Tracks changes of Django models.
 
     Tracks which fields have changed in the save() function, so that

extensions/fields.py

@@ -22,7 +22,7 @@ class VersionStringField(SemanticVersionField):
         return str(value)
 
     def value_to_string(self, obj):
-        value = self._get_val_from_obj(obj)
+        value = self.value_from_object(obj)
         return self.get_prep_value(value)
 
     def from_json(self, json_str):

extensions/models.py

@@ -10,7 +10,7 @@ from django.db.models import F, Q, Count
 from django.urls import reverse
 
 from common.fields import FilterableManyToManyField
-from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
+from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin, TrackChangesMixin
 from constants.base import (
     AUTHOR_ROLE_CHOICES,
     AUTHOR_ROLE_DEV,
@@ -651,7 +651,7 @@ class Maintainer(CreatedModifiedMixin, models.Model):
         ]
 
 
-class Preview(CreatedModifiedMixin, models.Model):
+class Preview(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
     extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
     file = models.ForeignKey(
         'files.File', related_name='extension_preview', on_delete=models.CASCADE

extensions/signals.py

@@ -18,15 +18,14 @@ User = get_user_model()
 @receiver(pre_delete, sender=extensions.models.Extension)
 @receiver(pre_delete, sender=extensions.models.Preview)
 @receiver(pre_delete, sender=extensions.models.Version)
-def _log_extension_delete(sender: object, instance: object, **kwargs: object) -> None:
+def _log_deletion(
-    cannot_be_deleted_reasons = instance.cannot_be_deleted_reasons
+    sender: object,
-    if len(cannot_be_deleted_reasons) > 0:
+    instance: Union[
-        # This shouldn't happen: prior validation steps should have taken care of this.
+        extensions.models.Extension, extensions.models.Version, extensions.models.Preview
-        # raise ValidationError({'__all__': cannot_be_deleted_reasons})
+    ],
-        args = {'sender': sender, 'pk': instance.pk, 'reasons': cannot_be_deleted_reasons}
+    **kwargs: object,
-        logger.error("%(sender)s pk=%(pk)s is being deleted but it %(reasons)s", args)
+) -> None:
-
+    instance.record_deletion()
-    logger.info('Deleting %s pk=%s "%s"', sender, instance.pk, str(instance))
 
 
 @receiver(post_delete, sender=extensions.models.Preview)

extensions/tests/test_delete.py

@@ -1,16 +1,21 @@
-from django.test import TestCase
+import json
+
+from django.contrib.admin.models import LogEntry, DELETION
+from django.test import TestCase  # , TransactionTestCase
 
 from common.tests.factories.extensions import create_approved_version, create_version
 from common.tests.factories.files import FileFactory
-from common.tests.factories.users import UserFactory
+from common.tests.factories.users import UserFactory, create_moderator
 import extensions.models
 import files.models
+import reviewers.models
 
 
 class DeleteTest(TestCase):
     fixtures = ['dev', 'licenses']
 
     def test_unlisted_unrated_extension_can_be_deleted_by_author(self):
+        self.maxDiff = None
         version = create_version(
             file__status=files.models.File.STATUSES.AWAITING_REVIEW,
             ratings=[],
@@ -29,6 +34,31 @@ class DeleteTest(TestCase):
         self.assertEqual(extension.cannot_be_deleted_reasons, [])
         preview_file = extension.previews.first()
         self.assertIsNotNone(preview_file)
+        # Create some ApprovalActivity as well
+        moderator = create_moderator()
+        approval_activity = reviewers.models.ApprovalActivity.objects.create(
+            extension=extension,
+            user=moderator,
+            message='This is a message in approval activity',
+        )
+        # Create a file validation record
+        file_validation = files.models.FileValidation.objects.create(
+            file=version_file, results={'deadbeef': 'foobar'}
+        )
+        object_reprs = list(
+            map(
+                repr,
+                [
+                    preview_file,
+                    version_file,
+                    file_validation,
+                    extension,
+                    approval_activity,
+                    preview_file.extension_preview.first(),
+                    version,
+                ],
+            )
+        )
 
         url = extension.get_delete_url()
         user = extension.authors.first()
@@ -49,6 +79,47 @@ class DeleteTest(TestCase):
         self.assertIsNone(extensions.models.Version.objects.filter(pk=version.pk).first())
         self.assertIsNone(files.models.File.objects.filter(pk=version_file.pk).first())
         self.assertIsNone(files.models.File.objects.filter(pk=preview_file.pk).first())
+
+        # Check that each of the deleted records was logged
+        deletion_log_entries_q = LogEntry.objects.filter(action_flag=DELETION)
+        self.assertEqual(deletion_log_entries_q.count(), 7)
+        self.assertEqual(
+            [_.object_repr for _ in deletion_log_entries_q],
+            object_reprs,
+        )
+        log_entry = deletion_log_entries_q.filter(object_repr__contains='Extension').first()
+        change_message_data = json.loads(log_entry.change_message)
+        self.assertEqual(
+            change_message_data[0]['deleted']['object'], f'<Extension: Add-on "{extension.name}">'
+        )
+        self.assertEqual(
+            set(change_message_data[0]['deleted']['old_state'].keys()),
+            {
+                'average_score',
+                'date_approved',
+                'date_created',
+                'date_modified',
+                'date_status_changed',
+                'description',
+                'download_count',
+                'extension_id',
+                'is_listed',
+                'name',
+                'pk',
+                'slug',
+                'status',
+                'support',
+                'team',
+                'type',
+                'view_count',
+                'website',
+            },
+        )
+        self.assertEqual(
+            log_entry.get_change_message(),
+            f'Deleted extension “<Extension: Add-on "{extension.name}">”.',
+        )
+
         # TODO: check that files were deleted from storage (create a temp one prior to the check)
 
     def test_publicly_listed_extension_cannot_be_deleted(self):

files/signals.py

@@ -36,5 +36,6 @@ def _scan_new_file(
 
 
 @receiver(pre_delete, sender=files.models.File)
-def _log_file_delete(sender: object, instance: files.models.File, **kwargs: object) -> None:
+@receiver(pre_delete, sender=files.models.FileValidation)
-    logger.info('Deleting file pk=%s source=%s', instance.pk, instance.source.name)
+def _log_deletion(sender: object, instance: files.models.File, **kwargs: object) -> None:
+    instance.record_deletion()

ratings/signals.py

@@ -1,5 +1,5 @@
 from actstream import action
-from django.db.models.signals import pre_save, post_save
+from django.db.models.signals import pre_save, post_save, pre_delete
 from django.dispatch import receiver
 
 from constants.activity import Verb
@@ -40,3 +40,8 @@ def _create_action_from_rating(
         action_object=instance,
         target=instance.extension,
     )
+
+
+@receiver(pre_delete, sender=Rating)
+def _log_deletion(sender: object, instance: Rating, **kwargs: object) -> None:
+    instance.record_deletion()

reviewers/models.py

@@ -9,7 +9,7 @@ from django.utils.translation import gettext_lazy as _
 
 import common.help_texts
 from extensions.models import Extension
-from common.model_mixins import CreatedModifiedMixin
+from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin
 from utils import absolutify, send_mail
 
 from constants.base import EXTENSION_TYPE_CHOICES
@@ -74,7 +74,7 @@ class ReviewerSubscription(CreatedModifiedMixin, models.Model):
         )
 
 
-class ApprovalActivity(CreatedModifiedMixin, models.Model):
+class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
     class ActivityType(models.TextChoices):
         COMMENT = "COM", _("Comment")
         APPROVED = "APR", _("Approved")

reviewers/signals.py

@@ -1,6 +1,6 @@
 from actstream import action
 from actstream.actions import follow
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, pre_delete
 from django.dispatch import receiver
 
 from constants.activity import Flag, Verb
@@ -37,3 +37,8 @@ def _create_action_from_review_and_follow(
         action_object=instance,
         target=instance.extension,
     )
+
+
+@receiver(pre_delete, sender=ApprovalActivity)
+def _log_deletion(sender: object, instance: ApprovalActivity, **kwargs: object) -> None:
+    instance.record_deletion()