diff --git a/hooks/__tests__/use-mailbox-drop.test.tsx b/hooks/__tests__/use-mailbox-drop.test.tsx index ccce2fd9a..8dde8efdb 100644 --- a/hooks/__tests__/use-mailbox-drop.test.tsx +++ b/hooks/__tests__/use-mailbox-drop.test.tsx @@ -91,12 +91,25 @@ describe('useMailboxDrop', () => { await drop(mailbox({}), ['m016', 't016']); expect(moveEmailsToMailbox.mock.calls).toEqual([ - [client016, ['m016', 't016'], 'f', 'userb@v10'], - [client016, ['m016', 't016'], 'f', 'userb@v016'], + [client016, ['m016', 't016'], 'f', 'userb@v10', drag.draggedEmails], + [client016, ['m016', 't016'], 'f', 'userb@v016', drag.draggedEmails], ]); expect(crossAccountMoveEmails).not.toHaveBeenCalled(); }); + // The team's `x` and usera's own `x` are unrelated messages; one of them + // is dragged. + it('hands the store the dragged row, not only its id, in a list that mixes accounts', async () => { + useEmailStore.setState({ isUnifiedView: true, selectedMailbox: '__unified_inbox' }); + drag.sourceMailboxId = '__unified_inbox'; + const dragged = email({ id: 'x', sourceClientAccountId: 'userb@v016', sourceAccountId: 'c' }); + drag.draggedEmails = [dragged]; + + await drop(mailbox({}), ['x']); + + expect(moveEmailsToMailbox).toHaveBeenCalledWith(client016, ['x'], 'f', 'userb@v016', [dragged]); + }); + it('moves a drop onto another login\'s folder across accounts, although both logins are account `e`', async () => { useAuthStore.setState({ getAllConnectedClients: () => new Map([['userb@v016', client016], ['userb@v10', client10]]), diff --git a/hooks/use-mailbox-drop.ts b/hooks/use-mailbox-drop.ts index 400cbfcf4..64a8bbec9 100644 --- a/hooks/use-mailbox-drop.ts +++ b/hooks/use-mailbox-drop.ts @@ -124,8 +124,9 @@ export function useMailboxDrop({ mailbox, accountId, onDropComplete, onSuccess, // A list that mixes accounts (unified, tag and "All folders" views) // stamps every row with its login and account. The store moves each // of those groups apart - in place only in the folder's own account, - // across accounts for the others. - await moveEmailsToMailbox(client, emailIds, mailbox.id, destAccountId); + // across accounts for the others. It gets the dragged rows: their + // ids alone can name other accounts' rows too. + await moveEmailsToMailbox(client, emailIds, mailbox.id, destAccountId, draggedEmails); } else { // Group dragged emails by source account. In single-account flows this // collapses to one bucket; in the Pro multi-account sidebar a single diff --git a/lib/__tests__/jmap-email-set-failures.test.ts b/lib/__tests__/jmap-email-set-failures.test.ts index 9aa0578fb..3552951f9 100644 --- a/lib/__tests__/jmap-email-set-failures.test.ts +++ b/lib/__tests__/jmap-email-set-failures.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { JMAPClient } from '../jmap/client'; +import { JMAPClient, RequestTimeoutError } from '../jmap/client'; +import { jmapErrorFailedIds } from '../jmap/jmap-error'; // #956: Email/set failures were swallowed by the delete/move helpers because // request() only rejects on HTTP errors. Method-level errors and per-id @@ -94,6 +95,51 @@ describe('JMAPClient Email/set failure surfacing (#956)', () => { await expect(client.batchMoveEmails(['e1'], 'archive')).rejects.toThrow(/Account not found/); }); + // Stalwart takes up to maxObjectsInSet ids per Email/set (500 by default + // on 0.16 and 1.0); a larger move goes out in batches. + describe('a move sent in batches', () => { + async function clientWithBatchesOf(size: number): Promise { + fetchSpy.mockResolvedValueOnce(jsonResponse({ ...makeSession(), capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInSet: size } } })); + const client = JMAPClient.withBearer('https://mail.example.com', 'token123', 'user@test.com'); + await client.connect(); + fetchSpy.mockReset(); + return client; + } + const moved = (ids: string[], refused: Record = {}) => jsonResponse({ + methodResponses: [['Email/set', { + updated: Object.fromEntries(ids.filter((id) => !(id in refused)).map((id) => [id, null])), + ...(Object.keys(refused).length ? { notUpdated: refused } : {}), + }, '0']], + }); + + it('moves the batches after a refused one and names only what was left where it was', async () => { + const client = await clientWithBatchesOf(2); + fetchSpy + .mockResolvedValueOnce(moved(['e1', 'e2'])) + .mockResolvedValueOnce(moved(['e3', 'e4'], { e3: { type: 'forbidden', description: 'No permission' } })) + .mockResolvedValueOnce(moved(['e5'])); + + const error = await client.batchMoveEmails(['e1', 'e2', 'e3', 'e4', 'e5'], 'archive').catch((e: unknown) => e); + + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(error).toBeInstanceOf(Error); + expect(jmapErrorFailedIds(error)).toEqual(['e3']); + }); + + it('names every message from the batch whose answer never came on', async () => { + const client = await clientWithBatchesOf(2); + fetchSpy + .mockResolvedValueOnce(moved(['e1', 'e2'])) + .mockRejectedValueOnce(new RequestTimeoutError(30_000)); + + const error = await client.batchMoveEmails(['e1', 'e2', 'e3', 'e4', 'e5'], 'archive').catch((e: unknown) => e); + + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(error).toBeInstanceOf(RequestTimeoutError); + expect(jmapErrorFailedIds(error)).toEqual(['e3', 'e4', 'e5']); + }); + }); + it('still resolves when the server accepted the change', async () => { const client = await connectedClient(); fetchSpy.mockResolvedValueOnce(success()); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 748b564d2..0d39c2d6e 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -3128,13 +3128,24 @@ export class JMAPClient implements IJMAPClient { if (markAsRead) patch["keywords/$seen"] = true; return patch; }; - for (const batch of batched(emailIds, this.getMaxObjectsInSet())) { + // A refused batch doesn't stop the others, and the error names every + // message left where it was: the batches after one that failed used to + // stay unsent while a caller counted them as moved. + const batches = batched(emailIds, this.getMaxObjectsInSet()); + const responses: JMAPResponse[] = []; + let requestError: unknown; + for (const batch of batches) { const updates = Object.fromEntries(batch.map(id => [id, buildPatch()])); - const response = await this.request([ - ["Email/set", { accountId: accountId || this.accountId, update: updates }, "0"], - ]); - this.assertEmailSetSucceeded(response, 'move emails'); + try { + responses.push(await this.request([ + ["Email/set", { accountId: accountId || this.accountId, update: updates }, "0"], + ])); + } catch (error) { + requestError = error; + break; + } } + this.assertEmailSetBatchesSucceeded(responses, batches, 'move emails', requestError); } async batchArchiveEmails( diff --git a/stores/__tests__/email-store-unified-actions.test.ts b/stores/__tests__/email-store-unified-actions.test.ts index 940bc39b8..b00c0b603 100644 --- a/stores/__tests__/email-store-unified-actions.test.ts +++ b/stores/__tests__/email-store-unified-actions.test.ts @@ -668,3 +668,209 @@ describe('unified-view batch actions with two logins that share account ids', () expect(cached['userb@v10|c']).toEqual(team10); }); }); + +// A tag view and the default "All folders" search stamp their rows with +// login and account like the unified views, but only the unified views cache +// their sources' folder lists; the Pro shell keeps the login browsed before a +// list that mixes accounts opens; and a selected id can be a member of an +// expanded thread only, or name rows of two accounts. +describe('lists that mix accounts, without the unified views\' cache and after browsing another login', () => { + let client016: IJMAPClient; // usera on 0.16.23, the active login + let client10: IJMAPClient; // usera on 1.0 + + // usera is account `d` on both servers, with the team `c` on 0.16.23. + // Folder ids repeat across servers: `f` is the 0.16.23 login's Archive and + // the 1.0 login's "Old stuff". + const own016 = [ + makeMailbox({ id: 'a', role: 'inbox', accountId: 'd' }), + makeMailbox({ id: 'b', name: 'Deleted Items', role: 'trash', accountId: 'd' }), + makeMailbox({ id: 'c', name: 'Junk Mail', role: 'junk', accountId: 'd' }), + makeMailbox({ id: 'f', name: 'Archive', role: 'archive', accountId: 'd' }), + ]; + const team016 = [ + makeMailbox({ id: 'c:a', originalId: 'a', name: 'Inbox', role: 'inbox', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:c', originalId: 'c', name: 'Junk Mail', role: 'junk', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:g', originalId: 'g', name: 'Archive', role: 'archive', isShared: true, accountId: 'c' }), + ]; + const own10 = [ + makeMailbox({ id: 'a', role: 'inbox', accountId: 'd' }), + makeMailbox({ id: 'k', name: 'Deleted Items', role: 'trash', accountId: 'd' }), + makeMailbox({ id: 'f', name: 'Old stuff', accountId: 'd' }), + makeMailbox({ id: 'h', name: 'Archive', role: 'archive', accountId: 'd' }), + ]; + const row = (id: string, login: string, account: string, extra: Partial = {}) => + makeEmail({ id, sourceClientAccountId: login, sourceAccountId: account, mailboxIds: { a: true }, ...extra }); + + beforeEach(() => { + client016 = Object.assign(makeClient(), { getAccountId: () => 'd' }); + client10 = Object.assign(makeClient(), { getAccountId: () => 'd' }); + useSettingsStore.setState({ archiveMode: 'single', deleteAction: 'trash' } as never); + useAuthStore.setState({ + activeAccountId: 'usera@v016', + getClientForAccount: (id: string) => + (id === 'usera@v016' ? client016 : id === 'usera@v10' ? client10 : undefined) as never, + } as never); + useEmailStore.setState({ + isUnifiedView: false, + unifiedRole: null, + crossView: null, + viewingAccountId: null, + selectedMailbox: 'a', + selectedKeyword: null, + searchQuery: '', + searchMailboxId: '', + mailboxes: [...own016, ...team016], + // Nothing cached by login and account: no unified view was built. The + // Pro sidebar holds the 1.0 login's folders under its login. + accountMailboxes: { 'usera@v10': own10 }, + emails: [row('m1', 'usera@v016', 'd'), row('m2', 'usera@v016', 'd')], + threadEmailsCache: new Map(), + selectedEmail: null, + selectedEmailIds: new Set(['m1', 'm2']), + error: null, + isLoading: false, + crossAccountMoveEmails: vi.fn().mockResolvedValue(undefined) as never, + refreshCurrentMailbox: vi.fn().mockResolvedValue(undefined) as never, + }); + }); + + afterEach(() => { + useEmailStore.setState({ crossAccountMoveEmails: realCrossAccountMoveEmails, refreshCurrentMailbox: realRefreshCurrentMailbox }); + }); + + const callsOf = (fn: unknown) => (fn as ReturnType).mock.calls; + const archiveCalls = (client: IJMAPClient) => callsOf(client.batchArchiveEmails) + .map(([emails, archiveId, , , accountId]) => [(emails as Array<{ id: string }>).map((e) => e.id), archiveId, accountId]); + const listed = () => useEmailStore.getState().emails.map((e) => `${e.sourceClientAccountId}/${e.sourceAccountId}/${e.id}`); + + it('archives an "All folders" search\'s own messages into the own Archive', async () => { + useEmailStore.setState({ searchQuery: 'invoice' }); + + await useEmailStore.getState().batchArchive(client016); + + expect(archiveCalls(client016)).toEqual([[['m1', 'm2'], 'f', 'd']]); + expect(listed()).toEqual([]); + expect(useEmailStore.getState().error).toBeNull(); + }); + + it('archives a tag view\'s own messages into the own Archive', async () => { + useEmailStore.setState({ selectedKeyword: 'work' }); + + await useEmailStore.getState().batchArchive(client016); + + expect(archiveCalls(client016)).toEqual([[['m1', 'm2'], 'f', 'd']]); + }); + + it('puts a tag view\'s messages back in their own Inbox with "Not spam", entered from Junk', async () => { + useEmailStore.setState({ selectedKeyword: 'work', selectedMailbox: 'c' }); + + await useEmailStore.getState().batchUndoSpam(client016, ['m1', 'm2']); + + expect(callsOf(client016.undoSpam)).toEqual([['m1', 'a', 'd'], ['m2', 'a', 'd']]); + }); + + // Pro shell: a folder of the 1.0 login browsed, then a tag. A unified view + // built earlier left the 0.16.23 login's list under the bare account id `d`. + describe('in a tag view of the 1.0 login browsed in the Pro shell', () => { + beforeEach(() => { + useEmailStore.setState({ + viewingAccountId: 'usera@v10', + selectedKeyword: 'work', + accountMailboxes: { 'usera@v10': own10, 'usera@v016|d': own016, d: own016 }, + emails: [row('m10', 'usera@v10', 'd')], + selectedEmailIds: new Set(['m10']), + }); + }); + + it('archives into that login\'s own Archive, not the folder with the other server\'s Archive id', async () => { + await useEmailStore.getState().batchArchive(client016); + + expect(archiveCalls(client016)).toEqual([]); + expect(archiveCalls(client10)).toEqual([[['m10'], 'h', 'd']]); + }); + + it('deletes into that login\'s own Deleted Items', async () => { + await useEmailStore.getState().batchDelete(client016, false); + + expect(callsOf(client10.batchMoveEmails)).toEqual([[['m10'], 'k', 'd', false]]); + expect(callsOf(client016.batchMoveEmails)).toEqual([]); + }); + }); + + // Pro shell: a folder of the 1.0 login browsed, then "All Inboxes". The + // "Move to" menus list the active login's folders. + it('moves into the folder "Move to" offers, the active login\'s, after another login was browsed', async () => { + useEmailStore.setState({ + isUnifiedView: true, + unifiedRole: 'inbox', + selectedMailbox: '__unified_inbox', + viewingAccountId: 'usera@v10', + emails: [row('m016', 'usera@v016', 'd'), row('m10', 'usera@v10', 'd')], + selectedEmailIds: new Set(['m016', 'm10']), + }); + + await useEmailStore.getState().batchMoveToMailbox(client016, 'f'); + + expect(callsOf(client016.batchMoveEmails)).toEqual([[['m016'], 'f', 'd']]); + expect(callsOf(client10.batchMoveEmails)).toEqual([]); + expect(callsOf(useEmailStore.getState().crossAccountMoveEmails)).toEqual([ + [new Map([['usera@v10', ['m10']]]), 'usera@v016', 'f', undefined, 'd'], + ]); + }); + + it('moves one dragged row, not the team\'s unrelated message with the same id', async () => { + const own = row('x', 'usera@v016', 'd'); + useEmailStore.setState({ + isUnifiedView: true, + unifiedRole: 'inbox', + selectedMailbox: '__unified_inbox', + emails: [own, row('x', 'usera@v016', 'c', { mailboxIds: { 'c:a': true } })], + selectedEmailIds: new Set(), + }); + + await useEmailStore.getState().moveEmailsToMailbox(client016, ['x'], 'f', 'usera@v016', [own]); + + expect(callsOf(client016.batchMoveEmails)).toEqual([[['x'], 'f', 'd']]); + expect(callsOf(useEmailStore.getState().crossAccountMoveEmails)).toEqual([]); + expect(listed()).toEqual(['usera@v016/c/x']); + }); + + // A thread of the team is expanded and one of its members that is not a + // row of the list is selected. Its id also names usera's own first message + // (`eaaaaab` on both test servers). + describe('with a selected member of an expanded thread', () => { + beforeEach(() => { + useEmailStore.setState({ + isUnifiedView: true, + unifiedRole: 'inbox', + selectedMailbox: '__unified_inbox', + emails: [row('other', 'usera@v016', 'c', { threadId: 'tt', mailboxIds: { 'c:a': true } })], + threadEmailsCache: new Map([['tt', [row('eaaaaab', 'usera@v016', 'c', { threadId: 'tt', mailboxIds: { 'c:a': true } })]]]), + selectedEmailIds: new Set(['eaaaaab']), + }); + }); + + it('marks it as spam in the team account', async () => { + await useEmailStore.getState().batchMarkAsSpam(client016, ['eaaaaab']); + + expect(callsOf(client016.markAsSpam)).toEqual([['eaaaaab', 'c', false]]); + }); + + it('puts it back in the team\'s Inbox with "Not spam" in "All Junk"', async () => { + useEmailStore.setState({ unifiedRole: 'junk', selectedMailbox: '__unified_junk' }); + + await useEmailStore.getState().batchUndoSpam(client016, ['eaaaaab']); + + expect(callsOf(client016.undoSpam)).toEqual([['eaaaaab', 'a', 'c']]); + }); + + it('reports a selected id found neither listed nor in a thread, and writes nothing for it', async () => { + useEmailStore.setState({ selectedEmailIds: new Set(['gone']) }); + + await expect(useEmailStore.getState().batchMarkAsSpam(client016, ['gone'])).rejects.toThrow(/not found/); + + expect(callsOf(client016.markAsSpam)).toEqual([]); + expect(useEmailStore.getState().selectedEmailIds).toEqual(new Set(['gone'])); + }); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index ed3728795..9d925d71d 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -230,10 +230,11 @@ interface EmailStore { moveToMailboxCrossAware: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; /** * `destLocalAccountId`: the login whose folder list `mailboxId` is from (a - * drop target in the multi-account sidebar); by default the viewed one's. - * Only a list that mixes accounts needs it. + * drop target in the multi-account sidebar); by default the active one's. + * `rows`: the rows that were dragged. Only a list that mixes accounts needs + * them: an id alone can name rows of several accounts there. */ - moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string, destLocalAccountId?: string | null) => Promise; + moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string, destLocalAccountId?: string | null, rows?: Email[]) => Promise; moveThreadToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; /** * Move emails across JMAP accounts. JMAP has no native cross-account move, @@ -1215,14 +1216,25 @@ function sourceMailboxes( } /** - * The folders a SourceBatch's role-based destinations (Archive, Junk, Inbox) - * come from: its source's own list, or the viewed account's for undecorated - * messages. + * The folders a SourceBatch's role-based destinations (Archive, Junk, Inbox, + * Deleted Items) come from: its source's own list, or the viewed account's + * for undecorated messages. A source's list is the one cached for its login + * and account, else that login's folder list narrowed to the account: a tag + * view and an "All folders" search stamp their rows too, but only the + * unified views fill that cache. Never the list cached under the bare + * account id, which holds whichever login's list was fetched last when two + * logins share account ids. */ function sourceBatchMailboxes(batch: SourceBatch): Mailbox[] { - return batch.accountId - ? (sourceMailboxes(useEmailStore.getState(), batch.clientAccountId, batch.accountId) ?? []) - : resolveActionMailboxes(); + if (!batch.accountId) return resolveActionMailboxes(); + if (!batch.clientAccountId) return []; + const state = useEmailStore.getState(); + const cached = state.accountMailboxes[sourceMailboxesKey(batch.clientAccountId, batch.accountId)]; + if (cached) return cached; + const loginMailboxes = batch.clientAccountId === useAuthStore.getState().activeAccountId + ? state.mailboxes + : state.accountMailboxes[batch.clientAccountId]; + return (loginMailboxes ?? []).filter((mailbox) => mailbox.accountId === batch.accountId); } /** A folder that messages of an aggregate list are moved to (resolveMoveDestination). */ @@ -1239,19 +1251,20 @@ interface MoveDestination { /** * The folder `storeId` names in the folder list of `localAccountId` - the - * login a drop target is listed under - or, by default, of the account being - * viewed, whose list "Move to" offers. + * login a drop target is listed under - or, by default, of the active login: + * the "Move to" menus offer its folders (`mailboxes`), whichever login was + * browsed before the list that mixes accounts was opened. */ function resolveMoveDestination(passedClient: IJMAPClient, storeId: string, localAccountId?: string | null): MoveDestination { const state = useEmailStore.getState(); const auth = useAuthStore.getState(); - const mailboxes = localAccountId - ? ((localAccountId === auth.activeAccountId ? state.mailboxes : state.accountMailboxes[localAccountId]) ?? []) - : resolveActionMailboxes(); + const login = localAccountId ?? auth.activeAccountId; + const isActive = login === auth.activeAccountId; + const mailboxes = (isActive ? state.mailboxes : (login ? state.accountMailboxes[login] : undefined)) ?? []; const mailbox = mailboxes.find((mb) => mb.id === storeId); - const client = localAccountId ? auth.getClientForAccount(localAccountId) : resolveActionClient(passedClient); + const client = (login ? auth.getClientForAccount(login) : undefined) ?? (isActive ? passedClient : undefined); return { - localAccountId: localAccountId ?? state.viewingAccountId ?? auth.activeAccountId, + localAccountId: login, storeId, accountId: mailbox?.accountId ?? client?.getAccountId(), mailboxId: mailbox?.originalId || storeId, @@ -1420,6 +1433,28 @@ async function archiveAggregateSelection(passedClient: IJMAPClient, selected: Em await useEmailStore.getState().refreshCurrentMailbox(passedClient); } +/** + * The rows selected `ids` name in a list that mixes accounts: listed rows, + * and members of expanded threads, which carry their thread's login and + * account. An id alone names a message in every account that has one, so + * an id found in neither is returned as `unknown` for the caller to report, + * never written in the account the list happens to be reached through. + */ +function aggregateRowsFor(ids: string[]): { rows: Email[]; unknown: string[] } { + const state = useEmailStore.getState(); + const wanted = new Set(ids); + const rows = new Map(); + const add = (email: Email) => { + if (wanted.has(email.id) && !rows.has(sourceRowKey(email))) rows.set(sourceRowKey(email), email); + }; + state.emails.forEach(add); + for (const members of state.threadEmailsCache.values()) { + for (const member of members) if (member.sourceAccountId) add(member); + } + const found = new Set([...rows.values()].map((email) => email.id)); + return { rows: [...rows.values()], unknown: [...wanted].filter((id) => !found.has(id)) }; +} + /** Messages of a batch spam write that live in one account, and how to reach it. */ interface SpamWriteTarget { messages: Email[]; @@ -1435,17 +1470,21 @@ interface SpamWriteTarget { * them, and updates the list: the messages that moved leave it and the * selection, from their folders' counters into the `role` folder of their * account; the others stay. Throws after that when any was left where it was, - * so the caller doesn't report the whole selection as moved. + * `unknown` ids included, so the caller doesn't report the whole selection + * as moved. */ async function writeSpamBatches( targets: SpamWriteTarget[], write: (target: SpamWriteTarget, email: Email) => Promise, counters: { role: string; arriveRead: boolean }, action: string, + unknown: string[] = [], ): Promise { const done: Email[] = []; const accountOf = new Map(); - const failures: BatchFailure[] = []; + const failures: BatchFailure[] = unknown.length > 0 + ? [{ ids: unknown, error: new Error('Message not found in the list') }] + : []; for (const target of targets) { if (!target.client) { failures.push({ ids: target.messages.map(e => e.id), error: new Error('Not connected') }); @@ -3014,16 +3053,17 @@ export const useEmailStore = create((set, get) => ({ } }, - moveEmailsToMailbox: async (client, emailIds, destinationMailboxId, destLocalAccountId) => { + moveEmailsToMailbox: async (client, emailIds, destinationMailboxId, destLocalAccountId, rows) => { if (emailIds.length === 0) return; const idSet = new Set(emailIds); if (isAggregateListView()) { // Emails may span logins and accounts: each group changes folders in // place only in the folder's own account (moveSourceBatches). Rows - // that could not be moved stay. + // that could not be moved stay. The rows a drag names, not every listed + // row with one of their ids: another account's message can share it. const failures = await moveRowsToMailbox( - get().emails.filter(e => idSet.has(e.id)), + rows?.length ? rows.filter(e => idSet.has(e.id)) : get().emails.filter(e => idSet.has(e.id)), resolveMoveDestination(client, destinationMailboxId, destLocalAccountId), client, ); @@ -3749,7 +3789,7 @@ export const useEmailStore = create((set, get) => ({ const mailboxesFor = (batch: SourceBatch) => !batch.accountId ? (viewAccountId ? (accountMailboxes[viewAccountId] ?? mailboxes) : mailboxes) - : (sourceMailboxes({ accountMailboxes }, batch.clientAccountId, batch.accountId) ?? mailboxes); + : sourceBatchMailboxes(batch); const jmapIdFor = (batch: SourceBatch) => batch.accountId ?? viewAccountId; if (forceDestroy) { @@ -4098,20 +4138,23 @@ export const useEmailStore = create((set, get) => ({ batchMarkAsSpam: async (client, emailIds) => { const { selectedMailbox, emails } = get(); const alsoMarkRead = useSettingsStore.getState().deleteAction === 'trash-and-read'; - const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); // Each message goes to the Junk of its own account: in a list that mixes // accounts the one it is stamped with (the selected folder is a virtual // one there), otherwise the viewed folder's. let targets: SpamWriteTarget[]; + let unknown: string[] = []; if (isAggregateListView()) { - targets = groupBySource(rows).map(batch => ({ + const resolved = aggregateRowsFor(emailIds); + unknown = resolved.unknown; + targets = groupBySource(resolved.rows).map(batch => ({ messages: batch.messages, client: sourceBatchClient(batch, client), accountId: batch.accountId, })); } else { const currentMailbox = resolveActionMailboxes().find(m => m.id === selectedMailbox); // Throw rather than return: the caller reports the mail as moved. (#695) if (!currentMailbox) throw new Error('Mailbox not found - cannot mark as spam'); + const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); targets = [{ messages: rows, client: resolveActionClient(client), accountId: currentMailbox.accountId }]; } @@ -4122,6 +4165,7 @@ export const useEmailStore = create((set, get) => ({ // After marking read in the same request, the email arrives in junk as read. { role: 'junk', arriveRead: alsoMarkRead }, 'mark as spam', + unknown, ); } catch (error) { console.error('Failed to batch mark as spam:', error); @@ -4131,14 +4175,16 @@ export const useEmailStore = create((set, get) => ({ batchUndoSpam: async (client: IJMAPClient, emailIds: string[]) => { const { selectedMailbox, emails } = get(); - const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); // Back to the Inbox of each message's own account (batch operations don't // preserve original mailboxes): in a list that mixes accounts ("All // Junk") the one it is stamped with, otherwise the viewed folder's. let targets: SpamWriteTarget[]; + let unknown: string[] = []; if (isAggregateListView()) { - targets = groupBySource(rows).map(batch => { + const resolved = aggregateRowsFor(emailIds); + unknown = resolved.unknown; + targets = groupBySource(resolved.rows).map(batch => { const inbox = sourceBatchMailboxes(batch).find(m => m.role === 'inbox' && (batch.accountId ? m.accountId === batch.accountId : !m.isShared)); return { @@ -4157,6 +4203,7 @@ export const useEmailStore = create((set, get) => ({ if (!inboxMailbox) { throw new Error('Inbox not found'); } + const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); targets = [{ messages: rows, client: resolveActionClient(client), accountId, mailboxId: inboxMailbox.originalId || inboxMailbox.id }]; } @@ -4169,6 +4216,7 @@ export const useEmailStore = create((set, get) => ({ }, { role: 'inbox', arriveRead: false }, 'mark as not spam', + unknown, ); } catch (error) { console.error('Failed to batch restore emails:', error);