From 49ff00fa85d040fbc437c22d42572db81814aebc Mon Sep 17 00:00:00 2001 From: Dave O'Connor Date: Tue, 18 Nov 2025 15:12:09 -0800 Subject: [PATCH 1/2] Move admin release report buttons from libraries to release report views. Refined release report publishing and added locking. Limited release report dropdown list. (#2001) --- config/celery.py | 2 +- docs/release_reports.md | 6 +- libraries/admin.py | 142 ++++++++++++++++-- libraries/forms.py | 20 ++- .../migrations/0036_releasereport_locked.py | 21 +++ libraries/models.py | 58 ++++++- libraries/tasks.py | 6 +- libraries/utils.py | 3 +- templates/admin/library_change_list.html | 1 - .../admin/releasereport_change_form.html | 16 ++ .../admin/releasereport_change_list.html | 17 +++ templates/admin/version_change_list.html | 6 - versions/admin.py | 19 +-- 13 files changed, 260 insertions(+), 57 deletions(-) create mode 100644 libraries/migrations/0036_releasereport_locked.py create mode 100644 templates/admin/releasereport_change_form.html create mode 100644 templates/admin/releasereport_change_list.html diff --git a/config/celery.py b/config/celery.py index 42834bab6..a23c34c3f 100644 --- a/config/celery.py +++ b/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. diff --git a/docs/release_reports.md b/docs/release_reports.md index cb089bb4d..be78da1fa 100644 --- a/docs/release_reports.md +++ b/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 diff --git a/libraries/admin.py b/libraries/admin.py index c9f36f42a..29478ff4d 100644 --- a/libraries/admin.py +++ b/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"] @@ -191,9 +195,32 @@ def generate_report(self): base_uri=f"{base_scheme}://{self.request.get_host()}", ) + 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,93 @@ 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 + + scheme = "http" if settings.LOCAL_DEVELOPMENT else "https" + release_tasks.delay( + base_uri=f"{scheme}://{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 +584,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) diff --git a/libraries/forms.py b/libraries/forms.py index da00b2140..86be35703 100644 --- a/libraries/forms.py +++ b/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." ) diff --git a/libraries/migrations/0036_releasereport_locked.py b/libraries/migrations/0036_releasereport_locked.py new file mode 100644 index 000000000..33acd0bc4 --- /dev/null +++ b/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.", + ), + ), + ] diff --git a/libraries/models.py b/libraries/models.py index d81edf478..5bbdc5c7f 100644 --- a/libraries/models.py +++ b/libraries/models.py @@ -565,6 +565,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 +593,61 @@ 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) - + @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 diff --git a/libraries/tasks.py b/libraries/tasks.py index 174b1edce..7b40fe47a 100644 --- a/libraries/tasks.py +++ b/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 diff --git a/libraries/utils.py b/libraries/utils.py index 6d204a55c..cdb88f029 100644 --- a/libraries/utils.py +++ b/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 diff --git a/templates/admin/library_change_list.html b/templates/admin/library_change_list.html index 3bad4eb61..0498a4bd4 100644 --- a/templates/admin/library_change_list.html +++ b/templates/admin/library_change_list.html @@ -7,7 +7,6 @@ {{ block.super }}
  • {% trans "Update Library Data" %}
  • {% trans "Update Authors & Maintainers" %}
    • -
  • {% trans "Get Release Report" %}
  • {% trans "Get Library Report" %}
    • {% endblock %} diff --git a/templates/admin/releasereport_change_form.html b/templates/admin/releasereport_change_form.html new file mode 100644 index 000000000..781b2abb2 --- /dev/null +++ b/templates/admin/releasereport_change_form.html @@ -0,0 +1,16 @@ +{% extends "admin/change_form.html" %} + +{% block content %} +
    + Publishing: + +
    +{{ block.super }} +{% endblock %} diff --git a/templates/admin/releasereport_change_list.html b/templates/admin/releasereport_change_list.html new file mode 100644 index 000000000..8270ac2c4 --- /dev/null +++ b/templates/admin/releasereport_change_list.html @@ -0,0 +1,17 @@ +{% extends "admin/change_list.html" %} +{% load i18n admin_urls %} + +{% block object-tools %} + +{% endblock %} diff --git a/templates/admin/version_change_list.html b/templates/admin/version_change_list.html index 4555a89f6..4a3650296 100644 --- a/templates/admin/version_change_list.html +++ b/templates/admin/version_change_list.html @@ -6,12 +6,6 @@ {% block object-tools-items %} {{ block.super }}
  • {% trans "Import New Releases" %}
    • -
  • - {% trans "Do it all" %} - -
    • {% endblock %} {% endblock %} diff --git a/versions/admin.py b/versions/admin.py index 23583e4f6..47e694511 100755 --- a/versions/admin.py +++ b/versions/admin.py @@ -4,7 +4,7 @@ from django.http import HttpRequest, HttpResponseRedirect from django.urls import path -from libraries.tasks import release_tasks, import_new_versions_tasks +from libraries.tasks import import_new_versions_tasks from . import models @@ -35,26 +35,9 @@ def get_urls(self): self.admin_site.admin_view(self.import_new_releases), name="import_new_releases", ), - path( - "release_tasks/", - self.admin_site.admin_view(self.release_tasks), - name="release_tasks", - ), ] return my_urls + urls - def release_tasks(self, request): - release_tasks.delay( - base_uri=f"https://{request.get_host()}", - user_id=request.user.id, - generate_report=True, - ) - self.message_user( - request, - "release_tasks has started, you will receive an email when the task finishes.", # noqa: E501 - ) - return HttpResponseRedirect("../") - def import_new_releases(self, request): import_new_versions_tasks.delay(user_id=request.user.id) msg = "New releases are being imported. You will receive an email when the task finishes." # noqa: E501 From 76dfef3fe90ad4c7d5fce59b37988e4de389b9a7 Mon Sep 17 00:00:00 2001 From: daveoconnor Date: Thu, 20 Nov 2025 11:24:24 -0800 Subject: [PATCH 2/2] Release page report link (#1895) (#2018) Merging to test one unified PR --- config/settings.py | 1 + libraries/admin.py | 7 +++---- libraries/models.py | 9 +++++++++ reports/generation.py | 22 +++++++++++++++------- templates/versions/detail.html | 9 +++++++++ versions/views.py | 21 ++++++++++++++++++--- 6 files changed, 55 insertions(+), 14 deletions(-) diff --git a/config/settings.py b/config/settings.py index 0509bec71..66952d9ea 100755 --- a/config/settings.py +++ b/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 = ( diff --git a/libraries/admin.py b/libraries/admin.py index 29478ff4d..9a5c78f00 100644 --- a/libraries/admin.py +++ b/libraries/admin.py @@ -188,11 +188,11 @@ 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): @@ -565,9 +565,8 @@ def get_urls(self): def release_tasks(self, request): from libraries.tasks import release_tasks - scheme = "http" if settings.LOCAL_DEVELOPMENT else "https" release_tasks.delay( - base_uri=f"{scheme}://{request.get_host()}", + base_uri=f"{settings.ACCOUNT_DEFAULT_HTTP_PROTOCOL}://{request.get_host()}", user_id=request.user.id, generate_report=False, ) diff --git a/libraries/models.py b/libraries/models.py index 5bbdc5c7f..9c6a68845 100644 --- a/libraries/models.py +++ b/libraries/models.py @@ -1,3 +1,4 @@ +import os import re import uuid from datetime import timedelta @@ -593,6 +594,14 @@ def rename_file_to(self, filename: str, allow_overwrite: bool = False): default_storage.delete(current_name) self.file.name = final_filename + def get_media_file(self): + return os.sep.join( + [ + settings.MEDIA_URL.rstrip("/"), + self.file.name, + ] + ) + @staticmethod def latest_published_locked( report_configuration: ReportConfiguration, diff --git a/reports/generation.py b/reports/generation.py index d4715a13c..d50d37fff 100644 --- a/reports/generation.py +++ b/reports/generation.py @@ -61,9 +61,13 @@ def generate_algolia_words( "index": version.stripped_boost_url_slug, "limit": 100, } - search_results = client.get_top_searches(**args).to_json() - search_data = json.loads(search_results) - return {r["search"]: r["count"] for r in search_data["searches"] if r["count"] > 1} + try: + search_results = client.get_top_searches(**args).to_json() + search_data = json.loads(search_results) + searches = search_data.get("searches") or [] + return {r["search"]: r["count"] for r in searches if r["count"] > 1} + except ValueError: + return {} def generate_wordcloud( @@ -173,10 +177,14 @@ def get_algolia_search_stats(client: AnalyticsClientSync, version: Version) -> d # search data search_response = client.get_searches_count(**default_args).to_json() search_data = json.loads(search_response) - # country data - country_results = client.get_top_countries(**default_args, limit=100).to_json() - country_data = json.loads(country_results) - country_stats = {r["country"]: r["count"] for r in country_data["countries"]} + try: + # country data + country_results = client.get_top_countries(**default_args, limit=100).to_json() + country_data = json.loads(country_results) + countries = country_data.get("countries") or [] + country_stats = {r["country"]: r["count"] for r in countries} + except ValueError: + country_stats = {} return { "total_searches": search_data.get("count"), "country_stats": country_stats, diff --git a/templates/versions/detail.html b/templates/versions/detail.html index ee502da6f..277f06f30 100644 --- a/templates/versions/detail.html +++ b/templates/versions/detail.html @@ -51,6 +51,15 @@ {{ version.github_url|cut:"https://" }} + {% if release_report_url %} +
    + + Release Report + {{ release_report_file_name }} + +
    + {% endif %} {% if not version.beta %} {% if deps.added or deps.removed %} diff --git a/versions/views.py b/versions/views.py index 216928826..dfe4b9242 100755 --- a/versions/views.py +++ b/versions/views.py @@ -17,7 +17,7 @@ from core.models import RenderedContent from libraries.constants import LATEST_RELEASE_URL_PATH_STR from libraries.mixins import VersionAlertMixin, BoostVersionMixin -from libraries.models import Commit, CommitAuthor +from libraries.models import Commit, CommitAuthor, ReleaseReport from libraries.tasks import generate_release_report from libraries.utils import ( set_selected_boost_version, @@ -70,6 +70,10 @@ def get_context_data(self, **kwargs): context["top_contributors_release"] = self.get_top_contributors_release(obj) context["documentation_url"] = obj.documentation_url + report_file_info = self.get_release_report_info() + if report_file_info: + context["release_report_file_name"] = report_file_info["file_name"] + context["release_report_url"] = report_file_info["file_path"] try: context["deps"] = self.get_library_version_dependencies(obj) except BoostImportedDataException: @@ -83,6 +87,18 @@ def get_context_data(self, **kwargs): context["version_alert"] = False return context + def get_release_report_info(self) -> dict | None: + try: + if report := ReleaseReport.objects.get( + report_configuration__version=self.object.name, published=True + ): + return { + "file_name": report.file.name.replace(ReleaseReport.upload_dir, ""), + "file_path": report.get_media_file(), + } + except ReleaseReport.DoesNotExist: + return {} + def get_library_version_dependencies(self, version: Version): diffs = version.get_dependency_diffs() added = [len(x["added"]) for x in diffs.values() if x["added"]] @@ -221,11 +237,10 @@ def get(self, request, *args, **kwargs): version_name = version.name cache_key = f"release-report-,,,,,,,-{version_name}" RenderedContent.objects.filter(cache_key=cache_key).delete() - scheme = "http" if settings.LOCAL_DEVELOPMENT else "https" generate_release_report.delay( user_id=request.user.id, params={"version": version.id}, - base_uri=f"{scheme}://{request.get_host()}", + base_uri=f"{settings.ACCOUNT_DEFAULT_HTTP_PROTOCOL}://{request.get_host()}", ) messages.success(request, "Report generation queued.") return redirect("release-report-preview", version_slug=version_name)