extensions/admin.py

@@ -83,6 +83,7 @@ class ExtensionAdmin(admin.ModelAdmin):
         'website',
         'icon',
         'featured_image',
+        'latest_version',
     )
     autocomplete_fields = ('team',)
 
@@ -104,6 +105,7 @@ class ExtensionAdmin(admin.ModelAdmin):
                     'description',
                     ('icon', 'featured_image'),
                     'status',
+                    'latest_version',
                 ),
             },
         ),

extensions/migrations/0031_extension_latest_version.py

@@ -0,0 +1,50 @@
+# Generated by Django 4.2.11 on 2024-05-27 12:42
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+from constants.base import (
+    EXTENSION_STATUS_CHOICES,
+    FILE_STATUS_CHOICES,
+)
+
+
+def valid_file_statuses(self):
+    if self.status == EXTENSION_STATUS_CHOICES.APPROVED:
+        return [FILE_STATUS_CHOICES.APPROVED]
+    return [FILE_STATUS_CHOICES.AWAITING_REVIEW, FILE_STATUS_CHOICES.APPROVED]
+
+
+def populate_latest_version(apps, schema_editor):
+    Extension = apps.get_model('extensions', 'Extension')
+    to_update = []
+    for extension in Extension.objects.prefetch_related(
+            'versions',
+            'versions__file',
+            ).all():
+        versions = extension.versions.all().order_by('-date_created')
+        latest_version = None
+        for version in versions:
+            if version.file.status not in valid_file_statuses(extension):
+                continue
+            latest_version = version
+            break
+        extension.latest_version = latest_version
+        to_update.append(extension)
+    Extension.objects.bulk_update(to_update, ['latest_version'])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('extensions', '0030_platform_version_platforms'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='extension',
+            name='latest_version',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='latest_version_of', to='extensions.version'),
+        ),
+        migrations.RunPython(populate_latest_version, reverse_code=migrations.RunPython.noop),
+    ]

extensions/models.py

@@ -19,6 +19,8 @@ from constants.base import (
     EXTENSION_TYPE_SLUGS_SINGULAR,
     FILE_STATUS_CHOICES,
 )
+from files.models import File
+from reviewers.models import ApprovalActivity
 import common.help_texts
 import extensions.fields
 
@@ -168,6 +170,13 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
         help_text='Whether the extension should be listed. It is kept in sync via signals.',
         default=False,
     )
+    latest_version = models.ForeignKey(
+        'Version',
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+        related_name='latest_version_of',
+    )
 
     featured_image = models.OneToOneField(
         'files.File',
@@ -228,10 +237,6 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
     def status_slug(self) -> str:
         return utils.slugify(EXTENSION_STATUS_CHOICES[self.status - 1][1])
 
-    def save(self, *args, **kwargs):
-        self.clean()
-        return super().save(*args, **kwargs)
-
     def update_metadata_from_version(self, version):
         update_fields = set()
         metadata = version.file.metadata
@@ -337,17 +342,6 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
             return [FILE_STATUS_CHOICES.APPROVED]
         return [FILE_STATUS_CHOICES.AWAITING_REVIEW, FILE_STATUS_CHOICES.APPROVED]
 
-    @property
-    def latest_version(self):
-        """Retrieve the latest version."""
-        versions = [
-            v for v in self.versions.all() if v.file and v.file.status in self.valid_file_statuses
-        ]
-        if not versions:
-            return None
-        versions = sorted(versions, key=lambda v: v.date_created, reverse=True)
-        return versions[0]
-
     def can_request_review(self):
         """Return whether an add-on can request a review or not."""
         if self.is_disabled or self.status in (
@@ -413,6 +407,40 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
             lookup_field = 'slug'
         return lookup_field
 
+    @transaction.atomic
+    def update_latest_version(self, skip_version=None):
+        versions = self.versions.select_related('file').order_by('-date_created')
+        latest_version = None
+        for version in versions:
+            if skip_version and version == skip_version:
+                continue
+            if version.file.status not in self.valid_file_statuses:
+                continue
+            latest_version = version
+            break
+        self.latest_version = latest_version
+        self.save(update_fields={'latest_version'})
+        if latest_version:
+            self.update_metadata_from_version(latest_version)
+
+    def update_is_listed(self):
+        should_be_listed = (
+            self.status == self.STATUSES.APPROVED
+            and self.versions.filter(file__status=File.STATUSES.APPROVED).count() > 0
+        )
+
+        # this method is called from post_save signal, this early return should prevent a loop
+        if self.is_listed == should_be_listed:
+            return
+
+        self.is_listed = should_be_listed
+        update_fields = {'is_listed'}
+        if self.status == self.STATUSES.APPROVED and not should_be_listed:
+            self.status = self.STATUSES.DRAFT
+            update_fields.add('status')
+
+        self.save(update_fields=update_fields)
+
 
 class VersionPermission(CreatedModifiedMixin, models.Model):
     name = models.CharField(max_length=128, null=False, blank=False, unique=True)
@@ -605,7 +633,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model
     @property
     def is_listed(self):
         # To be public, version file must have a public status.
-        return self.file is not None and self.file.status == self.file.STATUSES.APPROVED
+        return self.file.status == self.file.STATUSES.APPROVED
 
     @property
     def cannot_be_deleted_reasons(self) -> List[str]:
@@ -662,6 +690,41 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model
             },
         )
 
