Enforce tag nesting depth and hierarchy validation

src/documents/models.py

@@ -99,6 +99,8 @@ class Correspondent(MatchingModel):
 
 class Tag(MatchingModel):
     color = models.CharField(_("color"), max_length=7, default="#a6cee3")
+    # Maximum allowed nesting depth for tags (root = 1, max depth = 5)
+    MAX_NESTING_DEPTH: Final[int] = 5
 
     is_inbox_tag = models.BooleanField(
         _("is inbox tag"),
@@ -136,9 +138,43 @@ class Tag(MatchingModel):
             ancestors.extend(self.parent.get_all_ancestors())
         return ancestors
 
+    def subtree_height(self, node) -> int:
+        children = list(node.children.all())
+        if not children:
+            return 0
+        return 1 + max(self.subtree_height(child) for child in children)
+
     def clean(self):
+        # Prevent self-parenting
         if self.parent == self:
-            raise ValidationError("Cannot set itself as parent.")
+            raise ValidationError(_("Cannot set itself as parent."))
+
+        # Prevent assigning a descendant as parent
+        if (
+            self.parent
+            and self.pk is not None
+            and any(
+                ancestor.pk == self.pk for ancestor in self.parent.get_all_ancestors()
+            )
+        ):
+            raise ValidationError(_("Cannot set parent to a descendant."))
+
+        # Enforce maximum nesting depth
+        new_parent_depth = 0
+        if self.parent:
+            new_parent_depth = len(self.parent.get_all_ancestors()) + 1
+        if self.pk is None:
+            # Unsaved tag cannot have children; treat as leaf
+            height = 0
+        else:
+            try:
+                height = self.subtree_height(self)
+            except RecursionError:
+                raise ValidationError(_("Invalid tag hierarchy."))
+        deepest_new_depth = (new_parent_depth + 1) + height
+        if deepest_new_depth > self.MAX_NESTING_DEPTH:
+            raise ValidationError(_("Maximum nesting depth exceeded."))
+
         return super().clean()
 
 

src/documents/serialisers.py

@@ -13,6 +13,7 @@ from django.conf import settings
 from django.contrib.auth.models import Group
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import ValidationError
 from django.core.validators import DecimalValidator
 from django.core.validators import MaxLengthValidator
 from django.core.validators import RegexValidator
@@ -579,6 +580,30 @@ class TagSerializer(MatchingModelSerializer, OwnedObjectSerializer):
             raise serializers.ValidationError(_("Invalid color."))
         return color
 
+    def validate(self, attrs):
+        # Validate when changing parent
+        parent = attrs.get("parent", self.instance.parent if self.instance else None)
+
+        if self.instance:
+            # Temporarily set parent on the instance if updating and use model clean()
+            original_parent = self.instance.parent
+            try:
+                self.instance.parent = parent
+                self.instance.clean()
+            except ValidationError as e:
+                raise serializers.ValidationError({"parent": list(e)})
+            finally:
+                self.instance.parent = original_parent
+        else:
+            # For new instances, create a transient Tag and validate
+            temp = Tag(parent=parent)
+            try:
+                temp.clean()
+            except ValidationError as e:
+                raise serializers.ValidationError({"parent": list(e)})
+
+        return super().validate(attrs)
+
 
 class CorrespondentField(serializers.PrimaryKeyRelatedField):
     def get_queryset(self):

src/documents/tests/test_tag_hierarchy.py

@@ -110,3 +110,84 @@ class TestTagHierarchy(APITestCase):
         self.document.refresh_from_db()
         tags = set(self.document.tags.values_list("pk", flat=True))
         assert tags == {self.parent.pk, orphan.pk}
+
+    def test_cannot_set_parent_to_self(self):
+        tag = Tag.objects.create(name="Selfie")
+        resp = self.client.patch(
+            f"/api/tags/{tag.pk}/",
+            {"parent": tag.pk},
+            format="json",
+        )
+        assert resp.status_code == 400
+        assert "parent" in resp.data
+
+    def test_cannot_set_parent_to_descendant(self):
+        a = Tag.objects.create(name="A")
+        b = Tag.objects.create(name="B", parent=a)
+        c = Tag.objects.create(name="C", parent=b)
+
+        # Attempt to set A's parent to C (descendant) should fail
+        resp = self.client.patch(
+            f"/api/tags/{a.pk}/",
+            {"parent": c.pk},
+            format="json",
+        )
+        assert resp.status_code == 400
+        assert "parent" in resp.data
+
+    def test_max_depth_on_create(self):
+        a = Tag.objects.create(name="A1")
+        b = Tag.objects.create(name="B1", parent=a)
+        c = Tag.objects.create(name="C1", parent=b)
+        d = Tag.objects.create(name="D1", parent=c)
+
+        # Creating E under D yields depth 5: allowed
+        resp_ok = self.client.post(
+            "/api/tags/",
+            {"name": "E1", "parent": d.pk},
+            format="json",
+        )
+        assert resp_ok.status_code in (200, 201)
+        e_id = (
+            resp_ok.data["id"] if resp_ok.status_code == 201 else resp_ok.data.get("id")
+        )
+        assert e_id is not None
+
+        # Creating F under E would yield depth 6: rejected
+        resp_fail = self.client.post(
+            "/api/tags/",
+            {"name": "F1", "parent": e_id},
+            format="json",
+        )
+        assert resp_fail.status_code == 400
+        assert "parent" in resp_fail.data
+        assert any("Maximum" in str(msg) for msg in resp_fail.data["parent"])
+
+    def test_max_depth_on_move_subtree(self):
+        a = Tag.objects.create(name="A2")
+        b = Tag.objects.create(name="B2", parent=a)
+        c = Tag.objects.create(name="C2", parent=b)
+        d = Tag.objects.create(name="D2", parent=c)
+
+        x = Tag.objects.create(name="X2")
+        y = Tag.objects.create(name="Y2", parent=x)
+        assert y.parent_id == x.id
+
+        # Moving X under D would make deepest node Y exceed depth 5 -> reject
+        resp_fail = self.client.patch(
+            f"/api/tags/{x.pk}/",
+            {"parent": d.pk},
+            format="json",
+        )
+        assert resp_fail.status_code == 400
+        assert "parent" in resp_fail.data
+
+        # Moving X under C (depth 3) should be allowed (deepest becomes 5)
+        resp_ok = self.client.patch(
+            f"/api/tags/{x.pk}/",
+            {"parent": c.pk},
+            format="json",
+        )
+        assert resp_ok.status_code in (200, 202)
+        x.refresh_from_db()
+        assert x.parent_id == c.id