mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 16:13:15 +00:00
Merge pull request #1176 from cybersmurf/fix/archive-auto-create
fix: create the Archive folder on first use when the account has none
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -140,10 +140,11 @@ export class DemoJMAPClient implements IJMAPClient {
|
||||
async getMailboxes(_accountId?: string): Promise<Mailbox[]> { return [...this.data.mailboxes]; }
|
||||
async getAllMailboxes(): Promise<Mailbox[]> { return [...this.data.mailboxes]; }
|
||||
|
||||
async createMailbox(name: string, parentId?: string, _accountId?: string): Promise<Mailbox> {
|
||||
async createMailbox(name: string, parentId?: string, _accountId?: string, options?: { role?: string }): Promise<Mailbox> {
|
||||
const mb: Mailbox = {
|
||||
id: generateDemoId('mailbox'),
|
||||
name,
|
||||
role: options?.role,
|
||||
sortOrder: 100,
|
||||
totalEmails: 0,
|
||||
unreadEmails: 0,
|
||||
|
||||
@@ -137,7 +137,7 @@ export interface IJMAPClient {
|
||||
getMailboxChanges?(sinceState: string, accountId?: string, maxChanges?: number): Promise<CollectionChanges | null>;
|
||||
/** Email/changes since `sinceState`; null when the server cannot compute the delta. */
|
||||
getEmailChanges?(sinceState: string, accountId?: string, maxChanges?: number): Promise<CollectionChanges | null>;
|
||||
createMailbox(name: string, parentId?: string, accountId?: string): Promise<Mailbox>;
|
||||
createMailbox(name: string, parentId?: string, accountId?: string, options?: { role?: string }): Promise<Mailbox>;
|
||||
updateMailbox(mailboxId: string, changes: { name?: string; parentId?: string | null; role?: string | null; sortOrder?: number }, accountId?: string): Promise<void>;
|
||||
// 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.
|
||||
|
||||
+5
-1
@@ -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<Mailbox> {
|
||||
async createMailbox(name: string, parentId?: string, accountId?: string, options?: { role?: string }): Promise<Mailbox> {
|
||||
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,
|
||||
|
||||
@@ -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> = {}): 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<typeof vi.fn> } {
|
||||
return { createMailbox: vi.fn(async () => ownArchive) } as unknown as IJMAPClient & { createMailbox: ReturnType<typeof vi.fn> };
|
||||
}
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
@@ -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> = {}): 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<typeof vi.fn>).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<typeof vi.fn>).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);
|
||||
});
|
||||
});
|
||||
|
||||
+43
-4
@@ -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<Mailbox[]>;
|
||||
}): Promise<Mailbox> {
|
||||
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<EmailStore>((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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user