Release report tweaks (#2001) (#2016)

Merging to test one unified PR

Author: daveoconnor

config/celery.py

@@ -98,7 +98,7 @@ def setup_periodic_tasks(sender, **kwargs):
     # Update data required for release report. Executes Saturday evenings.
     sender.add_periodic_task(
         crontab(day_of_week="sat", hour=20, minute=3),
-        app.signature("libraries.tasks.release_tasks", generate_report=True),
+        app.signature("libraries.tasks.release_tasks", generate_report=False),
     )
 
     # Update users' profile photos from GitHub. Executes daily at 3:30 AM.

config/settings.py

@@ -427,6 +427,7 @@
         ]
 
 
+ACCOUNT_DEFAULT_HTTP_PROTOCOL = "http"
 if not LOCAL_DEVELOPMENT:
     ACCOUNT_DEFAULT_HTTP_PROTOCOL = "https"
     SECURE_PROXY_SSL_HEADER = (

docs/release_reports.md

@@ -6,9 +6,9 @@
    1. Ask Sam for a copy of the "subscribe" data.
    2. In the Django admin interface go to "Subscription datas" under "MAILING_LIST".
    3. At the top of the page click on the "IMPORT 'SUBSCRIBE' DATA" button.
-2. To update the mailing list counts, if you haven't already run the "DO IT ALL" button:
-   1. Go to "Versions" under "VERSIONS" in the admin interface
-   2. At the top of the page click on the "DO IT ALL" button.
+2. To update the mailing list counts, if you haven't already run the "GET RELEASE REPORT DATA" button:
+   1. Go to "Release Reports" under "VERSIONS" in the admin interface
+   2. At the top of the page click on the "GET RELEASE REPORT DATA" button.
 
 ## Report Creation
 

libraries/admin.py

@@ -1,6 +1,7 @@
+import structlog
 from django.conf import settings
-from django.contrib import admin
-from django.core.files.storage import default_storage
+from django.contrib import admin, messages
+from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.db.models import F, Count, OuterRef, Window
 from django.db.models.functions import RowNumber
@@ -43,6 +44,9 @@
 from .utils import generate_release_report_filename
 
 
+logger = structlog.get_logger()
+
+
 @admin.register(Commit)
 class CommitAdmin(admin.ModelAdmin):
     list_display = ["library_version", "sha", "author"]
@@ -184,16 +188,39 @@ def get_context_data(self, **kwargs):
         return context
 
     def generate_report(self):
-        base_scheme = "http" if settings.LOCAL_DEVELOPMENT else "https"
+        uri = f"{settings.ACCOUNT_DEFAULT_HTTP_PROTOCOL}://{self.request.get_host()}"
         generate_release_report.delay(
             user_id=self.request.user.id,
             params=self.request.GET,
-            base_uri=f"{base_scheme}://{self.request.get_host()}",
+            base_uri=uri,
         )
 
+    def locked_publish_check(self):
+        form = self.get_form()
+        form.is_valid()
+        publish = form.cleaned_data["publish"]
+        report_configuration = form.cleaned_data["report_configuration"]
+        if publish and ReleaseReport.latest_published_locked(report_configuration):
+            msg = (
+                f"A release report already exists with locked status for "
+                f"{report_configuration.display_name}. Delete or unlock the most "
+                f"recent report."
+            )
+            raise ValueError(msg)
+
     def get(self, request, *args, **kwargs):
         form = self.get_form()
         if form.is_valid():
+            try:
+                self.locked_publish_check()
+            except ValueError as e:
+                messages.error(request, str(e))
+                return TemplateResponse(
+                    request,
+                    self.form_template,
+                    self.get_context_data(),
+                )
+
             if form.cleaned_data["no_cache"]:
                 params = request.GET.copy()
                 form.cache_clear()
@@ -462,28 +489,92 @@ class Meta:
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        if self.instance.pk and not self.instance.published:
-            file_name = generate_release_report_filename(
-                self.instance.report_configuration.get_slug()
+        if not self.is_publish_editable():
+            # we require users to intentionally manually delete existing reports
+            self.fields["published"].disabled = True
+            self.fields["published"].help_text = (
+                "⚠️ A published PDF already exists for this Report Configuration. See "
+                '"Publishing" notes at the top of this page.'
             )
-            published_filename = f"{ReleaseReport.upload_dir}{file_name}"
-            if default_storage.exists(published_filename):
-                # we require users to intentionally manually delete existing reports
-                self.fields["published"].disabled = True
-                self.fields["published"].help_text = (
-                    f"⚠️ A published '{file_name}' already exists. To prevent accidents "
-                    "you must manually delete that file before publishing this report."
+
+    def is_publish_editable(self) -> bool:
+        # in play here are currently published and previously published rows because of
+        # filename collision risk.
+        if self.instance.published:
+            return True
+
+        published_filename = generate_release_report_filename(
+            version_slug=self.instance.report_configuration.get_slug(),
+            published_format=True,
+        )
+        reports = ReleaseReport.objects.filter(
+            report_configuration=self.instance.report_configuration,
+            file=f"{ReleaseReport.upload_dir}{published_filename}",
+        )
+
+        if reports.count() == 0 or reports.latest("created_at") == self.instance:
+            return True
+
+        return False
+
+    def clean(self):
+        cleaned_data = super().clean()
+        if not self.is_publish_editable():
+            raise ValidationError("This file is not publishable.")
+        if cleaned_data.get("published"):
+            report_configuration = cleaned_data.get("report_configuration")
+            if ReleaseReport.latest_published_locked(
+                report_configuration, self.instance
+            ):
+                raise ValidationError(
+                    f"A release report already exists with locked status for "
+                    f"{report_configuration.display_name}. Delete or unlock the most "
+                    f"recent report."
                 )
 
+        return cleaned_data
+
 
 @admin.register(ReleaseReport)
 class ReleaseReportAdmin(admin.ModelAdmin):
     form = ReleaseReportAdminForm
-    list_display = ["__str__", "created_at", "published", "published_at"]
-    list_filter = ["published", ReportConfigurationFilter, StaffUserCreatedByFilter]
+    list_display = ["__str__", "created_at", "published", "published_at", "locked"]
+    list_filter = [
+        "published",
+        "locked",
+        ReportConfigurationFilter,
+        StaffUserCreatedByFilter,
+    ]
     search_fields = ["file"]
     readonly_fields = ["created_at", "created_by"]
     ordering = ["-created_at"]
+    change_list_template = "admin/releasereport_change_list.html"
+    change_form_template = "admin/releasereport_change_form.html"
+
+    def get_urls(self):
+        urls = super().get_urls()
+        my_urls = [
+            path(
+                "release_tasks/",
+                self.admin_site.admin_view(self.release_tasks),
+                name="release_tasks",
+            ),
+        ]
+        return my_urls + urls
+
+    def release_tasks(self, request):
+        from libraries.tasks import release_tasks
+
+        release_tasks.delay(
+            base_uri=f"{settings.ACCOUNT_DEFAULT_HTTP_PROTOCOL}://{request.get_host()}",
+            user_id=request.user.id,
+            generate_report=False,
+        )
+        self.message_user(
+            request,
+            "release_tasks has started, you will receive an email when the task finishes.",  # noqa: E501
+        )
+        return HttpResponseRedirect("../")
 
     def has_add_permission(self, request):
         return False
@@ -492,3 +583,25 @@ def save_model(self, request, obj, form, change):
         if not change:
             obj.created_by = request.user
         super().save_model(request, obj, form, change)
+
+    @staticmethod
+    def clear_other_report_files(release_report: ReleaseReport):
+        if release_report.file:
+            other_reports = ReleaseReport.objects.filter(
+                file=release_report.file.name
+            ).exclude(pk=release_report.pk)
+
+            if other_reports.exists():
+                release_report.file = None
+                release_report.save()
+
+    def delete_model(self, request, obj):
+        # check if another report uses the same file
+        self.clear_other_report_files(obj)
+        super().delete_model(request, obj)
+
+    def delete_queryset(self, request, queryset):
+        # clear file reference, prevents deletion of the file if it's linked elsewhere
+        for obj in queryset:
+            self.clear_other_report_files(obj)
+        super().delete_queryset(request, queryset)

libraries/forms.py

@@ -244,13 +244,25 @@ class CreateReportForm(CreateReportFullForm):
     """Form for creating a report for a specific release."""
 
     html_template_name = "admin/release_report_detail.html"
-
-    report_configuration = ModelChoiceField(
-        queryset=ReportConfiguration.objects.order_by("-version")
-    )
+    # queryset will be set in __init__
+    report_configuration = ModelChoiceField(queryset=ReportConfiguration.objects.none())
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        # we want to allow master, develop, the latest release, the latest beta, along
+        # with any report configuration matching no Version, exclude all others.
+        exclusion_versions = []
+        if betas := Version.objects.filter(beta=True).order_by("-release_date")[1:]:
+            exclusion_versions += betas
+        if older_releases := Version.objects.filter(
+            active=True, full_release=True
+        ).order_by("-release_date")[1:]:
+            exclusion_versions += older_releases
+        qs = ReportConfiguration.objects.exclude(
+            version__in=[v.name for v in exclusion_versions]
+        ).order_by("-version")
+
+        self.fields["report_configuration"].queryset = qs
         self.fields["library_1"].help_text = (
             "If none are selected, all libraries will be selected."
         )

libraries/migrations/0036_releasereport_locked.py

@@ -0,0 +1,21 @@
+# Generated by Django 5.2.7 on 2025-11-11 22:39
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("libraries", "0035_releasereport"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="releasereport",
+            name="locked",
+            field=models.BooleanField(
+                default=False,
+                help_text="Can't be overwritten during release report publish. Blocks task-based publishing.",
+            ),
+        ),
+    ]

libraries/models.py

@@ -1,3 +1,4 @@
+import os
 import re
 import uuid
 from datetime import timedelta
@@ -565,6 +566,10 @@ class ReleaseReport(models.Model):
 
     published = models.BooleanField(default=False)
     published_at = models.DateTimeField(blank=True, null=True)
+    locked = models.BooleanField(
+        default=False,
+        help_text="Can't be overwritten during release report publish. Blocks task-based publishing.",
+    )
 
     def __str__(self):
         return f"{self.file.name.replace(self.upload_dir, "")}"
@@ -589,17 +594,69 @@ def rename_file_to(self, filename: str, allow_overwrite: bool = False):
         default_storage.delete(current_name)
         self.file.name = final_filename
 
-    def save(self, allow_overwrite=False, *args, **kwargs):
-        super().save(*args, **kwargs)
+    def get_media_file(self):
+        return os.sep.join(
+            [
+                settings.MEDIA_URL.rstrip("/"),
+                self.file.name,
+            ]
+        )
 
+    @staticmethod
+    def latest_published_locked(
+        report_configuration: ReportConfiguration,
+        release_report_exclusion=None,
+    ) -> bool:
+        release_reports_qs = ReleaseReport.objects.filter(
+            report_configuration__version=report_configuration.version,
+            published=True,
+        )
+        if release_report_exclusion:
+            release_reports_qs = release_reports_qs.exclude(
+                pk=release_report_exclusion.id
+            )
+        if release_reports_qs:
+            return release_reports_qs.first().locked
+        return False
+
+    def unpublish_previous_reports(self):
+        for r in ReleaseReport.objects.filter(
+            report_configuration__version=self.report_configuration.version,
+            published=True,
+        ).exclude(pk=self.id):
+            r.published = False
+            r.save()
+
+    def save(self, allow_published_overwrite=False, *args, **kwargs):
+        """
+        Args:
+            allow_published_overwrite (bool): If True, allows overwriting of published
+                reports (locked checks still apply)
+            *args: Additional positional arguments passed to the superclass save method
+            **kwargs: Additional keyword arguments passed to the superclass save method
+
+        Raises:
+            ValueError: Raised if there is an existing locked release report for the configuration, preventing publication
+                        of another one without resolving the conflict.
+        """
         is_being_published = self.published and not self.published_at
+        if not is_being_published:
+            super().save(*args, **kwargs)
         if is_being_published and self.file:
+            if ReleaseReport.latest_published_locked(self.report_configuration, self):
+                msg = (
+                    f"A release report already exists with locked status for "
+                    f"{self.report_configuration.display_name}. Delete or unlock the "
+                    f"most recent report."
+                )
+                raise ValueError(msg)
+            self.unpublish_previous_reports()
             new_filename = generate_release_report_filename(
                 self.report_configuration.get_slug(), self.published
             )
-            self.rename_file_to(new_filename, allow_overwrite)
+            self.rename_file_to(new_filename, allow_published_overwrite)
             self.published_at = timezone.now()
-            super().save(update_fields=["published_at", "file"])
+            super().save()
 
 
 # Signal handler to delete files when ReleaseReport is deleted

libraries/tasks.py

@@ -316,9 +316,9 @@ def generate_release_report_pdf(
         release_report.file.save(filename, ContentFile(pdf_bytes), save=True)
         if publish:
             release_report.published = True
-            release_report.save(allow_overwrite=True)
-        logger.info(f"{release_report_id=} updated with PDF {filename=}")
-
+            release_report.save(allow_published_overwrite=True)
+    except ValueError as e:
+        logger.error(f"Failed to publish release: {e}")
     except Exception as e:
         logger.error(f"Failed to generate PDF: {e}", exc_info=True)
         raise

libraries/utils.py

@@ -368,6 +368,5 @@ def generate_release_report_filename(version_slug: str, published_format: bool =
     filename_data = ["release-report", version_slug]
     if not published_format:
         filename_data.append(datetime.now(timezone.utc).isoformat())
-
-    filename = f"{"-".join(filename_data)}.pdf"
+    filename = f"{'-'.join(filename_data)}.pdf"
     return filename