abuse/migrations/0006_remove_abusereport_date_deleted.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.11 on 2024-04-18 08:59
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('abuse', '0005_alter_abusereport_type'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='abusereport',
+            name='date_deleted',
+        ),
+    ]

abuse/models.py

@@ -9,13 +9,13 @@ from extended_choices import Choices
 from geoip2.errors import GeoIP2Error
 
 from constants.base import ABUSE_TYPE, ABUSE_TYPE_EXTENSION, ABUSE_TYPE_RATING
-from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
+from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
 import extensions.fields
 
 User = get_user_model()
 
 
-class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
+class AbuseReport(CreatedModifiedMixin, TrackChangesMixin, models.Model):
     TYPE = ABUSE_TYPE
 
     REASONS = Choices(

common/model_mixins.py

@@ -3,7 +3,7 @@ import copy
 import logging
 
 from django.contrib.contenttypes.models import ContentType
-from django.db import models, transaction
+from django.db import models
 from django.shortcuts import reverse
 from django.utils import timezone
 
@@ -152,43 +152,3 @@ class TrackChangesMixin(models.Model):
             self.name = markdown.sanitize(self.name)
             if update_fields is not None:
                 kwargs['update_fields'] = kwargs['update_fields'].union({'name'})
-
-
-class SoftDeleteMixin(models.Model):
-    """Model with soft-deletion functionality."""
-
-    class Meta:
-        abstract = True
-
-    date_deleted = models.DateTimeField(null=True, blank=True, editable=False)
-
-    @property
-    def is_deleted(self) -> bool:
-        return self.date_deleted is not None
-
-    @transaction.atomic
-    def delete(self, hard=False):
-        if hard:
-            super().delete()
-        else:
-            if hasattr(self, 'file'):
-                # TODO(oleg) move to an archived directory, add random suffix
-                self.file.delete()
-            self.date_deleted = timezone.now()
-            self.save()
-
-        logger.info('%r pk=%r deleted', self.__class__, self.pk)
-
-    def delete_queryset(self, request, queryset):
-        """Given a queryset, soft-delete it from the database."""
-        queryset.update(date_deleted=timezone.now())
-
-    def undelete(self, save=True):
-        if not self.date_deleted:
-            logger.warning('%r pk=%r is not deleted, cannot undelete', self.__class__, self.pk)
-            return
-        self.date_deleted = None
-        if save:
-            self.save()
-
-        logger.info('%r pk=%r undeleted', self.__class__, self.pk)

common/tests/factories/extensions.py

@@ -94,4 +94,5 @@ def create_version(**kwargs) -> 'Version':
 def create_approved_version(**kwargs) -> 'Version':
     version = create_version(**kwargs)
     version.extension.approve()
+    version.refresh_from_db()
     return version

extensions/admin.py

@@ -53,7 +53,6 @@ class ExtensionAdmin(admin.ModelAdmin):
         'date_created',
         'date_status_changed',
         'date_approved',
-        'date_deleted',
         'date_modified',
         'average_score',
         'text_ratings_count',
@@ -76,7 +75,6 @@ class ExtensionAdmin(admin.ModelAdmin):
                         'date_status_changed',
                         'date_approved',
                         'date_modified',
-                        'date_deleted',
                     ),
                     'name',
                     'slug',
@@ -135,7 +133,6 @@ class VersionAdmin(admin.ModelAdmin):
         'tagline',
         'date_created',
         'date_modified',
-        'date_deleted',
         'average_score',
         'download_count',
     )
@@ -147,7 +144,7 @@ class VersionAdmin(admin.ModelAdmin):
                 'fields': (
                     'id',
                     'tagline',
-                    ('date_created', 'date_modified', 'date_deleted'),
+                    ('date_created', 'date_modified'),
                     'extension',
                     'version',
                     'blender_version_min',
@@ -185,8 +182,8 @@ class VersionAdmin(admin.ModelAdmin):
 
 class MaintainerAdmin(admin.ModelAdmin):
     model = Maintainer
-    list_display = ('extension', 'user', 'date_deleted')
+    list_display = ('extension', 'user')
-    readonly_fields = ('extension', 'user', 'date_deleted')
+    readonly_fields = ('extension', 'user')
 
 
 class LicenseAdmin(admin.ModelAdmin):

extensions/forms.py

@@ -76,9 +76,6 @@ class AddPreviewFileForm(forms.ModelForm):
         ):
             logger.warning('Found an existing %s pk=%s', model, existing_image.pk)
             self.instance = existing_image
-            # Undelete the instance, if necessary
-            if self.instance.is_deleted:
-                self.instance.undelete(save=False)
 
         # Fill in missing fields from request and the source file
         self.instance.user = self.request.user

