From cf558c5f0adf5b7c06d673e080263dd48a9fc8ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Pe=C3=B1a?= Date: Thu, 6 Aug 2026 17:36:52 +0200 Subject: [PATCH] fix(api): make tenant deletion cleanup atomic (#12379) --- .../tenant-deletion-transaction.fixed.md | 1 + api/src/backend/api/tests/test_views.py | 47 ++++++++++++++++++- api/src/backend/api/v1/views.py | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 api/changelog.d/tenant-deletion-transaction.fixed.md diff --git a/api/changelog.d/tenant-deletion-transaction.fixed.md b/api/changelog.d/tenant-deletion-transaction.fixed.md new file mode 100644 index 0000000000..5679bc4564 --- /dev/null +++ b/api/changelog.d/tenant-deletion-transaction.fixed.md @@ -0,0 +1 @@ +Tenant deletion no longer leaves memberships partially removed when exclusive-user cleanup fails diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index cc9247d9a0..f1ae5177d2 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -78,8 +78,9 @@ from conftest import ( today_after_n_days, ) from django.conf import settings -from django.db import close_old_connections, connection +from django.db import close_old_connections, connection, connections from django.db.models import Count +from django.db.models.signals import pre_delete from django.http import JsonResponse from django.test import RequestFactory from django.test.utils import CaptureQueriesContext @@ -519,6 +520,50 @@ class TestUserViewSet: assert error_field in response.json()["errors"][0]["source"]["pointer"] +@pytest.mark.requires_test_admin_alias +@pytest.mark.django_db(transaction=True, databases=["default", "admin"]) +class TestTenantDeletionTransactions: + @patch("api.v1.views.delete_tenant_task.apply_async") + def test_delete_rolls_back_memberships_when_user_cleanup_fails( + self, + delete_tenant_mock, + authenticated_client, + tenants_fixture, + ): + assert connections["default"] is not connections["admin"] + + _, tenant, _ = tenants_fixture + exclusive_user = User.objects.create_user( + name="exclusive user", + password=TEST_PASSWORD, + email="exclusive-user@example.com", + ) + membership = Membership.objects.create( + user=exclusive_user, + tenant=tenant, + role=Membership.RoleChoices.MEMBER, + ) + + def fail_user_cleanup(*, instance, **kwargs): + if instance.pk == exclusive_user.pk: + raise RuntimeError("Simulated user cleanup failure.") + + pre_delete.connect(fail_user_cleanup, sender=User) + try: + with ( + patch.object(MainRouter, "admin_db", "admin"), + pytest.raises(RuntimeError, match=r"Simulated user cleanup failure\."), + ): + authenticated_client.delete( + reverse("tenant-detail", kwargs={"pk": tenant.id}) + ) + finally: + pre_delete.disconnect(fail_user_cleanup, sender=User) + + assert Membership.objects.using("admin").filter(pk=membership.pk).exists() + delete_tenant_mock.assert_not_called() + + @pytest.mark.django_db class TestTenantViewSet: @pytest.fixture diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index f239512234..71a5faa431 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -1442,7 +1442,7 @@ class TenantViewSet(BaseTenantViewset): if not membership or membership.role != Membership.RoleChoices.OWNER: raise PermissionDenied("Only owners can delete a tenant.") - with transaction.atomic(): + with transaction.atomic(using=MainRouter.admin_db): # Collect user IDs from this tenant's memberships before deleting them tenant_user_ids = set( Membership.objects.using(MainRouter.admin_db)