+    @transaction.atomic
+    def save(self, *args, **kwargs):
+        is_new = self.pk is None
+        update_fields = kwargs.get('update_fields', None)
+        was_changed, old_state = self.pre_save_record(update_fields=update_fields)
+        self.record_status_change(was_changed, old_state, **kwargs)
+
+        super().save(*args, **kwargs)
+
+        if not is_new:
+            return
+
+        # auto approving our file if extension is already listed (i.e. have been approved)
+        if self.extension.is_listed:
+            args = {'f_id': self.file.pk, 'pk': self.pk, 's': self.file.source.name}
+            log.info('Auto-approving file pk=%(f_id)s of Version pk=%(pk)s source=%(s)s', args)
+            self.file.status = File.STATUSES.APPROVED
+            self.file.save(update_fields={'status', 'date_modified'})
+
+            ApprovalActivity(
+                type=ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION,
+                user=self.file.user,
+                extension=self.extension,
+                message=f'uploaded new version: {self.version}',
+            ).save()
+
+        self.extension.update_latest_version()
+
+    @transaction.atomic
+    def delete(self, *args, **kwargs):
+        if self == self.extension.latest_version:
+            # make sure self is not a candidate for latest_version, since it's being deleted
+            self.extension.update_latest_version(skip_version=self)
+        super().delete(*args, **kwargs)
+
 
 class Maintainer(CreatedModifiedMixin, models.Model):
     extension = models.ForeignKey(Extension, on_delete=models.CASCADE)

extensions/signals.py

@@ -4,11 +4,11 @@ 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, pre_delete, post_delete
+from django.db import transaction
+from django.db.models.signals import m2m_changed, pre_save, post_save, pre_delete
 from django.dispatch import receiver
 
 from constants.activity import Flag
-from reviewers.models import ApprovalActivity
 import extensions.models
 import files.models
 