extensions/migrations/0026_remove_extension_date_deleted_and_more.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.11 on 2024-04-18 08:59
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('extensions', '0025_alter_tag_type'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='extension',
+            name='date_deleted',
+        ),
+        migrations.RemoveField(
+            model_name='maintainer',
+            name='date_deleted',
+        ),
+        migrations.RemoveField(
+            model_name='version',
+            name='date_deleted',
+        ),
+    ]

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, SoftDeleteMixin
+from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
 from constants.base import (
     AUTHOR_ROLE_CHOICES,
     AUTHOR_ROLE_DEV,
@@ -106,43 +106,33 @@ class License(CreatedModifiedMixin, models.Model):
 
 
 class ExtensionManager(models.Manager):
-    @property
-    def exclude_deleted(self):
-        return self.filter(date_deleted__isnull=True)
-
     @property
     def listed(self):
-        return self.exclude_deleted.filter(
+        return self.filter(
             status=self.model.STATUSES.APPROVED,
             is_listed=True,
         )
 
     @property
     def unlisted(self):
-        return self.exclude_deleted.exclude(status=self.model.STATUSES.APPROVED)
+        return self.exclude(status=self.model.STATUSES.APPROVED)
 
     def authored_by(self, user_id: int):
-        return self.exclude_deleted.filter(
+        return self.filter(maintainer__user_id=user_id)
-            maintainer__user_id=user_id, maintainer__date_deleted__isnull=True
-        )
 
     def listed_or_authored_by(self, user_id: int):
-        return self.exclude_deleted.filter(
+        return self.filter(
-            Q(status=self.model.STATUSES.APPROVED)
+            Q(status=self.model.STATUSES.APPROVED) | Q(maintainer__user_id=user_id)
-            | Q(maintainer__user_id=user_id, maintainer__date_deleted__isnull=True)
         ).distinct()
 
 
-class Extension(
+class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model):
-    CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMixin, models.Model
-):
     track_changes_to_fields = {
         'status',
         'name',
         'description',
         'support',
         'website',
-        'date_deleted',
     }
     TYPES = EXTENSION_TYPE_CHOICES
     STATUSES = EXTENSION_STATUS_CHOICES
@@ -176,7 +166,6 @@ class Extension(
         User,
         through='Maintainer',
         related_name='extensions',
-        q_filter=Q(maintainer__date_deleted__isnull=True),
     )
     team = models.ForeignKey('teams.Team', null=True, blank=True, on_delete=models.SET_NULL)
 
@@ -190,8 +179,7 @@ class Extension(
         ordering = ['-average_score', '-date_created', 'name']
 
     def __str__(self):
-        label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
+        return f'{self.get_type_display()} "{self.name}"'
-        return f'{self.get_type_display()} "{self.name}"{label_deleted}'
 
     @property
     def type_slug(self) -> str:
@@ -236,12 +224,19 @@ class Extension(
         self.status = self.STATUSES.APPROVED
         self.save()
 
-    @transaction.atomic
+    @property
-    def delete(self, hard=False):
+    def cannot_be_deleted_reasons(self) -> List[str]:
-        versions = self.versions.filter(date_deleted__isnull=True)
+        """Return a list of reasons why this extension cannot be deleted."""
-        for v in versions:
+        reasons = []
-            v.delete(hard=hard)
+        if self.is_listed:
-        super().delete(hard=hard)
+            reasons.append('is_listed')
+        if self.ratings.count() > 0:
+            reasons.append('has_ratings')
+        if self.abusereport_set.count() > 0:
+            reasons.append('has_abuse_reports')
+        for v in self.versions.all():
+            reasons.extend(v.cannot_be_deleted_reasons)
+        return reasons
 
     def get_absolute_url(self):
         return reverse('extensions:detail', args=[self.type_slug, self.slug])
@@ -295,7 +290,6 @@ class Extension(
         """Retrieve the latest version."""
         return (
             self.versions.filter(
-                date_deleted__isnull=True,
                 file__status__in=self.valid_file_statuses,
                 file__isnull=False,
             )
@@ -312,7 +306,7 @@ class Extension(
 
         If the add-on has not been created yet or is deleted, it returns None.
         """
-        if not self.id or self.is_deleted:
+        if not self.id:
             return None
         try:
             return self.version
@@ -322,14 +316,9 @@ class Extension(
 
     def can_request_review(self):
         """Return whether an add-on can request a review or not."""
-        if (
+        if self.is_disabled or self.status in (
-            self.is_deleted
+            self.STATUSES.APPROVED,
-            or self.is_disabled
+            self.STATUSES.AWAITING_REVIEW,
-            or self.status
-            in (
-                self.STATUSES.APPROVED,
-                self.STATUSES.AWAITING_REVIEW,
-            )
         ):
             return False
 
@@ -360,9 +349,7 @@ class Extension(
         """Return True if given user is listed as a maintainer."""
         if user is None or user.is_anonymous:
             return False
-        return self.authors.filter(
+        return self.authors.filter(maintainer__user_id=user.pk).exists()
-            maintainer__user_id=user.pk, maintainer__date_deleted__isnull=True
-        ).exists()
 
     def can_rate(self, user) -> bool:
         """Return True if given user can rate this extension.
@@ -430,17 +417,13 @@ class Tag(CreatedModifiedMixin, models.Model):
 
 
 class VersionManager(models.Manager):
-    @property
-    def exclude_deleted(self):
-        return self.filter(date_deleted__isnull=True)
-
     @property
     def listed(self):
-        return self.exclude_deleted.filter(file__status=FILE_STATUS_CHOICES.APPROVED)
+        return self.filter(file__status=FILE_STATUS_CHOICES.APPROVED)
 
     @property
     def unlisted(self):
-        return self.exclude_deleted.exclude(file__status=FILE_STATUS_CHOICES.APPROVED)
+        return self.exclude(file__status=FILE_STATUS_CHOICES.APPROVED)
 
     def update_or_create(self, *args, **kwargs):
         # Stash the ManyToMany to be created after the Version has a valid ID already
@@ -457,11 +440,10 @@ class VersionManager(models.Manager):
         return version, result
 
 
-class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
+class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model):
     track_changes_to_fields = {
         'blender_version_min',
         'blender_version_max',
-        'date_deleted',
         'permissions',
         'version',
         'licenses',
@@ -582,21 +564,22 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
             self.tags.add(tag)
 
     def __str__(self) -> str:
-        label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
+        return f'{self.extension} v{self.version}'
-        return f'{self.extension} v{self.version}{label_deleted}'
 
+    @property
     def is_listed(self):
-        # To be public, a version must not be deleted, must belong to a public
+        # To be public, version file must have a public status.
-        # extension, and its attached file must have a public status.
+        return self.file is not None and self.file.status == self.file.STATUSES.APPROVED
-        try:
+
-            return (
+    @property
-                not self.is_deleted
+    def cannot_be_deleted_reasons(self) -> List[str]:
-                and self.extension.is_listed
+        """Return a list of reasons why this version cannot be deleted."""
-                and self.file is not None
+        reasons = []
-                and self.file.status == self.file.STATUSES.APPROVED
+        if self.is_listed:
-            )
+            reasons.append('version_is_listed')
-        except models.ObjectDoesNotExist:
+        if self.ratings.count() > 0:
-            return False
+            reasons.append('version_has_ratings')
+        return reasons
 
     @property
     def pending_rejection(self):
@@ -656,7 +639,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, SoftDeleteMi
         )
 
 
-class Maintainer(CreatedModifiedMixin, SoftDeleteMixin, models.Model):
+class Maintainer(CreatedModifiedMixin, models.Model):
     extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
     user = models.ForeignKey(User, on_delete=models.CASCADE)
     role = models.SmallIntegerField(default=AUTHOR_ROLE_DEV, choices=AUTHOR_ROLE_CHOICES)
@@ -680,6 +663,11 @@ class Preview(CreatedModifiedMixin, models.Model):
         ordering = ('position', 'date_created')
         unique_together = [['extension', 'file']]
 
+    @property
+    def cannot_be_deleted_reasons(self) -> List[str]:
+        """Return a list of reasons why this preview cannot be deleted."""
+        return []
+
 
 class ExtensionReviewerFlags(models.Model):
     extension = models.OneToOneField(

extensions/signals.py

@@ -1,21 +1,42 @@
 from typing import Union
+import logging
 
 from actstream.actions import follow, unfollow
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group
-from django.db.models.signals import m2m_changed, pre_save, post_save, post_delete
+from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete, post_delete
 from django.dispatch import receiver
 
 from constants.activity import Flag
 import extensions.models
 import files.models
 
+logger = logging.getLogger(__name__)
 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:
+    cannot_be_deleted_reasons = instance.cannot_be_deleted_reasons
+    if len(cannot_be_deleted_reasons) > 0:
+        # This shouldn't happen: prior validation steps should have taken care of this.
+        # raise ValidationError({'__all__': cannot_be_deleted_reasons})
+        args = {'sender': sender, 'pk': instance.pk, 'reasons': cannot_be_deleted_reasons}
+        logger.error("%(sender)s pk=%(pk)s is being deleted but it %(reasons)s", args)
+
+    logger.info('Deleting %s pk=%s "%s"', sender, instance.pk, str(instance))
+
+
 @receiver(post_delete, sender=extensions.models.Preview)
-def _delete_file(sender: object, instance: extensions.models.Preview, **kwargs: object) -> None:
+@receiver(post_delete, sender=extensions.models.Version)
-    instance.file.delete()
+def _delete_file(sender: object, instance: object, **kwargs: object) -> None:
+    f = instance.file
+    args = {'f_id': f.pk, 'h': f.hash, 'pk': instance.pk, 'sender': sender, 's': f.source.name}
+    logger.info('Deleting file pk=%(f_id)s s=%(s)s hash=%(h)s linked to %(sender)s pk=%(pk)s', args)
+    f.delete()
+    # TODO: this doesn't mean that the file was deleted from disk
 
 
 @receiver(pre_save, sender=extensions.models.Extension)
@@ -45,7 +66,6 @@ def extension_should_be_listed(extension):
     return (
         extension.latest_version is not None
         and extension.latest_version.is_listed
-        and extension.latest_version.date_deleted is None
         and extension.status == extension.STATUSES.APPROVED
     )
 
@@ -80,6 +100,7 @@ def _set_is_listed(
     if extension.status == extensions.models.Extension.STATUSES.APPROVED and not new_is_listed:
         extension.status = extensions.models.Extension.STATUSES.INCOMPLETE
 
+    logger.info('Extension pk=%s becomes listed', extension.pk)
     extension.is_listed = new_is_listed
     extension.save()
 
@@ -118,3 +139,30 @@ def _update_authors_follow(instance, action, model, reverse, pk_set, **kwargs):
                 unfollow(user, extension, send_action=False, flag=Flag.AUTHOR)
             elif action == 'post_add':
                 follow(user, extension, send_action=False, flag=Flag.AUTHOR)
+
+
+@receiver(post_save, sender=extensions.models.Preview)
+@receiver(post_save, sender=extensions.models.Version)
+def _auto_approve_subsequent_uploads(
+    sender: object,
+    instance: Union[extensions.models.Preview, extensions.models.Version],
+    created: bool,
+    raw: bool,
+    **kwargs: object,
+):
+    if raw:
+        return
+    if not created:
+        return
+    if not instance.file_id:
+        return
+
+    # N.B.: currently, subsequent version and preview uploads get approved automatically,
+    # if extension is currently listed (meaning, it was approved by a human already).
+    extension = instance.extension
+    file = instance.file
+    if extension.is_listed:
+        file.status = files.models.File.STATUSES.APPROVED
+        args = {'f_id': file.pk, 'pk': instance.pk, 'sender': sender, 's': file.source.name}
+        logger.info('Auto-approving file pk=%(f_id)s of %(sender)s pk=%(pk)s source=%(s)s', args)
+        file.save(update_fields={'status', 'date_modified'})

extensions/templates/extensions/confirm_delete.html

@@ -9,8 +9,7 @@
         </h2>
         <p>
           {% blocktranslate with extension_name=extension_name %}
-          By deleting <strong>{{ extension_name }}</strong> you will lose the files,
+            All extension previews and versions files will be deleted.
-          download count, reviews as well ratings for the whole extension.
           {% endblocktranslate %}
         </p>
         <div class="btn-row-fluid">

extensions/templates/extensions/draft_finalise.html

@@ -90,6 +90,15 @@
                         <i class="i-send"></i>
                         <span>{% trans 'Submit for Approval' %}</span>
                       </button>
+
+                      {% with cannot_be_deleted_reasons=extension.cannot_be_deleted_reasons %}
+                        {% if not cannot_be_deleted_reasons %}
+                          <a href="{{ extension.get_delete_url }}" class="btn btn-danger">
+                            <i class="i-trash"></i>
+                            <span>{% trans 'Delete Extension' %}</span>
+                          </a>
+                        {% endif %}
+                      {% endwith %}
                     </div>
                   </section>
                 </div>

extensions/templates/extensions/version_confirm_delete.html

@@ -8,9 +8,8 @@
           {% blocktranslate with extension_name=extension_name %} Delete {{ extension_name }} {{ version }}?{% endblocktranslate %}
         </h2>
         <p>
-          {% blocktranslate with extension_name=extension_name version=version.version %}
+          {% blocktranslate with extension_name=extension_name %}
-          By deleting <strong>{{ extension_name }} {{ version }}</strong> you will lose the files,
+            Files belonging to this version will be deleted.
-          download count, reviews as well ratings for this version.
           {% endblocktranslate %}
         </p>
         <div class="btn-row-fluid">

extensions/templates/extensions/version_list.html

@@ -21,7 +21,7 @@
 
   <div class="row ext-version-history pb-3">
     <div class="col">
-      {% for version in extension.versions.exclude_deleted %}
+      {% for version in extension.versions.all %}
         {% if version.is_listed or is_maintainer %}
         <details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}">
           <summary>
@@ -103,10 +103,14 @@
                 <a href="{{ version.update_url }}" class="btn">
                   <i class="i-edit"></i><span>Edit</span>
                 </a>
-                <a href="{{ version.get_delete_url }}" class="btn btn-danger">
+                {% with cannot_be_deleted_reasons=version.cannot_be_deleted_reasons %}
-                  <i class="i-trash"></i>
+                  {% if not cannot_be_deleted_reasons %}
-                  <span>{% trans "Delete Version" %}</span>
+                    <a href="{{ version.get_delete_url }}" class="btn btn-danger">
-                </a>
+                      <i class="i-trash"></i>
+                      <span>{% trans "Delete Version" %}</span>
+                    </a>
+                  {% endif %}
+                {% endwith %}
                 {% endif %}
               </div>
             </div>

extensions/tests/test_delete.py

@@ -1,14 +1,34 @@
 from django.test import TestCase
 
-from common.tests.factories.extensions import create_approved_version
+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
+import extensions.models
+import files.models
 
 
 class DeleteTest(TestCase):
     fixtures = ['dev', 'licenses']
 
-    def test_happy_path(self):
+    def test_unlisted_unrated_extension_can_be_deleted_by_author(self):
-        extension = create_approved_version().extension
+        version = create_version(
+            file__status=files.models.File.STATUSES.AWAITING_REVIEW,
+            ratings=[],
+            extension__previews=[
+                FileFactory(
+                    type=files.models.File.TYPES.IMAGE,
+                    source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
+                )
+            ],
+        )
+        extension = version.extension
+        version_file = version.file
+        self.assertEqual(version_file.get_status_display(), 'Awaiting Review')
+        self.assertEqual(extension.get_status_display(), 'Incomplete')
+        self.assertFalse(extension.is_listed)
+        self.assertEqual(extension.cannot_be_deleted_reasons, [])
+        preview_file = extension.previews.first()
+        self.assertIsNotNone(preview_file)
 
         url = extension.get_delete_url()
         user = extension.authors.first()
@@ -16,9 +36,71 @@ class DeleteTest(TestCase):
         response = self.client.post(url)
 
         self.assertEqual(response.status_code, 302)
-        extension.refresh_from_db()
+        # All relevant records should have been deleted
-        self.assertIsNotNone(extension.date_deleted)
+        with self.assertRaises(extensions.models.Extension.DoesNotExist):
-        self.assertTrue(all(v.date_deleted is not None for v in extension.versions.all()))
+            extension.refresh_from_db()
+        with self.assertRaises(extensions.models.Version.DoesNotExist):
+            version.refresh_from_db()
+        with self.assertRaises(files.models.File.DoesNotExist):
+            version_file.refresh_from_db()
+        with self.assertRaises(files.models.File.DoesNotExist):
+            preview_file.refresh_from_db()
+        self.assertIsNone(extensions.models.Extension.objects.filter(pk=extension.pk).first())
+        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())
+        # 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):
+        version = create_approved_version(ratings=[])
+        self.assertTrue(version.is_listed)
+        extension = version.extension
+        self.assertTrue(extension.is_listed)
+        self.assertEqual(extension.get_status_display(), 'Approved')
+
+        self.assertEqual(version.cannot_be_deleted_reasons, ['version_is_listed'])
+        self.assertEqual(extension.cannot_be_deleted_reasons, ['is_listed', 'version_is_listed'])
+
+        url = extension.get_delete_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        response = self.client.post(url)
+
+        self.assertEqual(response.status_code, 403)
+
+    def test_rated_extension_cannot_be_deleted(self):
+        version = create_version(file__status=files.models.File.STATUSES.AWAITING_REVIEW)
+        self.assertFalse(version.is_listed)
+        extension = version.extension
+        self.assertFalse(extension.is_listed)
+        self.assertEqual(extension.get_status_display(), 'Incomplete')
+
+        self.assertEqual(version.cannot_be_deleted_reasons, ['version_has_ratings'])
+        self.assertEqual(
+            extension.cannot_be_deleted_reasons, ['has_ratings', 'version_has_ratings']
+        )
+
+        url = extension.get_delete_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        response = self.client.post(url)
+
+        self.assertEqual(response.status_code, 403)
+
+    def test_reported_extension_cannot_be_deleted(self):  # TODO
+        pass
+
+    def test_extension_with_ratings_cannot_be_deleted(self):
+        version = create_approved_version()
+        extension = version.extension
+        self.assertEqual(extension.status, extension.STATUSES.APPROVED)
+
+        url = extension.get_delete_url()
+        user = extension.authors.first()
+        self.client.force_login(user)
+        response = self.client.post(url)
+
+        self.assertEqual(response.status_code, 403)
 
     def test_random_user_cant_delete(self):
         extension = create_approved_version().extension
@@ -30,5 +112,3 @@ class DeleteTest(TestCase):
 
         self.assertEqual(response.status_code, 403)
         extension.refresh_from_db()
-        self.assertIsNone(extension.date_deleted)
-        self.assertTrue(all(v.date_deleted is None for v in extension.versions.all()))

extensions/tests/test_models.py

@@ -47,7 +47,6 @@ class ExtensionTest(TestCase):
                         'name': 'Extension name',
                         'status': 1,
                         'support': 'https://example.com/',
-                        'date_deleted': None,
                     },
                 }
             },

extensions/tests/test_submit.py

@@ -352,7 +352,7 @@ class SubmitFinaliseTest(TestCase):
         self.assertEqual(version.blender_version_max, None)
         self.assertEqual(version.schema_version, '1.0.0')
         self.assertEqual(version.release_notes, data['release_notes'])
-        self.assertEqual(version.file.get_status_display(), 'Approved')
+        self.assertEqual(version.file.get_status_display(), 'Awaiting Review')
         # We cannot check for the ManyToMany yet (tags, licences, permissions)
 
         # Check that author can access the page they are redirected to

extensions/tests/test_views.py

@@ -98,27 +98,6 @@ class ExtensionDetailViewTest(_BaseTestCase):
 
         self._check_detail_page(response, extension)
 
-    def test_cannot_view_deleted_extension_anonymously(self):
-        extension = _create_extension()
-        extension.delete()
-        self.assertTrue(extension.is_deleted)
-
-        url = extension.get_absolute_url()
-        response = self.client.get(url)
-
-        self.assertEqual(response.status_code, 404)
-
-    def test_can_view_deleted_extension_if_staff(self):
-        staff_user = UserFactory(is_staff=True)
-        extension = _create_extension()
-        extension.delete()
-        self.assertTrue(extension.is_deleted)
-
-        self.client.force_login(staff_user)
-        response = self.client.get(extension.get_absolute_url())
-
-        self._check_detail_page(response, extension)
-
     def test_can_view_unlisted_extension_if_maintaner(self):
         extension = _create_extension()
 
@@ -135,6 +114,14 @@ class ExtensionDetailViewTest(_BaseTestCase):
 
         self._check_detail_page(response, extension)
 
+    def test_can_view_publicly_listed_extension_versions_anonymously(self):
+        extension = _create_extension()
+        extension.approve()
+
+        response = self.client.get(extension.get_versions_url())
+
+        self._check_detail_page(response, extension)
+
     def test_can_view_publicly_listed_extension_ratings_anonymously(self):
         extension = _create_extension()
         extension.approve()
@@ -201,25 +188,11 @@ class ListedExtensionsTest(_BaseTestCase):
         create_approved_version(extension=self.extension)
         self.assertEqual(self._listed_extensions_count(), 1)
 
-    def test_delete_extension(self):
-        self.extension.delete()
-        self.assertEqual(self._listed_extensions_count(), 0)
-
     def test_moderate_extension(self):
         self.extension.status = Extension.STATUSES.DISABLED
         self.extension.save()
         self.assertEqual(self._listed_extensions_count(), 0)
 
-    def test_soft_delete_only_version(self):
-        self.version.date_deleted = '1994-01-02 0:0:0+00:00'
-        self.version.save()
-        self.assertFalse(self.extension.is_listed)
-        self.assertEqual(self._listed_extensions_count(), 0)
-
-    def test_delete_only_version(self):
-        self.version.delete()
-        self.assertEqual(self._listed_extensions_count(), 0)
-
     def test_moderate_only_version(self):
         self.version.file.status = File.STATUSES.DISABLED
         self.version.file.save()

extensions/views/manage.py

@@ -180,13 +180,21 @@ class DeleteExtensionView(
         return context
 
     def test_func(self) -> bool:
+        obj = self.get_object()
         # Only maintainers allowed
-        return self.get_object().has_maintainer(self.request.user)
+        if not obj.has_maintainer(self.request.user):
+            return False
+        # Unless this extension cannot be deleted anymore
+        cannot_be_deleted_reasons = obj.cannot_be_deleted_reasons
+        if len(cannot_be_deleted_reasons) > 0:
+            return False
+        return True
 
 
 class VersionDeleteView(
     LoginRequiredMixin,
     MaintainedExtensionMixin,
+    UserPassesTestMixin,
     DeleteView,
 ):
     model = Version
@@ -226,6 +234,14 @@ class VersionDeleteView(
         context['confirm_url'] = version.get_delete_url()
         return context
 
+    def test_func(self) -> bool:
+        obj = self.get_object()
+        # Unless this version cannot be deleted anymore
+        cannot_be_deleted_reasons = obj.cannot_be_deleted_reasons
+        if len(cannot_be_deleted_reasons) > 0:
+            return False
+        return True
+
 
 class ManageVersionsView(
     LoginRequiredMixin,
@@ -355,8 +371,9 @@ class DraftExtensionView(
     def get_initial(self):
         """Return initial values for the version, based on the file."""
         initial = super().get_initial()
-        initial['file'] = self.version.file
+        if self.version:
-        initial.update(**self.version.file.parsed_version_fields)
+            initial['file'] = self.version.file
+            initial.update(**self.version.file.parsed_version_fields)
         return initial
 
     def get_context_data(self, form=None, extension_form=None, add_preview_formset=None, **kwargs):

extensions/views/mixins.py

@@ -1,6 +1,5 @@
 from django.contrib.auth.mixins import UserPassesTestMixin
 from django.shortcuts import get_object_or_404, redirect
-from django.core.exceptions import BadRequest
 
 from extensions.models import Extension
 from files.models import File
@@ -51,9 +50,7 @@ class ExtensionMixin:
     """Fetch an extension by slug in the URL before dispatching the view."""
 
     def dispatch(self, *args, **kwargs):
-        self.extension = get_object_or_404(
+        self.extension = get_object_or_404(Extension, slug=self.kwargs['slug'])
-            Extension.objects.exclude_deleted, slug=self.kwargs['slug']
-        )
         return super().dispatch(*args, **kwargs)
 
 
@@ -62,9 +59,6 @@ class DraftVersionMixin:
 
     def dispatch(self, *args, **kwargs):
         self.version = self.extension.versions.first()
-        if self.version is None:
-            error_message = f'Extension "{self.extension.name}" has no versions.'
-            raise BadRequest(error_message)
         return super().dispatch(*args, **kwargs)
 
 

extensions/views/public.py

@@ -2,7 +2,6 @@ import logging
 
 from django.contrib.auth import get_user_model
 from django.db.models import Q
-from django.http import Http404
 from django.shortcuts import get_object_or_404, redirect
 from django.views.generic.list import ListView
 
@@ -54,8 +53,6 @@ class HomeView(ListedExtensionsView):
 def extension_version_download(request, type_slug, slug, version):
     """Download an extension version and count downloads."""
     extension_version = get_object_or_404(Version, extension__slug=slug, version=version)
-    if extension_version.date_deleted is not None:
-        raise Http404("This extension version has been deleted")
     ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id)
     VersionDownload.create_from_request(request, object_id=extension_version.pk)
     return redirect(extension_version.downloadable_signed_url)

files/admin.py

@@ -36,7 +36,6 @@ class FileAdmin(admin.ModelAdmin):
         'status',
         'date_status_changed',
         'date_approved',
-        'date_deleted',
     )
     list_display = ('original_name', 'extension', 'user', 'date_created', 'type', 'status', 'is_ok')
 
@@ -45,7 +44,6 @@ class FileAdmin(admin.ModelAdmin):
     readonly_fields = (
         'id',
         'date_created',
-        'date_deleted',
         'date_modified',
         'date_approved',
         'date_status_changed',
@@ -84,7 +82,6 @@ class FileAdmin(admin.ModelAdmin):
                     'date_modified',
                     'date_status_changed',
                     'date_approved',
-                    'date_deleted',
                 )
             },
         ),

files/migrations/0006_remove_file_date_deleted.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.11 on 2024-04-18 08:59
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('files', '0005_rename_validation_filevalidation_results_and_more'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='file',
+            name='date_deleted',
+        ),
+    ]

files/migrations/0007_alter_file_status.py

@@ -0,0 +1,47 @@
+# Generated by Django 4.2.11 on 2024-04-18 10:10
+import logging
+
+from django.db import migrations, models
+
+logger = logging.getLogger(__name__)
+
+
+def change_first_uploads_to_awaiting_review(apps, schema_editor):
+    from constants.base import FILE_STATUS_CHOICES
+
+    File = apps.get_model('files', 'File')
+    to_update = []
+    for obj in File.objects.filter(status=FILE_STATUS_CHOICES.APPROVED):
+        # Unless this is a subsequent upload of a version/preview of an already approved extension,
+        # it should have a default status (Awaiting Review).
+        extension = (
+            obj.extension_preview.first() and obj.extension_preview.first().extension
+            or (hasattr(obj, 'version') and obj.version.extension)
+        )
+        if extension.is_listed:
+            continue
+        logger.info(
+            'Will change file pk=%s status from %s to %s', obj.pk, obj.get_status_display(),
+            'Awaiting Review',
+        )
+        obj.status = FILE_STATUS_CHOICES.AWAITING_REVIEW
+        to_update.append(obj)
+
+    logger.info('Updating %s files', len(to_update))
+    File.objects.bulk_update(to_update, batch_size=500, fields={'status'})
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('files', '0006_remove_file_date_deleted'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='file',
+            name='status',
+            field=models.PositiveSmallIntegerField(choices=[(2, 'Awaiting Review'), (3, 'Approved'), (4, 'Disabled by staff'), (5, 'Disabled by author')], default=2),
+        ),
+        migrations.RunPython(change_first_uploads_to_awaiting_review),
+    ]

files/models.py

@@ -5,7 +5,7 @@ import logging
 from django.contrib.auth import get_user_model
 from django.db import models
 
-from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
+from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
 from files.utils import get_sha256, guess_mimetype_from_ext
 from constants.base import (
     FILE_STATUS_CHOICES,
@@ -48,8 +48,8 @@ def thumbnail_upload_to(instance, filename):
     return path
 
 
-class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
+class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
-    track_changes_to_fields = {'status', 'size_bytes', 'hash', 'date_deleted'}
+    track_changes_to_fields = {'status', 'size_bytes', 'hash'}
 
     TYPES = FILE_TYPE_CHOICES
     STATUSES = FILE_STATUS_CHOICES
@@ -76,7 +76,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
             'as guessed from its contents.'
         ),
     )
