diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index 75512841d3..0c04beb744 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -3425,7 +3425,7 @@ class NotesImage(models.Model): @receiver(post_delete, sender=NotesImage, dispatch_uid='notesimage_post_delete') def after_notesimage_deleted(sender, instance, **kwargs): - """Remove the image file from storage once a NotesImage row is deleted. + """Remove the image file after the NotesImage deletion commits. A signal (rather than an overridden delete()) is required here: a NotesImage row is usually removed via a cascade - e.g. deleting its parent Note, or @@ -3435,7 +3435,9 @@ def after_notesimage_deleted(sender, instance, **kwargs): started from a single instance.delete() or a bulk QuerySet.delete(). """ if instance.image: - instance.image.delete(save=False) + transaction.on_commit( + lambda: instance.image.delete(save=False), using=kwargs.get('using') + ) class BarcodeScanResult(InvenTree.models.InvenTreeModel): diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index 1f9fe9935c..f32736b66d 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -17,6 +17,7 @@ from django.core.exceptions import ValidationError from django.core.files.base import ContentFile from django.core.files.storage import default_storage from django.core.files.uploadedfile import SimpleUploadedFile +from django.db import transaction from django.test import Client, TestCase from django.test.utils import override_settings from django.urls import reverse @@ -1691,6 +1692,52 @@ class CurrencyAPITests(InvenTreeAPITestCase): class NotesImageTest(InvenTreeAPITestCase): """Tests for uploading images to be used in markdown notes.""" + def test_rollback_preserves_image_files(self): + """Rolled-back note edits and cascaded deletions preserve image files.""" + for action in ['edit', 'delete_note', 'delete_part']: + with self.subTest(action=action): + part = Part.objects.create(name=f'Rollback {action}', active=False) + note = Note.objects.create( + model_type=ContentType.objects.get_for_model(Part), + model_id=part.pk, + title='Rollback image cleanup', + ) + with io.BytesIO() as buf: + Image.new('RGB', (2, 2)).save(buf, format='PNG') + image = NotesImage.objects.create( + note=note, + image=ContentFile( + buf.getvalue(), name=f'rollback_{action}.png' + ), + ) + note.content = f'
Image
Image removed
' + note.save() + elif action == 'delete_note': + note.delete() + else: + part.delete() + self.assertFalse( + NotesImage.objects.filter(pk=image_pk).exists() + ) + raise ValueError('Abort transaction') + + self.assertTrue(Part.objects.filter(pk=part_pk).exists()) + self.assertEqual(Note.objects.get(pk=note_pk).content, original_content) + self.assertEqual( + NotesImage.objects.get(pk=image_pk).image.name, image_name + ) + self.assertTrue(default_storage.exists(image_name)) + def test_invalid_files(self): """Test that invalid files are rejected.""" n = NotesImage.objects.count() @@ -1776,7 +1823,8 @@ class NotesImageTest(InvenTreeAPITestCase): # Remove the second image from the content and save note.content = f'