@@ -46,42 +46,24 @@ def _log_deletion(
 
 
 @receiver(pre_save, sender=extensions.models.Extension)
-@receiver(pre_save, sender=extensions.models.Version)
 def _record_changes(
     sender: object,
-    instance: Union[extensions.models.Extension, extensions.models.Version],
+    instance: extensions.models.Extension,
     update_fields: object,
     **kwargs: object,
 ) -> None:
     was_changed, old_state = instance.pre_save_record(update_fields=update_fields)
-
+    instance.sanitize('name', was_changed, old_state, **kwargs)
-    if hasattr(instance, 'name'):
+    instance.sanitize('description', was_changed, old_state, **kwargs)
-        instance.sanitize('name', was_changed, old_state, **kwargs)
-    if hasattr(instance, 'description'):
-        instance.sanitize('description', was_changed, old_state, **kwargs)
-
     instance.record_status_change(was_changed, old_state, **kwargs)
 
 
-@receiver(post_save, sender=extensions.models.Extension)
+# TODO? move this out into version.approve that would take care of updating file.status and
-def _update_search_index(sender, instance, **kw):
+# recomputing extension's is_listed and latest_version fields
-    pass  # TODO: update search index
-
-
-def extension_should_be_listed(extension):
-    return (
-        extension.latest_version is not None
-        and extension.latest_version.is_listed
-        and extension.status == extension.STATUSES.APPROVED
-    )
-
-
-@receiver(post_save, sender=extensions.models.Extension)
-@receiver(post_save, sender=extensions.models.Version)
 @receiver(post_save, sender=files.models.File)
-def _set_is_listed(
+def _update_version(
     sender: object,
-    instance: Union[extensions.models.Extension, extensions.models.Version, files.models.File],
+    instance: files.models.File,
     raw: bool,
     *args: object,
     **kwargs: object,
@@ -89,26 +71,27 @@ def _set_is_listed(
     if raw:
         return
 
-    if isinstance(instance, extensions.models.Extension):
+    if hasattr(instance, 'version'):
-        extension = instance
+        extension = instance.version.extension
-    else:
+        with transaction.atomic():
-        # Since signals is called very early on, we can't assume file.extension will be available.
+            # it's important to update is_listed before computing latest_version
-        extension = instance.extension
+            # because latest_version for listed and unlisted extensions are defined differently
-    if not extension:
+            extension.update_is_listed()
+            extension.update_latest_version()
+
+
+@receiver(post_save, sender=extensions.models.Extension)
+def _set_is_listed(
+    sender: object,
+    instance: extensions.models.Extension,
+    raw: bool,
+    *args: object,
+    **kwargs: object,
+) -> None:
+    if raw:
         return
 
-    old_is_listed = extension.is_listed
+    instance.update_is_listed()
-    new_is_listed = extension_should_be_listed(extension)
-
-    if old_is_listed == new_is_listed:
-        return
-
-    if extension.status == extensions.models.Extension.STATUSES.APPROVED and not new_is_listed:
-        extension.status = extensions.models.Extension.STATUSES.DRAFT
-
-    logger.info('Extension pk=%s becomes listed', extension.pk)
-    extension.is_listed = new_is_listed
-    extension.save()
 
 
 @receiver(post_save, sender=extensions.models.Extension)
@@ -148,10 +131,9 @@ def _update_authors_follow(instance, action, model, reverse, pk_set, **kwargs):
 
 
 @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],
+    instance: extensions.models.Preview,
     created: bool,
     raw: bool,
     **kwargs: object,
@@ -163,7 +145,7 @@ def _auto_approve_subsequent_uploads(
     if not instance.file_id:
         return
 
-    # N.B.: currently, subsequent version and preview uploads get approved automatically,
+    # N.B.: currently, subsequent preview uploads get approved automatically,
     # if extension is currently listed (meaning, it was approved by a human already).
     extension = instance.extension
     file = instance.file
@@ -172,45 +154,3 @@ def _auto_approve_subsequent_uploads(
         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'})
-
-
-@receiver(post_save, sender=extensions.models.Version)
-def _create_approval_activity_for_new_version_if_listed(
-    sender: object,
-    instance: extensions.models.Version,
-    created: bool,
-    raw: bool,
-    **kwargs: object,
-):
-    if raw:
-        return
-    if not created:
-        return
-    extension = instance.extension
-    if not extension.is_listed or not instance.file:
-        return
-    ApprovalActivity(
-        type=ApprovalActivity.ActivityType.UPLOADED_NEW_VERSION,
-        user=instance.file.user,
-        extension=instance.extension,
-        message=f'uploaded new version: {instance.version}',
-    ).save()
-
-
-@receiver(post_delete, sender=extensions.models.Version)
-@receiver(post_save, sender=extensions.models.Version)
-def _update_extension_metadata_from_latest_version(
-    sender: object,
-    instance: extensions.models.Version,
-    **kwargs: object,
-):
-    # this code will also be triggered when an extension is deleted
-    # and it deletes all related versions
-    extension = instance.extension
-    latest_version = extension.latest_version
-
-    # should check in case we are deleting the latest version, then no need to update anything
-    if not latest_version:
-        return
-
-    extension.update_metadata_from_version(latest_version)

extensions/tests/test_delete.py

@@ -111,6 +111,7 @@ class DeleteTest(TestCase):
                 'featured_image',
                 'icon',
                 'is_listed',
+                'latest_version',
                 'name',
                 'pk',
                 'slug',

extensions/tests/test_manifest.py

@@ -698,6 +698,7 @@ class VersionPermissionsTest(CreateFileTest):
         _file.status = File.STATUSES.APPROVED
         _file.save()
 
+        extension.refresh_from_db()
         self.assertNotEqual(version_original, extension.latest_version)
         self.assertEqual(Extension.objects.count(), 1)
         self.assertEqual(Version.objects.count(), 2)

extensions/views/public.py

@@ -37,13 +37,12 @@ class HomeView(ListedExtensionsView):
             .get_queryset()
             .prefetch_related(
                 'authors',
+                'latest_version__file',
+                'latest_version__tags',
                 'preview_set',
                 'preview_set__file',
                 'ratings',
                 'team',
-                'versions',
-                'versions__file',
-                'versions__tags',
             )
         )
         context['addons'] = q.filter(type=EXTENSION_TYPE_CHOICES.BPY)[:8]
@@ -77,7 +76,9 @@ class SearchView(ListedExtensionsView):
     def get_queryset(self):
         queryset = super().get_queryset()
         if self.kwargs.get('tag_slug'):
-            queryset = queryset.filter(versions__tags__slug=self.kwargs['tag_slug']).distinct()
+            queryset = queryset.filter(
+                latest_version__tags__slug=self.kwargs['tag_slug']
+            ).distinct()
         if self.kwargs.get('team_slug'):
             queryset = queryset.filter(team__slug=self.kwargs['team_slug'])
         if self.kwargs.get('user_id'):
@@ -95,18 +96,17 @@ class SearchView(ListedExtensionsView):
                     Q(slug__icontains=token)
                     | Q(name__icontains=token)
                     | Q(description__icontains=token)
-                    | Q(versions__tags__name__icontains=token)
+                    | Q(latest_version__tags__name__icontains=token)
                 )
             queryset = queryset.filter(search_query).distinct()
         return queryset.prefetch_related(
             'authors',
+            'latest_version__file',
+            'latest_version__tags',
             'preview_set',
             'preview_set__file',
             'ratings',
             'team',
-            'versions',
-            'versions__file',
-            'versions__tags',
         )
 
     def get_context_data(self, **kwargs):

reviewers/models.py

@@ -3,7 +3,6 @@ from django.db import models
 from django.utils.translation import gettext_lazy as _
 
 import common.help_texts
-from extensions.models import Extension
 from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin
 
 from constants.base import EXTENSION_TYPE_CHOICES
@@ -38,7 +37,7 @@ class ApprovalActivity(CreatedModifiedMixin, RecordDeletionMixin, models.Model):
 
     user = models.ForeignKey(User, on_delete=models.CASCADE, blank=True, null=True)
     extension = models.ForeignKey(
-        Extension,
+        'extensions.Extension',
         on_delete=models.CASCADE,
         related_name='review_activity',
     )