diff --git a/releases/admin.py b/releases/admin.py index 106e87683..d0c2579d9 100644 --- a/releases/admin.py +++ b/releases/admin.py @@ -1,7 +1,33 @@ +from django import forms from django.contrib import admin from .models import Release +_ARTIFACTS = ["checksum", "tarball", "wheel"] + + +class ReleaseAdminForm(forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Add `accept` attributes to the artifact file fields to make it a bit + # easier to pick the right files in the browser's 'filepicker + extensions = {"tarball": ".tar.gz", "wheel": ".whl", "checksum": ".asc,.txt"} + for field, accept in extensions.items(): + widget = self.fields[field].widget + widget.attrs.setdefault("accept", accept) + + self._previous_file_fields = {a: getattr(self.instance, a) for a in _ARTIFACTS} + + # Extending _save_m2m() instead of save() lets us support both save(commit=True/False) + def _save_m2m(self): + super()._save_m2m() + + # Delete any files from storage that might have been cleared + for a in _ARTIFACTS: + if self._previous_file_fields[a] and not getattr(self.instance, a): + self._previous_file_fields[a].delete(save=False) + @admin.register(Release) class ReleaseAdmin(admin.ModelAdmin): @@ -10,6 +36,7 @@ class ReleaseAdmin(admin.ModelAdmin): ("Dates", {"fields": ["date", "eol_date"]}), ("Artifacts", {"fields": ["tarball", "wheel", "checksum"]}), ] + form = ReleaseAdminForm list_display = ( "version", "show_is_published", @@ -25,16 +52,6 @@ class ReleaseAdmin(admin.ModelAdmin): list_filter = ("status", "is_lts", "is_active") ordering = ("-major", "-minor", "-micro", "-status", "-iteration") - def get_form(self, request, obj=None, change=False, **kwargs): - form_class = super().get_form(request, obj=obj, change=change, **kwargs) - # Add `accept` attributes to the artifact file fields to make it a bit - # easier to pick the right files in the browser's 'filepicker - extensions = {"tarball": ".tar.gz", "wheel": ".whl", "checksum": ".asc,.txt"} - for field, accept in extensions.items(): - widget = form_class.base_fields[field].widget - widget.attrs.setdefault("accept", accept) - return form_class - @admin.display( description="status", ordering="status", diff --git a/releases/tests.py b/releases/tests.py index 093189bf0..957eddf8a 100644 --- a/releases/tests.py +++ b/releases/tests.py @@ -1,5 +1,8 @@ import datetime import re +import shutil +import tempfile +from pathlib import Path from django.contrib import admin from django.core.exceptions import ValidationError @@ -422,6 +425,43 @@ def test_file_upload_renames_correctly(self): ) self.assertEqual(release.checksum.name, "pgp/Django-1.2.3.checksum.txt") + def test_clearing_also_deletes_file(self, commit_save=True): + tempdir = Path(tempfile.mkdtemp(prefix="djangoprojectcom_")) + self.addCleanup(shutil.rmtree, tempdir, ignore_errors=True) + self.enterContext(override_settings(MEDIA_ROOT=tempdir)) + + files = { + "checksum": tempdir / "checksum.txt", + "tarball": tempdir / "tarball.tar.gz", + "wheel": tempdir / "wheel.whl", + } + # Create the files on disk: + for f in files.values(): + f.touch() + + release = Release.objects.create( + version="1.0", **{a: f.name for a, f in files.items()} + ) + data = {"version": "2.0", **{f"{a}-clear": True for a in files.keys()}} + form = self.form_class(instance=release, data=data) + self.assertTrue(form.is_valid(), form.errors) + form.save(commit=commit_save) + if not commit_save: + for artifact, tmpfile in files.items(): + with self.subTest(artifact=artifact): + self.assertTrue(tmpfile.exists()) + release.save() + form.save_m2m() + release.refresh_from_db() + + for artifact, tmpfile in files.items(): + with self.subTest(artifact=artifact): + self.assertFalse(getattr(release, artifact)) + self.assertFalse(tmpfile.exists()) + + def test_clearing_also_deletes_file_commit_false(self): + self.test_clearing_also_deletes_file(commit_save=False) + class RedirectViewTestCase(TestCase): def test_redirect(self):