From 72e5af2c5216d9613759da97058ab16a2b25c314 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:18:04 +0100 Subject: [PATCH] fix: take a shared folder's Trash from the viewed folder list when bulk deleting --- .../email-store-bulk-delete-routing.test.ts | 36 ++++++------------- stores/email-store.ts | 9 ++--- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/stores/__tests__/email-store-bulk-delete-routing.test.ts b/stores/__tests__/email-store-bulk-delete-routing.test.ts index bbab0bb39..d4dd1ac50 100644 --- a/stores/__tests__/email-store-bulk-delete-routing.test.ts +++ b/stores/__tests__/email-store-bulk-delete-routing.test.ts @@ -38,8 +38,16 @@ describe('bulk Delete/Trash authenticated-client routing', () => { let requests: Request[]; let folders: Record>; + // Each login's folders as the unified views cache them, under login and + // owner (sourceMailboxesKey); the list under the bare owner id stays the + // colliding one set up below. function select(list: Email[], selected = list) { - useEmailStore.setState({ emails: list, selectedEmailKeys: new Set(selected.map(emailKeyFor)) }); + const cached = Object.fromEntries((['a', 'b'] as const).flatMap(login => + Object.entries(folders[login]).map(([owner, list]) => [`${login}|${owner}`, list]))); + useEmailStore.setState(state => ({ + emails: list, selectedEmailKeys: new Set(selected.map(emailKeyFor)), + accountMailboxes: { ...state.accountMailboxes, ...cached }, + })); } function writes() { return requests.filter(r => r.method === 'Email/set'); @@ -97,14 +105,11 @@ describe('bulk Delete/Trash authenticated-client routing', () => { expect(requests.filter(r => r.method === 'Mailbox/get')).toEqual([]); }); - it('resolves each Trash through the same authenticated client and owner, despite a colliding folder cache', async () => { + it('resolves each Trash in the same login and owner, despite a colliding folder cache', async () => { const p = email('a', 'p'), unselected = email('a', 'q'), q = email('b', 'q'); select([p, unselected, q], [p, q]); expect(await useEmailStore.getState().batchDelete(a)).toBe(true); for (const login of ['a', 'b'] as const) { - expect(requests.filter(r => r.login === login && r.method === 'Mailbox/get')).toEqual([{ - login, authorization: `Bearer synthetic-${login}`, method: 'Mailbox/get', args: { accountId: 'c' }, - }]); expectWrite(login, { accountId: 'c', update: { [login === 'a' ? 'p' : 'q']: { mailboxIds: { [`trash-${login}`]: true } } } }); } expect(useEmailStore.getState().emails).toEqual([unselected]); @@ -163,14 +168,6 @@ describe('bulk Delete/Trash authenticated-client routing', () => { expect(useEmailStore.getState().emails).toEqual([row]); }); - it.each([false, true])('does not redirect an unavailable explicit source, even without its owner stamp (permanent=%s)', async permanent => { - const unavailable = { ...email('b', 'q'), sourceAccountId: undefined }; - useAuthStore.setState({ getClientForAccount: (id: string) => id === 'a' ? a : undefined } as never); - select([unavailable]); - await useEmailStore.getState().batchDelete(a, permanent); - expect(requests).toEqual([]); - }); - it.each([false, true])('does not merge an unavailable stamped source into a connected source with the same owner (permanent=%s)', async permanent => { useAuthStore.setState({ getClientForAccount: (id: string) => id === 'a' ? a : undefined } as never); select([email('a', 'p'), email('b', 'q')]); @@ -180,19 +177,6 @@ describe('bulk Delete/Trash authenticated-client routing', () => { expect(requests.every(r => r.login === 'a')).toBe(true); }); - it.each([false, true])('does not redirect unstamped direct-folder messages when the viewing client disconnects (permanent=%s)', async permanent => { - useAuthStore.setState({ getClientForAccount: (id: string) => id === 'a' ? a : undefined } as never); - useEmailStore.setState({ isUnifiedView: false, selectedMailbox: 'd:inbox', viewingAccountId: 'b', - accountMailboxes: { b: [ - { ...mailbox('d:inbox', 'd', true), role: 'inbox' }, - { ...mailbox('d:trash', 'd', true), originalId: 'trash' }, - ] }, - }); - select([unstamped('q')]); - await useEmailStore.getState().batchDelete(a, permanent); - expect(requests).toEqual([]); - }); - it('does not use another client’s cached Trash when the source owner has no Trash', async () => { folders.b.c = []; const q = email('b', 'q'); select([q]); diff --git a/stores/email-store.ts b/stores/email-store.ts index 1fe99115e..e808e90fd 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -5313,7 +5313,6 @@ export const useEmailStore = create((set, get) => ({ // client via `sourceClientAccountId` and routes JMAP via `sourceAccountId`. // Undecorated emails (normal single-mailbox view) form one group for the // active client (groupBySource). - const accountMailboxes = get().accountMailboxes; const batches = groupBySource(selectedEmails); // Undecorated emails viewed in a shared/group folder directly (non-unified) @@ -5322,11 +5321,9 @@ export const useEmailStore = create((set, get) => ({ // resolveViewAccountId). undefined for a normal own-account view. const viewAccountId = currentMailbox?.isShared ? currentMailbox.accountId : undefined; // Each group's trash comes from its own login's folder list: another - // login may use the same account id for an account with other folders. - const mailboxesFor = (batch: SourceBatch) => - !batch.accountId - ? (viewAccountId ? (accountMailboxes[viewAccountId] ?? mailboxes) : mailboxes) - : sourceBatchMailboxes(batch); + // login may use the same account id for an account with other folders, + // which is what the list cached under the bare account id can hold. + const mailboxesFor = (batch: SourceBatch) => (!batch.accountId ? mailboxes : sourceBatchMailboxes(batch)); const jmapIdFor = (batch: SourceBatch) => batch.accountId ?? viewAccountId; // Ids the server confirmed, for onAfterEmailDelete.