-    status = models.PositiveSmallIntegerField(choices=STATUSES, default=STATUSES.APPROVED)
+    status = models.PositiveSmallIntegerField(choices=STATUSES, default=STATUSES.AWAITING_REVIEW)
 
     user = models.ForeignKey(
         User, related_name='files', null=False, blank=False, on_delete=models.CASCADE
@@ -103,8 +103,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mode
     objects = FileManager()
 
     def __str__(self) -> str:
-        label_deleted = f'{self.date_deleted and " (DELETED ❌)" or ""}'
+        return f'{self.original_name} ({self.get_status_display()})'
-        return f'{self.original_name} ({self.get_status_display()}){label_deleted}'
 
     @property
     def has_been_validated(self):

files/signals.py

@@ -1,6 +1,6 @@
 import logging
 
-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
 
 import files.models
@@ -32,3 +32,8 @@ def _scan_new_file(
         return
 
     schedule_scan(instance)
+
+
+@receiver(pre_delete, sender=files.models.File)
+def _log_file_delete(sender: object, instance: files.models.File, **kwargs: object) -> None:
+    logger.info('Deleting file pk=%s source=%s', instance.pk, instance.source.name)

files/tests/test_models.py

@@ -45,7 +45,6 @@ class FileTest(TestCase):
                         'status': 2,
                         'hash': 'foobar',
                         'size_bytes': 7149,
-                        'date_deleted': None,
                     },
                 }
             },

