From 45c7e2c3604193c8467daad61c9457b5da0303f7 Mon Sep 17 00:00:00 2001 From: sud Date: Tue, 6 Oct 2026 06:54:06 +0200 Subject: [PATCH] fix: create the Archive folder on first use when the account has none Stalwart's default folder set has no archive-role mailbox, so on a fresh account Archive did nothing but raise the not-found toast (#578). Both archive paths now go through ensureArchiveMailbox(), which creates "Archive" with the archive role in the account the action targets (own, the selected shared folder's owner, or the pinned account in unified view), reloads the mailbox list and continues with the store's own object. createMailbox() takes an optional role for this. A refused creation or a folder that does not come back still surfaces as an error. Co-Authored-By: Claude Fable 5.1 --- components/mail/mail-app.tsx | 24 +++-- lib/demo/demo-client.ts | 3 +- lib/jmap/client-interface.ts | 2 +- lib/jmap/client.ts | 6 +- .../email-store-archive-auto-create.test.ts | 99 +++++++++++++++++++ .../email-store-archive-missing.test.ts | 38 ++++++- stores/email-store.ts | 47 ++++++++- 7 files changed, 200 insertions(+), 19 deletions(-) create mode 100644 stores/__tests__/email-store-archive-auto-create.test.ts diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index b51b830d9..81bd343a2 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -31,7 +31,7 @@ import { usePolicyStore } from "@/stores/policy-store"; import type { UnifiedAccountClient } from "@/lib/unified-mailbox"; import { connectedAccountsGrew } from "@/lib/unified-mailbox"; import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal"; -import { useEmailStore, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, findArchiveMailbox, resolveUnstampedEmailAccountId, emptyFolderMovesToTrash } from "@/stores/email-store"; +import { useEmailStore, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveUnstampedEmailAccountId, emptyFolderMovesToTrash } from "@/stores/email-store"; import { groupSearchScopeFolders, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders"; import { toast } from "@/stores/toast-store"; import { formatRejectedRecipients, type JMAPClient } from "@/lib/jmap/client"; @@ -2362,17 +2362,23 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const currentMailboxes = readMailboxes(); - // Scope like batchArchive: the owning account in unified view, otherwise - // the selected shared folder's owner, otherwise the user's own archive. (#889) - const archiveMailbox = findArchiveMailbox(currentMailboxes, selectedMailbox, archiveAccountId); - if (!archiveMailbox) { - toast.error(t('email_viewer.archive_mailbox_not_found')); - return; - } - const { archiveMode } = useSettingsStore.getState(); try { + // Scope like batchArchive: the owning account in unified view, otherwise + // the selected shared folder's owner, otherwise the user's own archive + // (#889); created on first use when the account has none (#578). + const archiveMailbox = await ensureArchiveMailbox({ + client: archiveClient, + mailboxes: currentMailboxes, + selectedMailboxId: selectedMailbox, + accountId: archiveAccountId, + refresh: async () => { + await refreshMailboxes(); + return readMailboxes(); + }, + }); + if (archiveMode === 'single') { await moveThreadToMailbox(client, emailToArchive.id, archiveMailbox.id); } else { diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index 312d408c8..f2398343f 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -140,10 +140,11 @@ export class DemoJMAPClient implements IJMAPClient { async getMailboxes(_accountId?: string): Promise { return [...this.data.mailboxes]; } async getAllMailboxes(): Promise { return [...this.data.mailboxes]; } - async createMailbox(name: string, parentId?: string, _accountId?: string): Promise { + async createMailbox(name: string, parentId?: string, _accountId?: string, options?: { role?: string }): Promise { const mb: Mailbox = { id: generateDemoId('mailbox'), name, + role: options?.role, sortOrder: 100, totalEmails: 0, unreadEmails: 0, diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 41eef9402..96cbe87f3 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -137,7 +137,7 @@ export interface IJMAPClient { getMailboxChanges?(sinceState: string, accountId?: string, maxChanges?: number): Promise; /** Email/changes since `sinceState`; null when the server cannot compute the delta. */ getEmailChanges?(sinceState: string, accountId?: string, maxChanges?: number): Promise; - createMailbox(name: string, parentId?: string, accountId?: string): Promise; + createMailbox(name: string, parentId?: string, accountId?: string, options?: { role?: string }): Promise; updateMailbox(mailboxId: string, changes: { name?: string; parentId?: string | null; role?: string | null; sortOrder?: number }, accountId?: string): Promise; // `removeEmails` destroys the folder's messages too (onDestroyRemoveEmails, // RFC 8621 ยง2.5) instead of failing with mailboxHasEmail. diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 63aa729b9..fbbcfd04c 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -3167,7 +3167,7 @@ export class JMAPClient implements IJMAPClient { this.assertEmailSetSucceeded(response, "mark as not spam"); } - async createMailbox(name: string, parentId?: string, accountId?: string): Promise { + async createMailbox(name: string, parentId?: string, accountId?: string, options?: { role?: string }): Promise { const targetAccountId = accountId || this.accountId; const createId = `new-${Date.now()}`; // Subscribe explicitly: IMAP clients that list folders via LSUB @@ -3177,6 +3177,9 @@ export class JMAPClient implements IJMAPClient { if (parentId) { createData.parentId = parentId; } + if (options?.role) { + createData.role = options.role; + } const response = await this.request([ ["Mailbox/set", { @@ -3205,6 +3208,7 @@ export class JMAPClient implements IJMAPClient { id: created.id, name, parentId, + role: options?.role, sortOrder: 0, totalEmails: 0, unreadEmails: 0, diff --git a/stores/__tests__/email-store-archive-auto-create.test.ts b/stores/__tests__/email-store-archive-auto-create.test.ts new file mode 100644 index 000000000..9e4f20e47 --- /dev/null +++ b/stores/__tests__/email-store-archive-auto-create.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it, vi } from 'vitest'; +import { ensureArchiveMailbox, ArchiveMailboxNotFoundError } from '../email-store'; +import type { Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +// A fresh Stalwart account has no archive-role mailbox, so Archive used to +// stop at the not-found toast. The folder is now created on first use, in the +// account the action targets. (#578) + +function makeMailbox(overrides: Partial = {}): Mailbox { + return { + id: 'inbox', + name: 'Inbox', + sortOrder: 0, + totalEmails: 0, + unreadEmails: 0, + totalThreads: 0, + unreadThreads: 0, + isSubscribed: true, + myRights: { + mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, + maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true, + }, + ...overrides, + }; +} + +const ownInbox = makeMailbox({ id: 'a-inbox', role: 'inbox' }); +const ownArchive = makeMailbox({ id: 'a-archive', name: 'Archive', role: 'archive' }); +const sharedInbox = makeMailbox({ + id: 'owner-x:x-inbox', originalId: 'x-inbox', role: 'inbox', isShared: true, accountId: 'owner-x', +}); +const sharedArchive = makeMailbox({ + id: 'owner-x:x-archive', originalId: 'x-archive', name: 'Archive', role: 'archive', isShared: true, accountId: 'owner-x', +}); + +function makeClient(): IJMAPClient & { createMailbox: ReturnType } { + return { createMailbox: vi.fn(async () => ownArchive) } as unknown as IJMAPClient & { createMailbox: ReturnType }; +} + +describe('ensureArchiveMailbox', () => { + it('returns the existing archive folder without creating one', async () => { + const client = makeClient(); + const refresh = vi.fn(); + const result = await ensureArchiveMailbox({ + client, mailboxes: [ownInbox, ownArchive], selectedMailboxId: 'a-inbox', refresh, + }); + expect(result).toBe(ownArchive); + expect(client.createMailbox).not.toHaveBeenCalled(); + expect(refresh).not.toHaveBeenCalled(); + }); + + it('creates an archive-role folder in the own account and returns it from the refreshed list', async () => { + const client = makeClient(); + const refresh = vi.fn(async () => [ownInbox, ownArchive]); + const result = await ensureArchiveMailbox({ + client, mailboxes: [ownInbox], selectedMailboxId: 'a-inbox', refresh, + }); + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, undefined, { role: 'archive' }); + expect(refresh).toHaveBeenCalledTimes(1); + expect(result).toBe(ownArchive); + }); + + it('creates the folder in the selected shared folder\'s owner account', async () => { + const client = makeClient(); + const refresh = vi.fn(async () => [ownInbox, ownArchive, sharedInbox, sharedArchive]); + const result = await ensureArchiveMailbox({ + client, mailboxes: [ownInbox, ownArchive, sharedInbox], selectedMailboxId: 'owner-x:x-inbox', refresh, + }); + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, 'owner-x', { role: 'archive' }); + // The store's namespaced object, not the bare one Mailbox/set returned. + expect(result).toBe(sharedArchive); + }); + + it('pins the account in unified view', async () => { + const client = makeClient(); + const refresh = vi.fn(async () => [ownInbox, ownArchive, sharedInbox, sharedArchive]); + await ensureArchiveMailbox({ + client, mailboxes: [ownInbox, ownArchive, sharedInbox], selectedMailboxId: 'unified-inbox', accountId: 'owner-x', refresh, + }); + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, 'owner-x', { role: 'archive' }); + }); + + it('still reports not-found when the created folder does not come back', async () => { + const client = makeClient(); + const refresh = vi.fn(async () => [ownInbox]); + await expect(ensureArchiveMailbox({ + client, mailboxes: [ownInbox], selectedMailboxId: 'a-inbox', refresh, + })).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + }); + + it('surfaces a server refusal instead of swallowing it', async () => { + const client = makeClient(); + client.createMailbox.mockRejectedValueOnce(new Error('Failed to create mailbox: forbidden')); + await expect(ensureArchiveMailbox({ + client, mailboxes: [ownInbox], selectedMailboxId: 'a-inbox', refresh: vi.fn(), + })).rejects.toThrow('forbidden'); + }); +}); diff --git a/stores/__tests__/email-store-archive-missing.test.ts b/stores/__tests__/email-store-archive-missing.test.ts index cce2d45f5..3a4924d04 100644 --- a/stores/__tests__/email-store-archive-missing.test.ts +++ b/stores/__tests__/email-store-archive-missing.test.ts @@ -6,7 +6,8 @@ import type { Email, Mailbox } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; // #578: archiving with no archive folder used to return silently, leaving the -// user with a shortcut/button that did nothing. The store must now report it. +// user with a shortcut/button that did nothing. The store now creates the +// folder on first use and archives into it; a refused creation is reported. function makeMailbox(overrides: Partial = {}): Mailbox { return { @@ -50,10 +51,17 @@ describe('batchArchive without an archive mailbox (#578)', () => { let client: IJMAPClient; beforeEach(() => { + const archive = makeMailbox({ id: 'a-archive', name: 'Archive', role: 'archive' }); client = { batchArchiveEmails: vi.fn().mockResolvedValue(undefined), + createMailbox: vi.fn().mockResolvedValue(archive), getEmails: vi.fn().mockResolvedValue({ emails: [], hasMore: false, total: 0 }), - getMailboxes: vi.fn().mockResolvedValue([]), + // The reload after creation is what brings the new folder into the store. + getMailboxes: vi.fn().mockResolvedValue([ + makeMailbox({ id: 'a-inbox', role: 'inbox' }), + makeMailbox({ id: 'a-trash', name: 'Trash', role: 'trash' }), + archive, + ]), } as unknown as IJMAPClient; useAuthStore.setState({ @@ -78,7 +86,22 @@ describe('batchArchive without an archive mailbox (#578)', () => { }); }); - it('rejects with ArchiveMailboxNotFoundError and records the error', async () => { + it('creates the archive folder and archives into it', async () => { + await useEmailStore.getState().batchArchive(client); + + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, undefined, { role: 'archive' }); + expect(client.batchArchiveEmails).toHaveBeenCalledWith( + [expect.objectContaining({ id: 'e1' })], 'a-archive', 'single', expect.any(Array), undefined, + ); + expect(useEmailStore.getState().error).toBeNull(); + expect(useEmailStore.getState().selectedEmailIds.size).toBe(0); + }); + + it('rejects with ArchiveMailboxNotFoundError when the created folder does not come back', async () => { + (client.getMailboxes as ReturnType).mockResolvedValue([ + makeMailbox({ id: 'a-inbox', role: 'inbox' }), + ]); + await expect(useEmailStore.getState().batchArchive(client)).rejects.toBeInstanceOf( ArchiveMailboxNotFoundError, ); @@ -88,4 +111,13 @@ describe('batchArchive without an archive mailbox (#578)', () => { // Nothing was archived, so the selection must survive. expect(useEmailStore.getState().selectedEmailIds.has('e1')).toBe(true); }); + + it('reports a refused creation and keeps the selection', async () => { + (client.createMailbox as ReturnType).mockRejectedValue(new Error('Failed to create mailbox: forbidden')); + + await expect(useEmailStore.getState().batchArchive(client)).rejects.toThrow('forbidden'); + expect(useEmailStore.getState().error).toMatch(/forbidden/); + expect(client.batchArchiveEmails).not.toHaveBeenCalled(); + expect(useEmailStore.getState().selectedEmailIds.has('e1')).toBe(true); + }); }); diff --git a/stores/email-store.ts b/stores/email-store.ts index 0a1d291e0..f40ca2d04 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -733,6 +733,36 @@ export function findArchiveMailbox( ); } +/** + * Archive folder for the action, created when the account has none. + * + * Stalwart's default folder set has no archive-role mailbox, so on a fresh + * account the Archive button and shortcut only raised the not-found toast. + * The folder is created in the account the action targets (see + * findArchiveMailbox) with the `archive` role, so the sidebar shows it under + * its translated name and other clients recognise it. `refresh` reloads the + * mailbox list so the caller gets the store's own object (namespaced id, + * `originalId`) rather than the bare one Mailbox/set returned. (#578) + */ +export async function ensureArchiveMailbox(opts: { + client: IJMAPClient; + mailboxes: Mailbox[]; + selectedMailboxId: string | null | undefined; + accountId?: string; + refresh: () => Promise; +}): Promise { + const existing = findArchiveMailbox(opts.mailboxes, opts.selectedMailboxId, opts.accountId); + if (existing) return existing; + + const viewMailbox = opts.mailboxes.find(m => m.id === opts.selectedMailboxId); + const scopeId = opts.accountId ?? (viewMailbox?.isShared ? viewMailbox.accountId : undefined); + await opts.client.createMailbox('Archive', undefined, scopeId, { role: 'archive' }); + + const created = findArchiveMailbox(await opts.refresh(), opts.selectedMailboxId, opts.accountId); + if (!created) throw new ArchiveMailboxNotFoundError(); + return created; +} + /** * JMAP accountId for opening an email that carries no source stamps. * @@ -3862,10 +3892,19 @@ export const useEmailStore = create((set, get) => ({ // Scope the archive folder to the viewed shared/group account (if any) so the // move lands on the owner account, not the user's own archive (which appears // first in the merged list); see resolveViewAccountId. Own view is unchanged. - const archiveMailbox = findArchiveMailbox(mailboxes, get().selectedMailbox); - if (!archiveMailbox) { - const error = new ArchiveMailboxNotFoundError(); - set({ error: error.message }); + let archiveMailbox: Mailbox; + try { + archiveMailbox = await ensureArchiveMailbox({ + client: resolveActionClient(client), + mailboxes, + selectedMailboxId: get().selectedMailbox, + refresh: async () => { + await refreshMailboxesForViewingAccount(client); + return resolveActionMailboxes(); + }, + }); + } catch (error) { + set({ error: error instanceof Error ? error.message : 'Failed to archive emails' }); throw error; }