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 a3fe17295..e107a513b 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 824da0708..6fcba87b2 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; // Many folder updates in as few Mailbox/set calls as the server allows. // Refused folders don't stop the rest: returns their ids with the SetError type. diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index a23d50ec7..8f0804c8b 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 4a41105c0..253c09fcc 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -735,6 +735,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. * @@ -3864,10 +3894,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; }