SIANXKE-380: avoid file deletion before Attachment removal

drf_attachments/handlers.py

@@ -1,5 +1,3 @@
-import os
-
 from django.db.models.signals import post_delete
 from django.dispatch import receiver
 

drf_attachments/models/models.py

@@ -145,7 +145,6 @@ def set_and_validate(self):
         self.set_default_context()  # set the default context if yet empty (and if default is defined)
         self.validate_file()  # validate the file and its mime_type, extension and size
         self.manage_uniqueness()  # remove any other Attachments for content_objects with
-        self.cleanup_file()  # remove the old file of a changed Attachment
 
     def set_default_context(self):
         """Set context to settings.ATTACHMENT_DEFAULT_CONTEXT (if defined) if it's still empty"""
@@ -313,20 +312,3 @@ def manage_uniqueness(self):
                 remove_file(attachment.file.path)
 
             to_delete.delete()
-
-    def cleanup_file(self):
-        """
-        If an Attachment is updated and receives a new file, remove the previous file from the storage
-        """
-        if self.pk:
-            try:
-                # on update delete the old file if a new one was inserted
-                # (delete_orphan only removes image on deletion of the whole attachment instance)
-                old_instance = Attachment.objects.get(pk=self.pk)
-
-                # on update delete the old file if a new one was inserted
-                if old_instance.file != self.file:
-                    remove_file(old_instance.file.path)
-            except Attachment.DoesNotExist:
-                # Do nothing if Attachment does not yet exist in DB
-                pass

drf_attachments/utils.py

@@ -36,6 +36,7 @@ def remove_file(file_path, raise_exceptions=False):
 def get_api_attachment_url(attachment_pk):
     return reverse("attachment-download", kwargs={"pk": attachment_pk})
 
+
 def get_admin_attachment_url(attachment_pk):
     return reverse(
         "admin:drf_attachments_attachment_download",

tests/testapp/tests/test_api.py

@@ -230,6 +230,7 @@ def test_unique_upload(self):
         self.assertEqual(1, attachments.count())
         attachment = attachments[0]
         first_attachment_file_path = attachment.file.path
+        self.assertTrue(os.path.isfile(first_attachment_file_path))
 
         # check diagram model
         diagram_attachments = self.diagram.attachments.all()
@@ -327,13 +328,17 @@ def test_attachment_object_is_deleted_with_content_object(self):
             content_object=self.file,
             file_name=DemoFile.PDF,
         )
+        attachment = Attachment.objects.first()
+        attachment_file_path = attachment.file.path
+        self.assertTrue(os.path.isfile(attachment_file_path))
 
         # delete the File model
         response = self.client.delete(f"/api/file/{self.file.pk}/")
         self.assertEqual(HTTP_204_NO_CONTENT, response.status_code, response.content)
 
-        # check that the attachment was deleted as well
+        # check that the attachment and its file were deleted as well
         self.assertEqual(0, len(Attachment.objects.all()))
+        self.assertFalse(os.path.isfile(attachment_file_path))
 
     def test_physical_file_is_deleted_with_content_object(self):
         # add an attachment to File model