From 0e0d99e7c39da3c8cc6176cf545d88315601f027 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 02:23:28 +0200 Subject: [PATCH] fix: send unified-view archive, move and spam to each message's account In the unified views, a tag view and an "All folders" search a selection can span logins and accounts, and message, account and folder ids repeat across both: Stalwart numbers the accounts and the system folders of every install alike. - Archive sent the whole selection through one login into the Archive of the selected folder's account, which is a virtual folder there, so the own one: messages of a group account or of another login were archived by their ids in the own account, where the same ids can name unrelated messages, or failed after the own ones had moved. The list dropped every selected row either way. Each login and account now archives into its own Archive. - "Move to" and dragging messages applied one folder id to every account, so another account's messages landed in whatever folder has that id there, and failed moves were dropped from the list anyway. Only messages in the folder's own login and account change folders in place now; the others take the cross-account move (Email/copy within one login, download and import across logins), a single message moved with "Move to" or dragged as well. - A drop goes to the login whose sidebar list shows the folder. It went to the first login whose own account id matched the folder's, which with two servers that both number the user `e` could be the other one. - "Spam" did nothing while the app said the messages were marked, and "Not spam" failed, because the selected folder is a virtual one there. Both now write each message in its own account and move it to that account's Junk or Inbox. The spam shortcut in "All Junk" means "Not spam", as the toolbar does. - What could not be archived, moved or marked stays listed and selected, and the error says why. --- components/layout/sidebar.tsx | 3 +- components/mail/mail-app.tsx | 11 +- hooks/__tests__/use-mailbox-drop.test.tsx | 117 ++++ hooks/use-mailbox-drop.ts | 146 ++-- .../email-store-shared-folder-actions.test.ts | 23 + .../email-store-unified-actions.test.ts | 227 +++++- stores/email-store.ts | 654 +++++++++++++----- 7 files changed, 917 insertions(+), 264 deletions(-) create mode 100644 hooks/__tests__/use-mailbox-drop.test.tsx diff --git a/components/layout/sidebar.tsx b/components/layout/sidebar.tsx index d6b2f3447..b557780ef 100644 --- a/components/layout/sidebar.tsx +++ b/components/layout/sidebar.tsx @@ -499,7 +499,7 @@ function MailboxTreeItem({ onContextMenu?: (e: React.MouseEvent, node: MailboxNode) => void; /** Connected-account id this folder is listed under; null = active account. * Travels in the folder-drag payload so a folder tab fetches through the - * right client. */ + * right client, and tells a message drop which login the folder is in. */ dragAccountId?: string | null; }) { const tNotifications = useTranslations('notifications'); @@ -533,6 +533,7 @@ function MailboxTreeItem({ const { isDragging: globalDragging } = useDragDropContext(); const { dropHandlers, isValidDropTarget, isInvalidDropTarget } = useMailboxDrop({ mailbox: node, + accountId: dragAccountId ?? undefined, onSuccess: (count, mailboxName) => { if (count === 1) { toast.success( diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 401caf525..bd0d92ac6 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -790,12 +790,15 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }, onToggleSpam: async () => { if (isScheduledView) return; - const currentMailbox = mailboxes.find(m => m.id === selectedMailbox); + // An aggregate role view ("All Junk") selects a virtual folder: its + // role decides, as for the list's batch actions. + const currentRole = mailboxes.find(m => m.id === selectedMailbox)?.role + ?? (isUnifiedView ? (unifiedRole ?? undefined) : undefined); // Marking your own outgoing mail as spam makes no sense - the toolbar // and menus hide the action in Sent/Drafts/Scheduled, so the shortcut // is a no-op there too. - if (['sent', 'drafts', 'scheduled'].includes(currentMailbox?.role || '')) return; - const isInJunk = currentMailbox?.role === 'junk'; + if (['sent', 'drafts', 'scheduled'].includes(currentRole || '')) return; + const isInJunk = currentRole === 'junk'; if (selectedEmailIds.size > 0 && client) { const ids = Array.from(selectedEmailIds); try { @@ -868,7 +871,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }, // eslint-disable-next-line react-hooks/exhaustive-deps - }), [activeEmails, selectedEmail, client, selectedMailbox, isMobile, isTablet, selectedEmailIds, mailboxes, isScheduledView]); + }), [activeEmails, selectedEmail, client, selectedMailbox, isMobile, isTablet, selectedEmailIds, mailboxes, isScheduledView, isUnifiedView, unifiedRole]); // Initialize keyboard shortcuts useKeyboardShortcuts({ diff --git a/hooks/__tests__/use-mailbox-drop.test.tsx b/hooks/__tests__/use-mailbox-drop.test.tsx new file mode 100644 index 000000000..ccce2fd9a --- /dev/null +++ b/hooks/__tests__/use-mailbox-drop.test.tsx @@ -0,0 +1,117 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; +import type { DragEvent } from 'react'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import { useEmailStore } from '@/stores/email-store'; +import { useAuthStore } from '@/stores/auth-store'; + +const drag = vi.hoisted(() => ({ + isDragging: true, + sourceMailboxId: '' as string | null, + draggedEmails: [] as Email[], + endDrag: () => {}, +})); +vi.mock('@/contexts/drag-drop-context', () => ({ useDragDropContext: () => drag })); + +const { useMailboxDrop } = await import('../use-mailbox-drop'); + +// userb is logged in to a Stalwart 0.16.23 and a 1.0 server and is account `e` +// on both: a folder's account id can't say which login it belongs to, only +// the sidebar list it is shown in can. Both have their own Archive as `f`. + +function mailbox(overrides: Partial): Mailbox { + return { + id: 'f', name: 'Archive', role: 'archive', sortOrder: 0, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, + myRights: { mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true }, + isSubscribed: true, isShared: false, accountId: 'e', + ...overrides, + }; +} + +function email(overrides: Partial): Email { + return { id: 'x', threadId: 't', subject: 's', receivedAt: '2026-09-01T08:00:00Z', keywords: {}, mailboxIds: { a: true }, ...overrides } as Email; +} + +function dropEvent(ids: string[]): DragEvent { + return { + preventDefault: () => {}, + stopPropagation: () => {}, + dataTransfer: { getData: () => JSON.stringify(ids) }, + } as unknown as DragEvent; +} + +describe('useMailboxDrop', () => { + const client016 = { getAccountId: () => 'e' } as unknown as IJMAPClient; + const client10 = { getAccountId: () => 'e' } as unknown as IJMAPClient; + let moveEmailsToMailbox: ReturnType; + let crossAccountMoveEmails: ReturnType; + + beforeEach(() => { + moveEmailsToMailbox = vi.fn().mockResolvedValue(undefined); + crossAccountMoveEmails = vi.fn().mockResolvedValue(undefined); + useAuthStore.setState({ + client: client016, + activeAccountId: 'userb@v016', + getClientForAccount: (id: string) => (id === 'userb@v016' ? client016 : id === 'userb@v10' ? client10 : undefined), + // The 1.0 login first: matching the folder's account id against each + // login's own used to pick it for the 0.16.23 login's folders too. + getAllConnectedClients: () => new Map([['userb@v10', client10], ['userb@v016', client016]]), + } as never); + useEmailStore.setState({ + isUnifiedView: false, + selectedKeyword: null, + searchQuery: '', + viewingAccountId: null, + selectedMailbox: 'a', + mailboxes: [mailbox({ id: 'a', name: 'Inbox', role: 'inbox' }), mailbox({})], + selectedEmailIds: new Set(), + moveEmailsToMailbox: moveEmailsToMailbox as never, + crossAccountMoveEmails: crossAccountMoveEmails as never, + refreshCurrentMailbox: vi.fn().mockResolvedValue(undefined) as never, + }); + drag.sourceMailboxId = 'a'; + drag.draggedEmails = [email({ id: 'm1' })]; + }); + + const drop = async (target: Mailbox, ids: string[], accountId?: string) => { + const { result } = renderHook(() => useMailboxDrop({ mailbox: target, accountId, onSuccess: () => {}, onError: () => {} })); + await act(async () => { result.current.dropHandlers.onDrop(dropEvent(ids)); }); + }; + + it('lets the store move a drop from the unified view, into the folder of the login it is listed under', async () => { + useEmailStore.setState({ isUnifiedView: true, selectedMailbox: '__unified_inbox' }); + drag.sourceMailboxId = '__unified_inbox'; + drag.draggedEmails = [ + email({ id: 'm016', accountId: 'userb@v016', sourceClientAccountId: 'userb@v016', sourceAccountId: 'e' }), + email({ id: 't016', accountId: 'c', sourceClientAccountId: 'userb@v016', sourceAccountId: 'c' }), + ]; + + await drop(mailbox({}), ['m016', 't016'], 'userb@v10'); + await drop(mailbox({}), ['m016', 't016']); + + expect(moveEmailsToMailbox.mock.calls).toEqual([ + [client016, ['m016', 't016'], 'f', 'userb@v10'], + [client016, ['m016', 't016'], 'f', 'userb@v016'], + ]); + expect(crossAccountMoveEmails).not.toHaveBeenCalled(); + }); + + it('moves a drop onto another login\'s folder across accounts, although both logins are account `e`', async () => { + useAuthStore.setState({ + getAllConnectedClients: () => new Map([['userb@v016', client016], ['userb@v10', client10]]), + } as never); + + await drop(mailbox({}), ['m1'], 'userb@v10'); + + expect(crossAccountMoveEmails).toHaveBeenCalledWith(new Map([['userb@v016', ['m1']]]), 'userb@v10', 'f', undefined, undefined); + expect(moveEmailsToMailbox).not.toHaveBeenCalled(); + }); + + it('moves a drop onto the active login\'s own folder in place', async () => { + await drop(mailbox({}), ['m1']); + + expect(moveEmailsToMailbox).toHaveBeenCalledWith(client016, ['m1'], 'f'); + expect(crossAccountMoveEmails).not.toHaveBeenCalled(); + }); +}); diff --git a/hooks/use-mailbox-drop.ts b/hooks/use-mailbox-drop.ts index 4954423ca..400cbfcf4 100644 --- a/hooks/use-mailbox-drop.ts +++ b/hooks/use-mailbox-drop.ts @@ -2,7 +2,7 @@ import { useCallback, useState, DragEvent } from "react"; import { Mailbox, Email } from "@/lib/jmap/types"; -import { useEmailStore } from "@/stores/email-store"; +import { useEmailStore, isAggregateListView } from "@/stores/email-store"; import { useAuthStore } from "@/stores/auth-store"; import { useDragDropContext } from "@/contexts/drag-drop-context"; import { toast } from "@/stores/toast-store"; @@ -21,29 +21,16 @@ function resolveSourceAccountId(email: Email | undefined): string | null { return useAuthStore.getState().activeAccountId; } -/** - * Returns the local accountId ("user@host") that owns the destination - * mailbox. `mailbox.accountId` is the JMAP server's opaque account id, but - * `clients`, `activeAccountId`, and `email.accountId` all live in the local - * namespace. We map back by matching the JMAP id against each connected - * client's `getAccountId()`. Falls back to the viewing/active account so - * single-account flows (no connected clients map entry yet, in-memory edits, - * etc.) still resolve correctly. - */ -function resolveDestAccountId(mailbox: Mailbox): string | null { - const jmapId = mailbox.accountId; - if (jmapId) { - const clients = useAuthStore.getState().getAllConnectedClients(); - for (const [localId, client] of clients) { - if (client.getAccountId() === jmapId) return localId; - } - } - return useEmailStore.getState().viewingAccountId - ?? useAuthStore.getState().activeAccountId; -} - interface UseMailboxDropOptions { mailbox: Mailbox; + /** + * The local account ("user@host") whose folder list `mailbox` is listed + * in: the multi-account sidebar lists every login's folders. Absent, the + * active account's. `mailbox.accountId` can't tell: JMAP account ids are + * only unique per server, and Stalwart numbers the accounts of every + * install alike. + */ + accountId?: string; onDropComplete?: () => void; // Translation callbacks for toast messages onSuccess?: (count: number, mailboxName: string) => void; @@ -62,7 +49,7 @@ interface UseMailboxDropReturn { isInvalidDropTarget: boolean; } -export function useMailboxDrop({ mailbox, onDropComplete, onSuccess, onError }: UseMailboxDropOptions): UseMailboxDropReturn { +export function useMailboxDrop({ mailbox, accountId, onDropComplete, onSuccess, onError }: UseMailboxDropOptions): UseMailboxDropReturn { const [isOver, setIsOver] = useState(false); const { client } = useAuthStore(); const { moveEmailsToMailbox, crossAccountMoveEmails, selectedEmailIds, clearSelection, refreshCurrentMailbox, mailboxes } = useEmailStore(); @@ -129,58 +116,69 @@ export function useMailboxDrop({ mailbox, onDropComplete, onSuccess, onError }: const emailIds: string[] = JSON.parse(emailIdsJson); - // Group dragged emails by source account. In single-account flows this - // collapses to one bucket; in unified view or the Pro multi-account - // sidebar a single drag can mix sources. - const destAccountId = resolveDestAccountId(mailbox); - const idToEmail = new Map(draggedEmails.map((em) => [em.id, em])); - const bySource = new Map(); - for (const id of emailIds) { - const srcAccountId = resolveSourceAccountId(idToEmail.get(id)); - if (!srcAccountId) continue; - if (!bySource.has(srcAccountId)) bySource.set(srcAccountId, []); - bySource.get(srcAccountId)!.push(id); - } + // The local account the folder belongs to: the one whose list it is in. + const destAccountId = accountId ?? useAuthStore.getState().activeAccountId; - // Decide whether to route via the cross-account (blob copy + import) - // pipeline. Two cases require it: - // 1. Destination is a delegated/shared mailbox whose owner JMAP - // account differs from the source mailbox's JMAP account. There's - // no atomic Email/set across accounts, even via the same client. - // 2. Destination is a primary mailbox on a different connected local - // account than the source — the historical multi-account case. - const sourceMb = useEmailStore.getState().mailboxes.find(mb => mb.id === sourceMailboxId); - const sourceJmapAccountId = sourceMb?.accountId; - const destJmapAccountId = mailbox.accountId; - const sourceAccountIds = Array.from(bySource.keys()); - - const isJmapCrossAccount = - !!sourceJmapAccountId && - !!destJmapAccountId && - sourceJmapAccountId !== destJmapAccountId; - const isLocalCrossAccount = - !mailbox.isShared && - !!destAccountId && - sourceAccountIds.some((src) => src !== destAccountId); - const isCrossAccount = isJmapCrossAccount || isLocalCrossAccount; - - if (isCrossAccount) { - if (!destAccountId) { - throw new Error('Could not resolve destination account'); - } - // For a shared destination there is no separately-connected client - // for the owner; we reuse the viewing user's client but tell the - // import call to target the owner's JMAP account. - const jmapDestId = mailbox.originalId || mailbox.id; - const destJmapOverride = mailbox.isShared ? mailbox.accountId : undefined; - // Mirror image for the source: a shared group mailbox is accessed - // through the viewing user's client, but the email/blob live in the - // owner's JMAP account, so the copy/delete must target it. - const sourceJmapOverride = sourceMb?.isShared ? sourceMb.accountId : undefined; - await crossAccountMoveEmails(bySource, destAccountId, jmapDestId, destJmapOverride, sourceJmapOverride); + let isCrossAccount = false; + if (isAggregateListView()) { + // A list that mixes accounts (unified, tag and "All folders" views) + // stamps every row with its login and account. The store moves each + // of those groups apart - in place only in the folder's own account, + // across accounts for the others. + await moveEmailsToMailbox(client, emailIds, mailbox.id, destAccountId); } else { - // Single-account or same-account-shared move: bulk JMAP request. - await moveEmailsToMailbox(client, emailIds, mailbox.id); + // Group dragged emails by source account. In single-account flows this + // collapses to one bucket; in the Pro multi-account sidebar a single + // drag can mix sources. + const idToEmail = new Map(draggedEmails.map((em) => [em.id, em])); + const bySource = new Map(); + for (const id of emailIds) { + const srcAccountId = resolveSourceAccountId(idToEmail.get(id)); + if (!srcAccountId) continue; + if (!bySource.has(srcAccountId)) bySource.set(srcAccountId, []); + bySource.get(srcAccountId)!.push(id); + } + + // Decide whether to route via the cross-account (blob copy + import) + // pipeline. Two cases require it: + // 1. Destination is a delegated/shared mailbox whose owner JMAP + // account differs from the source mailbox's JMAP account. There's + // no atomic Email/set across accounts, even via the same client. + // 2. Destination is a primary mailbox on a different connected local + // account than the source — the historical multi-account case. + const sourceMb = useEmailStore.getState().mailboxes.find(mb => mb.id === sourceMailboxId); + const sourceJmapAccountId = sourceMb?.accountId; + const destJmapAccountId = mailbox.accountId; + const sourceAccountIds = Array.from(bySource.keys()); + + const isJmapCrossAccount = + !!sourceJmapAccountId && + !!destJmapAccountId && + sourceJmapAccountId !== destJmapAccountId; + const isLocalCrossAccount = + !mailbox.isShared && + !!destAccountId && + sourceAccountIds.some((src) => src !== destAccountId); + isCrossAccount = isJmapCrossAccount || isLocalCrossAccount; + + if (isCrossAccount) { + if (!destAccountId) { + throw new Error('Could not resolve destination account'); + } + // For a shared destination there is no separately-connected client + // for the owner; we reuse the viewing user's client but tell the + // import call to target the owner's JMAP account. + const jmapDestId = mailbox.originalId || mailbox.id; + const destJmapOverride = mailbox.isShared ? mailbox.accountId : undefined; + // Mirror image for the source: a shared group mailbox is accessed + // through the viewing user's client, but the email/blob live in the + // owner's JMAP account, so the copy/delete must target it. + const sourceJmapOverride = sourceMb?.isShared ? sourceMb.accountId : undefined; + await crossAccountMoveEmails(bySource, destAccountId, jmapDestId, destJmapOverride, sourceJmapOverride); + } else { + // Single-account or same-account-shared move: bulk JMAP request. + await moveEmailsToMailbox(client, emailIds, mailbox.id); + } } // Clear selection if any selected emails were moved @@ -222,7 +220,7 @@ export function useMailboxDrop({ mailbox, onDropComplete, onSuccess, onError }: } finally { endDrag(); } - }, [client, mailbox, mailboxes, isValidTarget, moveEmailsToMailbox, crossAccountMoveEmails, draggedEmails, sourceMailboxId, selectedEmailIds, clearSelection, refreshCurrentMailbox, endDrag, onDropComplete, onSuccess, onError]); + }, [client, mailbox, accountId, mailboxes, isValidTarget, moveEmailsToMailbox, crossAccountMoveEmails, draggedEmails, sourceMailboxId, selectedEmailIds, clearSelection, refreshCurrentMailbox, endDrag, onDropComplete, onSuccess, onError]); const valid = isValidTarget(); diff --git a/stores/__tests__/email-store-shared-folder-actions.test.ts b/stores/__tests__/email-store-shared-folder-actions.test.ts index e778c807c..94aa0cd63 100644 --- a/stores/__tests__/email-store-shared-folder-actions.test.ts +++ b/stores/__tests__/email-store-shared-folder-actions.test.ts @@ -66,6 +66,8 @@ function makeClient() { batchDeleteEmails: vi.fn().mockResolvedValue(undefined), batchMoveEmails: vi.fn().mockResolvedValue(undefined), batchArchiveEmails: vi.fn().mockResolvedValue(undefined), + markAsSpam: vi.fn().mockResolvedValue(undefined), + undoSpam: vi.fn().mockResolvedValue(undefined), getEmails: vi.fn().mockResolvedValue({ emails: [], hasMore: false, total: 0 }), getMailboxes: vi.fn().mockResolvedValue([]), } as unknown as IJMAPClient; @@ -155,6 +157,27 @@ describe('non-unified shared-folder batch action routing', () => { ); }); + it('batchMarkAsSpam moves shared emails to junk on the owner account', async () => { + await useEmailStore.getState().batchMarkAsSpam(activeClient, ['e1', 'e2']); + + expect((activeClient.markAsSpam as ReturnType).mock.calls).toEqual([['e1', 'owner-x', false], ['e2', 'owner-x', false]]); + expect(useEmailStore.getState().emails).toEqual([]); + }); + + it('batchMarkAsSpam rejects, instead of letting the caller report a move, when the viewed folder is unknown', async () => { + useEmailStore.setState({ selectedMailbox: 'owner-x:gone' }); + + await expect(useEmailStore.getState().batchMarkAsSpam(activeClient, ['e1'])).rejects.toThrow('Mailbox not found'); + expect(activeClient.markAsSpam).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e1', 'e2']); + }); + + it('batchUndoSpam puts shared emails back in the owner account Inbox', async () => { + await useEmailStore.getState().batchUndoSpam(activeClient, ['e1', 'e2']); + + expect((activeClient.undoSpam as ReturnType).mock.calls).toEqual([['e1', 'x-inbox', 'owner-x'], ['e2', 'x-inbox', 'owner-x']]); + }); + it('single-email archive resolves the SHARED archive when a shared inbox is selected (#889)', async () => { // mail-app's handleArchive resolves its target with the same helper as // batchArchive; with the shared inbox selected it must pick the owner's diff --git a/stores/__tests__/email-store-unified-actions.test.ts b/stores/__tests__/email-store-unified-actions.test.ts index 26fd79e23..940bc39b8 100644 --- a/stores/__tests__/email-store-unified-actions.test.ts +++ b/stores/__tests__/email-store-unified-actions.test.ts @@ -1,7 +1,8 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { useEmailStore, buildUnifiedAccountClients } from '../email-store'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore, buildUnifiedAccountClients, ArchiveMailboxNotFoundError } from '../email-store'; import { useAuthStore } from '../auth-store'; import { useAccountStore } from '../account-store'; +import { useSettingsStore } from '../settings-store'; import type { Email, Mailbox } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; import type { UnifiedAccountClient } from '@/lib/unified-mailbox'; @@ -60,9 +61,17 @@ function makeClient() { batchMarkAsRead: vi.fn().mockResolvedValue(undefined), batchDeleteEmails: vi.fn().mockResolvedValue(undefined), batchMoveEmails: vi.fn().mockResolvedValue(undefined), + batchArchiveEmails: vi.fn().mockResolvedValue(undefined), + markAsSpam: vi.fn().mockResolvedValue(undefined), + undoSpam: vi.fn().mockResolvedValue(undefined), + getMailboxes: vi.fn().mockResolvedValue([]), } as unknown as IJMAPClient; } +// Some tests replace these with spies. +const realCrossAccountMoveEmails = useEmailStore.getState().crossAccountMoveEmails; +const realRefreshCurrentMailbox = useEmailStore.getState().refreshCurrentMailbox; + describe('unified-view single-email action routing (#281)', () => { let activeClient: IJMAPClient; // account-a, also the "passed" client let accountBClient: IJMAPClient; @@ -354,10 +363,31 @@ describe('unified-view batch actions with two logins that share account ids', () // Two installs need not number their folders alike: here the 1.0 login's // Deleted Items are `k` (own) and `q` (team), the 0.16.23 login's `b`. - const own016 = [makeMailbox({ id: 'a', role: 'inbox', accountId: 'e' }), makeMailbox({ id: 'b', name: 'Deleted Items', role: 'trash', accountId: 'e' })]; - const own10 = [makeMailbox({ id: 'a', role: 'inbox', accountId: 'e' }), makeMailbox({ id: 'k', name: 'Deleted Items', role: 'trash', accountId: 'e' })]; - const team016 = [makeMailbox({ id: 'c:b', originalId: 'b', name: 'Deleted Items', role: 'trash', isShared: true, accountId: 'c' })]; - const team10 = [makeMailbox({ id: 'c:q', originalId: 'q', name: 'Deleted Items', role: 'trash', isShared: true, accountId: 'c' })]; + // Folders the users made get the next free id of their account: the + // 0.16.23 login's own Archive is `f`, and so is the team's "Projects". + const own016 = [ + makeMailbox({ id: 'a', role: 'inbox', accountId: 'e' }), + makeMailbox({ id: 'b', name: 'Deleted Items', role: 'trash', accountId: 'e' }), + makeMailbox({ id: 'c', name: 'Junk Mail', role: 'junk', accountId: 'e' }), + makeMailbox({ id: 'f', name: 'Archive', role: 'archive', accountId: 'e' }), + ]; + const own10 = [ + makeMailbox({ id: 'a', role: 'inbox', accountId: 'e' }), + makeMailbox({ id: 'k', name: 'Deleted Items', role: 'trash', accountId: 'e' }), + makeMailbox({ id: 'c', name: 'Junk Mail', role: 'junk', accountId: 'e' }), + makeMailbox({ id: 'h', name: 'Archive', role: 'archive', accountId: 'e' }), + ]; + const team016 = [ + makeMailbox({ id: 'c:a', originalId: 'a', name: 'Inbox', role: 'inbox', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:b', originalId: 'b', name: 'Deleted Items', role: 'trash', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:f', originalId: 'f', name: 'Projects', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:g', originalId: 'g', name: 'Archive', role: 'archive', isShared: true, accountId: 'c' }), + ]; + const team10 = [ + makeMailbox({ id: 'c:a', originalId: 'a', name: 'Inbox', role: 'inbox', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:q', originalId: 'q', name: 'Deleted Items', role: 'trash', isShared: true, accountId: 'c' }), + makeMailbox({ id: 'c:i', originalId: 'i', name: 'Archive', role: 'archive', isShared: true, accountId: 'c' }), + ]; beforeEach(() => { client016 = makeClient(); @@ -398,10 +428,20 @@ describe('unified-view batch actions with two logins that share account ids', () unifiedScope: [], unifiedCounts: [], crossUnreadCount: 0, + error: null, + // The cross-account move and the list refresh have tests of their own. + crossAccountMoveEmails: vi.fn().mockResolvedValue(undefined) as never, + refreshCurrentMailbox: vi.fn().mockResolvedValue(undefined) as never, }); }); + afterEach(() => { + useEmailStore.setState({ crossAccountMoveEmails: realCrossAccountMoveEmails, refreshCurrentMailbox: realRefreshCurrentMailbox }); + }); + const callsOf = (fn: unknown) => (fn as ReturnType).mock.calls; + const crossMoves = () => callsOf(useEmailStore.getState().crossAccountMoveEmails); + const listed = () => useEmailStore.getState().emails.map((e) => `${e.sourceClientAccountId}/${e.sourceAccountId}/${e.id}`); it('marks each login\'s messages read through its own client', async () => { await useEmailStore.getState().batchMarkAsRead(client016, true); @@ -424,18 +464,177 @@ describe('unified-view batch actions with two logins that share account ids', () expect(callsOf(client10.batchMoveEmails)).toEqual([[['m10'], 'k', 'e', false], [['t10'], 'q', 'c', false]]); }); - it('moves a selection through each login\'s own client', async () => { - await useEmailStore.getState().batchMoveToMailbox(client016, 'archive'); + // "Move to" offers the viewed login's folders. A folder id means nothing in + // another account: the team's `f` is its "Projects", and the 1.0 server's + // `e` has folders of its own. + it('moves a selection in place only in the folder\'s own login and account (0.16.23 and 1.0 logins)', async () => { + await useEmailStore.getState().batchMoveToMailbox(client016, 'f'); - expect(callsOf(client016.batchMoveEmails)).toEqual([[['m016'], 'archive', 'e'], [['t016'], 'archive', 'c']]); - expect(callsOf(client10.batchMoveEmails)).toEqual([[['m10'], 'archive', 'e'], [['t10'], 'archive', 'c']]); + expect(callsOf(client016.batchMoveEmails)).toEqual([[['m016'], 'f', 'e']]); + expect(callsOf(client10.batchMoveEmails)).toEqual([]); + // The others are moved across accounts, each from its own login and account. + expect(crossMoves()).toEqual([ + [new Map([['userb@v10', ['m10']]]), 'userb@v016', 'f', undefined, 'e'], + [new Map([['userb@v016', ['t016']]]), 'userb@v016', 'f', undefined, 'c'], + [new Map([['userb@v10', ['t10']]]), 'userb@v016', 'f', undefined, 'c'], + ]); + expect(listed()).not.toContain('userb@v016/e/m016'); + expect(useEmailStore.getState().error).toBeNull(); }); - it('moves dropped messages through each login\'s own client', async () => { - await useEmailStore.getState().moveEmailsToMailbox(client016, ['m016', 'm10', 't10'], 'archive'); + it('moves a selection into a group folder in place only in that group account of the same login', async () => { + useEmailStore.setState({ mailboxes: [...own016, ...team016] }); - expect(callsOf(client016.batchMoveEmails)).toEqual([[['m016'], 'archive', 'e']]); - expect(callsOf(client10.batchMoveEmails)).toEqual([[['m10'], 'archive', 'e'], [['t10'], 'archive', 'c']]); + await useEmailStore.getState().batchMoveToMailbox(client016, 'c:g'); + + expect(callsOf(client016.batchMoveEmails)).toEqual([[['t016'], 'g', 'c']]); + expect(callsOf(client10.batchMoveEmails)).toEqual([]); + expect(crossMoves()).toEqual([ + [new Map([['userb@v016', ['m016']]]), 'userb@v016', 'g', 'c', 'e'], + [new Map([['userb@v10', ['m10']]]), 'userb@v016', 'g', 'c', 'e'], + [new Map([['userb@v10', ['t10']]]), 'userb@v016', 'g', 'c', 'c'], + ]); + }); + + it('keeps what could not be moved listed and selected, and says why', async () => { + const refusal = jmapError('Failed to move emails: forbidden', { type: 'forbidden' }); + refusal.jmapFailedIds = ['m016']; + (client016.batchMoveEmails as ReturnType).mockRejectedValue(refusal); + // As crossAccountMoveEmails does: what it moved leaves the list and the selection. + useEmailStore.setState({ + crossAccountMoveEmails: vi.fn(async (bySource: Map) => { + const [[login, ids]] = [...bySource]; + if (login === 'userb@v10' && ids.includes('m10')) throw new Error('Failed to move email: Failed to import email'); + useEmailStore.setState((s) => ({ + emails: s.emails.filter((e) => !ids.includes(e.id)), + selectedEmailIds: new Set([...s.selectedEmailIds].filter((id) => !ids.includes(id))), + })); + }) as never, + }); + + await useEmailStore.getState().batchMoveToMailbox(client016, 'f'); + + const s = useEmailStore.getState(); + expect(listed()).toEqual(['userb@v016/e/m016', 'userb@v10/e/m10']); + expect(s.selectedEmailIds).toEqual(new Set(['m016', 'm10'])); + expect(s.error).toBe('Failed to move 2 emails; first error: Failed to move emails: forbidden'); + }); + + it('moves dropped messages in place only in the login the folder is listed under', async () => { + // Dropped on the 1.0 login's Archive (`h`) in the multi-account sidebar. + await useEmailStore.getState().moveEmailsToMailbox(client016, ['m016', 'm10', 't10'], 'h', 'userb@v10'); + + expect(callsOf(client10.batchMoveEmails)).toEqual([[['m10'], 'h', 'e']]); + expect(callsOf(client016.batchMoveEmails)).toEqual([]); + expect(crossMoves()).toEqual([ + [new Map([['userb@v016', ['m016']]]), 'userb@v10', 'h', undefined, 'e'], + [new Map([['userb@v10', ['t10']]]), 'userb@v10', 'h', undefined, 'c'], + ]); + expect(listed()).toEqual(['userb@v016/e/m016', 'userb@v016/c/t016', 'userb@v10/c/t10']); + }); + + it('moves one dropped group message across accounts, not into the group folder with the same id', async () => { + await useEmailStore.getState().moveEmailsToMailbox(client016, ['t016'], 'f'); + + expect(callsOf(client016.moveEmail)).toEqual([]); + expect(callsOf(client016.batchMoveEmails)).toEqual([]); + expect(crossMoves()).toEqual([[new Map([['userb@v016', ['t016']]]), 'userb@v016', 'f', undefined, 'c']]); + }); + + it('throws for a drop that left messages where they were, keeping their rows', async () => { + (client10.batchMoveEmails as ReturnType).mockRejectedValue(new TypeError('Failed to fetch')); + + await expect(useEmailStore.getState().moveEmailsToMailbox(client016, ['m10'], 'h', 'userb@v10')) + .rejects.toThrow('Failed to fetch'); + expect(listed()).toContain('userb@v10/e/m10'); + }); + + it('"Move to" on one group message moves it across accounts, and on an own one in place', async () => { + await useEmailStore.getState().moveToMailboxCrossAware(client016, 't016', 'f'); + await useEmailStore.getState().moveToMailboxCrossAware(client016, 'm016', 'f'); + + expect(callsOf(client016.moveEmail)).toEqual([]); + expect(crossMoves()).toEqual([[new Map([['userb@v016', ['t016']]]), 'userb@v016', 'f', undefined, 'c']]); + expect(callsOf(client016.batchMoveEmails)).toEqual([[['m016'], 'f', 'e']]); + }); + + const archiveCalls = (client: IJMAPClient) => callsOf(client.batchArchiveEmails) + .map(([emails, archiveId, mode, , accountId]) => [(emails as Array<{ id: string }>).map((e) => e.id), archiveId, mode, accountId]); + + it('archives each login\'s and account\'s messages into their own Archive (0.16.23 and 1.0 logins)', async () => { + useSettingsStore.setState({ archiveMode: 'single' } as never); + + await useEmailStore.getState().batchArchive(client016); + + expect(archiveCalls(client016)).toEqual([[['m016'], 'f', 'single', 'e'], [['t016'], 'g', 'single', 'c']]); + expect(archiveCalls(client10)).toEqual([[['m10'], 'h', 'single', 'e'], [['t10'], 'i', 'single', 'c']]); + expect(listed()).toEqual([]); + expect(useEmailStore.getState().selectedEmailIds.size).toBe(0); + }); + + it('archives in a tag view by each message\'s account, not by the group folder still selected underneath', async () => { + useSettingsStore.setState({ archiveMode: 'single' } as never); + useEmailStore.setState({ isUnifiedView: false, unifiedRole: null, selectedKeyword: 'work', selectedMailbox: 'c:a', mailboxes: [...own016, ...team016] }); + + await useEmailStore.getState().batchArchive(client016); + + expect(archiveCalls(client016)).toEqual([[['m016'], 'f', 'single', 'e'], [['t016'], 'g', 'single', 'c']]); + expect(archiveCalls(client10)).toEqual([[['m10'], 'h', 'single', 'e'], [['t10'], 'i', 'single', 'c']]); + }); + + it('keeps the messages of an account without an Archive listed and selected, archiving the others', async () => { + useSettingsStore.setState({ archiveMode: 'single' } as never); + useEmailStore.setState((s) => ({ + accountMailboxes: { ...s.accountMailboxes, 'userb@v10|c': team10.filter((m) => m.role !== 'archive') }, + })); + + await useEmailStore.getState().batchArchive(client016); + + const s = useEmailStore.getState(); + expect(listed()).toEqual(['userb@v10/c/t10']); + expect(s.selectedEmailIds).toEqual(new Set(['t10'])); + expect(s.error).toMatch(/archive mailbox not found/i); + }); + + it('rejects with ArchiveMailboxNotFoundError when no message could be archived', async () => { + useEmailStore.setState({ + accountMailboxes: {}, + selectedEmailIds: new Set(['m10']), + }); + + await expect(useEmailStore.getState().batchArchive(client016)).rejects.toBeInstanceOf(ArchiveMailboxNotFoundError); + expect(callsOf(client10.batchArchiveEmails)).toEqual([]); + expect(listed()).toContain('userb@v10/e/m10'); + expect(useEmailStore.getState().selectedEmailIds).toEqual(new Set(['m10'])); + }); + + it('marks each message as spam in its own login and account', async () => { + useSettingsStore.setState({ deleteAction: 'trash' } as never); + + await useEmailStore.getState().batchMarkAsSpam(client016, ['m016', 'm10', 't016', 't10']); + + expect(callsOf(client016.markAsSpam)).toEqual([['m016', 'e', false], ['t016', 'c', false]]); + expect(callsOf(client10.markAsSpam)).toEqual([['m10', 'e', false], ['t10', 'c', false]]); + expect(listed()).toEqual([]); + }); + + it('rejects for a message the server would not mark as spam and keeps it, moving the others', async () => { + useSettingsStore.setState({ deleteAction: 'trash' } as never); + (client10.markAsSpam as ReturnType).mockRejectedValueOnce(new Error('Failed to mark as spam: forbidden')); + + await expect(useEmailStore.getState().batchMarkAsSpam(client016, ['m016', 'm10'])).rejects.toThrow('Failed to mark as spam: forbidden'); + + expect(listed()).toEqual(['userb@v10/e/m10', 'userb@v016/c/t016', 'userb@v10/c/t10']); + }); + + it('puts each message back in the Inbox of its own login and account ("All Junk")', async () => { + useEmailStore.setState({ unifiedRole: 'junk' }); + + await useEmailStore.getState().batchUndoSpam(client016, ['m016', 'm10', 't016', 't10']); + + expect(callsOf(client016.undoSpam)).toEqual([['m016', 'a', 'e'], ['t016', 'a', 'c']]); + expect(callsOf(client10.undoSpam)).toEqual([['m10', 'a', 'e'], ['t10', 'a', 'c']]); + expect(listed()).toEqual([]); }); it('deletes a single message into its own login\'s Deleted Items', async () => { diff --git a/stores/email-store.ts b/stores/email-store.ts index 14d68f7c5..ed3728795 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -228,7 +228,12 @@ interface EmailStore { * single-account `moveToMailbox` when source and destination share an account. */ moveToMailboxCrossAware: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; - moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string) => Promise; + /** + * `destLocalAccountId`: the login whose folder list `mailboxId` is from (a + * drop target in the multi-account sidebar); by default the viewed one's. + * Only a list that mixes accounts needs it. + */ + moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string, destLocalAccountId?: string | null) => Promise; moveThreadToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; /** * Move emails across JMAP accounts. JMAP has no native cross-account move, @@ -1101,18 +1106,44 @@ function isUnscopedSearchActive( * folder: the unified / cross-account views, a tag view (#1038) and an * "All folders" search (#1082). */ -function isAggregateListView(): boolean { +export function isAggregateListView(): boolean { const s = useEmailStore.getState(); return s.isUnifiedView || !!s.selectedKeyword || isUnscopedSearchActive(s); } /** A batch write's share for one source of an aggregate list (see groupBySource). */ -interface SourceBatch { +interface SourceBatch { /** Login that reaches the messages (`sourceClientAccountId`). */ clientAccountId?: string; /** JMAP account that owns them; undefined for undecorated messages. */ accountId?: string; ids: string[]; + messages: T[]; +} + +type SourceStamped = { id: string; sourceClientAccountId?: string; sourceAccountId?: string }; + +/** The login and account a message was stamped with; '' for an undecorated one. */ +function sourceKey(message: { sourceClientAccountId?: string; sourceAccountId?: string }): string { + return message.sourceAccountId ? `${message.sourceClientAccountId ?? ''}|${message.sourceAccountId}` : ''; +} + +/** + * One row of an aggregate list: its source and its id. The id alone can name + * rows of several accounts, since ids repeat across accounts and servers. + */ +function sourceRowKey(message: SourceStamped): string { + return `${sourceKey(message)}#${message.id}`; +} + +/** + * Whether the open message is one of the rows `rowKeys` names (sourceRowKey). + * A copy without the list's source stamps goes by its id. + */ +function isOneOfRows(email: SourceStamped | null | undefined, rowKeys: Set): boolean { + if (!email) return false; + if (email.sourceAccountId) return rowKeys.has(sourceRowKey(email)); + return [...rowKeys].some(key => key.endsWith(`#${email.id}`)); } /** @@ -1124,21 +1155,35 @@ interface SourceBatch { * message ids name other messages. Messages without source stamps (a plain * folder view) form one group without an account, for the caller's client. */ -function groupBySource( - messages: Array<{ id: string; sourceClientAccountId?: string; sourceAccountId?: string }>, -): SourceBatch[] { - const groups = new Map(); +function groupBySource(messages: T[]): SourceBatch[] { + const groups = new Map>(); for (const message of messages) { const accountId = message.sourceAccountId || undefined; const clientAccountId = accountId ? message.sourceClientAccountId : undefined; - const key = accountId ? `${clientAccountId ?? ''}|${accountId}` : ''; - const group = groups.get(key) ?? { clientAccountId, accountId, ids: [] }; + const key = sourceKey(message); + const group = groups.get(key) ?? { clientAccountId, accountId, ids: [], messages: [] }; group.ids.push(message.id); + group.messages.push(message); groups.set(key, group); } return [...groups.values()]; } +/** Messages of a batch write that were left as they were, and why. */ +type BatchFailure = { ids: string[]; error: unknown }; + +/** + * What a batch write reports when some of its groups failed: the reason when + * one group failed, else how many messages were left as they were and the + * first reason. + */ +function batchFailureMessage(failures: BatchFailure[], action: string): string { + const reason = (error: unknown) => (error instanceof Error && error.message ? error.message : `Failed to ${action}`); + if (failures.length === 1) return reason(failures[0].error); + const count = failures.reduce((sum, failure) => sum + failure.ids.length, 0); + return `Failed to ${action} ${count} emails; first error: ${reason(failures[0].error)}`; +} + /** * The client for a SourceBatch: the stamped login's, or the caller's for * undecorated messages. Undefined when the login is gone - never another @@ -1169,6 +1214,272 @@ function sourceMailboxes( ?? state.accountMailboxes[jmapAccountId]; } +/** + * The folders a SourceBatch's role-based destinations (Archive, Junk, Inbox) + * come from: its source's own list, or the viewed account's for undecorated + * messages. + */ +function sourceBatchMailboxes(batch: SourceBatch): Mailbox[] { + return batch.accountId + ? (sourceMailboxes(useEmailStore.getState(), batch.clientAccountId, batch.accountId) ?? []) + : resolveActionMailboxes(); +} + +/** A folder that messages of an aggregate list are moved to (resolveMoveDestination). */ +interface MoveDestination { + /** The login whose folder list the folder was picked from. */ + localAccountId: string | null; + /** The store id the folder was picked by. */ + storeId: string; + /** The account that owns the folder, and the folder's id there. */ + accountId: string | undefined; + mailboxId: string; + isShared: boolean; +} + +/** + * The folder `storeId` names in the folder list of `localAccountId` - the + * login a drop target is listed under - or, by default, of the account being + * viewed, whose list "Move to" offers. + */ +function resolveMoveDestination(passedClient: IJMAPClient, storeId: string, localAccountId?: string | null): MoveDestination { + const state = useEmailStore.getState(); + const auth = useAuthStore.getState(); + const mailboxes = localAccountId + ? ((localAccountId === auth.activeAccountId ? state.mailboxes : state.accountMailboxes[localAccountId]) ?? []) + : resolveActionMailboxes(); + const mailbox = mailboxes.find((mb) => mb.id === storeId); + const client = localAccountId ? auth.getClientForAccount(localAccountId) : resolveActionClient(passedClient); + return { + localAccountId: localAccountId ?? state.viewingAccountId ?? auth.activeAccountId, + storeId, + accountId: mailbox?.accountId ?? client?.getAccountId(), + mailboxId: mailbox?.originalId || storeId, + isShared: !!mailbox?.isShared, + }; +} + +/** + * Moves the messages of an aggregate list into one folder. Only the messages + * of the login and account that own the folder change folders in place: a + * mailbox id means nothing in another account, where the same id - Stalwart + * numbers the folders of every account alike - names that account's own + * folder, or another one. The others go through the cross-account move + * (crossAccountMoveEmails): Email/copy within one login, a download and an + * import across logins. Messages whose login is gone stay where they are. + * + * Returns the rows moved in place (the cross-account move takes the rows it + * moved off the list itself) and what failed. + */ +async function moveSourceBatches( + batches: SourceBatch[], + destination: MoveDestination, + passedClient: IJMAPClient, +): Promise<{ moved: T[]; failures: BatchFailure[] }> { + const callerLogin = useEmailStore.getState().viewingAccountId ?? useAuthStore.getState().activeAccountId; + const moved: T[] = []; + const failures: BatchFailure[] = []; + const inPlace: Array<{ batch: SourceBatch; client: IJMAPClient }> = []; + const across: Array<{ batch: SourceBatch; login: string }> = []; + for (const batch of batches) { + const client = sourceBatchClient(batch, passedClient); + const login = batch.accountId ? batch.clientAccountId : callerLogin; + if (!client || !login) { + failures.push({ ids: batch.ids, error: new Error('Not connected') }); + } else if (login === destination.localAccountId && (batch.accountId ?? client.getAccountId()) === destination.accountId) { + inPlace.push({ batch, client }); + } else { + across.push({ batch, login }); + } + } + + const outcomes = await Promise.allSettled( + inPlace.map(({ batch, client }) => client.batchMoveEmails(batch.ids, destination.mailboxId, batch.accountId)), + ); + outcomes.forEach((outcome, i) => { + const { batch } = inPlace[i]; + if (outcome.status === 'fulfilled') { + moved.push(...batch.messages); + return; + } + // A refusal names the messages the server left alone; the rest moved. + const refused = new Set(jmapErrorFailedIds(outcome.reason) ?? batch.ids); + moved.push(...batch.messages.filter((message) => !refused.has(message.id))); + failures.push({ ids: batch.ids.filter((id) => refused.has(id)), error: outcome.reason }); + }); + + // One at a time: each run reports through the store's loading state. + for (const { batch, login } of across) { + try { + if (!destination.localAccountId) throw new Error('Destination account is not connected'); + await useEmailStore.getState().crossAccountMoveEmails( + new Map([[login, batch.ids]]), + destination.localAccountId, + destination.mailboxId, + destination.isShared ? destination.accountId : undefined, + batch.accountId, + ); + } catch (error) { + const listed = new Set(useEmailStore.getState().emails.map((email) => email.id)); + failures.push({ ids: batch.ids.filter((id) => listed.has(id)), error }); + } + } + return { moved, failures }; +} + +/** + * Moves rows of an aggregate list into one folder (moveSourceBatches). The + * rows that moved leave the list and the selection, with their folders' + * counters; the others stay. Returns what failed. + */ +async function moveRowsToMailbox(rows: Email[], destination: MoveDestination, passedClient: IJMAPClient): Promise { + const { moved, failures } = await moveSourceBatches(groupBySource(rows), destination, passedClient); + if (moved.length === 0) return failures; + const movedKeys = new Set(moved.map(sourceRowKey)); + const unread = moved.filter((email) => !email.keywords?.$seen).length; + const arrive = (mailbox: Mailbox) => (mailbox.id === destination.storeId + ? { ...mailbox, totalEmails: mailbox.totalEmails + moved.length, unreadEmails: mailbox.unreadEmails + unread } + : mailbox); + useEmailStore.setState((state) => { + // Out of each message's own account list, into the folder's login's list. + const patch = applyBatchMailboxCounterUpdate(state, moved, applyMoveOutCounters); + const activeId = useAuthStore.getState().activeAccountId; + if (!destination.localAccountId || destination.localAccountId === activeId) { + patch.mailboxes = patch.mailboxes.map(arrive); + } else if (patch.accountMailboxes[destination.localAccountId]) { + patch.accountMailboxes[destination.localAccountId] = patch.accountMailboxes[destination.localAccountId].map(arrive); + } + const selectedEmailIds = new Set(state.selectedEmailIds); + for (const email of moved) selectedEmailIds.delete(email.id); + return { + emails: state.emails.filter((email) => !movedKeys.has(sourceRowKey(email))), + selectedEmail: isOneOfRows(state.selectedEmail, movedKeys) ? null : state.selectedEmail, + selectedEmailIds, + ...patch, + }; + }); + return failures; +} + +/** + * Batch archive in a list that mixes accounts: each login and account + * 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`. Throws when nothing was archived. + */ +async function archiveAggregateSelection(passedClient: IJMAPClient, selected: Email[]): Promise { + if (selected.length === 0) return; + const mode = useSettingsStore.getState().archiveMode; + useEmailStore.setState({ isLoading: true, error: null }); + + const archived: Email[] = []; + const failures: BatchFailure[] = []; + await Promise.all(groupBySource(selected).map(async (batch) => { + const client = sourceBatchClient(batch, passedClient); + const mailboxes = sourceBatchMailboxes(batch); + const archive = batch.accountId + ? findArchiveMailbox(mailboxes, undefined, batch.accountId) + : findArchiveMailbox(mailboxes, useEmailStore.getState().selectedMailbox); + try { + if (!client) throw new Error('Not connected'); + if (!archive) throw new ArchiveMailboxNotFoundError(); + await client.batchArchiveEmails( + batch.messages.map(e => ({ id: e.id, receivedAt: e.receivedAt })), + archive.originalId || archive.id, + mode, + mailboxes, + archive.accountId, + ); + archived.push(...batch.messages); + } catch (error) { + failures.push({ ids: batch.ids, error }); + } + })); + + const error = failures.length > 0 ? batchFailureMessage(failures, 'archive') : null; + if (archived.length === 0) { + useEmailStore.setState({ error, isLoading: false }); + throw failures[0].error; + } + const archivedKeys = new Set(archived.map(sourceRowKey)); + useEmailStore.setState((state) => { + const selectedEmailIds = new Set(state.selectedEmailIds); + for (const email of archived) selectedEmailIds.delete(email.id); + return { + emails: state.emails.filter(e => !archivedKeys.has(sourceRowKey(e))), + selectedEmailIds, + isLoading: false, + error, + }; + }); + + // A year/month archive can create new sub-folders. + await refreshMailboxesForViewingAccount(passedClient); + await useEmailStore.getState().refreshCurrentMailbox(passedClient); +} + +/** Messages of a batch spam write that live in one account, and how to reach it. */ +interface SpamWriteTarget { + messages: Email[]; + client: IJMAPClient | undefined; + /** Their account; undefined for the client's own. */ + accountId: string | undefined; + /** "Not spam": the id of that account's Inbox, if it has one. */ + mailboxId?: string; +} + +/** + * Runs a batch "Spam" or "Not spam" message by message, as the client moves + * them, and updates the list: the messages that moved leave it and the + * selection, from their folders' counters into the `role` folder of their + * account; the others stay. Throws after that when any was left where it was, + * so the caller doesn't report the whole selection as moved. + */ +async function writeSpamBatches( + targets: SpamWriteTarget[], + write: (target: SpamWriteTarget, email: Email) => Promise, + counters: { role: string; arriveRead: boolean }, + action: string, +): Promise { + const done: Email[] = []; + const accountOf = new Map(); + const failures: BatchFailure[] = []; + for (const target of targets) { + if (!target.client) { + failures.push({ ids: target.messages.map(e => e.id), error: new Error('Not connected') }); + continue; + } + for (const email of target.messages) { + try { + await write(target, email); + done.push(email); + accountOf.set(email, target.accountId); + } catch (error) { + failures.push({ ids: [email.id], error }); + } + } + } + + if (done.length > 0) { + const doneKeys = new Set(done.map(sourceRowKey)); + useEmailStore.setState(state => { + const patch = applyBatchMailboxCounterUpdate( + state, done, applyRoleMoveCounters(counters.role, email => accountOf.get(email), counters.arriveRead)); + const selectedEmailIds = new Set(state.selectedEmailIds); + for (const email of done) selectedEmailIds.delete(email.id); + return { + emails: state.emails.filter(e => !doneKeys.has(sourceRowKey(e))), + selectedEmail: isOneOfRows(state.selectedEmail, doneKeys) ? null : state.selectedEmail, + selectedEmailIds, + ...patch, + }; + }); + } + if (failures.length > 0) throw new Error(batchFailureMessage(failures, action)); +} + /** * After a mailbox-list mutation (create/rename/delete/etc.), refresh the * cache for whichever account we're operating on. Writes the result to the @@ -1334,6 +1645,55 @@ function applyDeleteCounters( }; } +// Per-mailbox counter map (for applyBatchMailboxCounterUpdate) for moving a +// group of emails out of their folders: message counts only, like the +// destination's gain. +function applyMoveOutCounters( + mailbox: Mailbox, + group: Array<{ sourceClientAccountId?: string }>, +): Mailbox { + let dTotal = 0; + let dUnread = 0; + for (const email of group as Email[]) { + if (emailInMailbox(email, mailbox)) { + dTotal--; + if (!email.keywords?.$seen) dUnread--; + } + } + return dTotal === 0 && dUnread === 0 ? mailbox : { + ...mailbox, + totalEmails: Math.max(0, mailbox.totalEmails + dTotal), + unreadEmails: Math.max(0, mailbox.unreadEmails + dUnread), + }; +} + +// Per-mailbox counter map (for applyBatchMailboxCounterUpdate) for emails +// moved to the folder with `role` (Junk, Inbox) of their own account: they +// leave their folders and join that one, unread unless `arriveRead`. +// `accountOf` names an email's account; undefined stands for the own one. +function applyRoleMoveCounters(role: string, accountOf: (email: Email) => string | undefined, arriveRead: boolean) { + return (mailbox: Mailbox, group: Array<{ sourceClientAccountId?: string }>): Mailbox => { + let dTotal = 0; + let dUnread = 0; + for (const email of group as Email[]) { + const unread = !email.keywords?.$seen; + const accountId = accountOf(email); + if (emailInMailbox(email, mailbox)) { + dTotal--; + if (unread) dUnread--; + } else if (mailbox.role === role && (accountId ? mailbox.accountId === accountId : !mailbox.isShared)) { + dTotal++; + if (unread && !arriveRead) dUnread++; + } + } + return dTotal === 0 && dUnread === 0 ? mailbox : { + ...mailbox, + totalEmails: Math.max(0, mailbox.totalEmails + dTotal), + unreadEmails: Math.max(0, mailbox.unreadEmails + dUnread), + }; + }; +} + // ─── Unified-badge live projection ──────────────────────────────────────────── // // The unified-section badges (unifiedCounts / crossUnreadCount) are a pure @@ -2546,6 +2906,15 @@ export const useEmailStore = create((set, get) => ({ const email = state.emails.find((e) => e.id === emailId); if (!email) return; + // A list that mixes accounts offers the viewed account's folders, and + // the message may be in any account (moveSourceBatches). Its own folder + // list would take the id for one of that account's folders. + if (isAggregateListView() && email.sourceAccountId) { + const failures = await moveRowsToMailbox([email], resolveMoveDestination(client, destinationMailboxId), client); + if (failures.length > 0) throw failures[0].error; + return; + } + const { mailboxes } = resolveEmailActionContext(email, client); const find = (id: string) => mailboxes.find((mb) => mb.id === id) ?? state.mailboxes.find((mb) => mb.id === id); @@ -2645,8 +3014,27 @@ export const useEmailStore = create((set, get) => ({ } }, - moveEmailsToMailbox: async (client, emailIds, destinationMailboxId) => { + moveEmailsToMailbox: async (client, emailIds, destinationMailboxId, destLocalAccountId) => { if (emailIds.length === 0) return; + const idSet = new Set(emailIds); + + if (isAggregateListView()) { + // Emails may span logins and accounts: each group changes folders in + // place only in the folder's own account (moveSourceBatches). Rows + // that could not be moved stay. + const failures = await moveRowsToMailbox( + get().emails.filter(e => idSet.has(e.id)), + resolveMoveDestination(client, destinationMailboxId, destLocalAccountId), + client, + ); + if (failures.length > 0) { + const error = new Error(batchFailureMessage(failures, 'move')); + set({ error: error.message }); + throw error; + } + return; + } + if (emailIds.length === 1) { await get().moveToMailbox(client, emailIds[0], destinationMailboxId); return; @@ -2657,24 +3045,11 @@ export const useEmailStore = create((set, get) => ({ const mailboxes = resolveActionMailboxes(); const destMailbox = mailboxes.find(mb => mb.id === destinationMailboxId); const jmapDestId = destMailbox?.originalId || destinationMailboxId; - const idSet = new Set(emailIds); const affected = emails.filter(e => idSet.has(e.id)); - if (isAggregateListView()) { - // In unified view, emails may span accounts – group by login and owning - // JMAP account and dispatch through the login client that can reach each - // one: `sourceClientAccountId` (a real AccountEntry.id) picks the client, - // `sourceAccountId` is passed for owner-scoped routing (groupBySource). - await Promise.all(groupBySource(affected).map(async (batch) => { - const acctClient = sourceBatchClient(batch, client); - if (!acctClient) return; - await acctClient.batchMoveEmails(batch.ids, jmapDestId, batch.accountId); - })); - } else { - const currentMailbox = mailboxes.find(mb => mb.id === selectedMailbox); - const accountId = currentMailbox?.isShared ? currentMailbox.accountId : undefined; - await resolveActionClient(client).batchMoveEmails(emailIds, jmapDestId, accountId); - } + const currentMailbox = mailboxes.find(mb => mb.id === selectedMailbox); + const accountId = currentMailbox?.isShared ? currentMailbox.accountId : undefined; + await resolveActionClient(client).batchMoveEmails(emailIds, jmapDestId, accountId); // Adjust counters and drop moved emails from the current view. let unreadDelta = 0; @@ -2685,21 +3060,7 @@ export const useEmailStore = create((set, get) => ({ set((state) => { // Decrement source folders per the email's own account list (#281). - const patch = applyBatchMailboxCounterUpdate(state, affected, (mb, group) => { - let dTotal = 0; - let dUnread = 0; - for (const e of group as Email[]) { - if (emailInMailbox(e, mb)) { - dTotal--; - if (!e.keywords?.$seen) dUnread--; - } - } - return dTotal === 0 && dUnread === 0 ? mb : { - ...mb, - totalEmails: Math.max(0, mb.totalEmails + dTotal), - unreadEmails: Math.max(0, mb.unreadEmails + dUnread), - }; - }); + const patch = applyBatchMailboxCounterUpdate(state, affected, applyMoveOutCounters); // The destination folder (picked from the active/viewing list) gains them. patch.mailboxes = patch.mailboxes.map(mb => mb.id === destinationMailboxId ? { ...mb, totalEmails: mb.totalEmails + movedCount, unreadEmails: mb.unreadEmails + unreadDelta } @@ -3476,17 +3837,15 @@ export const useEmailStore = create((set, get) => ({ const emailIdsArray = Array.from(selectedEmailIds); if (isAggregateListView()) { - // Group by login and owning JMAP account; dispatch through the reaching - // login client (groupBySource). - const destMailbox = resolveActionMailboxes().find(mb => mb.id === toMailboxId); - const jmapDestId = destMailbox?.originalId || toMailboxId; - const batches = groupBySource(emailIdsArray.map(id => emails.find(e => e.id === id) ?? { id })); - const promises = batches.map(async (batch) => { - const acctClient = sourceBatchClient(batch, client); - if (!acctClient) return; - await acctClient.batchMoveEmails(batch.ids, jmapDestId, batch.accountId); - }); - await Promise.allSettled(promises); + // Only messages in the folder's own login and account change folders + // in place; the others are moved across accounts (moveSourceBatches). + // What could not be moved stays listed and selected. + const failures = await moveRowsToMailbox( + emails.filter(e => selectedEmailIds.has(e.id)), + resolveMoveDestination(client, toMailboxId), + client, + ); + set({ isLoading: false, error: failures.length > 0 ? batchFailureMessage(failures, 'move') : null }); } else { // Non-unified: route to the viewed shared/group account (if any), reached // through the active client. Resolve the destination to its bare owner id @@ -3495,17 +3854,17 @@ export const useEmailStore = create((set, get) => ({ const destMailbox = resolveActionMailboxes().find(mb => mb.id === toMailboxId); const jmapDestId = destMailbox?.originalId || toMailboxId; await resolveActionClient(client).batchMoveEmails(emailIdsArray, jmapDestId, viewAccountId); + + // Update local state - remove from current view since they moved + const remainingEmails = emails.filter(e => !selectedEmailIds.has(e.id)); + + set({ + emails: remainingEmails, + selectedEmailIds: new Set(), + isLoading: false + }); } - // Update local state - remove from current view since they moved - const remainingEmails = emails.filter(e => !selectedEmailIds.has(e.id)); - - set({ - emails: remainingEmails, - selectedEmailIds: new Set(), - isLoading: false - }); - // Refresh emails to get updated list (honors active search/filters) if (!get().isUnifiedView) { await get().refreshCurrentMailbox(client); @@ -3523,6 +3882,11 @@ export const useEmailStore = create((set, get) => ({ const mailboxes = resolveActionMailboxes(); if (selectedEmailIds.size === 0) return; + if (isAggregateListView()) { + await archiveAggregateSelection(client, emails.filter(e => selectedEmailIds.has(e.id))); + return; + } + // 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. @@ -3733,67 +4097,32 @@ export const useEmailStore = create((set, get) => ({ batchMarkAsSpam: async (client, emailIds) => { const { selectedMailbox, emails } = get(); - const mailboxes = resolveActionMailboxes(); - const effectiveClient = resolveActionClient(client); - - const currentMailbox = mailboxes.find(m => m.id === selectedMailbox); - if (!currentMailbox) return; - const alsoMarkRead = useSettingsStore.getState().deleteAction === 'trash-and-read'; + const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); + + // Each message goes to the Junk of its own account: in a list that mixes + // accounts the one it is stamped with (the selected folder is a virtual + // one there), otherwise the viewed folder's. + let targets: SpamWriteTarget[]; + if (isAggregateListView()) { + targets = groupBySource(rows).map(batch => ({ + messages: batch.messages, client: sourceBatchClient(batch, client), accountId: batch.accountId, + })); + } else { + const currentMailbox = resolveActionMailboxes().find(m => m.id === selectedMailbox); + // Throw rather than return: the caller reports the mail as moved. (#695) + if (!currentMailbox) throw new Error('Mailbox not found - cannot mark as spam'); + targets = [{ messages: rows, client: resolveActionClient(client), accountId: currentMailbox.accountId }]; + } try { - for (const emailId of emailIds) { - const email = emails.find(e => e.id === emailId); - const markRead = alsoMarkRead && !!email && !email.keywords?.$seen; - await effectiveClient.markAsSpam(emailId, currentMailbox.accountId, markRead); - } - - // Adjust counters and drop the emails from the current view. Junk is - // resolved the same way the client did for the move itself. - const affected = emails.filter(e => emailIds.includes(e.id)); - const junkMailbox = mailboxes.find(m => - currentMailbox.accountId - ? (m.role === 'junk' && m.accountId === currentMailbox.accountId) - : (m.role === 'junk' && !m.isShared) - ); - let junkTotal = 0; - let junkUnread = 0; - for (const e of affected) { - junkTotal += 1; + await writeSpamBatches( + targets, + (target, email) => target.client!.markAsSpam(email.id, target.accountId, alsoMarkRead && !!email.keywords && !email.keywords.$seen), // After marking read in the same request, the email arrives in junk as read. - if (!e.keywords?.$seen && !alsoMarkRead) junkUnread += 1; - } - - set(state => { - // Decrement source folders per the email's own account list (#281). - const patch = applyBatchMailboxCounterUpdate(state, affected, (mb, group) => { - let dTotal = 0; - let dUnread = 0; - for (const e of group as Email[]) { - if (emailInMailbox(e, mb)) { - dTotal--; - if (!e.keywords?.$seen) dUnread--; - } - } - return dTotal === 0 && dUnread === 0 ? mb : { - ...mb, - totalEmails: Math.max(0, mb.totalEmails + dTotal), - unreadEmails: Math.max(0, mb.unreadEmails + dUnread), - }; - }); - // The junk folder (picked from the active/viewing list) gains them. - if (junkMailbox) { - patch.mailboxes = patch.mailboxes.map(mb => mb.id === junkMailbox.id - ? { ...mb, totalEmails: mb.totalEmails + junkTotal, unreadEmails: mb.unreadEmails + junkUnread } - : mb); - } - return { - emails: state.emails.filter(e => !emailIds.includes(e.id)), - selectedEmail: emailIds.includes(state.selectedEmail?.id || '') ? null : state.selectedEmail, - selectedEmailIds: new Set(), - ...patch, - }; - }); + { role: 'junk', arriveRead: alsoMarkRead }, + 'mark as spam', + ); } catch (error) { console.error('Failed to batch mark as spam:', error); throw error; @@ -3801,63 +4130,46 @@ export const useEmailStore = create((set, get) => ({ }, batchUndoSpam: async (client: IJMAPClient, emailIds: string[]) => { - const { selectedMailbox } = get(); - const mailboxes = resolveActionMailboxes(); - const effectiveClient = resolveActionClient(client); + const { selectedMailbox, emails } = get(); + const rows = emailIds.map(id => emails.find(e => e.id === id) ?? ({ id } as Email)); - // Find inbox (batch operations don't preserve original mailboxes) - const currentMailbox = mailboxes.find(m => m.id === selectedMailbox); - const accountId = currentMailbox?.accountId; - - const inboxMailbox = mailboxes.find(m => - m.role === 'inbox' && - (accountId ? m.accountId === accountId : !m.accountId) - ); - - if (!inboxMailbox) { - throw new Error('Inbox not found'); + // Back to the Inbox of each message's own account (batch operations don't + // preserve original mailboxes): in a list that mixes accounts ("All + // Junk") the one it is stamped with, otherwise the viewed folder's. + let targets: SpamWriteTarget[]; + if (isAggregateListView()) { + targets = groupBySource(rows).map(batch => { + const inbox = sourceBatchMailboxes(batch).find(m => + m.role === 'inbox' && (batch.accountId ? m.accountId === batch.accountId : !m.isShared)); + return { + messages: batch.messages, client: sourceBatchClient(batch, client), accountId: batch.accountId, + mailboxId: inbox ? (inbox.originalId || inbox.id) : undefined, + }; + }); + } else { + const mailboxes = resolveActionMailboxes(); + const currentMailbox = mailboxes.find(m => m.id === selectedMailbox); + const accountId = currentMailbox?.accountId; + const inboxMailbox = mailboxes.find(m => + m.role === 'inbox' && + (accountId ? m.accountId === accountId : !m.accountId) + ); + if (!inboxMailbox) { + throw new Error('Inbox not found'); + } + targets = [{ messages: rows, client: resolveActionClient(client), accountId, mailboxId: inboxMailbox.originalId || inboxMailbox.id }]; } try { - for (const emailId of emailIds) { - await effectiveClient.undoSpam(emailId, inboxMailbox.originalId || inboxMailbox.id, accountId); - } - - // Adjust counters and drop the restored emails from the junk view. - const affected = get().emails.filter(e => emailIds.includes(e.id)); - let unreadDelta = 0; - for (const e of affected) { - if (!e.keywords?.$seen) unreadDelta += 1; - } - - set(state => { - // Decrement source folders per the email's own account list (#281). - const patch = applyBatchMailboxCounterUpdate(state, affected, (mb, group) => { - let dTotal = 0; - let dUnread = 0; - for (const e of group as Email[]) { - if (emailInMailbox(e, mb)) { - dTotal--; - if (!e.keywords?.$seen) dUnread--; - } - } - return dTotal === 0 && dUnread === 0 ? mb : { - ...mb, - totalEmails: Math.max(0, mb.totalEmails + dTotal), - unreadEmails: Math.max(0, mb.unreadEmails + dUnread), - }; - }); - // The inbox (picked from the active/viewing list) gains them. - patch.mailboxes = patch.mailboxes.map(mb => mb.id === inboxMailbox.id - ? { ...mb, totalEmails: mb.totalEmails + affected.length, unreadEmails: mb.unreadEmails + unreadDelta } - : mb); - return { - emails: state.emails.filter(e => !emailIds.includes(e.id)), - selectedEmail: emailIds.includes(state.selectedEmail?.id || '') ? null : state.selectedEmail, - selectedEmailIds: new Set(), - ...patch, - }; - }); + await writeSpamBatches( + targets, + (target, email) => { + if (!target.mailboxId) throw new Error('Inbox not found'); + return target.client!.undoSpam(email.id, target.mailboxId, target.accountId); + }, + { role: 'inbox', arriveRead: false }, + 'mark as not spam', + ); } catch (error) { console.error('Failed to batch restore emails:', error); throw error;