Fix: wait to apply tag changes until other changes saved with multiple workflows...

diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py
index 21aef4cb62bea46fe8f0204aa36dfc1fbd7634db..2cf73ca41c9de081c2741721ee0a84867b126ca8 100644 (file)
--- 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 1dfb9e47a716f04ea83bf6df9e333e9b7e25dd2f..c410f76a2c196ded4d73ea140a30541e86db42ac 100644 (file)
--- 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())