ratings/admin.py

@@ -16,12 +16,12 @@ class RatingTypeFilter(admin.SimpleListFilter):
     parameter_name = 'type'
 
     def lookups(self, request, model_admin):
-        """
+        """Return a list of lookup option tuples.
-        Returns a list of tuples. The first element in each
+
-        tuple is the coded value for the option that will
+        The first element in each tuple is the coded value
-        appear in the URL query. The second element is the
+        for the option that will appear in the URL query.
-        human-readable name for the option that will appear
+        The second element is the human-readable name for
-        in the right sidebar.
+        the option that will appear in the right sidebar.
         """
         return (
             ('rating', 'User Rating'),
@@ -29,10 +29,10 @@ class RatingTypeFilter(admin.SimpleListFilter):
         )
 
     def queryset(self, request, queryset):
-        """
+        """Return the filtered queryset.
-        Returns the filtered queryset based on the value
+
-        provided in the query string and retrievable via
+        Filter based on the value provided in the query string
-        `self.value()`.
+        and retrievable via `self.value()`.
         """
         if self.value() == 'rating':
             return queryset.filter(reply_to__isnull=True)
@@ -53,7 +53,6 @@ class RatingAdmin(admin.ModelAdmin):
     readonly_fields = (
         'date_created',
         'date_modified',
-        'date_deleted',
         'extension',
         'version',
         'text',
@@ -63,7 +62,6 @@ class RatingAdmin(admin.ModelAdmin):
     fields = ('status',) + readonly_fields
     list_display = (
         'date_created',
-        'is_deleted',
         'user',
         'ip_address',
         'score',
@@ -71,7 +69,7 @@ class RatingAdmin(admin.ModelAdmin):
         'status',
         'truncated_text',
     )
-    list_filter = ('status', RatingTypeFilter, 'score', 'date_deleted')
+    list_filter = ('status', RatingTypeFilter, 'score')
     actions = ('delete_selected',)
     list_select_related = ('user',)  # For extension/reply_to see get_queryset()
 
@@ -94,11 +92,5 @@ class RatingAdmin(admin.ModelAdmin):
     is_reply.boolean = True
     is_reply.admin_order_field = 'reply_to'
 
-    def is_deleted(self, obj):
-        return bool(obj.date_deleted)
-
-    is_deleted.boolean = True
-    is_deleted.admin_order_field = 'date_deleted'
-
 
 admin.site.register(Rating, RatingAdmin)

ratings/migrations/0005_remove_rating_date_deleted.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.11 on 2024-04-18 08:59
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ratings', '0004_alter_rating_status'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='rating',
+            name='date_deleted',
+        ),
+    ]

