From 3fac196d3ff43c6d5e766fdd88eb34a3b2967333 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:08:37 +0200 Subject: [PATCH] fix: ask plugins before creating archive folders and marking whole folders read --- components/mail/mail-app.tsx | 23 ++++-- components/settings/reading-settings.tsx | 58 ++++++++------- lib/filters/quick-rule-flow.ts | 4 +- lib/plugin-mail-hooks.ts | 23 ++++++ .../email-store-plugin-mailbox-hooks.test.ts | 73 ++++++++++++++++++- stores/email-store.ts | 49 +++++++++++-- 6 files changed, 191 insertions(+), 39 deletions(-) diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index b2ef72ec2..f9ae1b1f9 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -120,6 +120,7 @@ import { PLUGIN_REPLY_EVENT, type PluginReplyRequest } from "@/lib/plugin-sandbo import { mailtoComposerDraft } from "./mailto-draft"; import { getQuoteBodies } from "@/lib/email-composer-utils"; import { emailHooks } from "@/lib/plugin-hooks"; +import { createMailboxWithHooks } from "@/lib/plugin-mail-hooks"; import { emailToReadView } from "@/lib/plugin-projection"; import { emitThreadExpand } from "@/lib/plugin-thread-expand"; import { announceSent, cancelAnnouncedSend, releaseAnnouncedSend } from "@/lib/plugin-after-send"; @@ -2519,6 +2520,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { return readMailboxes(); }, }); + if (!archiveMailbox) return; // a plugin refused the Archive folder let archived: boolean; if (archiveMode === 'single') { @@ -2531,9 +2533,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { let yearMailbox = currentMailboxes.find( m => m.name === year && m.parentId === archiveId - ); + ) ?? null; if (!yearMailbox) { - yearMailbox = await archiveClient.createMailbox(year, archiveId, archiveAccountId); + yearMailbox = await createMailboxWithHooks(archiveClient, year, archiveId, archiveAccountId); + if (!yearMailbox) return; // a plugin refused the folder await refreshMailboxes(); } @@ -2544,9 +2547,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const afterYear = readMailboxes(); let monthMailbox = afterYear.find( m => m.name === month && m.parentId === yearId - ); + ) ?? null; if (!monthMailbox) { - monthMailbox = await archiveClient.createMailbox(month, yearId, archiveAccountId); + monthMailbox = await createMailboxWithHooks(archiveClient, month, yearId, archiveAccountId); + if (!monthMailbox) return; // a plugin refused the folder await refreshMailboxes(); } archived = await moveThreadToMailbox(client, emailToArchive.id, monthMailbox.id); @@ -2970,6 +2974,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (!client) return; try { const count = await markMailboxAsRead(client, mailboxId); + if (count === null) return; // a plugin stopped it and said why await fetchMailboxes(client); if (selectedMailbox === mailboxId) await fetchEmails(client, mailboxId); if (count > 0) { @@ -3002,9 +3007,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { try { const ids = collectIds(mailboxId); let total = 0; + let stopped = false; for (const id of ids) { - total += await markMailboxAsRead(client, id); + const count = await markMailboxAsRead(client, id); + if (count === null) stopped = true; + else total += count; } + // Every folder a plugin stopped: it said why, nothing to report. + if (stopped && total === 0) return; await fetchMailboxes(client); if (selectedMailbox && ids.includes(selectedMailbox)) await fetchEmails(client, selectedMailbox); if (total > 0) { @@ -3029,7 +3039,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (!confirmed) return; try { - const total = await client.markAllAsRead(); + const total = await useEmailStore.getState().markAllMailboxesAsRead(client); + if (total === null) return; // a plugin stopped it and said why await fetchMailboxes(client); if (selectedMailbox) await fetchEmails(client, selectedMailbox); if (total > 0) { diff --git a/components/settings/reading-settings.tsx b/components/settings/reading-settings.tsx index 25728ea30..b36b42ff7 100644 --- a/components/settings/reading-settings.tsx +++ b/components/settings/reading-settings.tsx @@ -12,6 +12,8 @@ import { cn } from '@/lib/utils'; import { SettingsSection, SettingItem, Select, ToggleSwitch } from './settings-section'; import { AlertTriangle, FolderSync, Loader2 } from '@/components/icons'; import { usePolicyStore } from '@/stores/policy-store'; +import { allowEmailMove, createMailboxWithHooks, emitEmailMove } from '@/lib/plugin-mail-hooks'; +import type { Mailbox } from '@/lib/jmap/types'; export function ReadingSettings() { const t = useTranslations('settings.email_behavior'); @@ -61,39 +63,45 @@ export function ReadingSettings() { try { const archiveId = archiveMailbox.originalId || archiveMailbox.id; const emails = await client.getEmailsInMailbox(archiveId); - let movedCount = 0; + // One group per destination ("2024" or "2024/03"): plugins are asked + // once per folder and move, and can refuse either; the groups they + // refuse stay where they are. + const groups = new Map(); for (const email of emails) { const emailDate = new Date(email.receivedAt); const year = emailDate.getFullYear().toString(); const month = (emailDate.getMonth() + 1).toString().padStart(2, '0'); + const key = archiveMode === 'year' ? year : `${year}/${month}`; + const group = groups.get(key) ?? { year, month, ids: [] }; + group.ids.push(email.id); + groups.set(key, group); + } - let currentMailboxes = useEmailStore.getState().mailboxes; + // An existing folder, or a new one; null when a plugin refused it. + const folder = async (name: string, parentId: string): Promise => { + const existing = useEmailStore.getState().mailboxes.find(m => m.name === name && m.parentId === parentId); + if (existing) return existing; + const created = await createMailboxWithHooks(client, name, parentId); + if (created) await fetchMailboxes(client); + return useEmailStore.getState().mailboxes.find(m => m.name === name && m.parentId === parentId) ?? created; + }; - let yearMailbox = currentMailboxes.find( - m => m.name === year && m.parentId === archiveId - ); - if (!yearMailbox) { - yearMailbox = await client.createMailbox(year, archiveId); - await fetchMailboxes(client); - currentMailboxes = useEmailStore.getState().mailboxes; - } + let movedCount = 0; + for (const { year, month, ids } of groups.values()) { + const yearMailbox = await folder(year, archiveId); + if (!yearMailbox) continue; + const destination = archiveMode === 'year' + ? yearMailbox + : await folder(month, yearMailbox.originalId || yearMailbox.id); + if (!destination) continue; - if (archiveMode === 'year') { - await client.moveEmail(email.id, yearMailbox.id); - movedCount++; - } else { - const yearId = yearMailbox.originalId || yearMailbox.id; - let monthMailbox = currentMailboxes.find( - m => m.name === month && m.parentId === yearId - ); - if (!monthMailbox) { - monthMailbox = await client.createMailbox(month, yearId); - await fetchMailboxes(client); - } - await client.moveEmail(email.id, monthMailbox.id); - movedCount++; - } + const known = useEmailStore.getState().mailboxes; + const move = { emailIds: ids, fromMailboxId: archiveMailbox.id, toMailboxId: destination.id }; + if (!(await allowEmailMove(move, known))) continue; + await client.batchMoveEmails(ids, destination.originalId || destination.id); + emitEmailMove(move, known); + movedCount += ids.length; } setReorganizeResult(t('archive_mode.reorganize_success', { count: movedCount })); diff --git a/lib/filters/quick-rule-flow.ts b/lib/filters/quick-rule-flow.ts index 37faafbf0..d49a85eba 100644 --- a/lib/filters/quick-rule-flow.ts +++ b/lib/filters/quick-rule-flow.ts @@ -8,6 +8,7 @@ import { useAuthStore } from '@/stores/auth-store'; import { useEmailStore, type BatchActionTarget } from '@/stores/email-store'; import { useFilterStore } from '@/stores/filter-store'; import { emailHooks } from '@/lib/plugin-hooks'; +import { createMailboxWithHooks } from '@/lib/plugin-mail-hooks'; import { useQuickRuleStore, type NewFolderRequest, type QuickRuleEditor } from '@/stores/quick-rule-store'; import { FiltersChangedError, @@ -394,11 +395,12 @@ export async function createFolderAndRunPreset( const { target } = request; let created; try { - created = await target.client.createMailbox(name.trim()); + created = await createMailboxWithHooks(target.client, name.trim()); } catch (error) { toast.error(text.notifications('rule_folder_failed'), error instanceof Error ? error.message : undefined); return; } + if (!created) return; // a plugin refused the folder and said why // The sidebar of the account on screen; other accounts follow their push. const email = useEmailStore.getState(); if (useAuthStore.getState().client === target.client && !email.viewingAccountId) { diff --git a/lib/plugin-mail-hooks.ts b/lib/plugin-mail-hooks.ts index 608b25c8f..d222ddf13 100644 --- a/lib/plugin-mail-hooks.ts +++ b/lib/plugin-mail-hooks.ts @@ -4,6 +4,7 @@ import { emailHooks } from './plugin-hooks'; import type { Mailbox } from './jmap/types'; +import type { IJMAPClient } from './jmap/client-interface'; import type { MailboxView, QuotaView } from './plugin-types'; export function mailboxToView(mailbox: Mailbox): MailboxView { @@ -98,3 +99,25 @@ export function emitEmailMove(move: EmailMove, mailboxes: Mailbox[]): void { void emailHooks.onEmailUnarchive.emit({ emailIds: [...move.emailIds], toMailboxId: move.toMailboxId }); } } + +/** + * Create a folder straight through a client (the Archive folder and its + * year/month folders, a quick rule's new folder), asking and telling plugins + * like the store's createMailbox does. Resolves null when a plugin said no. + * `parentId` is the server's id of the parent, as the client takes it. + */ +export async function createMailboxWithHooks( + client: IJMAPClient, + name: string, + parentId?: string, + accountId?: string, + options?: { role?: string }, +): Promise { + if (emailHooks.onBeforeMailboxCreate.size > 0 + && !(await emailHooks.onBeforeMailboxCreate.intercept({ name, parentId: parentId ?? null }))) { + return null; + } + const created = await client.createMailbox(name, parentId, accountId, options); + void emailHooks.onMailboxCreate.emit({ mailboxId: created?.id ?? null, name, parentId: parentId ?? null }); + return created; +} diff --git a/stores/__tests__/email-store-plugin-mailbox-hooks.test.ts b/stores/__tests__/email-store-plugin-mailbox-hooks.test.ts index 272683cfd..09a57ee1c 100644 --- a/stores/__tests__/email-store-plugin-mailbox-hooks.test.ts +++ b/stores/__tests__/email-store-plugin-mailbox-hooks.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { useEmailStore } from '../email-store'; +import { ensureArchiveMailbox, useEmailStore } from '../email-store'; import { useAuthStore } from '../auth-store'; import { useSettingsStore } from '@/stores/settings-store'; import { clearAllHooks, emailHooks } from '@/lib/plugin-hooks'; @@ -46,6 +46,12 @@ beforeEach(() => { getAllMailboxes: vi.fn().mockResolvedValue(mailboxes), // A fresh object per read, as from the server. getQuota: vi.fn().mockImplementation(async () => ({ used: 50, total: 200 })), + markMailboxAsRead: vi.fn().mockResolvedValue(2), + markAllAsRead: vi.fn().mockResolvedValue(5), + queryEmailFields: vi.fn().mockResolvedValue([{ id: 'u1' }, { id: 'u2' }]), + batchMarkAsRead: vi.fn().mockResolvedValue(undefined), + getMailboxes: vi.fn().mockResolvedValue(mailboxes), + fetchTagCounts: vi.fn(), } as unknown as IJMAPClient; useAuthStore.setState({ activeAccountId: 'acct', getClientForAccount: () => client as never } as never); useSettingsStore.setState({ deleteAction: 'trash' } as never); @@ -190,3 +196,68 @@ describe('quota hook', () => { ]); }); }); + +// Marking a whole folder read is one server call that doesn't say which +// messages it touched, so plugins listening get the ids listed first. +describe('marking a folder read', () => { + it('uses the one bulk call when no plugin listens', async () => { + expect(await useEmailStore.getState().markMailboxAsRead(client, 'inbox')).toBe(2); + expect(client.markMailboxAsRead).toHaveBeenCalled(); + expect(client.queryEmailFields).not.toHaveBeenCalled(); + }); + + it('asks plugins with the unread ids and tells them afterwards', async () => { + const before = vi.fn(() => true); + const after = vi.fn(); + emailHooks.onBeforeEmailReadStateChange.register('t', before); + emailHooks.onEmailReadStateChange.register('t', after); + + expect(await useEmailStore.getState().markMailboxAsRead(client, 'inbox')).toBe(2); + + expect(client.queryEmailFields).toHaveBeenCalledWith({ inMailbox: 'inbox', notKeyword: '$seen' }, [], undefined, expect.any(Number)); + expect(before).toHaveBeenCalledWith({ emailIds: ['u1', 'u2'], read: true }); + expect(client.batchMarkAsRead).toHaveBeenCalledWith(['u1', 'u2'], true, undefined); + expect(after).toHaveBeenCalledWith({ emailIds: ['u1', 'u2'], read: true }); + expect(client.markMailboxAsRead).not.toHaveBeenCalled(); + }); + + it('leaves the folder unread when a plugin says no', async () => { + emailHooks.onBeforeEmailReadStateChange.register('t', () => false); + + expect(await useEmailStore.getState().markMailboxAsRead(client, 'inbox')).toBeNull(); + expect(client.batchMarkAsRead).not.toHaveBeenCalled(); + expect(client.markMailboxAsRead).not.toHaveBeenCalled(); + expect(useEmailStore.getState().mailboxes.find(m => m.id === 'inbox')?.unreadEmails).toBe(1); + }); + + it('asks the same way for every folder at once', async () => { + emailHooks.onBeforeEmailReadStateChange.register('t', () => false); + + expect(await useEmailStore.getState().markAllMailboxesAsRead(client)).toBeNull(); + expect(client.queryEmailFields).toHaveBeenCalledWith({ notKeyword: '$seen' }, [], undefined, expect.any(Number)); + expect(client.markAllAsRead).not.toHaveBeenCalled(); + }); +}); + +describe('folders created outside the store', () => { + it('asks before creating the Archive folder, and archives nothing when refused', async () => { + emailHooks.onBeforeMailboxCreate.register('t', () => false); + const refresh = vi.fn(async () => mailboxes); + + const archive = await ensureArchiveMailbox({ client, mailboxes, selectedMailboxId: 'inbox', refresh }); + + expect(archive).toBeNull(); + expect(client.createMailbox).not.toHaveBeenCalled(); + }); + + it('reports the Archive folder it created', async () => { + const created = vi.fn(); + emailHooks.onMailboxCreate.register('t', created); + const archive = makeMailbox({ id: 'archive', name: 'Archive', role: 'archive' }); + const refresh = vi.fn(async () => [...mailboxes, archive]); + + expect(await ensureArchiveMailbox({ client, mailboxes, selectedMailboxId: 'inbox', refresh })).toBe(archive); + expect(client.createMailbox).toHaveBeenCalledWith('Archive', undefined, undefined, { role: 'archive' }); + expect(created).toHaveBeenCalledWith(expect.objectContaining({ name: 'Archive', parentId: null })); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index e89c7d5aa..ac536f195 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -7,7 +7,7 @@ import { useCalendarStore } from "@/stores/calendar-store"; import { orderKeywords, placeHeldRows, type HeldKeywords, type SortLevel } from "@/lib/message-list-order"; import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty } from "@/lib/jmap/search-utils"; import { emailHooks } from "@/lib/plugin-hooks"; -import { allowEmailMove, emitEmailMove, idsFromSelectionKeys, keywordChangeLists, mailboxToView, quotaToView } from "@/lib/plugin-mail-hooks"; +import { allowEmailMove, createMailboxWithHooks, emitEmailMove, idsFromSelectionKeys, keywordChangeLists, mailboxToView, quotaToView } from "@/lib/plugin-mail-hooks"; import { emailSummary, emailToReadView } from "@/lib/plugin-projection"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { emailKeyFor, threadKeyFor, threadIdFromKey, KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils"; @@ -429,7 +429,10 @@ interface EmailStore { /** Reparents several folders at once; resolves with the ids the server refused. */ moveMailboxes: (client: IJMAPClient, mailboxIds: string[], newParentId: string | null) => Promise; emptyMailbox: (client: IJMAPClient, mailboxId: string) => Promise; - markMailboxAsRead: (client: IJMAPClient, mailboxId: string) => Promise; + /** How many messages were marked read; null when a plugin stopped it. */ + markMailboxAsRead: (client: IJMAPClient, mailboxId: string) => Promise; + /** Every unread message in the account; null when a plugin stopped it. */ + markAllMailboxesAsRead: (client: IJMAPClient) => Promise; // Unified mailbox operations fetchUnifiedEmails: (accounts: UnifiedAccountClient[], role: UnifiedMailboxRole) => Promise; @@ -878,13 +881,14 @@ export async function ensureArchiveMailbox(opts: { selectedMailboxId: string | null | undefined; accountId?: string; refresh: () => Promise; -}): 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' }); + // null: a plugin refused the new folder (and said why). + if (!(await createMailboxWithHooks(opts.client, 'Archive', undefined, scopeId, { role: 'archive' }))) return null; const created = findArchiveMailbox(await opts.refresh(), opts.selectedMailboxId, opts.accountId); if (!created) throw new ArchiveMailboxNotFoundError(); @@ -1630,6 +1634,30 @@ function findRoleMailbox(mailboxes: Mailbox[], role: string, accountId: string | * Throws when any message could not be moved; the ones that were moved still * leave the list. */ +/** + * Mark every unread message matching `filter` read. The server-side bulk + * call doesn't say which messages it touched, so when a plugin listens the + * ids are listed first, plugins are asked once, and told afterwards. + * Resolves null when a plugin said no. + */ +async function markReadAskingPlugins( + client: IJMAPClient, + filter: Record, + accountId: string | undefined, + bulk: (client: IJMAPClient) => Promise, +): Promise { + const asks = emailHooks.onBeforeEmailReadStateChange.size > 0; + const tells = emailHooks.onEmailReadStateChange.size > 0; + if (!asks && !tells) return bulk(client); + const unread = await client.queryEmailFields({ ...filter, notKeyword: '$seen' }, [], accountId, Number.MAX_SAFE_INTEGER); + const emailIds = unread.map(e => String(e.id)); + if (emailIds.length === 0) return 0; + if (asks && !(await emailHooks.onBeforeEmailReadStateChange.intercept({ emailIds, read: true }))) return null; + await client.batchMarkAsRead(emailIds, true, accountId); + void emailHooks.onEmailReadStateChange.emit({ emailIds, read: true }); + return emailIds.length; +} + async function runBatchSpamAction( get: StoreGet, set: StoreSet, @@ -4416,7 +4444,7 @@ 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. - let archiveMailbox: Mailbox; + let archiveMailbox: Mailbox | null; try { archiveMailbox = await ensureArchiveMailbox({ client: resolveActionClient(client), @@ -4431,6 +4459,7 @@ export const useEmailStore = create((set, get) => ({ set({ error: error instanceof Error ? error.message : 'Failed to archive emails' }); throw error; } + if (!archiveMailbox) return false; const mode = useSettingsStore.getState().archiveMode; const archiveId = archiveMailbox.originalId || archiveMailbox.id; @@ -5810,13 +5839,21 @@ export const useEmailStore = create((set, get) => ({ return true; }, + markAllMailboxesAsRead: async (client) => { + return markReadAskingPlugins(client, {}, undefined, (actionClient) => actionClient.markAllAsRead()); + }, + markMailboxAsRead: async (client, mailboxId) => { try { const mailbox = resolveActionMailboxes().find(mb => mb.id === mailboxId); const accountId = mailbox?.isShared ? mailbox.accountId : undefined; const jmapMailboxId = mailbox?.originalId || mailboxId; - const count = await resolveActionClient(client).markMailboxAsRead(jmapMailboxId, accountId); + const count = await markReadAskingPlugins( + resolveActionClient(client), { inMailbox: jmapMailboxId }, accountId, + (actionClient) => actionClient.markMailboxAsRead(jmapMailboxId, accountId), + ); + if (count === null) return null; // Update local state: mark all emails currently visible in this mailbox as read, // and zero-out the mailbox unread counter.