diff --git a/stores/__tests__/email-store-bulk-archive.test.ts b/stores/__tests__/email-store-bulk-archive.test.ts index 93a2df635..f801c9eec 100644 --- a/stores/__tests__/email-store-bulk-archive.test.ts +++ b/stores/__tests__/email-store-bulk-archive.test.ts @@ -35,6 +35,18 @@ const email = (id: string, owner: string, login = 'login-a') => ({ mailboxIds: { inbox: true }, keywords: {}, size: 100, hasAttachment: false, }) as Email; +const NO_ARCHIVE = new ArchiveMailboxNotFoundError().message; + +/** + * A bulk archive that archived some groups resolves and reports the others + * through `error`, which runBatchEmailAction toasts; it throws only when + * nothing was archived. + */ +async function archivedPartly(client: IJMAPClient, error: string | RegExp): Promise { + expect(await useEmailStore.getState().batchArchive(client)).toBe(true); + expect(useEmailStore.getState().error).toMatch(error); +} + function makeClient(folders: Record) { return { batchArchiveEmails: vi.fn().mockResolvedValue(undefined), @@ -111,7 +123,7 @@ describe('bulk archive ownership and reconciliation', () => { Object.assign(client, { getAccountCapability }); const failed = useEmailStore.getState().emails[1]; - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(getAccountCapability).toHaveBeenCalledWith('urn:ietf:params:jmap:mail', 'shared'); expect(client.createMailbox).not.toHaveBeenCalled(); @@ -161,7 +173,7 @@ describe('bulk archive ownership and reconciliation', () => { selectedEmailKeys: new Set([emailKeyFor(failed), emailKeyFor(email('other-email', 'primary', 'login-b'))]), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(deniedCapability).toHaveBeenCalledWith('urn:ietf:params:jmap:mail', 'primary'); expect(allowedCapability).toHaveBeenCalledWith('urn:ietf:params:jmap:mail', 'primary'); @@ -191,7 +203,7 @@ describe('bulk archive ownership and reconciliation', () => { expandedThreadIds: new Set([threadKeyFor(archived), threadKeyFor(failed)]), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(getAccountCapability).toHaveBeenCalledExactlyOnceWith('urn:ietf:params:jmap:mail', failed.sourceAccountId); expect(deniedClient.createMailbox).not.toHaveBeenCalled(); @@ -235,7 +247,7 @@ describe('bulk archive ownership and reconciliation', () => { folders.shared = [mailbox('shared', 'inbox', true)]; vi.mocked(client.createMailbox).mockRejectedValue(new Error('Archive creation forbidden')); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('Archive creation forbidden'); + await archivedPartly(client, 'Archive creation forbidden'); expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, 'shared', { role: 'archive' }); expect(client.batchArchiveEmails).toHaveBeenCalledTimes(1); @@ -244,7 +256,7 @@ describe('bulk archive ownership and reconciliation', () => { expect(refresh).toHaveBeenCalledOnce(); }); - it('publishes a newly created Archive even when its email move fails', async () => { + it('keeps the row selected when the move into a newly created Archive fails', async () => { folders.primary = [mailbox('primary', 'inbox')]; useEmailStore.setState({ emails: [email('primary-email', 'primary')], selectedEmailKeys: new Set([emailKeyFor(email('primary-email', 'primary'))]) }); vi.mocked(client.createMailbox).mockImplementation(async () => { @@ -256,7 +268,7 @@ describe('bulk archive ownership and reconciliation', () => { await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('Email move forbidden'); - expect(useEmailStore.getState().accountMailboxes['login-a'].map(m => m.id)).toContain('primary-archive'); + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, 'primary', { role: 'archive' }); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(email('primary-email', 'primary'))]); expect(refresh).not.toHaveBeenCalled(); }); @@ -302,34 +314,19 @@ describe('bulk archive ownership and reconciliation', () => { }); vi.mocked(client.batchArchiveEmails).mockRejectedValue(new ArchiveEmailsError('forbidden', ['accepted'])); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('forbidden'); + await archivedPartly(client, 'forbidden'); expect(useEmailStore.getState().emails).toEqual([failed]); expect(useEmailStore.getState().selectedEmail).toBe(failed); - expect([...useEmailStore.getState().retainedInViewIds]).toEqual(['refused']); expect(useEmailStore.getState().threadEmailsCache.get(threadKeyFor(failed))).toEqual([failed]); expect(useEmailStore.getState().expandedThreadIds.has(threadKeyFor(failed))).toBe(true); }); - it('preserves a failed selected row when the actual refresh omits it', async () => { - const failed = email('shared-email', 'shared'); - useEmailStore.setState({ selectedEmail: failed, refreshCurrentMailbox: originalRefresh }); - folders.shared = [mailbox('shared', 'inbox', true)]; - fetchUnifiedMock.mockResolvedValue({ emails: [], total: 0, hasMore: false, errors: new Map() }); - - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); - - expect(useEmailStore.getState().emails).toEqual([failed]); - expect(useEmailStore.getState().selectedEmail).toBe(failed); - expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(email('shared-email', 'shared'))]); - expect(useEmailStore.getState().totalEmails).toBe(1); - }); - it('leaves the shared row selected when its Archive is missing, while primary succeeds', async () => { folders.shared = [mailbox('shared', 'inbox', true)]; useEmailStore.setState({ accountMailboxes: { ...folders }, mailboxes: Object.values(folders).flat() }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(client.batchArchiveEmails).toHaveBeenCalledTimes(1); expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['shared-email']); @@ -344,7 +341,7 @@ describe('bulk archive ownership and reconciliation', () => { if (owner === 'primary') throw new Error('forbidden'); }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('forbidden'); + await archivedPartly(client, 'forbidden'); expect(client.batchArchiveEmails).toHaveBeenCalledTimes(2); expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['primary-email']); @@ -375,7 +372,6 @@ describe('bulk archive ownership and reconciliation', () => { expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(failed)]); expect(useEmailStore.getState().selectedEmail).toBeNull(); expect(refresh).toHaveBeenCalledOnce(); - expect(vi.mocked(client.getMailboxes).mock.calls).toEqual([['primary'], ['primary']]); expect(errorToast).toHaveBeenCalledWith('archive failed', 'Failed to archive emails: notFound'); expect(successToast).not.toHaveBeenCalled(); }); @@ -457,9 +453,7 @@ describe('bulk archive ownership and reconciliation', () => { }); const viewer = useEmailStore.getState().selectedEmail; - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow( - failure === 'missing Archive' ? /archive mailbox not found/i : /forbidden/, - ); + await archivedPartly(client, failure === 'missing Archive' ? NO_ARCHIVE : /forbidden/); expect(useEmailStore.getState().emails).toEqual([failed]); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(failed)]); @@ -483,7 +477,7 @@ describe('bulk archive ownership and reconciliation', () => { selectedEmailKeys: new Set([emailKeyFor(archived), emailKeyFor(refused), emailKeyFor(shared)]), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('notFound'); + await archivedPartly(client, /^Failed to archive 2 emails/); expect(useEmailStore.getState().emails).toEqual([refused, shared]); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(refused), emailKeyFor(shared)]); @@ -500,7 +494,7 @@ describe('bulk archive ownership and reconciliation', () => { selectedEmailKeys: new Set([emailKeyFor(archived), emailKeyFor(failed)]), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(useEmailStore.getState().emails).toEqual([failed]); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(failed)]); @@ -523,7 +517,7 @@ describe('bulk archive ownership and reconciliation', () => { crossView: view === 'cross-account' ? 'all' : null, }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(loader).toHaveBeenCalledOnce(); expect(useEmailStore.getState().emails).toEqual([failed]); @@ -537,7 +531,7 @@ describe('bulk archive ownership and reconciliation', () => { emails: [email('primary-email', 'primary'), email('shared-email', 'shared', 'disconnected')], selectedEmailKeys: new Set([emailKeyFor(email('primary-email', 'primary')), emailKeyFor(email('shared-email', 'shared', 'disconnected'))]), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('not connected'); + await archivedPartly(client, /not connected/i); expect(client.batchArchiveEmails).toHaveBeenCalledTimes(1); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(email('shared-email', 'shared', 'disconnected'))]); expect(refresh).toHaveBeenCalledOnce(); @@ -551,19 +545,17 @@ describe('bulk archive ownership and reconciliation', () => { emails: [failed], total: 1, hasMore: false, errors: new Map(), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(fetchUnifiedMock).toHaveBeenCalledOnce(); expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['shared-email']); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(email('shared-email', 'shared'))]); expect(useEmailStore.getState().error).toMatch(/archive mailbox not found/i); - expect(useEmailStore.getState().accountMailboxes.primary).toEqual(folders.primary); - expect(useEmailStore.getState().mailboxes.find(m => m.id === 'shared:archive')).toBeDefined(); }); it('leaves a read-only shared Archive untouched without blocking the primary owner', async () => { folders.shared[1].myRights.mayAddItems = false; - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); expect(client.batchArchiveEmails).toHaveBeenCalledTimes(1); expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['shared-email']); expect([...useEmailStore.getState().selectedEmailKeys]).toEqual([emailKeyFor(email('shared-email', 'shared'))]); @@ -647,14 +639,12 @@ describe('bulk archive ownership and reconciliation', () => { }), }); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + await archivedPartly(client, NO_ARCHIVE); const state = useEmailStore.getState(); expect(state.emails).toEqual([row]); expect([...state.selectedEmailKeys]).toEqual([emailKeyFor(member)]); expect(state.selectedEmail).toBe(member); - expect(state.threadEmailsCache.get(threadKeyFor(row))).toEqual([row, member]); - expect(state.expandedThreadIds.has(threadKeyFor(row))).toBe(true); }); it.each(['folder creation', 'archive move'] as const)('keeps plugin cancellation and confirmed notifications per owner: %s', async veto => { @@ -685,26 +675,9 @@ describe('bulk archive ownership and reconciliation', () => { emailHooks.onEmailArchive.register('test', after); vi.mocked(client.batchArchiveEmails).mockRejectedValue(new ArchiveEmailsError('forbidden', [accepted.id, 'unrequested-id'])); - await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('forbidden'); + await archivedPartly(client, 'forbidden'); expect(after).toHaveBeenCalledExactlyOnceWith({ emailIds: [accepted.id] }); expect(useEmailStore.getState().emails).toEqual([refused]); }); - - it('preserves other owners in the viewed login cache when refreshing a successful owner', async () => { - const other = mailbox('other', 'inbox', true); - const merged = [...folders.primary, ...folders.shared, other]; - useEmailStore.setState({ - emails: [email('shared-email', 'shared')], - selectedEmailKeys: new Set([emailKeyFor(email('shared-email', 'shared'))]), - viewingAccountId: 'login-a', accountMailboxes: { 'login-a': merged }, - }); - - await useEmailStore.getState().batchArchive(client); - - expect(useEmailStore.getState().accountMailboxes['login-a']).toEqual([ - ...folders.primary, other, ...folders.shared, - ]); - }); - }); diff --git a/stores/__tests__/email-store-unified-actions.test.ts b/stores/__tests__/email-store-unified-actions.test.ts index 7ebbfd0dc..7d2c7706e 100644 --- a/stores/__tests__/email-store-unified-actions.test.ts +++ b/stores/__tests__/email-store-unified-actions.test.ts @@ -74,6 +74,17 @@ function makeClient() { } as unknown as IJMAPClient; } +/** What `client` reads with Mailbox/get, by account (bulk archive reads its folders live). */ +function servesFolders(client: IJMAPClient, byAccount: Record): IJMAPClient { + (client.getMailboxes as ReturnType).mockImplementation(async (accountId: string) => byAccount[accountId] ?? []); + return client; +} + +/** An account without an Archive gets one, unless the server refuses it. */ +function refuseArchiveCreation(client: IJMAPClient): void { + Object.assign(client, { getAccountCapability: () => ({ mayCreateTopLevelMailbox: false }) }); +} + // Some tests replace these with spies. const realCrossAccountMoveEmails = useEmailStore.getState().crossAccountMoveEmails; const realRefreshCurrentMailbox = useEmailStore.getState().refreshCurrentMailbox; @@ -405,8 +416,8 @@ describe('unified-view batch actions with two logins that share account ids', () ]; beforeEach(() => { - client016 = makeClient(); - client10 = makeClient(); + client016 = servesFolders(makeClient(), { e: own016, c: team016 }); + client10 = servesFolders(makeClient(), { e: own10, c: team10 }); useAuthStore.setState({ activeAccountId: 'userb@v016', getClientForAccount: (id: string) => @@ -605,6 +616,8 @@ describe('unified-view batch actions with two logins that share account ids', () it('keeps the messages of an account without an Archive listed and selected, archiving the others', async () => { useSettingsStore.setState({ archiveMode: 'single' } as never); + servesFolders(client10, { e: own10, c: team10.filter((m) => m.role !== 'archive') }); + refuseArchiveCreation(client10); useEmailStore.setState((s) => ({ accountMailboxes: { ...s.accountMailboxes, 'userb@v10|c': team10.filter((m) => m.role !== 'archive') }, })); @@ -618,6 +631,8 @@ describe('unified-view batch actions with two logins that share account ids', () }); it('rejects with ArchiveMailboxNotFoundError when no message could be archived', async () => { + servesFolders(client10, { e: own10.filter((m) => m.role !== 'archive') }); + refuseArchiveCreation(client10); useEmailStore.setState({ accountMailboxes: {}, selectedEmailKeys: new Set([keyOf('userb@v10', 'e', 'm10')]), @@ -724,8 +739,8 @@ describe('lists that mix accounts, without the unified views\' cache and after b makeEmail({ id, sourceClientAccountId: login, sourceAccountId: account, mailboxIds: { a: true }, ...extra }); beforeEach(() => { - client016 = Object.assign(makeClient(), { getAccountId: () => 'd' }); - client10 = Object.assign(makeClient(), { getAccountId: () => 'd' }); + client016 = servesFolders(Object.assign(makeClient(), { getAccountId: () => 'd' }), { d: own016, c: team016 }); + client10 = servesFolders(Object.assign(makeClient(), { getAccountId: () => 'd' }), { d: own10 }); useSettingsStore.setState({ archiveMode: 'single', deleteAction: 'trash' } as never); useAuthStore.setState({ activeAccountId: 'usera@v016', diff --git a/stores/email-store.ts b/stores/email-store.ts index 2c58fc34d..1fe99115e 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -2078,40 +2078,63 @@ async function moveRowsToMailbox(rows: Email[], destination: MoveDestination, pa * archives into its own Archive. One Archive for the whole selection moved * every message by its id in that Archive's account, where the ids of the * other accounts' messages name unrelated messages - or none. The rows that - * were archived leave the list and the selection, the others stay and the - * failure is reported through `error`, `notFound` selection keys - * (resolveSelectionRows) included. Throws when nothing was archived. + * were archived leave the list, the selection, the viewer and expanded + * threads; the others stay and the failure is reported through `error`, + * `notFound` selection keys (resolveSelectionRows) included. Throws when + * nothing was archived; resolves false when a plugin stopped a group. + * + * Each group reads its folders through its own login and creates its Archive + * when the account has none, as the folder view does (Stalwart makes no + * Archive). Plugins are asked per group, each about its own Archive. */ async function archiveAggregateSelection(passedClient: IJMAPClient, selected: Email[], notFound: string[] = []): Promise { if (selected.length === 0 && notFound.length === 0) return true; const mode = useSettingsStore.getState().archiveMode; - - const batches = groupBySource(selected).map((batch) => { - const mailboxes = sourceBatchMailboxes(batch); - const archive = batch.accountId - ? findArchiveMailbox(mailboxes, undefined, batch.accountId) - : findArchiveMailbox(mailboxes, useEmailStore.getState().selectedMailbox); - return { batch, mailboxes, archive }; - }); - // Plugins are asked once for the whole selection, though each group goes - // to its own account's Archive. - const firstArchive = batches.find(({ archive }) => archive)?.archive; - const hookMailboxes = [...resolveActionMailboxes(), ...batches.flatMap(({ archive }) => (archive ? [archive] : []))]; - const move = firstArchive ? { - emailIds: [...new Set(selected.map(e => e.id))], - fromMailboxId: moveSourceMailboxId(selected, hookMailboxes), - toMailboxId: firstArchive.id, - } : null; - if (move && !(await allowEmailMove(move, hookMailboxes))) return false; + const { selectedMailbox } = useEmailStore.getState(); useEmailStore.setState({ isLoading: true, error: null }); const archived: Email[] = []; const failures: BatchFailure[] = []; - await Promise.all(batches.map(async ({ batch, mailboxes, archive }) => { + let stoppedByPlugin = false; + await Promise.all(groupBySource(selected).map(async (batch) => { const client = sourceBatchClient(batch, passedClient); + let move: Parameters[0] | null = null; + let hookMailboxes: Mailbox[] = []; try { if (!client) throw new Error('Not connected'); - if (!archive) throw new ArchiveMailboxNotFoundError(); + // The source's folders as its login reads them now, by the server's own + // ids: batchArchiveEmails finds year/month folders by their parent's id + // there, not by the store's namespaced one. + const folders = async (refresh: boolean): Promise => { + if (!batch.accountId) { + if (refresh) await refreshMailboxesForViewingAccount(passedClient); + return resolveActionMailboxes(); + } + return (await client.getMailboxes(batch.accountId)).map((m) => (m.originalId && m.accountId ? { + ...m, id: m.originalId, + parentId: m.parentId?.startsWith(`${m.accountId}:`) ? m.parentId.slice(m.accountId.length + 1) : m.parentId, + } : m)); + }; + let mailboxes = await folders(false); + const archive = await ensureArchiveMailbox({ + client, mailboxes, selectedMailboxId: selectedMailbox, accountId: batch.accountId, + refresh: async () => (mailboxes = await folders(true)), + }); + if (!archive) { + stoppedByPlugin = true; + return; + } + if (archive.myRights?.mayAddItems === false) throw new ArchiveMailboxNotFoundError(); + hookMailboxes = [...mailboxes, archive]; + move = { + emailIds: [...new Set(batch.ids)], + fromMailboxId: moveSourceMailboxId(batch.messages, hookMailboxes), + toMailboxId: archive.id, + }; + if (!(await allowEmailMove(move, hookMailboxes))) { + stoppedByPlugin = true; + return; + } await client.batchArchiveEmails( batch.messages.map(e => ({ id: e.id, receivedAt: e.receivedAt })), archive.originalId || archive.id, @@ -2120,8 +2143,13 @@ async function archiveAggregateSelection(passedClient: IJMAPClient, selected: Em archive.accountId, ); archived.push(...batch.messages); + emitEmailMove(move, hookMailboxes); } catch (error) { - failures.push({ ids: batch.ids, error }); + // An archive that failed part way names the messages it did move. + const moved = new Set(error instanceof ArchiveEmailsError ? error.updatedIds : []); + archived.push(...batch.messages.filter(e => moved.has(e.id))); + failures.push({ ids: batch.ids.filter(id => !moved.has(id)), error }); + if (move && moved.size > 0) emitEmailMove({ ...move, emailIds: move.emailIds.filter(id => moved.has(id)) }, hookMailboxes); } })); failures.push(...notFoundFailures(notFound)); @@ -2129,25 +2157,39 @@ async function archiveAggregateSelection(passedClient: IJMAPClient, selected: Em const error = failures.length > 0 ? batchFailureMessage(failures, 'archive') : null; if (archived.length === 0) { useEmailStore.setState({ error, isLoading: false }); - throw failures[0].error; + if (failures.length > 0) throw failures[0].error; + return false; } const archivedKeys = new Set(archived.map(sourceRowKey)); useEmailStore.setState((state) => { const selectedEmailKeys = new Set(state.selectedEmailKeys); for (const email of archived) selectedEmailKeys.delete(emailKeyFor(email)); + const threadEmailsCache = new Map(state.threadEmailsCache); + const expandedThreadIds = new Set(state.expandedThreadIds); + for (const [key, members] of threadEmailsCache) { + const kept = members.filter(e => !archivedKeys.has(sourceRowKey(e))); + if (kept.length === members.length) continue; + if (kept.length > 0) threadEmailsCache.set(key, kept); + else { + threadEmailsCache.delete(key); + expandedThreadIds.delete(key); + } + } return { emails: state.emails.filter(e => !archivedKeys.has(sourceRowKey(e))), + selectedEmail: isOneOfRows(state.selectedEmail, archivedKeys) ? null : state.selectedEmail, selectedEmailKeys, + threadEmailsCache, + expandedThreadIds, isLoading: false, error, }; }); - if (move) emitEmailMove({ ...move, emailIds: [...new Set(archived.map(e => e.id))] }, hookMailboxes); // A year/month archive can create new sub-folders. await refreshMailboxesForViewingAccount(passedClient); await useEmailStore.getState().refreshCurrentMailbox(passedClient); - return true; + return !stoppedByPlugin; } /**