ratings/models.py

@@ -1,12 +1,12 @@
 import logging
 
 from django.contrib.auth import get_user_model
-from django.db import models, transaction
+from django.db import models
 from django.template.defaultfilters import truncatechars
 from django.utils.translation import gettext_lazy as _
 from django.urls import reverse
 
-from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin
+from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
 from common.templatetags import common
 from constants.base import RATING_STATUS_CHOICES, RATING_SCORE_CHOICES
 from utils import send_mail
@@ -17,28 +17,20 @@ log = logging.getLogger(__name__)
 
 class RatingManager(models.Manager):
     # TODO: figure out how to retrieve reviews "annotated" with replies, if any
-    @property
-    def exclude_deleted(self):
-        return self.filter(date_deleted__isnull=True)
-
     @property
     def listed(self):
-        return self.exclude_deleted.filter(
+        return self.filter(status=self.model.STATUSES.APPROVED, reply_to__isnull=True)
-            status=self.model.STATUSES.APPROVED, reply_to__isnull=True
-        )
 
     @property
     def unlisted(self):
-        return self.exclude_deleted.exclude(
+        return self.exclude(status=self.models.STATUSES.APPROVED, reply_to__isnull=True)
-            status=self.models.STATUSES.APPROVED, reply_to__isnull=True
-        )
 
     @property
     def listed_texts(self):
         return self.listed.filter(text__isnull=False)
 
 
