Deleted release artifact files when they're cleared in admin

Author: bmispelon

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",

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):