From ff3360310bdafdbacff634c1fac2ff99d8031038 Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Sat, 16 May 2026 17:14:37 -0700 Subject: [PATCH] Fix: Defer password removal workflow action until the file is in place (#12814) --- src/documents/tests/test_workflows.py | 69 +++++++++++++++++++++++---- src/documents/workflows/actions.py | 15 +++--- 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 2e31608bb..1bfe294d0 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -4301,10 +4301,11 @@ class TestWorkflows( title="Protected", ) - document_consumption_finished.send( - sender=self.__class__, - document=doc, - ) + with self.captureOnCommitCallbacks(execute=True): + document_consumption_finished.send( + sender=self.__class__, + document=doc, + ) assert mock_remove_password.call_count == 2 mock_remove_password.assert_has_calls( @@ -4325,12 +4326,64 @@ class TestWorkflows( ) # ensure handler disconnected after first run - document_consumption_finished.send( - sender=self.__class__, - document=doc, - ) + with self.captureOnCommitCallbacks(execute=True): + document_consumption_finished.send( + sender=self.__class__, + document=doc, + ) assert mock_remove_password.call_count == 2 + @mock.patch("documents.bulk_edit.remove_password") + def test_password_removal_deferred_until_transaction_commit( + self, + mock_remove_password, + ) -> None: + """ + GIVEN: + - Workflow password removal action triggered during consumption + - document_consumption_finished signal fires inside a transaction + WHEN: + - Signal fires but transaction has not yet committed + THEN: + - Password removal is not attempted until the transaction commits + """ + from django.db import transaction + + action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.PASSWORD_REMOVAL, + passwords=["secret"], + ) + + temp_dir = Path(tempfile.mkdtemp()) + original_file = temp_dir / "file.pdf" + original_file.write_bytes(b"pdf content") + consumable = ConsumableDocument( + source=DocumentSource.ApiUpload, + original_file=original_file, + ) + + execute_password_removal_action(action, consumable, logging_group=None) + + doc = Document.objects.create( + checksum="pw-checksum-on-commit", + title="Protected", + ) + + with self.captureOnCommitCallbacks(execute=True): + with transaction.atomic(): + document_consumption_finished.send( + sender=self.__class__, + document=doc, + ) + mock_remove_password.assert_not_called() + + mock_remove_password.assert_called_once_with( + [doc.id], + password="secret", + update_document=True, + user=doc.owner, + ) + def test_workflow_trash_action_soft_delete(self) -> None: """ GIVEN: diff --git a/src/documents/workflows/actions.py b/src/documents/workflows/actions.py index b6adc35b7..948642dcb 100644 --- a/src/documents/workflows/actions.py +++ b/src/documents/workflows/actions.py @@ -4,6 +4,7 @@ from pathlib import Path from django.conf import settings from django.contrib.auth.models import User +from django.db import transaction from django.utils import timezone from documents.data_models import ConsumableDocument @@ -295,13 +296,15 @@ def execute_password_removal_action( # hook the consumption-finished signal to attempt password removal later def handler(sender, **kwargs): consumed_document: Document = kwargs.get("document") - if consumed_document is not None: - execute_password_removal_action( - action, - consumed_document, - logging_group, - ) document_consumption_finished.disconnect(handler) + if consumed_document is not None: + transaction.on_commit( + lambda: execute_password_removal_action( + action, + consumed_document, + logging_group, + ), + ) document_consumption_finished.connect(handler, weak=False) return