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;