extensions/migrations/0031_extension_latest_version.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.11 on 2024-05-24 17:06
+# Generated by Django 4.2.11 on 2024-05-27 12:42
 
 from django.db import migrations, models
 import django.db.models.deletion
@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
         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_for', to='extensions.version'),
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='latest_version_of', to='extensions.version'),
         ),
     ]

extensions/models.py

@@ -175,7 +175,7 @@ class Extension(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Mod
         null=True,
         blank=True,
         on_delete=models.SET_NULL,
-        related_name='latest_version_for',
+        related_name='latest_version_of',
     )
 
     featured_image = models.OneToOneField(
@@ -237,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
@@ -411,6 +407,22 @@ 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.all().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)
+
 
 class VersionPermission(CreatedModifiedMixin, models.Model):
     name = models.CharField(max_length=128, null=False, blank=False, unique=True)
@@ -603,7 +615,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]:
@@ -697,18 +709,8 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model
     @transaction.atomic
     def delete(self, *args, **kwargs):
         if self == self.extension.latest_version:
-            versions = self.extension.versions.all().order_by('-date_created')
-            latest_version = None
-            for version in versions:
-                if version == self:
-                    continue
-                if version.file.status not in self.extension.valid_file_statuses:
-                    continue
-                latest_version = version
-                break
-            self.extension.latest_version = latest_version
-            self.extension.save(update_fields={'latest_version'})
-            self.extension.update_metadata_from_version(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)
 
 

extensions/signals.py

@@ -59,17 +59,50 @@ def _record_changes(
 
 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
+        extension.status == extension.STATUSES.APPROVED
+        and extension.versions.filter(file__status=files.models.File.STATUSES.APPROVED).count() > 0
     )
 
 
-@receiver(post_save, sender=extensions.models.Extension)
+def update_is_listed(extension):
+    old_is_listed = extension.is_listed
+    new_is_listed = extension_should_be_listed(extension)
+
+    if old_is_listed == new_is_listed:
+        return
+
+    extension.is_listed = new_is_listed
+    update_fields = {'is_listed'}
+    if extension.status == extensions.models.Extension.STATUSES.APPROVED and not new_is_listed:
+        extension.status = extensions.models.Extension.STATUSES.DRAFT
+        update_fields.add('status')
+
+    extension.save(update_fields=update_fields)
+
+
+# TODO? split this out into version.approve that would take care of updating file.status and
+# recomputing extension.latest_version
 @receiver(post_save, sender=files.models.File)
-def _set_is_listed(
+def _update_version(
     sender: object,
-    instance: Union[extensions.models.Extension, files.models.File],
+    instance: files.models.File,
+    raw: bool,
+    *args: object,
+    **kwargs: object,
+) -> None:
+    if raw:
+        return
+
+    if hasattr(instance, 'version'):
+        extension = instance.version.extension
+        update_is_listed(extension)
+        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,
@@ -77,28 +110,8 @@ def _set_is_listed(
     if raw:
         return
 
-    if isinstance(instance, extensions.models.Extension):
     extension = instance
-    else:
-        # Since signals is called very early on, we can't assume file.extension will be available.
-        extension = instance.extension
-    if not extension:
-        return
-
-    # TODO if file was approved, we need to recompute latest_version
-
-    old_is_listed = extension.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()
+    update_is_listed(extension)
 
 
 @receiver(post_save, sender=extensions.models.Extension)