diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 21aef4cb6..2cf73ca41 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -535,7 +535,9 @@ def run_workflow( ): def assignment_action(): if action.assign_tags.all().count() > 0: - document.tags.add(*action.assign_tags.all()) + doc_tag_ids.extend( + list(action.assign_tags.all().values_list("pk", flat=True)), + ) if action.assign_correspondent is not None: document.correspondent = action.assign_correspondent @@ -641,12 +643,12 @@ def run_workflow( def removal_action(): if action.remove_all_tags: - document.tags.clear() + doc_tag_ids.clear() else: for tag in action.remove_tags.filter( pk__in=list(document.tags.values_list("pk", flat=True)), ).all(): - document.tags.remove(tag.pk) + doc_tag_ids.remove(tag.pk) if action.remove_all_correspondents or ( document.correspondent @@ -747,6 +749,7 @@ def run_workflow( # 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.all().values_list("pk", flat=True)) if matching.document_matches_workflow( document, workflow, @@ -765,7 +768,9 @@ def run_workflow( elif action.type == WorkflowAction.WorkflowActionType.REMOVAL: removal_action() + # save first before setting tags document.save() + document.tags.set(doc_tag_ids) @before_task_publish.connect diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index 1dfb9e47a..c410f76a2 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -1758,3 +1758,52 @@ class TestWorkflows(DirectoriesMixin, FileSystemAssertsMixin, APITestCase): info = cm.output[0] expected_str = f"Document matched {trigger} from {w}" self.assertIn(expected_str, info) + + def test_workflow_with_tag_actions_doesnt_overwrite_other_actions(self): + """ + GIVEN: + - Document updated workflow filtered by has tag with two actions, first adds owner, second removes a tag + WHEN: + - File that matches is consumed + THEN: + - Both actions are applied correctly + """ + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + ) + trigger.filter_has_tags.add(self.t1) + action1 = WorkflowAction.objects.create( + assign_owner=self.user2, + ) + action2 = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.REMOVAL, + ) + action2.remove_tags.add(self.t1) + w = Workflow.objects.create( + name="Workflow Add Owner and Remove Tag", + order=0, + ) + w.triggers.add(trigger) + w.actions.add(action1) + w.actions.add(action2) + w.save() + + doc = Document.objects.create( + title="sample test", + correspondent=self.c, + original_filename="sample.pdf", + ) + + superuser = User.objects.create_superuser("superuser") + self.client.force_authenticate(user=superuser) + + self.client.patch( + f"/api/documents/{doc.id}/", + {"tags": [self.t1.id, self.t2.id]}, + format="json", + ) + + doc.refresh_from_db() + self.assertEqual(doc.owner, self.user2) + self.assertEqual(doc.tags.all().count(), 1) + self.assertIn(self.t2, doc.tags.all())