starred/webmail
1
0
Fork 0
mirror of https://github.com/bulwarkmail/webmail.git synced 2026-10-31 16:13:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: act on the clicked row when another account has a message with the same id (GHSA-8x23-qw3w-mhrx)

Browse Source
This commit is contained in:
Linus Rath
2026-10-18 17:54:48 +02:00
parent fbae8acfb8
commit bfe98b49a0
7 changed files with 239 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/email/email-list.tsx
+9 -5
View File
@@ -64,8 +64,9 @@ interface EmailListProps {
onTogglePinned?: (email: Email) => void;
onDelete?: (email: Email) => void;
onArchive?: (email: Email) => void;
onSetTag?: (emailId: string, tagId: string | null) => void;
onMoveToMailbox?: (emailId: string, mailboxId: string) => void;
/** `emailKey` (emailKeyFor of the row) tells namesakes in other accounts apart. */
onSetTag?: (emailId: string, tagId: string | null, emailKey?: string) => void;
onMoveToMailbox?: (emailId: string, mailboxId: string, emailKey?: string) => void;
onMarkAsSpam?: (email: Email) => void;
onUndoSpam?: (email: Email) => void;
onOpenAttachment?: (email: Email, attachment: Attachment) => void;
@@ -252,8 +253,11 @@ export function EmailList({
* the message it was handed when it opened, but tags can be applied from
* inside it without dismissing it, so what it draws has to keep up.
*/
// The live copy of the row the menu was opened on, by its key: in a merged
// view the bare id is also another account's message, and every action of
// the menu would go to that namesake instead.
const contextMenuEmail = contextMenu.data
? emails.find((email) => email.id === contextMenu.data!.id) ?? contextMenu.data
? emails.find((email) => emailKeyFor(email) === emailKeyFor(contextMenu.data!)) ?? contextMenu.data
: null;
/**
* Whose sender the menu's Rules entry uses: every selected message, or the
@@ -1034,8 +1038,8 @@ export function EmailList({
onTogglePinned={onTogglePinned ? () => onTogglePinned(contextMenuEmail!) : undefined}
onDelete={() => onDelete?.(contextMenuEmail!)}
onArchive={() => onArchive?.(contextMenuEmail!)}
onSetTag={(color) => onSetTag?.(contextMenuEmail!.id, color)}
onMoveToMailbox={(mailboxId) => onMoveToMailbox?.(contextMenuEmail!.id, mailboxId)}
onSetTag={(color) => onSetTag?.(contextMenuEmail!.id, color, emailKeyFor(contextMenuEmail!))}
onMoveToMailbox={(mailboxId) => onMoveToMailbox?.(contextMenuEmail!.id, mailboxId, emailKeyFor(contextMenuEmail!))}
onRemoveFromFolder={() => void removeFromFolder([contextMenuEmail!])}
onMarkAsSpam={() => onMarkAsSpam?.(contextMenuEmail!)}
onUndoSpam={() => onUndoSpam?.(contextMenuEmail!)}
components/email/thread-list-item.tsx
+3 -3
View File
@@ -304,7 +304,7 @@ interface ThreadListItemProps {
onMarkAsRead?: (email: Email, read: boolean) => void;
onDelete?: (email: Email) => void;
onArchive?: (email: Email) => void;
onSetTag?: (emailId: string, tagId: string | null) => void;
onSetTag?: (emailId: string, tagId: string | null, emailKey?: string) => void;
onMarkAsSpam?: (email: Email) => void;
onUndoSpam?: (email: Email) => void;
onOpenAttachment?: (email: Email, attachment: Attachment) => void;
@@ -1030,7 +1030,7 @@ export const ThreadListItem = React.forwardRef<HTMLDivElement, ThreadListItemPro
onMarkAsRead={onMarkAsRead ? (read) => onMarkAsRead(latestEmail, read) : undefined}
onDelete={onDelete ? () => onDelete(latestEmail) : undefined}
onArchive={onArchive ? () => onArchive(latestEmail) : undefined}
onSetTag={onSetTag ? (color) => onSetTag(latestEmail.id, color) : undefined}
onSetTag={onSetTag ? (color) => onSetTag(latestEmail.id, color, emailKeyFor(latestEmail)) : undefined}
onMarkAsSpam={onMarkAsSpam ? () => onMarkAsSpam(latestEmail) : undefined}
onUndoSpam={onUndoSpam ? () => onUndoSpam(latestEmail) : undefined}
onOpenAttachment={onOpenAttachment ? (a) => onOpenAttachment(latestEmail, a) : undefined}
@@ -1537,7 +1537,7 @@ export const ThreadListItem = React.forwardRef<HTMLDivElement, ThreadListItemPro
onMarkAsRead={onMarkAsRead ? (read) => onMarkAsRead(latestEmail, read) : undefined}
onDelete={onDelete ? () => onDelete(latestEmail) : undefined}
onArchive={onArchive ? () => onArchive(latestEmail) : undefined}
onSetTag={onSetTag ? (color) => onSetTag(latestEmail.id, color) : undefined}
onSetTag={onSetTag ? (color) => onSetTag(latestEmail.id, color, emailKeyFor(latestEmail)) : undefined}
onMarkAsSpam={onMarkAsSpam ? () => onMarkAsSpam(latestEmail) : undefined}
onUndoSpam={onUndoSpam ? () => onUndoSpam(latestEmail) : undefined}
isInJunk={currentMailboxRole === 'junk'}
components/mail/mail-app.tsx
+37 -34
View File
@@ -838,7 +838,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const keyboardHandlers = useMemo(() => ({
onNextEmail: () => {
if (activeEmails.length === 0) return;
const currentIndex = selectedEmail ? activeEmails.findIndex(e => e.id === selectedEmail.id) : -1;
const currentIndex = selectedEmail ? activeEmails.findIndex(e => emailKeyFor(e) === emailKeyFor(selectedEmail)) : -1;
const nextIndex = currentIndex < activeEmails.length - 1 ? currentIndex + 1 : currentIndex;
if (nextIndex >= 0 && nextIndex < activeEmails.length) {
void handleEmailSelect(activeEmails[nextIndex]);
@@ -847,7 +847,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
},
onPreviousEmail: () => {
if (activeEmails.length === 0) return;
const currentIndex = selectedEmail ? activeEmails.findIndex(e => e.id === selectedEmail.id) : activeEmails.length;
const currentIndex = selectedEmail ? activeEmails.findIndex(e => emailKeyFor(e) === emailKeyFor(selectedEmail)) : activeEmails.length;
const prevIndex = currentIndex > 0 ? currentIndex - 1 : 0;
if (prevIndex >= 0 && prevIndex < activeEmails.length) {
void handleEmailSelect(activeEmails[prevIndex]);
@@ -946,7 +946,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (shortcutsTargetSelection) {
await batchMarkAsRead(client, false);
} else if (selectedEmail) {
await markAsRead(client, selectedEmail.id, false);
await markAsRead(client, selectedEmail.id, false, emailKeyFor(selectedEmail));
}
},
onMarkAsRead: async () => {
@@ -955,7 +955,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (shortcutsTargetSelection) {
await batchMarkAsRead(client, true);
} else if (selectedEmail) {
await markAsRead(client, selectedEmail.id, true);
await markAsRead(client, selectedEmail.id, true, emailKeyFor(selectedEmail));
}
},
onToggleSpam: async () => {
@@ -1953,13 +1953,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
} else if (markAsReadDelay === 0) {
// Mark as read instantly
debug.log('email', '[Mark as Read] Instant mode - marking as read now');
markAsRead(client, selectedEmail.id, true);
markAsRead(client, selectedEmail.id, true, emailKeyFor(selectedEmail));
} else {
// Mark as read after delay
debug.log('email', '[Mark as Read] Delayed mode - will mark as read in', markAsReadDelay, 'ms');
markAsReadTimeoutRef.current = setTimeout(() => {
debug.log('email', '[Mark as Read] Timeout fired - marking as read now');
markAsRead(client, selectedEmail.id, true);
markAsRead(client, selectedEmail.id, true, emailKeyFor(selectedEmail));
markAsReadTimeoutRef.current = null;
}, markAsReadDelay);
}
@@ -2798,7 +2798,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
let archived: boolean;
if (archiveMode === 'single') {
archived = await moveThreadToMailbox(client, emailToArchive.id, archiveMailbox.id);
archived = await moveThreadToMailbox(client, emailToArchive.id, archiveMailbox.id, emailKeyFor(emailToArchive));
} else {
const emailDate = new Date(emailToArchive.receivedAt);
const year = emailDate.getFullYear().toString();
@@ -2815,7 +2815,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
}
if (archiveMode === 'year') {
archived = await moveThreadToMailbox(client, emailToArchive.id, yearMailbox.id);
archived = await moveThreadToMailbox(client, emailToArchive.id, yearMailbox.id, emailKeyFor(emailToArchive));
} else {
const yearId = yearMailbox.originalId || yearMailbox.id;
const afterYear = readMailboxes();
@@ -2827,7 +2827,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (!monthMailbox) return; // a plugin refused the folder
await refreshMailboxes();
}
archived = await moveThreadToMailbox(client, emailToArchive.id, monthMailbox.id);
archived = await moveThreadToMailbox(client, emailToArchive.id, monthMailbox.id, emailKeyFor(emailToArchive));
}
}
if (!archived) return;
@@ -2855,13 +2855,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const folderToLeave = useEmailStore(removalFolder);
const openEmailCanLeaveFolder = !!selectedEmail && canRemoveFromFolder(selectedEmail, folderToLeave);
const handleMoveToMailbox = async (emailId: string, mailboxId: string) => {
const handleMoveToMailbox = async (emailId: string, mailboxId: string, emailKey?: string) => {
if (!client) return;
const destinationList = [mailboxes, ...Object.values(accountMailboxes)]
.find(list => list.some(m => m.id === mailboxId));
const destination = destinationList?.find(m => m.id === mailboxId);
try {
if (!(await moveToMailboxCrossAware(client, emailId, mailboxId))) return;
if (!(await moveToMailboxCrossAware(client, emailId, mailboxId, emailKey))) return;
toast.success(
t('notifications.email_moved'),
destination && destinationList
@@ -2878,7 +2878,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (!client || !selectedEmail) return;
try {
await toggleStar(client, selectedEmail.id);
await toggleStar(client, selectedEmail.id, emailKeyFor(selectedEmail));
} catch (error) {
console.error("Failed to toggle star:", error);
toast.error(t('notifications.error_updating'));
@@ -2891,14 +2891,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const emailId = emailToMark.id;
try {
if (!(await markAsSpam(client, emailId))) return;
if (!(await markAsSpam(client, emailId, emailKeyFor(emailToMark)))) return;
toast.success(t('email_viewer.spam.toast_success'), {
action: {
label: t('email_viewer.spam.toast_undo'),
onClick: async () => {
try {
if (await undoSpam(client, emailId)) toast.success(t('notifications.email_moved'));
if (await undoSpam(client, emailId, emailKeyFor(emailToMark))) toast.success(t('notifications.email_moved'));
} catch (_error) {
console.error("Failed to undo spam:", _error);
toast.error(t('email_viewer.spam.error'));
@@ -2917,7 +2917,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (!client || !emailToRestore) return;
try {
if (!(await undoSpam(client, emailToRestore.id))) return;
if (!(await undoSpam(client, emailToRestore.id, emailKeyFor(emailToRestore)))) return;
toast.success(t('email_viewer.spam.toast_not_spam_success'));
} catch (_error) {
@@ -2930,7 +2930,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (!client) return;
try {
const email = emails.find(e => e.id === emailToPin.id) ?? emailToPin;
const email = emails.find(e => emailKeyFor(e) === emailKeyFor(emailToPin)) ?? emailToPin;
const isPinned = email.keywords?.['$pinned'] === true;
// Same routing as tags: the write goes to the email's own account, and
@@ -2940,7 +2940,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
// would replace a tag-filtered list (refreshCurrentMailbox fetches by
// folder only). (#281)
// Only $pinned changes; the row's other keywords may be stale.
await patchEmailKeywords(client, email.id, { $pinned: !isPinned });
await patchEmailKeywords(client, email.id, { $pinned: !isPinned }, emailKeyFor(email));
if (!isUnifiedView && !useEmailStore.getState().selectedKeyword) {
void refreshCurrentMailbox(client);
}
@@ -2950,12 +2950,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
}
};
const handleSetTag = async (emailId: string, tagId: string | null) => {
const handleSetTag = async (emailId: string, tagId: string | null, emailKey?: string) => {
if (!client) return;
try {
// Remove any existing tag keywords
const email = emails.find(e => e.id === emailId);
// Remove any existing tag keywords. By the row's key when given: the bare
// id can also be another account's message in a merged view.
const email = emailKey ? emails.find(e => emailKeyFor(e) === emailKey) : emails.find(e => e.id === emailId);
if (!email) return;
// Only the tag keywords change: writing the whole set from this row
@@ -2991,7 +2992,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
// call site covered only the unified view, so a tag set on a directly
// selected shared folder was written to the reaching account and silently
// dropped by the server. (#281)
await patchEmailKeywords(client, emailId, changes);
await patchEmailKeywords(client, emailId, changes, emailKeyFor(email));
// Refresh tag counts
fetchTagCounts(client);
@@ -3971,9 +3972,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
if (!client || target.keywords?.$seen) return;
try {
if (useEmailStore.getState().emails.some((e) => e.id === target.id)) {
await markAsRead(client, target.id, true);
await markAsRead(client, target.id, true, emailKeyFor(target));
// Still unread: a plugin stopped it, so the copy here stays unread too.
if (!useEmailStore.getState().emails.find((e) => e.id === target.id)?.keywords?.$seen) return;
if (!useEmailStore.getState().emails.find((e) => emailKeyFor(e) === emailKeyFor(target))?.keywords?.$seen) return;
} else {
// Not in the store's list, so the store action can't ask plugins.
if (emailHooks.onBeforeEmailReadStateChange.size > 0
@@ -4212,7 +4213,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
};
// Navigate to next/previous email in the list
const selectedEmailIndex = selectedEmail ? activeEmails.findIndex(e => e.id === selectedEmail.id) : -1;
const selectedEmailIndex = selectedEmail ? activeEmails.findIndex(e => emailKeyFor(e) === emailKeyFor(selectedEmail)) : -1;
const handleNavigateNext = selectedEmailIndex >= 0 && selectedEmailIndex < activeEmails.length - 1
? () => handleEmailSelect(activeEmails[selectedEmailIndex + 1])
@@ -4709,9 +4710,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
// The open message when it is in this list, else the
// first row - as an email, so the selection is keyed by
// its owning account and not by a colliding id.
const target = (selectedEmail && activeEmails.some((e) => e.id === selectedEmail.id)
? activeEmails.find((e) => e.id === selectedEmail.id)
: activeEmails[0])!;
const openKey = selectedEmail ? emailKeyFor(selectedEmail) : null;
const target = activeEmails.find((e) => emailKeyFor(e) === openKey) ?? activeEmails[0];
toggleEmailSelection(target);
}
}}
@@ -5058,12 +5058,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
}}
onMarkAsRead={async (email, read) => {
if (client) {
await markAsRead(client, email.id, read);
await markAsRead(client, email.id, read, emailKeyFor(email));
}
}}
onToggleStar={async (email) => {
if (client) {
await toggleStar(client, email.id);
await toggleStar(client, email.id, emailKeyFor(email));
}
}}
onTogglePinned={async (email) => {
@@ -5075,8 +5075,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
onArchive={archiveActionEnabled ? async (email) => {
await handleArchive(email);
} : undefined}
onSetTag={(emailId, color) => {
handleSetTag(emailId, color);
onSetTag={(emailId, color, emailKey) => {
handleSetTag(emailId, color, emailKey);
}}
onMoveToMailbox={handleMoveToMailbox}
onMarkAsSpam={async (email) => {
@@ -5212,7 +5212,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
onDownloadAttachment={handleDownloadAttachment}
onMarkAsRead={async (emailId, read) => {
if (client) {
await markAsRead(client, emailId, read);
const target = conversationEmails.find((e) => e.id === emailId);
await markAsRead(client, emailId, read, target ? emailKeyFor(target) : undefined);
}
}}
mailboxRole={mailboxes.find(m => m.id === selectedMailbox)?.role ?? (isUnifiedView ? (unifiedRole ?? undefined) : undefined)}
@@ -5246,8 +5247,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
onUndoSpam={() => handleUndoSpam()}
onMarkAsRead={async (emailId, read) => {
if (client) {
await markAsRead(client, emailId, read);
const nowRead = !!useEmailStore.getState().emails.find((e) => e.id === emailId)?.keywords?.$seen;
const emailKey = selectedEmail?.id === emailId ? emailKeyFor(selectedEmail) : undefined;
await markAsRead(client, emailId, read, emailKey);
const nowRead = !!useEmailStore.getState().emails
.find((e) => (emailKey ? emailKeyFor(e) === emailKey : e.id === emailId))?.keywords?.$seen;
if (nowRead !== read) return; // a plugin stopped it
// Marking the open message unread returns to the list
// (Gmail-style, gated on returnToListAfterAction). Staying
components/pro/pro-email-tab-body.tsx
+4 -4
View File
@@ -19,7 +19,7 @@ import type { IJMAPClient } from "@/lib/jmap/client-interface";
import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix";
import { getQuoteBodies } from "@/lib/email-composer-utils";
import { buildForwardAsAttachmentPayload } from "@/lib/forward-as-attachment";
import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY } from "@/lib/thread-utils";
import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, emailKeyFor } from "@/lib/thread-utils";
import { cn } from "@/lib/utils";
import { getDraftTextBody } from '@/lib/email-body-selection';
import { messagePriorityFromHeaders } from '@/lib/message-priority';
@@ -252,7 +252,7 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
const archiveMb = mailboxes.find((m) => m.role === 'archive');
if (!archiveMb) return;
try {
if (!(await moveToMailbox(client, email.id, archiveMb.id))) return;
if (!(await moveToMailbox(client, email.id, archiveMb.id, emailKeyFor(email)))) return;
toast.success(tNotifications('email_archived'));
onClose();
} catch (err) {
@@ -263,7 +263,7 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
const handleToggleStar = useCallback(async () => {
if (!client || !email) return;
try {
if (!(await toggleStar(client, email.id))) return;
if (!(await toggleStar(client, email.id, emailKeyFor(email)))) return;
// Reflect locally - the viewer re-reads from email-store's selectedEmail
// shape only for the mail tab; here we update our local copy too.
setEmail((prev) => prev ? {
@@ -320,7 +320,7 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
const handleMoveToMailbox = useCallback(async (mailboxId: string) => {
if (!client || !email) return;
try {
if (await moveToMailbox(client, email.id, mailboxId)) onClose();
if (await moveToMailbox(client, email.id, mailboxId, emailKeyFor(email))) onClose();
} catch (err) {
console.error('Move failed:', err);
}
stores/__tests__/email-single-namesakes.test.ts
+111
View File
@@ -36,6 +36,11 @@ const makeClient = () => ({
moveToTrash: vi.fn().mockResolvedValue(undefined),
deleteEmail: vi.fn().mockResolvedValue(undefined),
getMailboxes: vi.fn().mockResolvedValue([]),
markAsRead: vi.fn().mockResolvedValue(undefined),
toggleStar: vi.fn().mockResolvedValue(undefined),
batchUpdateKeywords: vi.fn().mockResolvedValue(undefined),
markAsSpam: vi.fn().mockResolvedValue(undefined),
undoSpam: vi.fn().mockResolvedValue(undefined),
}) as unknown as IJMAPClient & Record<string, ReturnType<typeof vi.fn>>;
// The list is newest first: account A's namesake sits right above the row
@@ -104,3 +109,109 @@ describe('single-email actions with a namesake in another account', () => {
expect(useEmailStore.getState().emails.map(e => e.sourceAccountId)).toEqual(['account-a']);
});
});
/**
* The other single-email actions resolved the row by its bare id as well, so in
* a merged view a hover star, a hover or menu read toggle, a tag or "spam" went
* to the FIRST row with that id - another account's message (measured against
* Stalwart 0.16: the hover star on one account's Sent message flagged another
* account's). With the row's key each reaches only the clicked row.
*/
describe('keyword and spam actions with a namesake in another account', () => {
beforeEach(() => {
useEmailStore.setState({
emails: [above, { ...clicked, keywords: {} } as Email],
selectedEmail: null,
processingReadStatus: new Set(),
} as never);
});
it('stars only the clicked row', async () => {
await useEmailStore.getState().toggleStar(clientA, clicked.id, emailKeyFor(clicked));
expect(clientB.toggleStar).toHaveBeenCalledWith('eiaaaabc', true, 'account-b');
expect(clientA.toggleStar).not.toHaveBeenCalled();
const [a, b] = useEmailStore.getState().emails;
expect(a.keywords.$flagged).toBeFalsy();
expect(b.keywords.$flagged).toBe(true);
});
it('marks only the clicked row read', async () => {
await useEmailStore.getState().markAsRead(clientA, clicked.id, true, emailKeyFor(clicked));
expect(clientB.markAsRead).toHaveBeenCalledWith('eiaaaabc', true, 'account-b');
expect(clientA.markAsRead).not.toHaveBeenCalled();
const [a, b] = useEmailStore.getState().emails;
expect(a.keywords.$seen).toBe(true);
expect(b.keywords.$seen).toBe(true);
});
it('marks only the clicked row unread and leaves the namesake read', async () => {
useEmailStore.setState({ emails: [above, clicked] } as never);
await useEmailStore.getState().markAsRead(clientA, clicked.id, false, emailKeyFor(clicked));
expect(clientB.markAsRead).toHaveBeenCalledWith('eiaaaabc', false, 'account-b');
expect(clientA.markAsRead).not.toHaveBeenCalled();
const [a, b] = useEmailStore.getState().emails;
expect(a.keywords.$seen).toBe(true);
expect(b.keywords.$seen).toBeFalsy();
});
it('writes a keyword change only to the clicked row', async () => {
await useEmailStore.getState().patchEmailKeywords(clientA, clicked.id, { $label1: true }, emailKeyFor(clicked));
expect(clientB.batchUpdateKeywords).toHaveBeenCalledWith(['eiaaaabc'], { 'keywords/$label1': true }, 'account-b');
expect(clientA.batchUpdateKeywords).not.toHaveBeenCalled();
const [a, b] = useEmailStore.getState().emails;
expect(a.keywords.$label1).toBeUndefined();
expect(b.keywords.$label1).toBe(true);
});
it('without a key keeps the previous single-account behaviour (bare id)', async () => {
useEmailStore.setState({ isUnifiedView: false, emails: [{ ...clicked, sourceClientAccountId: undefined, sourceAccountId: undefined } as Email] } as never);
await useEmailStore.getState().toggleStar(clientA, clicked.id);
expect(clientA.toggleStar).toHaveBeenCalledWith('eiaaaabc', true, undefined);
});
});
/**
* Spam and "not spam" took the row by its bare id too, the toast's undo cache
* was keyed by it, and the next open message was picked by it: with a namesake
* listed above, both rows dropped out of the selection walk.
*/
describe('spam actions with a namesake in another account', () => {
it('reports only the clicked row and opens the namesake next', async () => {
await useEmailStore.getState().markAsSpam(clientA, clicked.id, emailKeyFor(clicked));
expect(clientB.markAsSpam).toHaveBeenCalledWith('eiaaaabc', 'account-b', false);
expect(clientA.markAsSpam).not.toHaveBeenCalled();
const state = useEmailStore.getState();
expect(state.emails.map(e => e.sourceAccountId)).toEqual(['account-a']);
expect(state.selectedEmail?.sourceAccountId).toBe('account-a');
});
it('moves only the clicked row out of junk with "not spam"', async () => {
// The list is refetched after the move; keep the local result in view.
useEmailStore.setState({ fetchUnifiedEmails: vi.fn() } as never);
await useEmailStore.getState().undoSpam(clientA, clicked.id, emailKeyFor(clicked));
expect(clientB.undoSpam).toHaveBeenCalledWith('eiaaaabc', 'a', 'account-b');
expect(clientA.undoSpam).not.toHaveBeenCalled();
const state = useEmailStore.getState();
expect(state.emails.map(e => e.sourceAccountId)).toEqual(['account-a']);
expect(state.selectedEmail?.sourceAccountId).toBe('account-a');
});
it('keeps one undo entry per row when both namesakes are reported', async () => {
useEmailStore.setState({ spamUndoCache: new Map() } as never);
await useEmailStore.getState().markAsSpam(clientA, clicked.id, emailKeyFor(clicked));
useEmailStore.setState({ emails: [above] } as never);
await useEmailStore.getState().markAsSpam(clientA, above.id, emailKeyFor(above));
await useEmailStore.getState().undoSpam(clientA, clicked.id, emailKeyFor(clicked));
expect(clientB.undoSpam).toHaveBeenCalledWith('eiaaaabc', 'a', 'account-b');
expect(clientA.undoSpam).not.toHaveBeenCalled();
});
});
stores/__tests__/email-store-cross-account-move.test.ts
+1 -1
View File
@@ -80,7 +80,7 @@ describe('email-store moveToMailboxCrossAware', () => {
await useEmailStore.getState().moveToMailboxCrossAware(client, 'e1', 'archive-A');
expect(crossSpy).not.toHaveBeenCalled();
expect(moveSpy).toHaveBeenCalledWith(client, 'e1', 'archive-A');
expect(moveSpy).toHaveBeenCalledWith(client, 'e1', 'archive-A', undefined);
});
it('reverse: shared → own also routes cross-account (source override set)', async () => {
stores/email-store.ts
+74 -58
View File
@@ -323,7 +323,8 @@ interface EmailStore {
sendRawEmail: (client: IJMAPClient, rawMimeBlob: Blob, identityId: string, delayedUntil?: string, envelopeRecipients?: string[]) => Promise<SendEmailResult>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
deleteEmail: (client: IJMAPClient, emailId: string, forceDelete?: boolean, emailKey?: string) => Promise<boolean>;
markAsRead: (client: IJMAPClient, emailId: string, read: boolean) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
markAsRead: (client: IJMAPClient, emailId: string, read: boolean, emailKey?: string) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
moveToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string, emailKey?: string) => Promise<boolean>;
/**
@@ -333,9 +334,10 @@ interface EmailStore {
* equivalent of what drag-and-drop already does. Falls back to the plain
* single-account `moveToMailbox` when source and destination share an account.
*/
moveToMailboxCrossAware: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise<boolean>;
moveToMailboxCrossAware: (client: IJMAPClient, emailId: string, mailboxId: string, emailKey?: string) => Promise<boolean>;
moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string) => Promise<boolean>;
moveThreadToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
moveThreadToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string, emailKey?: string) => Promise<boolean>;
/**
* Take messages out of the open folder only, leaving them in their other
* folders (#615). Acts on those of `emails` that removalFolder and
@@ -391,15 +393,18 @@ interface EmailStore {
scopeSearchToOpenFolder: () => void;
clearSearchFilters: () => void;
toggleAdvancedSearch: () => void;
toggleStar: (client: IJMAPClient, emailId: string) => Promise<boolean>;
setEmailKeywords: (client: IJMAPClient, emailId: string, keywords: Record<string, boolean>) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
toggleStar: (client: IJMAPClient, emailId: string, emailKey?: string) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
setEmailKeywords: (client: IJMAPClient, emailId: string, keywords: Record<string, boolean>, emailKey?: string) => Promise<boolean>;
/**
* Set (true) or clear (false) just the named keywords. Every other keyword
* is left as the server has it.
*/
patchEmailKeywords: (client: IJMAPClient, emailId: string, changes: Record<string, boolean>) => Promise<boolean>;
patchEmailKeywords: (client: IJMAPClient, emailId: string, changes: Record<string, boolean>, emailKey?: string) => Promise<boolean>;
markEmailKeyword: (client: IJMAPClient, emailId: string, keyword: string) => Promise<void>;
setEmailKeywordsLocal: (emailId: string, keywords: Record<string, boolean>) => void;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
setEmailKeywordsLocal: (emailId: string, keywords: Record<string, boolean>, emailKey?: string) => void;
// Batch operations
/** Acts on the selection, or on `target`'s messages when given. */
@@ -423,8 +428,10 @@ interface EmailStore {
// owning JMAP account (passed to JMAP), `sourceClientAccountId` is the login the
// email is reachable through (used to pick the right client). (#281)
spamUndoCache: Map<string, { emailId: string; originalMailboxId: string; accountId?: string; sourceClientAccountId?: string }>;
markAsSpam: (client: IJMAPClient, emailId: string) => Promise<boolean>;
undoSpam: (client: IJMAPClient, emailId: string) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
markAsSpam: (client: IJMAPClient, emailId: string, emailKey?: string) => Promise<boolean>;
/** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */
undoSpam: (client: IJMAPClient, emailId: string, emailKey?: string) => Promise<boolean>;
batchMarkAsSpam: (client: IJMAPClient, emailIds: string[]) => Promise<boolean>;
batchUndoSpam: (client: IJMAPClient, emailIds: string[]) => Promise<boolean>;
@@ -532,10 +539,6 @@ function getNextSelectedEmailAfterRemoval(state: { emails: Email[]; selectedEmai
return null;
}
function getNextSelectedEmail(state: { emails: Email[]; selectedEmail: Email | null }, removedEmailId: string): Email | null {
return getNextSelectedEmailAfterRemoval(state, new Set([removedEmailId]));
}
/**
* Matches the list row a single-email action is about. Email ids repeat across
* accounts, so in a merged view (unified inbox, tags) the bare id also matches
@@ -1348,10 +1351,14 @@ export function resolveEmailActionContext(
function resolveKeywordActionContext(
emailId: string,
passedClient: IJMAPClient,
emailKey?: string,
): { client: IJMAPClient; accountId: string | undefined } {
const state = useEmailStore.getState();
const email = state.emails.find(e => e.id === emailId)
?? (state.selectedEmail?.id === emailId ? state.selectedEmail : undefined);
// By the row's key when given: in a merged view the bare id also matches
// another account's namesake, whose account would receive the write.
const isTarget = singleEmailMatcher(emailId, emailKey);
const email = state.emails.find(isTarget)
?? (state.selectedEmail && isTarget(state.selectedEmail) ? state.selectedEmail : undefined);
if (!email) return { client: passedClient, accountId: undefined };
const { client, accountId } = resolveEmailActionContext(email, passedClient);
return { client, accountId };
@@ -3378,17 +3385,18 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
return true;
},
markAsRead: async (client, emailId, read): Promise<boolean> => {
markAsRead: async (client, emailId, read, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
try {
// Check if this email is already being processed
const processingKey = `${emailId}-${read}`;
const processingKey = `${emailKey ?? emailId}-${read}`;
const currentProcessing = get().processingReadStatus;
if (currentProcessing.has(processingKey)) {
return true; // Already being processed
}
// Get the email to check its current state and mailboxes
const email = get().emails.find(e => e.id === emailId);
const email = get().emails.find(isTarget);
if (!email) return false;
// Check if already in the desired state
@@ -3425,7 +3433,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
newProcessingSet.delete(processingKey);
// Only update counters if the state is actually changing
const emailInState = state.emails.find(e => e.id === emailId);
const emailInState = state.emails.find(isTarget);
if (!emailInState) return { processingReadStatus: newProcessingSet };
const wasRead = emailInState.keywords?.$seen;
@@ -3458,9 +3466,9 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
return {
emails: state.emails.map(e =>
e.id === emailId ? { ...e, keywords: { ...e.keywords, $seen: read } } : e
isTarget(e) ? { ...e, keywords: { ...e.keywords, $seen: read } } : e
),
selectedEmail: state.selectedEmail?.id === emailId
selectedEmail: (!!state.selectedEmail && isTarget(state.selectedEmail))
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
: state.selectedEmail,
retainedInViewIds,
@@ -3476,7 +3484,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
let updated = false;
const newCache = new Map(state.threadEmailsCache);
for (const [tid, cachedEmails] of newCache) {
const idx = cachedEmails.findIndex(e => e.id === emailId);
const idx = cachedEmails.findIndex(isTarget);
if (idx !== -1) {
newCache.set(tid, cachedEmails.map((e, i) =>
i === idx ? { ...e, keywords: { ...e.keywords, $seen: read } } : e
@@ -3506,9 +3514,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
return true;
},
moveToMailboxCrossAware: async (client, emailId, destinationMailboxId): Promise<boolean> => {
moveToMailboxCrossAware: async (client, emailId, destinationMailboxId, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
const state = get();
const email = state.emails.find((e) => e.id === emailId);
const email = state.emails.find(isTarget);
if (!email) return false;
const { mailboxes } = resolveEmailActionContext(email, client);
@@ -3528,7 +3537,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
sourceMailbox.accountId !== destMailbox.accountId;
if (!isCrossAccount) {
return await get().moveToMailbox(client, emailId, destinationMailboxId);
return await get().moveToMailbox(client, emailId, destinationMailboxId, emailKey);
}
const destAccountId = resolveDestLocalAccountId(destMailbox!);
@@ -3536,7 +3545,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
email.accountId ?? state.viewingAccountId ?? useAuthStore.getState().activeAccountId;
if (!destAccountId || !sourceAccountId) {
// Can't resolve the local endpoints — fall back rather than drop the mail.
return await get().moveToMailbox(client, emailId, destinationMailboxId);
return await get().moveToMailbox(client, emailId, destinationMailboxId, emailKey);
}
// JMAP has no cross-account move: copy the raw message into the destination
@@ -3971,14 +3980,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
}
},
moveThreadToMailbox: async (client, emailId, destinationMailboxId): Promise<boolean> => {
moveThreadToMailbox: async (client, emailId, destinationMailboxId, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
try {
const state = get();
const email = state.emails.find(e => e.id === emailId)
?? (state.selectedEmail?.id === emailId ? state.selectedEmail : null);
const email = state.emails.find(isTarget)
?? ((!!state.selectedEmail && isTarget(state.selectedEmail)) ? state.selectedEmail : null);
if (!email?.threadId) {
return await get().moveToMailbox(client, emailId, destinationMailboxId);
return await get().moveToMailbox(client, emailId, destinationMailboxId, emailKey);
}
// In unified view route to the email's own account (client + that
@@ -3991,7 +4001,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
const threadEmailIds = thread?.emailIds?.length ? thread.emailIds : [emailId];
if (threadEmailIds.length <= 1) {
return await get().moveToMailbox(client, emailId, destinationMailboxId);
return await get().moveToMailbox(client, emailId, destinationMailboxId, emailKey);
}
const move = { emailIds: threadEmailIds, fromMailboxId: moveSourceMailboxId([email], mailboxes), toMailboxId: destinationMailboxId };
@@ -4307,9 +4317,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
set((state) => ({ isAdvancedSearchOpen: !state.isAdvancedSearchOpen }));
},
toggleStar: async (client, emailId): Promise<boolean> => {
toggleStar: async (client, emailId, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
try {
const email = get().emails.find(e => e.id === emailId);
const email = get().emails.find(isTarget);
if (!email) return false;
const isFlagged = email.keywords.$flagged || false;
@@ -4325,9 +4336,9 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// Update local state
set((state) => ({
emails: state.emails.map(e =>
e.id === emailId ? { ...e, keywords: { ...e.keywords, $flagged: !isFlagged } } : e
isTarget(e) ? { ...e, keywords: { ...e.keywords, $flagged: !isFlagged } } : e
),
selectedEmail: state.selectedEmail?.id === emailId
selectedEmail: (!!state.selectedEmail && isTarget(state.selectedEmail))
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $flagged: !isFlagged } }
: state.selectedEmail,
// In the starred cross view, un-starring drops the mail out of the
@@ -4364,7 +4375,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
void emailHooks.onEmailKeywordChange.emit({ emailIds: [emailId], added: [keyword], removed: [] });
},
setEmailKeywords: async (client, emailId, keywords): Promise<boolean> => {
setEmailKeywords: async (client, emailId, keywords, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
// Route the write to the email's OWN account, exactly like toggleStar and
// every other single-email action. `resolveEmailActionContext` covers both
// shapes: in an aggregate view the owner comes from the email's
@@ -4381,54 +4393,56 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// Only the keywords that differ from the row are written (see
// patchEmailKeywords): replacing the whole set from the row undid what
// another client had set since, such as `$seen` or `$answered`.
const previous = (get().emails.find(e => e.id === emailId) ?? get().selectedEmail)?.keywords ?? {};
const previous = (get().emails.find(isTarget) ?? get().selectedEmail)?.keywords ?? {};
const changes: Record<string, boolean> = {};
for (const key of new Set([...Object.keys(previous), ...Object.keys(keywords)])) {
if (!!previous[key] !== !!keywords[key]) changes[key] = !!keywords[key];
}
return await get().patchEmailKeywords(client, emailId, changes);
return await get().patchEmailKeywords(client, emailId, changes, emailKey);
},
patchEmailKeywords: async (client, emailId, changes): Promise<boolean> => {
patchEmailKeywords: async (client, emailId, changes, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
const keys = Object.keys(changes);
if (keys.length === 0) return true;
if (emailHooks.onBeforeEmailKeywordChange.size > 0
&& !(await emailHooks.onBeforeEmailKeywordChange.intercept({ emailIds: [emailId], ...keywordChangeLists(changes) }))) {
return false;
}
const { client: actionClient, accountId } = resolveKeywordActionContext(emailId, client);
const { client: actionClient, accountId } = resolveKeywordActionContext(emailId, client, emailKey);
// `keywords/<name>` pointers touch only these keywords on the server;
// a cleared keyword is removed (null), since a keyword map holds `true`
// values only.
const patch = Object.fromEntries(keys.map(key => [keywordPointer(key), changes[key] ? true : null]));
await actionClient.batchUpdateKeywords([emailId], patch, accountId);
const previous = (get().emails.find(e => e.id === emailId) ?? get().selectedEmail)?.keywords ?? {};
const previous = (get().emails.find(isTarget) ?? get().selectedEmail)?.keywords ?? {};
const next: Record<string, boolean> = { ...previous };
for (const key of keys) {
if (changes[key]) next[key] = true;
else delete next[key];
}
get().setEmailKeywordsLocal(emailId, next);
get().setEmailKeywordsLocal(emailId, next, emailKey);
void emailHooks.onEmailKeywordChange.emit({ emailIds: [emailId], ...keywordChangeLists(changes) });
refillAfterKeywordChange(get, client, keys.filter(k => !!previous[k] !== !!changes[k]));
return true;
},
setEmailKeywordsLocal: (emailId, keywords) => {
setEmailKeywordsLocal: (emailId, keywords, emailKey) => {
const isTarget = singleEmailMatcher(emailId, emailKey);
set((state) => {
// This patch replaces the whole keyword map, so it can flip $seen as well
// as labels. Only a genuine read-state change moves the tag unread counts.
const previous = state.emails.find(e => e.id === emailId) ?? state.selectedEmail;
const previous = state.emails.find(isTarget) ?? state.selectedEmail;
const wasRead = previous?.keywords?.$seen ?? false;
const isRead = keywords.$seen ?? false;
const delta = wasRead === isRead ? 0 : (isRead ? -1 : 1);
return {
emails: state.emails.map(e =>
e.id === emailId ? { ...e, keywords: { ...keywords } } : e
isTarget(e) ? { ...e, keywords: { ...keywords } } : e
),
selectedEmail: state.selectedEmail?.id === emailId
selectedEmail: (!!state.selectedEmail && isTarget(state.selectedEmail))
? { ...state.selectedEmail, keywords: { ...keywords } }
: state.selectedEmail,
tagCounts: applyTagCountReadDelta(state.tagCounts, [{ keywords, delta }]),
@@ -5127,14 +5141,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
},
// Spam operations
markAsSpam: async (client, emailId): Promise<boolean> => {
markAsSpam: async (client, emailId, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
// The viewer may show an email the list no longer holds (e.g. the unread
// quick-filter dropped it once it was read), so fall back to the selected
// email like moveThreadToMailbox does. Throw rather than no-op so the
// caller's success toast cannot fire for a message that was not moved. (#695)
const state = get();
const email = state.emails.find(e => e.id === emailId)
?? (state.selectedEmail?.id === emailId ? state.selectedEmail : null);
const email = state.emails.find(isTarget)
?? ((!!state.selectedEmail && isTarget(state.selectedEmail)) ? state.selectedEmail : null);
if (!email) {
throw new Error('Email not found - cannot mark as spam');
}
@@ -5159,7 +5174,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// still move the message, and undo falls back to the inbox. Returning
// here let the caller show a success toast for a message never moved.
if (currentMailbox) {
get().spamUndoCache.set(emailId, {
get().spamUndoCache.set(emailKey ?? emailId, {
emailId,
originalMailboxId: currentMailbox.originalId || currentMailbox.id,
accountId,
@@ -5205,8 +5220,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
});
return {
emails: state.emails.filter(e => e.id !== emailId),
selectedEmail: getNextSelectedEmail(state, emailId),
emails: state.emails.filter(e => !isTarget(e)),
selectedEmail: getNextSelectedEmailAfterRemovingRow(state, isTarget),
...mailboxPatch,
};
});
@@ -5218,7 +5233,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
return true;
},
undoSpam: async (client, emailId): Promise<boolean> => {
undoSpam: async (client, emailId, emailKey): Promise<boolean> => {
const isTarget = singleEmailMatcher(emailId, emailKey);
if (emailHooks.onBeforeEmailSpamToggle.size > 0
&& !(await emailHooks.onBeforeEmailSpamToggle.intercept({ emailIds: [emailId], spam: false }))) {
return false;
@@ -5227,7 +5243,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
const mailboxes = resolveActionMailboxes();
// Try cache first (preserves exact original mailbox for toast undo)
const cachedData = get().spamUndoCache.get(emailId);
const cachedData = get().spamUndoCache.get(emailKey ?? emailId);
let targetMailboxId: string;
let accountId: string | undefined;
@@ -5242,13 +5258,13 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
if (cachedData.sourceClientAccountId) {
undoClient = useAuthStore.getState().getClientForAccount(cachedData.sourceClientAccountId) ?? undoClient;
}
get().spamUndoCache.delete(emailId);
get().spamUndoCache.delete(emailKey ?? emailId);
} else {
// Generic "not spam" (button/menu, no undo cache). In aggregate views
// (e.g. "All Junk") route to the email's own account so shared/group
// messages move back to *their* inbox via *their* client, not the active
// account's. (#281)
const listEmail = get().emails.find(e => e.id === emailId);
const listEmail = get().emails.find(isTarget);
let inboxMailboxes = mailboxes;
if (listEmail?.sourceClientAccountId && listEmail?.sourceAccountId) {
undoClient = useAuthStore.getState().getClientForAccount(listEmail.sourceClientAccountId) ?? undoClient;
@@ -5279,8 +5295,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// message to the next one, mirroring markAsSpam. Without this the viewer
// stays stuck on the mail that just left the folder.
set(state => ({
emails: state.emails.filter(e => e.id !== emailId),
selectedEmail: getNextSelectedEmail(state, emailId),
emails: state.emails.filter(e => !isTarget(e)),
selectedEmail: getNextSelectedEmailAfterRemovingRow(state, isTarget),
}));
void emailHooks.onEmailSpamToggle.emit({ emailIds: [emailId], spam: false });