Skip to content

Commit cf558c5

Browse files
authored
fix(api): make tenant deletion cleanup atomic (#12379)
1 parent e2cae35 commit cf558c5

3 files changed

Lines changed: 48 additions & 2 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Tenant deletion no longer leaves memberships partially removed when exclusive-user cleanup fails

api/src/backend/api/tests/test_views.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@
7878
today_after_n_days,
7979
)
8080
from django.conf import settings
81-
from django.db import close_old_connections, connection
81+
from django.db import close_old_connections, connection, connections
8282
from django.db.models import Count
83+
from django.db.models.signals import pre_delete
8384
from django.http import JsonResponse
8485
from django.test import RequestFactory
8586
from django.test.utils import CaptureQueriesContext
@@ -519,6 +520,50 @@ def test_users_create_invalid_fields(
519520
assert error_field in response.json()["errors"][0]["source"]["pointer"]
520521

521522

523+
@pytest.mark.requires_test_admin_alias
524+
@pytest.mark.django_db(transaction=True, databases=["default", "admin"])
525+
class TestTenantDeletionTransactions:
526+
@patch("api.v1.views.delete_tenant_task.apply_async")
527+
def test_delete_rolls_back_memberships_when_user_cleanup_fails(
528+
self,
529+
delete_tenant_mock,
530+
authenticated_client,
531+
tenants_fixture,
532+
):
533+
assert connections["default"] is not connections["admin"]
534+
535+
_, tenant, _ = tenants_fixture
536+
exclusive_user = User.objects.create_user(
537+
name="exclusive user",
538+
password=TEST_PASSWORD,
539+
email="exclusive-user@example.com",
540+
)
541+
membership = Membership.objects.create(
542+
user=exclusive_user,
543+
tenant=tenant,
544+
role=Membership.RoleChoices.MEMBER,
545+
)
546+
547+
def fail_user_cleanup(*, instance, **kwargs):
548+
if instance.pk == exclusive_user.pk:
549+
raise RuntimeError("Simulated user cleanup failure.")
550+
551+
pre_delete.connect(fail_user_cleanup, sender=User)
552+
try:
553+
with (
554+
patch.object(MainRouter, "admin_db", "admin"),
555+
pytest.raises(RuntimeError, match=r"Simulated user cleanup failure\."),
556+
):
557+
authenticated_client.delete(
558+
reverse("tenant-detail", kwargs={"pk": tenant.id})
559+
)
560+
finally:
561+
pre_delete.disconnect(fail_user_cleanup, sender=User)
562+
563+
assert Membership.objects.using("admin").filter(pk=membership.pk).exists()
564+
delete_tenant_mock.assert_not_called()
565+
566+
522567
@pytest.mark.django_db
523568
class TestTenantViewSet:
524569
@pytest.fixture

api/src/backend/api/v1/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,7 @@ def destroy(self, request, *args, **kwargs):
14421442
if not membership or membership.role != Membership.RoleChoices.OWNER:
14431443
raise PermissionDenied("Only owners can delete a tenant.")
14441444

1445-
with transaction.atomic():
1445+
with transaction.atomic(using=MainRouter.admin_db):
14461446
# Collect user IDs from this tenant's memberships before deleting them
14471447
tenant_user_ids = set(
14481448
Membership.objects.using(MainRouter.admin_db)

0 commit comments

Comments
 (0)