-class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Model):
+class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
     track_changes_to_fields = {'status'}
 
     STATUSES = RATING_STATUS_CHOICES
@@ -107,7 +99,7 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mo
     @classmethod
     def get_for(cls, user_id: int, extension_id: int):
         """Get rating left by a given user for a given extension."""
-        return cls.objects.exclude_deleted.filter(
+        return cls.objects.filter(
             reply_to=None,
             user_id=user_id,
             extension_id=extension_id,
@@ -141,25 +133,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, SoftDeleteMixin, models.Mo
         # user_responsible=user.
         self.save()
 
-    @transaction.atomic
-    def delete(self, user_responsible=None, send_post_save_signal=True):
-        if user_responsible is None:
-            user_responsible = self.user
-
-        for flag in self.ratingflag_set.all():
-            flag.delete()
-
-        SoftDeleteMixin.delete(self)
-        log.warning(
-            'Rating deleted: user pk=%s (%s) deleted id:%s by pk=%s (%s) ("%s")',
-            user_responsible.pk,
-            str(user_responsible),
-            self.pk,
-            self.user.pk,
-            str(self.user),
-            str(self.text),
-        )
-
     @classmethod
     def get_replies(cls, ratings):
         ratings = [r.id for r in ratings]

ratings/tests/test_views.py

@@ -27,12 +27,6 @@ class RatingsViewTest(TestCase):
                 text='this rating is also approved',
                 status=Rating.STATUSES.APPROVED,
             ),
-            RatingFactory(
-                version=version,
-                date_deleted='2022-01-01 01:01:01Z',
-                text='this rating is deleted',
-                status=Rating.STATUSES.APPROVED,
-            ),
         ]
 
         url = version.extension.get_ratings_url()

reviewers/tests/test_views.py

@@ -17,14 +17,6 @@ class CommentsViewTest(TestCase):
         self.assertEqual(r.status_code, 200)
         self.assertEqual(len(r.context['object_list']), 1)
 
-        # Deleted extensions don't show up in the approval queue
-        self.assertIsNone(self.default_version.extension.date_deleted)
-        self.default_version.extension.delete()
-        self.assertIsNotNone(self.default_version.extension.date_deleted)
-        r = self.client.get(reverse('reviewers:approval-queue'))
-        self.assertEqual(r.status_code, 200)
-        self.assertEqual(len(r.context['object_list']), 0)
-
     # Page is visible for every extension and does not require authentication
     def test_visibility(self):
         r = self.client.get(

reviewers/views.py

@@ -18,10 +18,8 @@ class ApprovalQueueView(ListView):
     paginate_by = 100
 
     def get_queryset(self):
-        return (
+        return Extension.objects.exclude(status=Extension.STATUSES.APPROVED).order_by(
-            Extension.objects.exclude_deleted
+            '-date_created'
-            .exclude(status=Extension.STATUSES.APPROVED)
-            .order_by('-date_created')
         )
 
     template_name = 'reviewers/extensions_review_list.html'