Skip to content

Fix supernav update on save #2941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JacobCoffee merged 3 commits into from
Feb 23, 2026
+83 −1
Merged

Fix supernav update on save #2941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
859bb08
save post, and piuuuurge
JacobCoffee Feb 23, 2026
53bca3f
be prettier
JacobCoffee Feb 23, 2026
d7cf0ff
test: add tests for ReleaseFile signal-triggered box updates
JacobCoffee Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion apps/downloads/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.conf import settings
from django.core.exceptions import ValidationError
from django.db import models
from django.db.models.signals import post_save
from django.db.models.signals import post_delete, post_save
from django.dispatch import receiver
from django.template.loader import render_to_string
from django.urls import reverse
Expand Down Expand Up @@ -332,6 +332,31 @@ def update_download_supernav_and_boxes(sender, instance, **kwargs):
update_homepage_download_box()


def _update_boxes_for_release_file(instance):
"""Update supernav and download boxes if the file's release is published."""
if instance.release_id and instance.release.is_published:
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_update_boxes_for_release_file() dereferences instance.release to check is_published, which can introduce an extra DB query on each ReleaseFile save/delete when the Release isn’t already cached. If this handler ends up being called frequently, consider checking publication status via a lightweight query (e.g., Release.objects.filter(pk=instance.release_id, is_published=True).exists()) to avoid loading the full Release object.

Suggested change
if instance.release_id and instance.release.is_published:
if instance.release_id and Release.objects.filter(pk=instance.release_id, is_published=True).exists():

Copilot uses AI. Check for mistakes.
update_supernav()
update_download_landing_sources_box()
update_homepage_download_box()
purge_url("/box/supernav-python-downloads/")
purge_url("/box/homepage-downloads/")
purge_url("/box/download-sources/")
Comment on lines +341 to +343
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of box purge URLs is duplicated here and in purge_fastly_download_pages() above. To avoid future drift (e.g., adding/removing a box in one place but not the other), consider centralizing these paths in a shared constant or a small helper used by both code paths.

Copilot uses AI. Check for mistakes.


@receiver(post_save, sender="downloads.ReleaseFile")
def update_boxes_on_release_file_save(sender, instance, **kwargs):
"""Refresh supernav when a release file is added or changed."""
if kwargs.get("raw", False):
return
_update_boxes_for_release_file(instance)


@receiver(post_delete, sender="downloads.ReleaseFile")
def update_boxes_on_release_file_delete(sender, instance, **kwargs):
"""Refresh supernav when a release file is deleted."""
_update_boxes_for_release_file(instance)


class ReleaseFile(ContentManageable, NameSlugModel):
"""Individual files in a release.

Expand Down
57 changes: 57 additions & 0 deletions apps/downloads/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import datetime as dt
from unittest.mock import patch

from apps.downloads.models import Release, ReleaseFile
from apps.downloads.tests.base import BaseDownloadTests
Expand Down Expand Up @@ -232,3 +233,59 @@ def test_update_supernav_skips_os_without_files(self):

# Android (no files) should not be present
self.assertNotIn("android", content.lower())

@patch("apps.downloads.models.update_supernav")
@patch("apps.downloads.models.update_download_landing_sources_box")
@patch("apps.downloads.models.update_homepage_download_box")
def test_release_file_save_triggers_box_updates(self, mock_home, mock_sources, mock_supernav):
"""Saving a ReleaseFile on a published release should update boxes."""
mock_supernav.reset_mock()
mock_sources.reset_mock()
mock_home.reset_mock()

ReleaseFile.objects.create(
os=self.windows,
release=self.python_3,
name="Windows installer",
url="/ftp/python/3.10.19/python-3.10.19.exe",
download_button=True,
)

mock_supernav.assert_called()
mock_sources.assert_called()
mock_home.assert_called()

@patch("apps.downloads.models.update_supernav")
@patch("apps.downloads.models.update_download_landing_sources_box")
@patch("apps.downloads.models.update_homepage_download_box")
def test_release_file_save_skips_unpublished_release(self, mock_home, mock_sources, mock_supernav):
"""Saving a ReleaseFile on a draft release should not update boxes."""
mock_supernav.reset_mock()
mock_sources.reset_mock()
mock_home.reset_mock()

ReleaseFile.objects.create(
os=self.windows,
release=self.draft_release,
name="Windows installer draft",
url="/ftp/python/9.7.2/python-9.7.2.exe",
)

mock_supernav.assert_not_called()
mock_sources.assert_not_called()
mock_home.assert_not_called()

@patch("apps.downloads.models.update_supernav")
@patch("apps.downloads.models.update_download_landing_sources_box")
@patch("apps.downloads.models.update_homepage_download_box")
def test_release_file_delete_triggers_box_updates(self, mock_home, mock_sources, mock_supernav):
"""Deleting a ReleaseFile on a published release should update boxes."""
mock_supernav.reset_mock()
mock_sources.reset_mock()
mock_home.reset_mock()

self.release_275_windows_32bit.delete()

mock_supernav.assert_called()
mock_sources.assert_called()
mock_home.assert_called()