From 85d2d47dc13d33fff0deeb7fd8ecd7bdb80c2e26 Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Sun, 3 Mar 2024 21:44:13 -0800
Subject: [PATCH] Apply suggestions from code review

---
 src/documents/consumer.py         | 33 ++++++++++++++++++++++++-------
 src/documents/signals/handlers.py |  4 +++-
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/documents/consumer.py b/src/documents/consumer.py
index be9aebc5d..1b1b1da0c 100644
--- a/src/documents/consumer.py
+++ b/src/documents/consumer.py
@@ -122,41 +122,60 @@ class WorkflowTriggerPlugin(
                         # Removal actions overwrite the current overrides
                         if action.remove_all_tags:
                             overrides.tag_ids = []
-                        elif action.remove_tags.all().count() > 0:
+                        elif (
+                            action.remove_tags.all().count() > 0
+                            and len(overrides.tag_ids) > 0
+                        ):
                             for tag in action.remove_tags.all():
-                                overrides.tag_ids.remove(tag.pk)
+                                if tag.pk in overrides.tag_ids:
+                                    overrides.tag_ids.remove(tag.pk)
 
                         if action.remove_all_correspondents:
                             overrides.correspondent_id = None
-                        elif action.remove_correspondents.all().count() > 0:
+                        elif (
+                            action.remove_correspondents.all().count() > 0
+                            and overrides.correspondent_id is not None
+                        ):
                             for correspondent in action.remove_correspondents.all():
                                 if overrides.correspondent_id == correspondent.pk:
                                     overrides.correspondent_id = None
 
                         if action.remove_all_document_types:
                             overrides.document_type_id = None
-                        elif action.remove_document_types.all().count() > 0:
+                        elif (
+                            action.remove_document_types.all().count() > 0
+                            and overrides.document_type_id is not None
+                        ):
                             for doc_type in action.remove_document_types.all():
                                 if overrides.document_type_id == doc_type.pk:
                                     overrides.document_type_id = None
 
                         if action.remove_all_storage_paths:
                             overrides.storage_path_id = None
-                        elif action.remove_storage_paths.all().count() > 0:
+                        elif (
+                            action.remove_storage_paths.all().count() > 0
+                            and overrides.storage_path_id is not None
+                        ):
                             for storage_path in action.remove_storage_paths.all():
                                 if overrides.storage_path_id == storage_path.pk:
                                     overrides.storage_path_id = None
 
                         if action.remove_all_custom_fields:
                             overrides.custom_field_ids = []
-                        elif action.remove_custom_fields.all().count() > 0:
+                        elif (
+                            action.remove_custom_fields.all().count() > 0
+                            and len(overrides.custom_field_ids) > 0
+                        ):
                             for field in action.remove_custom_fields.all():
                                 if field.pk in overrides.custom_field_ids:
                                     overrides.custom_field_ids.remove(field.pk)
 
                         if action.remove_all_owners:
                             overrides.owner_id = None
-                        elif action.remove_owners.all().count() > 0:
+                        elif (
+                            action.remove_owners.all().count() > 0
+                            and overrides.owner_id is not None
+                        ):
                             for owner in action.remove_owners.all():
                                 if overrides.owner_id == owner.pk:
                                     overrides.owner_id = None
diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py
index fc88eaf47..a0a004f5a 100644
--- a/src/documents/signals/handlers.py
+++ b/src/documents/signals/handlers.py
@@ -661,7 +661,9 @@ def run_workflow(
                     if action.remove_all_tags:
                         document.tags.clear()
                     elif action.remove_tags.all().count() > 0:
-                        document.tags.remove(*action.remove_tags.all())
+                        for tag in action.remove_tags.all():
+                            if tag in document.tags.all():
+                                document.tags.remove(tag)
 
                     if action.remove_all_correspondents or (
                         document.correspondent in action.remove_correspondents.all()