Fixes #22821: Prevent Tenant Group deletion from creating duplicate ungrouped Tenant names or slugs (#22830)

docs/models/tenancy/tenantgroup.md

@@ -4,6 +4,8 @@
 
 Tenant groups may be nested recursively to achieve a multi-level hierarchy. For example, you might have a group called "Customers" containing subgroups of individual tenants grouped by product or account team.
 
+A tenant group cannot be deleted if ungrouping its tenants, including those of any nested groups, would result in duplicate tenant names or slugs among ungrouped tenants.
+
 ## Fields
 
 ### Parent

netbox/tenancy/models/tenants.py

@@ -1,5 +1,5 @@
 from django.db import models
-from django.db.models import Q
+from django.db.models import Count, ProtectedError, Q
 from django.utils.translation import gettext_lazy as _
 
 from netbox.models import NestedGroupModel, PrimaryModel
@@ -35,6 +35,24 @@ class TenantGroup(NestedGroupModel):
         verbose_name = _('tenant group')
         verbose_name_plural = _('tenant groups')
 
+    def delete(self, *args, **kwargs):
+        # Ungrouping the tenants of this group and its descendants can violate tenant name and slug uniqueness.
+        ungrouped = Tenant.objects.filter(
+            Q(group__isnull=True) | Q(group__in=self.get_descendants(include_self=True))
+        )
+        duplicate_names = ungrouped.values('name').annotate(count=Count('pk')).filter(count__gt=1).values('name')
+        duplicate_slugs = ungrouped.values('slug').annotate(count=Count('pk')).filter(count__gt=1).values('slug')
+        if conflicts := set(ungrouped.filter(Q(name__in=duplicate_names) | Q(slug__in=duplicate_slugs))):
+            raise ProtectedError(
+                _(
+                    "Unable to delete tenant group {tenant_group}. Ungrouping its tenants, including those of any "
+                    "nested groups, would create duplicate tenant names or slugs."
+                ).format(tenant_group=self),
+                conflicts,
+            )
+
+        return super().delete(*args, **kwargs)
+
 
 class Tenant(ContactsMixin, PrimaryModel):
     """

netbox/tenancy/tests/test_api.py

@@ -1,9 +1,14 @@
+import json
+import logging
+
+from django.test import tag
 from django.urls import reverse
+from rest_framework import status
 
 from dcim.models import Site
 from tenancy.choices import *
 from tenancy.models import *
-from utilities.testing import APITestCase, APIViewTestCases
+from utilities.testing import APITestCase, APIViewTestCases, disable_logging
 
 
 class AppTestCase(APITestCase):
@@ -60,6 +65,52 @@ class TenantGroupTestCase(APIViewTestCases.APIViewTestCase):
             },
         ]
 
+    @tag('regression')  # Ref: #22821
+    def test_delete_tenant_group_with_conflicting_tenants(self):
+        """
+        Attempt and fail to delete a tenant group whose tenants cannot be ungrouped.
+        """
+        group = TenantGroup.objects.create(name='Tenant Group 7', slug='tenant-group-7')
+        Tenant.objects.create(name='Tenant 1', slug='tenant-1a', group=group)
+        Tenant.objects.create(name='Tenant 1', slug='tenant-1b')
+
+        self.add_permissions('tenancy.delete_tenantgroup')
+        url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': group.pk})
+        with disable_logging(level=logging.WARNING):
+            response = self.client.delete(url, **self.header)
+
+        self.assertHttpStatus(response, status.HTTP_409_CONFLICT)
+
+        content = json.loads(response.content.decode('utf-8'))
+        self.assertIn('detail', content)
+        self.assertTrue(content['detail'].startswith('Unable to delete object.'))
+        self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
+
+    @tag('regression')  # Ref: #22821
+    def test_bulk_delete_tenant_groups_with_conflicting_tenants(self):
+        """
+        Attempt and fail to bulk delete two tenant groups whose tenants conflict only once both are
+        ungrouped, leaving every group and assignment intact.
+        """
+        group1 = TenantGroup.objects.create(name='Tenant Group 8', slug='tenant-group-8')
+        group2 = TenantGroup.objects.create(name='Tenant Group 9', slug='tenant-group-9')
+        tenant1 = Tenant.objects.create(name='Tenant 2', slug='tenant-2a', group=group1)
+        tenant2 = Tenant.objects.create(name='Tenant 2', slug='tenant-2b', group=group2)
+
+        self.add_permissions('tenancy.delete_tenantgroup')
+        data = [{'id': group1.pk}, {'id': group2.pk}]
+        with disable_logging(level=logging.WARNING):
+            response = self.client.delete(self._get_list_url(), data, format='json', **self.header)
+
+        self.assertHttpStatus(response, status.HTTP_409_CONFLICT)
+
+        # The rolled back batch must not leave the first group deleted
+        self.assertEqual(TenantGroup.objects.filter(pk__in=(group1.pk, group2.pk)).count(), 2)
+        tenant1.refresh_from_db()
+        tenant2.refresh_from_db()
+        self.assertEqual(tenant1.group, group1)
+        self.assertEqual(tenant2.group, group2)
+
 
 class TenantTestCase(APIViewTestCases.APIViewTestCase):
     model = Tenant

netbox/tenancy/tests/test_models.py

@@ -1,6 +1,104 @@
-from django.test import TestCase
+from django.db.models import ProtectedError
+from django.test import TestCase, tag
 
-from tenancy.models import Contact, ContactGroup
+from tenancy.models import Contact, ContactGroup, Tenant, TenantGroup
+
+
+class TenantGroupTestCase(TestCase):
+
+    @tag('regression')  # Ref: #22821
+    def test_tenantgroup_deletion_blocked_by_duplicate_ungrouped_name(self):
+        """
+        Deleting a tenant group must raise ProtectedError when ungrouping its tenant would duplicate
+        the name of an already ungrouped tenant.
+        """
+        group = TenantGroup.objects.create(name='Tenant Group 1', slug='tenant-group-1')
+        tenant1 = Tenant.objects.create(name='Tenant 1', slug='tenant-1a', group=group)
+        tenant2 = Tenant.objects.create(name='Tenant 1', slug='tenant-1b')
+
+        with self.assertRaises(ProtectedError) as cm:
+            group.delete()
+
+        self.assertEqual(
+            cm.exception.args[0],
+            'Unable to delete tenant group Tenant Group 1. Ungrouping its tenants, including those of any nested '
+            'groups, would create duplicate tenant names or slugs.'
+        )
+        self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
+
+        # The failed deletion must leave the group and its tenant assignment intact
+        self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
+        tenant1.refresh_from_db()
+        self.assertEqual(tenant1.group, group)
+
+    @tag('regression')  # Ref: #22821
+    def test_tenantgroup_deletion_blocked_by_duplicate_ungrouped_slug(self):
+        """
+        Deleting a tenant group must raise ProtectedError for a slug collision alone, when the
+        colliding tenants have differing names.
+        """
+        group = TenantGroup.objects.create(name='Tenant Group 2', slug='tenant-group-2')
+        tenant1 = Tenant.objects.create(name='Tenant 2', slug='duplicate-slug', group=group)
+        tenant2 = Tenant.objects.create(name='Tenant 3', slug='duplicate-slug')
+
+        with self.assertRaises(ProtectedError) as cm:
+            group.delete()
+
+        self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
+        self.assertTrue(TenantGroup.objects.filter(pk=group.pk).exists())
+
+    @tag('regression')  # Ref: #22821
+    def test_tenantgroup_deletion_blocked_by_duplicate_name_in_descendants(self):
+        """
+        Deleting a parent tenant group must raise ProtectedError when ungrouping the tenants of its
+        descendant groups would duplicate a name.
+        """
+        parent = TenantGroup.objects.create(name='Parent Group', slug='parent-group')
+        child1 = TenantGroup.objects.create(name='Child Group 1', slug='child-group-1', parent=parent)
+        child2 = TenantGroup.objects.create(name='Child Group 2', slug='child-group-2', parent=parent)
+        tenant1 = Tenant.objects.create(name='Tenant 4', slug='tenant-4a', group=child1)
+        tenant2 = Tenant.objects.create(name='Tenant 4', slug='tenant-4b', group=child2)
+
+        with self.assertRaises(ProtectedError) as cm:
+            parent.delete()
+
+        self.assertEqual(set(cm.exception.protected_objects), {tenant1, tenant2})
+        self.assertTrue(TenantGroup.objects.filter(pk=parent.pk).exists())
+        self.assertEqual(TenantGroup.objects.filter(pk__in=(child1.pk, child2.pk)).count(), 2)
+
+    def test_tenantgroup_deletion_ungroups_tenants(self):
+        """
+        Deleting a tenant group whose tenants can be ungrouped safely must succeed and clear the group
+        assignment across the whole subtree.
+        """
+        parent = TenantGroup.objects.create(name='Parent Group', slug='parent-group')
+        child = TenantGroup.objects.create(name='Child Group', slug='child-group', parent=parent)
+        tenant1 = Tenant.objects.create(name='Tenant 5', slug='tenant-5', group=parent)
+        tenant2 = Tenant.objects.create(name='Tenant 6', slug='tenant-6', group=child)
+
+        parent.delete()
+
+        self.assertFalse(TenantGroup.objects.filter(pk__in=(parent.pk, child.pk)).exists())
+        tenant1.refresh_from_db()
+        tenant2.refresh_from_db()
+        self.assertIsNone(tenant1.group)
+        self.assertIsNone(tenant2.group)
+
+    def test_tenantgroup_deletion_ignores_tenants_in_unrelated_groups(self):
+        """
+        Deleting a tenant group must succeed when a tenant outside its subtree shares a name and slug
+        with one of its own tenants, as that tenant remains grouped.
+        """
+        group = TenantGroup.objects.create(name='Tenant Group 3', slug='tenant-group-3')
+        unrelated = TenantGroup.objects.create(name='Unrelated Group', slug='unrelated-group')
+        tenant = Tenant.objects.create(name='Tenant 7', slug='tenant-7', group=group)
+        Tenant.objects.create(name='Tenant 7', slug='tenant-7', group=unrelated)
+
+        group.delete()
+
+        self.assertFalse(TenantGroup.objects.filter(pk=group.pk).exists())
+        tenant.refresh_from_db()
+        self.assertIsNone(tenant.group)
 
 
 class ContactGroupTestCase(TestCase):