diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 94504fcd0..648052797 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1247,30 +1247,31 @@ class DocumentSerializer( validated_data["tags"] = list(final_tags) if validated_data.get("remove_inbox_tags"): - tag_ids_being_added = ( - [ - tag.id - for tag in validated_data["tags"] - if tag not in instance.tags.all() - ] + current_tag_ids = {t.pk for t in instance.tags.all()} + tags = ( + validated_data["tags"] if "tags" in validated_data - else [] + else list(instance.tags.all()) ) - inbox_tags_not_being_added = Tag.objects.filter(is_inbox_tag=True).exclude( - id__in=tag_ids_being_added, - ) - if "tags" in validated_data: - validated_data["tags"] = [ - tag - for tag in validated_data["tags"] - if tag not in inbox_tags_not_being_added - ] - else: - validated_data["tags"] = [ - tag - for tag in instance.tags.all() - if tag not in inbox_tags_not_being_added - ] + + # Tags newly added in this update, plus their ancestors, are kept + keep_ids: set[int] = set() + for tag in tags: + if tag.pk not in current_tag_ids: + keep_ids.add(tag.pk) + keep_ids.update(int(pk) for pk in tag.get_ancestors_pks()) + + # Remove inbox tags and their descendants, except those being kept + remove_ids: set[int] = set() + for inbox_tag in ( + Tag.objects.filter(is_inbox_tag=True) + .exclude(pk__in=keep_ids) + .only("pk", "tn_descendants_pks") + ): + remove_ids.add(inbox_tag.pk) + remove_ids.update(int(pk) for pk in inbox_tag.get_descendants_pks()) + + validated_data["tags"] = [t for t in tags if t.pk not in remove_ids] if settings.AUDIT_LOG_ENABLED: with set_actor(self.user): diff --git a/src/documents/tests/test_tag_hierarchy.py b/src/documents/tests/test_tag_hierarchy.py index b439d5d70..ed0bff351 100644 --- a/src/documents/tests/test_tag_hierarchy.py +++ b/src/documents/tests/test_tag_hierarchy.py @@ -2,6 +2,7 @@ from unittest import mock from django.contrib.auth.models import Permission from django.contrib.auth.models import User +from rest_framework import status from rest_framework.test import APITestCase from documents import bulk_edit @@ -108,6 +109,44 @@ class TestTagHierarchy(DirectoriesMixin, APITestCase): self.document.refresh_from_db() assert self.document.tags.count() == 0 + def test_remove_inbox_tags_removes_nested_children(self) -> None: + inbox = Tag.objects.create(name="Inbox", is_inbox_tag=True) + nested = Tag.objects.create(name="Nested", tn_parent=inbox) + self.document.add_nested_tags([nested]) + + resp = self.client.patch( + f"/api/documents/{self.document.pk}/", + {"title": "new title", "remove_inbox_tags": True}, + format="json", + ) + assert resp.status_code == status.HTTP_200_OK + self.document.refresh_from_db() + assert self.document.tags.count() == 0 + + # A subsequent save must not re-add the inbox tag as an ancestor + resp = self.client.patch( + f"/api/documents/{self.document.pk}/", + {"title": "another title", "tags": [], "remove_inbox_tags": True}, + format="json", + ) + assert resp.status_code == status.HTTP_200_OK + self.document.refresh_from_db() + assert self.document.tags.count() == 0 + + def test_remove_inbox_tags_keeps_inbox_when_nested_child_added(self) -> None: + inbox = Tag.objects.create(name="Inbox", is_inbox_tag=True) + nested = Tag.objects.create(name="Nested", tn_parent=inbox) + self.document.add_nested_tags([inbox]) + + self.client.patch( + f"/api/documents/{self.document.pk}/", + {"tags": [nested.pk], "remove_inbox_tags": True}, + format="json", + ) + self.document.refresh_from_db() + tags = set(self.document.tags.values_list("pk", flat=True)) + assert tags == {inbox.pk, nested.pk} + def test_bulk_edit_respects_hierarchy(self) -> None: bulk_edit.add_tag([self.document.pk], self.child.pk) self.document.refresh_from_db()