From 7cbd33dbc870f64f0a1ef736f1133119ba335b53 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:51:19 +0200 Subject: [PATCH] fix: act on the ticked account's mail in batch actions #1061 Batch delete grouped the selection by bare email id, so with the same id in two accounts, permanent-deleting B's row destroyed A's message and left B's alone. Copy had the same lookup and went out through the wrong account. - resolve the selection to email objects by key, including ticked members of an expanded thread that are not list rows - delete, move, mark read, tag and copy group by those objects - rows, thread checkboxes, drag and drop compare by key, so ticked rows show as checked in unified and tag views - the Mountain View toolbar uses the same resolver --- .../email/__tests__/thread-list-item.test.tsx | 66 +++++++- components/email/email-list.tsx | 14 +- components/email/mv-list-toolbar.tsx | 6 +- components/email/thread-email-item.tsx | 4 +- components/email/thread-list-item.tsx | 14 +- components/mail/mail-app.tsx | 8 +- hooks/use-email-drag.ts | 11 +- hooks/use-mailbox-drop.ts | 6 +- .../__tests__/email-batch-namesakes.test.ts | 154 ++++++++++++++++++ stores/email-store.ts | 77 ++++++--- 10 files changed, 306 insertions(+), 54 deletions(-) create mode 100644 stores/__tests__/email-batch-namesakes.test.ts diff --git a/components/email/__tests__/thread-list-item.test.tsx b/components/email/__tests__/thread-list-item.test.tsx index 2807d0bfd..aabbd2415 100644 --- a/components/email/__tests__/thread-list-item.test.tsx +++ b/components/email/__tests__/thread-list-item.test.tsx @@ -4,7 +4,7 @@ import { ThreadListItem } from '../thread-list-item'; import { useSettingsStore, DEFAULT_KEYWORDS } from '@/stores/settings-store'; import { useEmailStore } from '@/stores/email-store'; import { useUIStore } from '@/stores/ui-store'; -import { groupEmailsByThread } from '@/lib/thread-utils'; +import { emailKeyFor, groupEmailsByThread } from '@/lib/thread-utils'; import type { Email } from '@/lib/jmap/types'; vi.mock('@/hooks/use-email-drag', () => ({ @@ -439,3 +439,67 @@ describe('ThreadListItem account row tint', () => { expect(row(container)!.className).toContain('bg-teal-100'); }); }); + +describe('ThreadListItem selection in a merged view', () => { + // Unified and tag views stamp each email with its owning account, and the + // selection holds `emailKeyFor` keys. A bare-id check never matches them. + const stamp = { sourceClientAccountId: 'account-b', sourceAccountId: 'account-b' }; + + beforeEach(() => { + useSettingsStore.setState({ + emailKeywords: [...DEFAULT_KEYWORDS], + showPreview: false, + mailLayout: 'split', + density: 'regular', + }); + useEmailStore.setState({ + isUnifiedView: true, + selectedEmailKeys: new Set(), + lastSelectedEmailKey: null, + selectedMailbox: '', + }); + }); + + it('shows a ticked stamped row as checked', () => { + const email = makeEmail({ ...stamp }); + useEmailStore.setState({ emails: [email], selectedEmailKeys: new Set([emailKeyFor(email)]) }); + renderRow(email); + + expect(screen.getByRole('checkbox')).toHaveAttribute('aria-checked', 'true'); + }); + + it('does not show a namesake row from another account as checked', () => { + const mine = makeEmail({ sourceClientAccountId: 'account-a', sourceAccountId: 'account-a' }); + const theirs = makeEmail({ ...stamp }); + useEmailStore.setState({ emails: [mine, theirs], selectedEmailKeys: new Set([emailKeyFor(mine)]) }); + renderRow(theirs); + + expect(screen.getByRole('checkbox')).toHaveAttribute('aria-checked', 'false'); + }); + + it('ticks a whole thread by key and shows it checked', () => { + const e1 = makeEmail({ id: 'e1', threadId: 't1', receivedAt: '2024-01-15T10:00:00Z', ...stamp }); + const e2 = makeEmail({ id: 'e2', threadId: 't1', receivedAt: '2024-01-15T11:00:00Z', ...stamp }); + useEmailStore.setState({ emails: [e1, e2] }); + const [thread] = groupEmailsByThread([e1, e2]); + const ui = ( + {}} onEmailSelect={() => {}} /> + ); + const { rerender } = render(ui); + + act(() => { + screen.getByRole('checkbox').dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + const { selectedEmailKeys, lastSelectedEmailKey } = useEmailStore.getState(); + expect([...selectedEmailKeys].sort()).toEqual([emailKeyFor(e1), emailKeyFor(e2)].sort()); + expect(lastSelectedEmailKey).toBe(emailKeyFor(e2)); + rerender(ui); + expect(screen.getByRole('checkbox')).toHaveAttribute('aria-checked', 'true'); + + act(() => { + screen.getByRole('checkbox').dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + expect(useEmailStore.getState().selectedEmailKeys.size).toBe(0); + }); +}); diff --git a/components/email/email-list.tsx b/components/email/email-list.tsx index 17d02e328..df676ebb3 100644 --- a/components/email/email-list.tsx +++ b/components/email/email-list.tsx @@ -13,7 +13,7 @@ import { Trash2, Mail, MailX, MailOpen, Loader2, SearchX, AlertTriangle, Calenda import { useState, useEffect, useLayoutEffect, useRef, useCallback, useMemo } from "react"; import { Button } from "@/components/ui/button"; import { ConfirmDialog } from "@/components/ui/confirm-dialog"; -import { useEmailStore, ArchiveMailboxNotFoundError } from "@/stores/email-store"; +import { useEmailStore, ArchiveMailboxNotFoundError, resolveSelectedEmails } from "@/stores/email-store"; import { useAuthStore } from "@/stores/auth-store"; import { useAccountStore } from "@/stores/account-store"; import { useSettingsStore } from "@/stores/settings-store"; @@ -400,7 +400,7 @@ export function EmailList({ if (!client || isProcessing) return; setIsProcessing(true); try { - const emailIds = emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))).map(e => e.id); + const emailIds = resolveSelectedEmails(useEmailStore.getState()).map(e => e.id); await batchUndoSpam(client, emailIds); toast.success(tSpam('toast_not_spam_batch', { count: emailIds.length })); } catch { @@ -812,9 +812,11 @@ export function EmailList({ onRescheduleScheduled={onRescheduleScheduled ? () => onRescheduleScheduled(contextMenuEmail!) : undefined} copyTargets={copyTargets} onCopyToAccount={async (accountId, mailboxId) => { + // Pass the emails, not their ids: an id alone cannot say which + // account's namesake to copy. const ids = selectedEmailKeys.has(emailKeyFor(contextMenuEmail!)) && selectedEmailKeys.size > 1 - ? emails.filter((email) => selectedEmailKeys.has(emailKeyFor(email))).map((email) => email.id) - : [contextMenuEmail!.id]; + ? resolveSelectedEmails(useEmailStore.getState()) + : [contextMenuEmail!]; const target = accountMailboxes[accountId]?.find((mb) => mb.id === mailboxId); await runBatchEmailAction(() => copyEmailsToAccount(ids, accountId, target?.originalId ?? mailboxId), { success: tNotifications('emails_copied', { count: ids.length }), @@ -854,7 +856,7 @@ export function EmailList({ }} onBatchMarkAsSpam={async () => { if (client) { - const emailIds = emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))).map(e => e.id); + const emailIds = resolveSelectedEmails(useEmailStore.getState()).map(e => e.id); try { await batchMarkAsSpam(client, emailIds); toast.success( @@ -867,7 +869,7 @@ export function EmailList({ }} onBatchUndoSpam={async () => { if (client) { - const emailIds = emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))).map(e => e.id); + const emailIds = resolveSelectedEmails(useEmailStore.getState()).map(e => e.id); try { await batchUndoSpam(client, emailIds); toast.success( diff --git a/components/email/mv-list-toolbar.tsx b/components/email/mv-list-toolbar.tsx index e8066ae74..d5fd11836 100644 --- a/components/email/mv-list-toolbar.tsx +++ b/components/email/mv-list-toolbar.tsx @@ -19,7 +19,7 @@ import { } from "@/components/icons"; import { useTranslations } from "next-intl"; import { useAuthStore } from "@/stores/auth-store"; -import { useEmailStore, ArchiveMailboxNotFoundError } from "@/stores/email-store"; +import { useEmailStore, ArchiveMailboxNotFoundError, resolveSelectedEmails } from "@/stores/email-store"; import { runBatchEmailAction } from "@/lib/email-action-toast"; import { toast } from "@/stores/toast-store"; import { useConfirmDialog } from "@/hooks/use-confirm-dialog"; @@ -247,7 +247,7 @@ export function MvListToolbar({ }); }); - const selectedEmails = emails.filter((email) => selectedEmailKeys.has(email.id)); + const selectedEmails = resolveSelectedEmails(useEmailStore.getState()); // The same targets the row's own context menu offers, with the account of // the selection leading (#1149). @@ -320,7 +320,7 @@ export function MvListToolbar({ const handleSpam = () => run(async () => { if (!client) return; - const ids = Array.from(selectedEmailKeys); + const ids = resolveSelectedEmails(useEmailStore.getState()).map((email) => email.id); try { if (isInJunk) { await batchUndoSpam(client, ids); diff --git a/components/email/thread-email-item.tsx b/components/email/thread-email-item.tsx index 4a4a726de..83f54244d 100644 --- a/components/email/thread-email-item.tsx +++ b/components/email/thread-email-item.tsx @@ -12,7 +12,7 @@ import { useLongPress } from "@/hooks/use-long-press"; import { useEmailStore } from "@/stores/email-store"; import { useSettingsStore } from "@/stores/settings-store"; import { useUIStore } from "@/stores/ui-store"; -import { getEmailTagIds } from "@/lib/thread-utils"; +import { emailKeyFor, getEmailTagIds } from "@/lib/thread-utils"; import { useKeywordFormat } from "@/hooks/use-keyword-format"; import { useTagDisplay } from "@/hooks/use-tag-display"; import { TagBadge } from "./tag-badge"; @@ -47,7 +47,7 @@ export function ThreadEmailItem({ const sender = email.from?.[0]; const { selectedMailbox, selectedEmailKeys, toggleEmailSelection, selectRangeEmails, clearSelection } = useEmailStore(); const density = useSettingsStore((state) => state.density); - const isChecked = selectedEmailKeys.has(email.id); + const isChecked = selectedEmailKeys.has(emailKeyFor(email)); const { dragHandlers, isDragging } = useEmailDrag({ email, diff --git a/components/email/thread-list-item.tsx b/components/email/thread-list-item.tsx index 655f9924b..5a2a9d638 100644 --- a/components/email/thread-list-item.tsx +++ b/components/email/thread-list-item.tsx @@ -17,7 +17,7 @@ import { accountTintKey, generateAvatarColor } from "@/lib/account-utils"; import { useUIStore } from "@/stores/ui-store"; import { useEmailStore } from "@/stores/email-store"; import { useAccountStore, type AccountEntry } from "@/stores/account-store"; -import { getThreadTagIds, getEmailTagIds } from "@/lib/thread-utils"; +import { emailKeyFor, getThreadTagIds, getEmailTagIds } from "@/lib/thread-utils"; import { useKeywordFormat } from "@/hooks/use-keyword-format"; import { useTagDisplay } from "@/hooks/use-tag-display"; import { TagBadge, TAG_GROUP_CLASS, TAG_LOZENGE_CLASS } from "./tag-badge"; @@ -306,7 +306,7 @@ const SingleEmailItem = React.forwardRef( const account = email.accountId ? getAccountById(email.accountId) : undefined; const accountColor = account?.avatarColor; const accountDescription = describeAccount(email.accountLabel, account); - const isChecked = selectedEmailKeys.has(email.id); + const isChecked = selectedEmailKeys.has(emailKeyFor(email)); const isMobile = useUIStore((state) => state.isMobile); // The horizontal one-line "focus" layout doesn't fit on narrow screens; fall back to multi-line on mobile. const isFocusedMailLayout = mailLayout === 'focus' && !isMobile; @@ -879,7 +879,7 @@ export const ThreadListItem = React.forwardRef e.id === selectedEmailId); - const isChecked = thread.emails.some(e => selectedEmailKeys.has(e.id)); + const isChecked = thread.emails.some(e => selectedEmailKeys.has(emailKeyFor(e))); if (emailCount === 1) { return ( @@ -909,16 +909,16 @@ export const ThreadListItem = React.forwardRef { - const allSelected = thread.emails.every(em => selectedEmailKeys.has(em.id)); + const allSelected = thread.emails.every(em => selectedEmailKeys.has(emailKeyFor(em))); const newSelection = new Set(selectedEmailKeys); thread.emails.forEach(em => { if (allSelected) { - newSelection.delete(em.id); + newSelection.delete(emailKeyFor(em)); } else { - newSelection.add(em.id); + newSelection.add(emailKeyFor(em)); } }); - useEmailStore.setState({ selectedEmailKeys: newSelection, lastSelectedEmailKey: latestEmail.id }); + useEmailStore.setState({ selectedEmailKeys: newSelection, lastSelectedEmailKey: emailKeyFor(latestEmail) }); }; const handleThreadCheckboxClick = (e: React.MouseEvent) => { diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 8ccb50440..a742c738f 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -32,7 +32,7 @@ import { usePolicyStore } from "@/stores/policy-store"; import type { UnifiedAccountClient } from "@/lib/unified-mailbox"; import { connectedAccountsGrew } from "@/lib/unified-mailbox"; import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal"; -import { useEmailStore, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveUnstampedEmailAccountId, resolveEmailBlobContext, emptyFolderMovesToTrash } from "@/stores/email-store"; +import { useEmailStore, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveSelectedEmails, resolveUnstampedEmailAccountId, resolveEmailBlobContext, emptyFolderMovesToTrash } from "@/stores/email-store"; import { groupSearchScopeFolders, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders"; import { toast } from "@/stores/toast-store"; import { formatRejectedRecipients, type JMAPClient } from "@/lib/jmap/client"; @@ -59,7 +59,7 @@ import { debug } from "@/lib/debug"; import { playNotificationSound } from "@/lib/notification-sound"; import { cn, getMailboxPath } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; -import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, emailKeyFor, groupEmailsByThread, threadKeyFor } from "@/lib/thread-utils"; +import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, groupEmailsByThread, threadKeyFor } from "@/lib/thread-utils"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { ErrorBoundary, @@ -849,8 +849,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const isInJunk = currentMailbox?.role === 'junk'; if (selectedEmailKeys.size > 0 && client) { // The spam actions take bare ids of the viewed account: resolve the - // selection keys through the loaded rows. - const ids = useEmailStore.getState().emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))).map(e => e.id); + // selection keys through the loaded rows and thread members. + const ids = resolveSelectedEmails(useEmailStore.getState()).map(e => e.id); try { if (isInJunk) { await batchUndoSpam(client, ids); diff --git a/hooks/use-email-drag.ts b/hooks/use-email-drag.ts index 0bea4c299..1a0c8cdee 100644 --- a/hooks/use-email-drag.ts +++ b/hooks/use-email-drag.ts @@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useRef, DragEvent } from "react"; import { Email } from "@/lib/jmap/types"; import { IJMAPClient } from "@/lib/jmap/client-interface"; import { useEmailStore } from "@/stores/email-store"; +import { emailKeyFor } from "@/lib/thread-utils"; import { useAuthStore } from "@/stores/auth-store"; import { useDragDropContext } from "@/contexts/drag-drop-context"; import { useUIStore } from "@/stores/ui-store"; @@ -195,10 +196,10 @@ export function useEmailDrag({ email, sourceMailboxId, threadEmails }: UseEmailD const handlePointerEnter = useCallback(() => { if (!dragOutEnabled || !client) return; - const isSelected = selectedEmailKeys.has(email.id); + const isSelected = selectedEmailKeys.has(emailKeyFor(email)); const isMulti = isSelected && selectedEmailKeys.size > 1; if (isMulti) { - const selected = emails.filter((em) => selectedEmailKeys.has(em.id)); + const selected = emails.filter((em) => selectedEmailKeys.has(emailKeyFor(em))); // Only worth bundling when at least one selected email has a blobId. if (selected.some((em) => em.blobId)) { prefetchEmailBundle(client, selected, filenameOptions, bundleOptions); @@ -206,16 +207,16 @@ export function useEmailDrag({ email, sourceMailboxId, threadEmails }: UseEmailD } else { prefetchSingle(); } - }, [dragOutEnabled, client, selectedEmailKeys, email.id, emails, prefetchSingle, filenameOptions, bundleOptions]); + }, [dragOutEnabled, client, selectedEmailKeys, email, emails, prefetchSingle, filenameOptions, bundleOptions]); const handleDragStart = useCallback((e: DragEvent) => { // Determine which emails to drag: // - If current email is selected, drag all selected // - If threadEmails provided (thread header), drag all thread emails // - Otherwise, drag only this email - const isSelected = selectedEmailKeys.has(email.id); + const isSelected = selectedEmailKeys.has(emailKeyFor(email)); const emailsToDrag = isSelected - ? emails.filter(em => selectedEmailKeys.has(em.id)) + ? emails.filter(em => selectedEmailKeys.has(emailKeyFor(em))) : threadEmails || [email]; e.dataTransfer.effectAllowed = "copyMove"; diff --git a/hooks/use-mailbox-drop.ts b/hooks/use-mailbox-drop.ts index 2b87b6665..61c6792f8 100644 --- a/hooks/use-mailbox-drop.ts +++ b/hooks/use-mailbox-drop.ts @@ -3,6 +3,7 @@ import { useCallback, useState, DragEvent } from "react"; import { Mailbox, Email } from "@/lib/jmap/types"; import { useEmailStore } from "@/stores/email-store"; +import { emailKeyFor } from "@/lib/thread-utils"; import { useAuthStore } from "@/stores/auth-store"; import { useDragDropContext } from "@/contexts/drag-drop-context"; import { toast } from "@/stores/toast-store"; @@ -183,8 +184,9 @@ export function useMailboxDrop({ mailbox, onDropComplete, onSuccess, onError }: await moveEmailsToMailbox(client, emailIds, mailbox.id); } - // Clear selection if any selected emails were moved - if (emailIds.some(id => selectedEmailKeys.has(id))) { + // Clear selection if any selected emails were moved. The selection is + // keyed by owning account, so compare the dragged emails, not their ids. + if (draggedEmails.some(em => selectedEmailKeys.has(emailKeyFor(em)))) { clearSelection(); } diff --git a/stores/__tests__/email-batch-namesakes.test.ts b/stores/__tests__/email-batch-namesakes.test.ts new file mode 100644 index 000000000..4ffae0142 --- /dev/null +++ b/stores/__tests__/email-batch-namesakes.test.ts @@ -0,0 +1,154 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore, resolveSelectedEmails } from '../email-store'; +import { useAuthStore } from '../auth-store'; +import { emailKeyFor } from '@/lib/thread-utils'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +/** + * Batch actions over a selection that holds a namesake: the same email id in + * two accounts of the unified view. Each action must reach the account of the + * row that was ticked, through that account's client, and leave the other + * account's message alone. + */ +const twin = (account: string, overrides: Partial = {}): Email => ({ + id: 'eiaaaabc', threadId: 'taaaabc', blobId: `blob-${account}`, + mailboxIds: { [`inbox-${account}`]: true }, keywords: {}, + from: [{ email: `someone@${account}` }], to: [], subject: account, + receivedAt: '2026-09-19T08:00:00Z', size: 42, preview: '', hasAttachment: false, + sourceClientAccountId: account, sourceAccountId: account, + ...overrides, +} as unknown as Email); + +const mailbox = (overrides: Partial): Mailbox => ({ + id: 'inbox', name: 'Inbox', 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, + ...overrides, +} as Mailbox); + +const makeClient = () => ({ + batchDeleteEmails: vi.fn().mockResolvedValue(undefined), + batchMoveEmails: vi.fn().mockResolvedValue(undefined), + batchMarkAsRead: vi.fn().mockResolvedValue(undefined), + getMailboxes: vi.fn().mockResolvedValue([]), +}) as unknown as IJMAPClient & Record>; + +const a = twin('account-a'); +const b = twin('account-b'); +let clientA: ReturnType; +let clientB: ReturnType; + +beforeEach(() => { + clientA = makeClient(); + clientB = makeClient(); + useAuthStore.setState({ + activeAccountId: 'account-a', + client: clientA, + getClientForAccount: (id: string) => + (id === 'account-a' ? clientA : id === 'account-b' ? clientB : undefined) as never, + } as never); + useEmailStore.setState({ + isUnifiedView: true, + selectedMailbox: '', + selectedKeyword: null, + emails: [a, b], + threadEmailsCache: new Map(), + selectedEmailKeys: new Set(), + lastSelectedEmailKey: null, + mailboxes: [], + accountMailboxes: { + 'account-a': [mailbox({ id: 'trash-a', name: 'Trash', role: 'trash', accountId: 'account-a' })], + 'account-b': [mailbox({ id: 'trash-b', name: 'Trash', role: 'trash', accountId: 'account-b' })], + }, + error: null, + }); +}); + +describe('batch actions with a namesake in another account', () => { + it('permanently deletes only the ticked account copy', async () => { + useEmailStore.getState().toggleEmailSelection(b); + await useEmailStore.getState().batchDelete(clientA, true); + + expect(clientB.batchDeleteEmails).toHaveBeenCalledWith(['eiaaaabc'], 'account-b'); + expect(clientA.batchDeleteEmails).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails.map(e => e.sourceAccountId)).toEqual(['account-a']); + }); + + it('moves only the ticked account copy to its own trash', async () => { + useEmailStore.getState().toggleEmailSelection(b); + await useEmailStore.getState().batchDelete(clientA); + + expect(clientB.batchMoveEmails).toHaveBeenCalledWith(['eiaaaabc'], 'trash-b', 'account-b', false); + expect(clientA.batchMoveEmails).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails.map(e => e.sourceAccountId)).toEqual(['account-a']); + }); + + it('keeps the namesake whose account has no trash when the other one moved', async () => { + useEmailStore.setState({ + accountMailboxes: { + 'account-a': [], + 'account-b': [mailbox({ id: 'trash-b', name: 'Trash', role: 'trash', accountId: 'account-b' })], + }, + }); + useEmailStore.getState().selectAllEmails(); + await useEmailStore.getState().batchDelete(clientA); + + expect(clientB.batchMoveEmails).toHaveBeenCalledTimes(1); + expect(clientA.batchMoveEmails).not.toHaveBeenCalled(); + const state = useEmailStore.getState(); + expect(state.emails.map(e => e.sourceAccountId)).toEqual(['account-a']); + expect(state.error).toMatch(/trash folder missing/); + }); + + it('marks only the ticked account copy read', async () => { + useEmailStore.getState().toggleEmailSelection(b); + await useEmailStore.getState().batchMarkAsRead(clientA, true); + + expect(clientB.batchMarkAsRead).toHaveBeenCalledWith(['eiaaaabc'], true, 'account-b'); + expect(clientA.batchMarkAsRead).not.toHaveBeenCalled(); + const [rowA, rowB] = useEmailStore.getState().emails; + expect(rowA.keywords.$seen).toBeUndefined(); + expect(rowB.keywords.$seen).toBe(true); + }); + + it('copies the ticked account copy from its own account', async () => { + const crossAccountMoveEmails = vi.fn().mockResolvedValue(undefined); + useEmailStore.setState({ crossAccountMoveEmails }); + await useEmailStore.getState().copyEmailsToAccount([b], 'account-c', 'dest'); + + expect(crossAccountMoveEmails).toHaveBeenCalledTimes(1); + const [bySource, , , , sourceJmap] = crossAccountMoveEmails.mock.calls[0]; + expect([...bySource]).toEqual([['account-b', ['eiaaaabc']]]); + expect(sourceJmap).toBe('account-b'); + }); +}); + +describe('selection reaching expanded thread members', () => { + const row = { ...a, id: 'row', threadId: 'thr' } as Email; + const member = { ...a, id: 'member', threadId: 'thr' } as Email; + + it('resolves a ticked member that is not a list row, once', () => { + const state = { + emails: [row], + threadEmailsCache: new Map([['k', [row, member]]]), + selectedEmailKeys: new Set([emailKeyFor(row), emailKeyFor(member)]), + }; + expect(resolveSelectedEmails(state).map(e => e.id)).toEqual(['row', 'member']); + }); + + it('deletes a ticked member that lives outside the list', async () => { + useEmailStore.setState({ + emails: [row], + threadEmailsCache: new Map([['account-a/account-a:thr', [row, member]]]), + selectedEmailKeys: new Set([emailKeyFor(member)]), + }); + await useEmailStore.getState().batchDelete(clientA, true); + + expect(clientA.batchDeleteEmails).toHaveBeenCalledWith(['member'], 'account-a'); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index 251435f27..299735205 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -56,6 +56,28 @@ function dropFromSelection( return next; } +/** + * The emails behind a key-based selection, each once. Looks in the list and in + * the loaded members of expanded threads: a thread member ticked in a folder + * view may live in another folder and so never be a list row. + */ +export function resolveSelectedEmails(state: { + emails: Email[]; + threadEmailsCache: Map; + selectedEmailKeys: Set; +}): Email[] { + const { selectedEmailKeys } = state; + if (selectedEmailKeys.size === 0) return []; + const found = new Map(); + const visit = (email: Email) => { + const key = emailKeyFor(email); + if (selectedEmailKeys.has(key) && !found.has(key)) found.set(key, email); + }; + state.emails.forEach(visit); + for (const members of state.threadEmailsCache.values()) members.forEach(visit); + return [...found.values()]; +} + /** Enough of an email to key a selection by its owning account. */ type SelectableEmail = Pick; @@ -307,7 +329,8 @@ interface EmailStore { * grouped by the login and JMAP account they live in, then copied through * crossAccountMoveEmails with `keepOriginal`. */ - copyEmailsToAccount: (emailIds: string[], destAccountId: string, destMailboxId: string) => Promise; + /** Pass emails rather than ids where you have them: an id alone can name another account's namesake. */ + copyEmailsToAccount: (emails: Array, destAccountId: string, destMailboxId: string) => Promise; searchEmails: (client: IJMAPClient, query: string) => Promise; advancedSearch: (client: IJMAPClient) => Promise; setSearchFilters: (filters: Partial) => void; @@ -1822,7 +1845,7 @@ export const useEmailStore = create((set, get) => ({ }, selectEmail: (email) => { const prev = get().selectedEmail; - set({ selectedEmail: email, lastSelectedEmailKey: email?.id ?? get().lastSelectedEmailKey }); + set({ selectedEmail: email, lastSelectedEmailKey: email ? emailKeyFor(email) : get().lastSelectedEmailKey }); if (prev && (!email || email.id !== prev.id)) { emailHooks.onEmailClose.emitSync(prev); } @@ -3149,8 +3172,9 @@ export const useEmailStore = create((set, get) => ({ // One group per (login, JMAP account): crossAccountMoveEmails takes a // single source JMAP account per call. const groups = new Map(); - for (const id of emailIds) { - const email = state.emails.find((e) => e.id === id) + for (const entry of emailIds) { + const id = typeof entry === 'string' ? entry : entry.id; + const email = typeof entry !== 'string' ? entry : state.emails.find((e) => e.id === id) ?? (state.selectedEmail?.id === id ? state.selectedEmail : undefined); // Aggregate views stamp each email with the login that reaches it and // its owning JMAP account; otherwise it lives in the open folder. @@ -3634,7 +3658,7 @@ export const useEmailStore = create((set, get) => ({ // exactly those emails, never the keys and never a lookup by id (which // is what let a namesake in another account ride along). A targeted call // names its messages, and the account they belong to, itself. - const selectedEmails = target ? [] : emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))); + const selectedEmails = target ? [] : resolveSelectedEmails(get()); const targetIds = target ? new Set(target.emailIds) : null; const emailIdsArray = targetIds ? Array.from(targetIds) : selectedEmails.map(e => e.id); if (emailIdsArray.length === 0) return; @@ -3731,7 +3755,7 @@ export const useEmailStore = create((set, get) => ({ }, batchSetTag: async (client, tagId, add) => { - const { selectedEmailKeys, emails } = get(); + const { selectedEmailKeys } = get(); if (selectedEmailKeys.size === 0) return; // Both prefixes name the same tag when read, so taking one off clears @@ -3745,8 +3769,7 @@ export const useEmailStore = create((set, get) => ({ // folder's owner in a shared folder. (#281) The selection is keyed by // owning account, so a namesake in another account is never written. const groups = new Map>(); - for (const email of emails) { - if (!selectedEmailKeys.has(emailKeyFor(email))) continue; + for (const email of resolveSelectedEmails(get())) { // A message already in the wanted state needs no write. if (tagKeys.some(key => email.keywords?.[key] === true) === add) continue; const { client: actionClient, accountId } = resolveEmailActionContext(email, client); @@ -3797,8 +3820,11 @@ export const useEmailStore = create((set, get) => ({ // The selection is keyed by owning account; JMAP takes the bare ids of // exactly those emails, never the keys and never a lookup by id (which // is what let a namesake in another account ride along). - const selectedEmails = emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))); - const emailIdsArray = selectedEmails.map(e => e.id); + const selectedEmails = resolveSelectedEmails(get()); + if (selectedEmails.length === 0) { + set({ selectedEmailKeys: new Set(), isLoading: false }); + return; + } // Determine if the current folder forces permanent deletion. const currentMailbox = mailboxes.find(m => m.id === selectedMailbox); @@ -3813,12 +3839,14 @@ export const useEmailStore = create((set, get) => ({ // `sourceClientAccountId` and routes JMAP via `sourceAccountId`. Undecorated // emails (normal single-mailbox view) fall into '__default__' = active client. const accountMailboxes = get().accountMailboxes; - const bySource = new Map(); - for (const emailId of emailIdsArray) { - const email = emails.find(e => e.id === emailId); - const key = email?.sourceAccountId || '__default__'; - if (!bySource.has(key)) bySource.set(key, { clientAccountId: email?.sourceClientAccountId, ids: [] }); - bySource.get(key)!.ids.push(emailId); + // Group the selected emails themselves: looking an id up in the list + // would find the first account's namesake and act on it instead. + const bySource = new Map(); + for (const email of selectedEmails) { + const key = email.sourceAccountId || '__default__'; + if (!bySource.has(key)) bySource.set(key, { clientAccountId: email.sourceClientAccountId, ids: [], keys: [] }); + bySource.get(key)!.ids.push(email.id); + bySource.get(key)!.keys.push(emailKeyFor(email)); } // Undecorated emails viewed in a shared/group folder directly (non-unified) @@ -3847,8 +3875,9 @@ export const useEmailStore = create((set, get) => ({ } else { // Move to trash per account. const failedAccounts: string[] = []; - const movedEmailIds = new Set(); - const promises = Array.from(bySource.entries()).map(async ([sourceAccountId, { clientAccountId, ids }]) => { + // Keys, not ids: a namesake in an account that failed must stay. + const movedKeys = new Set(); + const promises = Array.from(bySource.entries()).map(async ([sourceAccountId, { clientAccountId, ids, keys }]) => { const acctClient = getClient(sourceAccountId, clientAccountId); if (!acctClient) { failedAccounts.push(sourceAccountId); @@ -3865,19 +3894,19 @@ export const useEmailStore = create((set, get) => ({ } const trashId = trashMailbox.originalId || trashMailbox.id; await acctClient.batchMoveEmails(ids, trashId, jmapIdFor(sourceAccountId), alsoMarkRead); - ids.forEach(id => movedEmailIds.add(id)); + keys.forEach(key => movedKeys.add(key)); }); await Promise.allSettled(promises); - if (failedAccounts.length > 0 && movedEmailIds.size === 0) { + if (failedAccounts.length > 0 && movedKeys.size === 0) { // Nothing moved - bail out so the UI doesn't drop the emails from view. throw new Error('Trash mailbox not found - cannot move emails to trash'); } // Only remove successfully moved emails from local state. - if (movedEmailIds.size < emailIdsArray.length) { - const deletedEmails = emails.filter(e => movedEmailIds.has(e.id)); - const remainingEmails = emails.filter(e => !movedEmailIds.has(e.id)); + if (movedKeys.size < selectedEmails.length) { + const deletedEmails = emails.filter(e => movedKeys.has(emailKeyFor(e))); + const remainingEmails = emails.filter(e => !movedKeys.has(emailKeyFor(e))); const mailboxPatch = applyBatchMailboxCounterUpdate(get(), deletedEmails, applyDeleteCounters); set({ emails: remainingEmails, @@ -3916,7 +3945,7 @@ export const useEmailStore = create((set, get) => ({ batchMoveToMailbox: async (client, toMailboxId, target) => { const { selectedEmailKeys, emails } = get(); // Keyed selection as in batchMarkAsRead; a targeted call names its own. - const selectedEmails = target ? [] : emails.filter(e => selectedEmailKeys.has(emailKeyFor(e))); + const selectedEmails = target ? [] : resolveSelectedEmails(get()); const targetIds = target ? new Set(target.emailIds) : null; const emailIdsArray = targetIds ? Array.from(targetIds) : selectedEmails.map(e => e.id); if (emailIdsArray.length === 0) return;