From 3e473c12b52b0eb89cb625074f84911370976bd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Lebleu?=
 <jeromelebleu@users.noreply.github.com>
Date: Thu, 12 May 2022 14:52:05 +0200
Subject: [PATCH] Fix #229 -- Delete old file after validation is passed

Previously files were deleted even if the from validation failed.

Co-Authored-By: codingjoe <johannes@maron.family>
---
 stdimage/models.py  | 15 ++++++++++++++-
 tests/forms.py      |  6 ++++++
 tests/models.py     |  6 +++++-
 tests/test_forms.py | 13 +++++++++++++
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/stdimage/models.py b/stdimage/models.py
index bed00d7..6e0083d 100644
--- a/stdimage/models.py
+++ b/stdimage/models.py
@@ -264,6 +264,9 @@ def __init__(
 
         super().__init__(verbose_name=verbose_name, name=name, **kwargs)
 
+        # The attribute name of the old file to use on the model object
+        self._old_attname = "_old_%s" % name
+
     def add_variation(self, name, params):
         variation = self.def_variation.copy()
         variation["kwargs"] = {}
@@ -313,9 +316,19 @@ def save_form_data(self, instance, data):
         if self.delete_orphans and (data is False or data is not None):
             file = getattr(instance, self.name)
             if file and file._committed and file != data:
-                file.delete(save=False)
+                # Store the old file which should be deleted if the new one is valid
+                setattr(instance, self._old_attname, file)
         super().save_form_data(instance, data)
 
+    def pre_save(self, model_instance, add):
+        if hasattr(model_instance, self._old_attname):
+            # Delete the old file and its variations from the storage
+            old_file = getattr(model_instance, self._old_attname)
+            old_file.delete_variations()
+            old_file.storage.delete(old_file.name)
+            delattr(model_instance, self._old_attname)
+        return super().pre_save(model_instance, add)
+
     def deconstruct(self):
         name, path, args, kwargs = super().deconstruct()
         return (
diff --git a/tests/forms.py b/tests/forms.py
index c70347d..ff3927f 100644
--- a/tests/forms.py
+++ b/tests/forms.py
@@ -7,3 +7,9 @@ class ThumbnailModelForm(forms.ModelForm):
     class Meta:
         model = models.ThumbnailModel
         fields = "__all__"
+
+
+class MinSizeModelForm(forms.ModelForm):
+    class Meta:
+        model = models.MinSizeModel
+        fields = "__all__"
diff --git a/tests/models.py b/tests/models.py
index 68a702f..4e91627 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -95,7 +95,11 @@ class MaxSizeModel(models.Model):
 
 
 class MinSizeModel(models.Model):
-    image = StdImageField(upload_to=upload_to, validators=[MinSizeValidator(200, 200)])
+    image = StdImageField(
+        upload_to=upload_to,
+        delete_orphans=True,
+        validators=[MinSizeValidator(200, 200)],
+    )
 
 
 class ForceMinSizeModel(models.Model):
diff --git a/tests/test_forms.py b/tests/test_forms.py
index 81b3360..03629b0 100644
--- a/tests/test_forms.py
+++ b/tests/test_forms.py
@@ -45,3 +45,16 @@ def test_save_form_data__none(self, db):
         obj = form.save()
         assert obj.image
         assert os.path.exists(org_path)
+
+    def test_save_form_data__invalid(self, db):
+        instance = models.MinSizeModel.objects.create(
+            image=self.fixtures["600x400.jpg"]
+        )
+        org_path = instance.image.path
+        assert os.path.exists(org_path)
+        form = forms.MinSizeModelForm(
+            files={"image": self.fixtures["100.gif"]},
+            instance=instance,
+        )
+        assert not form.is_valid()
+        assert os.path.exists(org_path)