diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index f8d670e30..4ff5a2b33 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -818,7 +818,6 @@ def run_workflows( # Refresh this so the matching data is fresh and instance fields are re-freshed # Otherwise, this instance might be behind and overwrite the work another process did document.refresh_from_db() - doc_tag_ids = list(document.tags.values_list("pk", flat=True)) if matching.document_matches_workflow(document, workflow, trigger_type): action: WorkflowAction @@ -836,14 +835,13 @@ def run_workflows( apply_assignment_to_document( action, document, - doc_tag_ids, logging_group, ) elif action.type == WorkflowAction.WorkflowActionType.REMOVAL: if use_overrides and overrides: apply_removal_to_overrides(action, overrides) else: - apply_removal_to_document(action, document, doc_tag_ids) + apply_removal_to_document(action, document) elif action.type == WorkflowAction.WorkflowActionType.EMAIL: context = build_workflow_action_context(document, overrides) execute_email_action( @@ -886,7 +884,6 @@ def run_workflows( "modified", ], ) - document.tags.set(doc_tag_ids) WorkflowRun.objects.create( workflow=workflow, diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 924533698..e4cc85087 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -3512,6 +3512,124 @@ class TestWorkflows( as_json=False, ) + @mock.patch("documents.signals.handlers.execute_webhook_action") + def test_workflow_webhook_action_does_not_overwrite_concurrent_tags( + self, + mock_execute_webhook_action, + ): + """ + GIVEN: + - A document updated workflow with only a webhook action + - A tag update that happens after run_workflows + WHEN: + - The workflow runs + THEN: + - The concurrent tag update is preserved + """ + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + ) + webhook_action = WorkflowActionWebhook.objects.create( + use_params=False, + body="Test message: {{doc_url}}", + url="http://paperless-ngx.com", + include_document=False, + ) + action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.WEBHOOK, + webhook=webhook_action, + ) + w = Workflow.objects.create( + name="Webhook workflow", + order=0, + ) + w.triggers.add(trigger) + w.actions.add(action) + w.save() + + inbox_tag = Tag.objects.create(name="inbox") + error_tag = Tag.objects.create(name="error") + doc = Document.objects.create( + title="sample test", + correspondent=self.c, + original_filename="sample.pdf", + ) + doc.tags.add(inbox_tag) + + def add_error_tag(*args, **kwargs): + Document.objects.get(pk=doc.pk).tags.add(error_tag) + + mock_execute_webhook_action.side_effect = add_error_tag + + run_workflows(WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, doc) + + doc.refresh_from_db() + self.assertCountEqual(doc.tags.all(), [inbox_tag, error_tag]) + + @mock.patch("documents.signals.handlers.execute_webhook_action") + def test_workflow_tag_actions_do_not_overwrite_concurrent_tags( + self, + mock_execute_webhook_action, + ): + """ + GIVEN: + - A document updated workflow that clears tags and assigns an inbox tag + - A later tag update that happens before the workflow finishes + WHEN: + - The workflow runs + THEN: + - The later tag update is preserved + """ + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + ) + removal_action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.REMOVAL, + remove_all_tags=True, + ) + assign_action = WorkflowAction.objects.create( + assign_owner=self.user2, + ) + assign_action.assign_tags.add(self.t1) + webhook_action = WorkflowActionWebhook.objects.create( + use_params=False, + body="Test message: {{doc_url}}", + url="http://paperless-ngx.com", + include_document=False, + ) + notify_action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.WEBHOOK, + webhook=webhook_action, + ) + w = Workflow.objects.create( + name="Workflow tag race", + order=0, + ) + w.triggers.add(trigger) + w.actions.add(removal_action) + w.actions.add(assign_action) + w.actions.add(notify_action) + w.save() + + doc = Document.objects.create( + title="sample test", + correspondent=self.c, + original_filename="sample.pdf", + owner=self.user3, + ) + doc.tags.add(self.t2, self.t3) + + def add_error_tag(*args, **kwargs): + Document.objects.get(pk=doc.pk).tags.add(self.t2) + + mock_execute_webhook_action.side_effect = add_error_tag + + run_workflows(WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, doc) + + doc.refresh_from_db() + self.assertEqual(doc.owner, self.user2) + self.assertCountEqual(doc.tags.all(), [self.t1, self.t2]) + @override_settings( PAPERLESS_URL="http://localhost:8000", ) diff --git a/src/documents/workflows/mutations.py b/src/documents/workflows/mutations.py index ef85dba0f..2612202e6 100644 --- a/src/documents/workflows/mutations.py +++ b/src/documents/workflows/mutations.py @@ -16,7 +16,6 @@ logger = logging.getLogger("paperless.workflows.mutations") def apply_assignment_to_document( action: WorkflowAction, document: Document, - doc_tag_ids: list[int], logging_group, ): """ @@ -25,12 +24,7 @@ def apply_assignment_to_document( action: WorkflowAction, annotated with 'has_assign_*' boolean fields """ if action.has_assign_tags: - tag_ids_to_add: set[int] = set() - for tag in action.assign_tags.all(): - tag_ids_to_add.add(tag.pk) - tag_ids_to_add.update(int(pk) for pk in tag.get_ancestors_pks()) - - doc_tag_ids[:] = list(set(doc_tag_ids) | tag_ids_to_add) + document.add_nested_tags(action.assign_tags.all()) if action.assign_correspondent: document.correspondent = action.assign_correspondent @@ -197,7 +191,6 @@ def apply_assignment_to_overrides( def apply_removal_to_document( action: WorkflowAction, document: Document, - doc_tag_ids: list[int], ): """ Apply removal actions to a Document instance. @@ -206,14 +199,15 @@ def apply_removal_to_document( """ if action.remove_all_tags: - doc_tag_ids.clear() + document.tags.clear() else: tag_ids_to_remove: set[int] = set() for tag in action.remove_tags.all(): tag_ids_to_remove.add(tag.pk) tag_ids_to_remove.update(int(pk) for pk in tag.get_descendants_pks()) - doc_tag_ids[:] = [t for t in doc_tag_ids if t not in tag_ids_to_remove] + if tag_ids_to_remove: + document.tags.remove(*tag_ids_to_remove) if action.remove_all_correspondents or ( document.correspondent