Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/changelog.d/tenant-deletion-transaction.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Tenant deletion no longer leaves memberships partially removed when exclusive-user cleanup fails
47 changes: 46 additions & 1 deletion api/src/backend/api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@
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
Expand Down Expand Up @@ -519,6 +520,50 @@ def test_users_create_invalid_fields(
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
Expand Down
2 changes: 1 addition & 1 deletion api/src/backend/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ def destroy(self, request, *args, **kwargs):
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):
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# Collect user IDs from this tenant's memberships before deleting them
tenant_user_ids = set(
Membership.objects.using(MainRouter.admin_db)
Expand Down
Loading