From 960c5f18e487cb7ab68e99d362f21c443d5d0788 Mon Sep 17 00:00:00 2001 From: Petr Ledvina Date: Thu, 17 Sep 2026 16:13:03 +0200 Subject: [PATCH] Preserve note image files when transactions roll back (#12867) --- src/backend/InvenTree/common/models.py | 6 ++- src/backend/InvenTree/common/tests.py | 56 ++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 5 deletions(-) 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

' + note.save() + part_pk, note_pk, image_pk = part.pk, note.pk, image.pk + original_content = note.content + image_name = image.image.name + + with self.captureOnCommitCallbacks(execute=True): + with self.assertRaisesMessage(ValueError, 'Abort transaction'): + with transaction.atomic(): + if action == 'edit': + note.content = '

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'' - note.save() + with self.captureOnCommitCallbacks(execute=True): + note.save() # The removed image must be gone from both the DB and the file system self.assertFalse(NotesImage.objects.filter(pk=ni2.pk).exists()) @@ -1820,7 +1868,8 @@ class NotesImageTest(InvenTreeAPITestCase): # Delete the *part*, not the note or image directly - this cascades # Part -> InvenTreeNoteMixin.delete() -> Note -> NotesImage - part.delete() + with self.captureOnCommitCallbacks(execute=True): + part.delete() self.assertFalse(NotesImage.objects.filter(pk=ni.pk).exists()) self.assertFalse(Note.objects.filter(pk=note.pk).exists()) @@ -1883,7 +1932,8 @@ class NotesImageTest(InvenTreeAPITestCase): # Deleting the source NotesImage must not remove the copied image # (files are independent; Django cascade does not call Python delete()) - ni.delete() + with self.captureOnCommitCallbacks(execute=True): + ni.delete() self.assertFalse(default_storage.exists(old_name)) self.assertTrue(default_storage.exists(new_img.image.name)) self.assertTrue(NotesImage.objects.filter(pk=new_img.pk).exists())