fix: create each account's Archive when bulk archiving in the unified inbox

This commit is contained in:
Linus Rath
2026-10-26 12:25:08 +01:00
parent e6c911899c
commit eeb6ccc4db
3 changed files with 118 additions and 88 deletions
@@ -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<void> {
expect(await useEmailStore.getState().batchArchive(client)).toBe(true);
expect(useEmailStore.getState().error).toMatch(error);
}
function makeClient(folders: Record<string, Mailbox[]>) {
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,
]);
});
});
@@ -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<string, Mailbox[]>): IJMAPClient {
(client.getMailboxes as ReturnType<typeof vi.fn>).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',
+69 -27
View File
@@ -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<boolean> {
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<typeof allowEmailMove>[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<Mailbox[]> => {
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;
}
/**