From 4b2b641f9275ad528dc0e7a7bdbdfcd70bcc83a8 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Sat, 19 Sep 2026 06:10:32 +0200 Subject: [PATCH] feat: pass the workspace id to the mailbox delete in the database-backed removal and erasure tests, which compiled with the owner's id but would answer not found now that Delete is scoped to the organization, and rename the cloud link stub's parameter to say what it carries --- internal/app/cloudlink/disconnect_managed_test.go | 4 ++-- internal/app/email/mailbox_erasure_live_test.go | 10 +++++----- internal/app/email/worker_removal_live_test.go | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/app/cloudlink/disconnect_managed_test.go b/internal/app/cloudlink/disconnect_managed_test.go index 78fff637d..b80d08c0c 100644 --- a/internal/app/cloudlink/disconnect_managed_test.go +++ b/internal/app/cloudlink/disconnect_managed_test.go @@ -45,12 +45,12 @@ type revokingEmails struct { refused *[]*errx.Error } -func (s revokingEmails) Delete(ctx context.Context, userID, accountID string) *errx.Error { +func (s revokingEmails) Delete(ctx context.Context, orgID, accountID string) *errx.Error { if xerr := s.svc.RevokeForDelete(ctx, s.org, uuid.MustParse(accountID)); xerr != nil { *s.refused = append(*s.refused, xerr) return xerr } - return s.stubEmailDeletes.Delete(ctx, userID, accountID) + return s.stubEmailDeletes.Delete(ctx, orgID, accountID) } // Disconnect revokes the instance before it deletes the managed mirrors, so the diff --git a/internal/app/email/mailbox_erasure_live_test.go b/internal/app/email/mailbox_erasure_live_test.go index a6715f809..2b1cba856 100644 --- a/internal/app/email/mailbox_erasure_live_test.go +++ b/internal/app/email/mailbox_erasure_live_test.go @@ -60,7 +60,7 @@ func TestLiveDeleteRecordsTheErasureItCannotPerform(t *testing.T) { exec(t, f, `INSERT INTO email_accounts_oauth (email_account_id, access_token, refresh_token, expires_at) VALUES ($1, 'sealed-access', 'sealed-refresh', now() + interval '1 hour')`, f.mailbox) - if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("delete: %v", xerr) } if f.mailboxExists(t) { @@ -91,7 +91,7 @@ func TestLiveDeleteQueuesErasureForAMailboxWithNoGrant(t *testing.T) { f := newRemovalLiveFixture(t) cleanErasure(t, f) - if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("delete: %v", xerr) } row := readErasure(t, f) @@ -110,7 +110,7 @@ func TestLiveAFailedDeleteQueuesNoErasure(t *testing.T) { cleanErasure(t, f) f.pub.removeErr = errBusDown - if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr == nil { + if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr == nil { t.Fatal("the delete was reported as succeeding") } if !f.mailboxExists(t) { @@ -157,7 +157,7 @@ func TestLiveDeleteTakesTheRowsThatHadNoForeignKey(t *testing.T) { exec(t, f, `INSERT INTO warmup_pending_engagements (email_account_id, payload, fire_at) VALUES ($1, '{}'::jsonb, now())`, f.mailbox) - if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("delete: %v", xerr) } @@ -222,7 +222,7 @@ func TestLiveDeleteClearsLabelsOnThreadsItEmptied(t *testing.T) { f.user, m.thread) } - if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("delete: %v", xerr) } diff --git a/internal/app/email/worker_removal_live_test.go b/internal/app/email/worker_removal_live_test.go index 105a4ad93..f9638f893 100644 --- a/internal/app/email/worker_removal_live_test.go +++ b/internal/app/email/worker_removal_live_test.go @@ -159,7 +159,7 @@ func TestLiveDisablingAMailboxRemovesItFromItsWorker(t *testing.T) { func TestLiveDeletingAMailboxRemovesItFromItsWorkerFirst(t *testing.T) { f := newRemovalLiveFixture(t) - if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("delete: %v", xerr) } @@ -199,10 +199,10 @@ func TestLiveDeleteThatMatchesNoRowRefundsNothing(t *testing.T) { } } -// The lookup that finds the mailbox is deliberately unscoped, so ownership is -// checked in the service. A teammate's user id must not delete this mailbox or -// publish a removal for it. -func TestLiveDeleteRefusesAMailboxTheCallerDoesNotOwn(t *testing.T) { +// The lookup that finds the mailbox is deliberately unscoped, so the workspace +// is checked in the service. Another workspace's id must not delete this +// mailbox or publish a removal for it. +func TestLiveDeleteRefusesAMailboxOfAnotherWorkspace(t *testing.T) { f := newRemovalLiveFixture(t) if xerr := f.svc.Delete(context.Background(), uuid.New().String(), f.mailbox.String()); xerr != errx.ErrNotFound { @@ -222,7 +222,7 @@ func TestLiveDeleteKeepsEverythingWhenTheWorkerCannotBeTold(t *testing.T) { f := newRemovalLiveFixture(t) f.pub.removeErr = errBusDown - xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()) + xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()) if xerr == nil || xerr.Code != errx.ServiceUnavailable { t.Fatalf("error = %v, want a 503 so the client retries", xerr) } @@ -259,7 +259,7 @@ func TestLiveDeletingAMailboxWithScheduledWork(t *testing.T) { t.Fatalf("fixture admin action: %v", err) } - if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil { + if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil { t.Fatalf("disconnecting a mailbox that has scheduled work failed: %v", xerr) } if f.mailboxExists(t) {