diff --git a/App.tsx b/App.tsx index 23c2676b..17772b27 100644 --- a/App.tsx +++ b/App.tsx @@ -45,6 +45,7 @@ import { useAuthStore } from './src/stores/auth-store'; import { useCalendarStore } from './src/stores/calendar-store'; import { useContactsStore } from './src/stores/contacts-store'; import { useEmailStore } from './src/stores/email-store'; +import { mailboxAccountId } from './src/lib/mailbox-tree'; import { useHasCalendar, useHasContacts, useHasFiles } from './src/lib/capabilities'; import { useSettingsStore } from './src/stores/settings-store'; import { useLocaleStore } from './src/stores/locale-store'; @@ -180,10 +181,14 @@ function MainTabsNavigator({ navigation }: NativeStackScreenProps navigation.navigate('Compose')} onEmailPress={(email) => { + // The list holds the open folder's mail: name its account so the + // viewer never has to guess it from whatever folder is open. + const { mailboxes: all, currentMailboxId } = useEmailStore.getState(); navigation.navigate('EmailThread', { emailId: email.id, threadId: email.threadId, subject: email.subject, + jmapAccountId: mailboxAccountId(all, currentMailboxId), }); }} /> diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index 9f448392..1a39f9b8 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -15,7 +15,7 @@ import { computeReplyThreadingHeaders } from '../../lib/email-threading'; import { findReceivingIdentity } from '../../lib/email-headers'; import { pickEmailBody, plainTextBody } from '../../lib/email-body'; import { htmlToPlainText } from '../../lib/compose-html'; -import { mailboxesForSiblingOf } from '../../lib/mailbox-tree'; +import { mailboxesOfAccount } from '../../lib/mailbox-tree'; import { emailDisplayDate, formatFullDateTime } from '../../lib/email-date'; interface Props { @@ -41,7 +41,6 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent }: P const identities = useSettingsStore((s) => s.identities); const sendDelaySeconds = useSettingsStore((s) => s.sendDelaySeconds); const mailboxes = useEmailStore((s) => s.mailboxes); - const currentMailboxId = useEmailStore((s) => s.currentMailboxId); const [text, setText] = React.useState(''); const [sending, setSending] = React.useState(false); @@ -58,7 +57,8 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent }: P Alert.alert(t('common.error', 'Error'), t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available')); return; } - const scoped = mailboxesForSiblingOf(mailboxes, currentMailboxId); + // Sent/Drafts of the account the reply is submitted from: the message's. + const scoped = mailboxesOfAccount(mailboxes, jmapAccountId); const sent = scoped.find((m) => m.role === 'sent'); const drafts = scoped.find((m) => m.role === 'drafts'); if (!sent) { diff --git a/src/components/email/ReadReceiptBanner.tsx b/src/components/email/ReadReceiptBanner.tsx index 417016eb..7c4b7d7b 100644 --- a/src/components/email/ReadReceiptBanner.tsx +++ b/src/components/email/ReadReceiptBanner.tsx @@ -10,7 +10,7 @@ import { useEmailStore } from '../../stores/email-store'; import { sendReadReceipt, patchKeywordsForEmails } from '../../api/email'; import { jmapClient } from '../../api/jmap-client'; import { findReceivingIdentity } from '../../lib/email-headers'; -import { mailboxesForSiblingOf } from '../../lib/mailbox-tree'; +import { mailboxesOfAccount } from '../../lib/mailbox-tree'; interface Props { email: Email; @@ -44,7 +44,6 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa const fetchIdentities = useSettingsStore((s) => s.fetchIdentities); const readReceiptResponse = useSettingsStore((s) => s.readReceiptResponse); const mailboxes = useEmailStore((s) => s.mailboxes); - const currentMailboxId = useEmailStore((s) => s.currentMailboxId); const [busy, setBusy] = React.useState(false); const [handledLocally, setHandledLocally] = React.useState(false); @@ -71,7 +70,8 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa const send = React.useCallback(async (automatic: boolean) => { if (!identity) return; - const scoped = mailboxesForSiblingOf(mailboxes, currentMailboxId); + // Sent of the account the receipt is submitted from: the message's. + const scoped = mailboxesOfAccount(mailboxes, jmapAccountId); const sent = scoped.find((m) => m.role === 'sent'); if (!sent) throw new Error(t('email_composer.no_sent_folder', 'No Sent folder found')); await sendReadReceipt({ @@ -93,7 +93,7 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa ), }); await flagSent(); - }, [identity, mailboxes, currentMailboxId, requestedBy, jmapAccountId, email.messageId, email.subject, t, flagSent]); + }, [identity, mailboxes, requestedBy, jmapAccountId, email.messageId, email.subject, t, flagSent]); // "always" mode: auto-send once when the message is actually displayed. React.useEffect(() => { diff --git a/src/components/email/UnsubscribeBanner.tsx b/src/components/email/UnsubscribeBanner.tsx index 66c87aa0..473c23eb 100644 --- a/src/components/email/UnsubscribeBanner.tsx +++ b/src/components/email/UnsubscribeBanner.tsx @@ -13,7 +13,7 @@ import { sendEmail } from '../../api/email'; import { isValidUnsubscribeUrl, parseMailtoUrl, isOneClickUnsubscribe } from '../../lib/unsubscribe'; import type { ListHeaders } from '../../lib/email-headers'; import { findReceivingIdentity } from '../../lib/email-headers'; -import { mailboxesForSiblingOf } from '../../lib/mailbox-tree'; +import { mailboxesOfAccount } from '../../lib/mailbox-tree'; const DISMISSED_KEY = 'webmail:unsubscribe-dismissed:v1'; const dismissed = new Set(); @@ -57,7 +57,6 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr const t = useLocaleStore((s) => s.t); const identities = useSettingsStore((s) => s.identities); const mailboxes = useEmailStore((s) => s.mailboxes); - const currentMailboxId = useEmailStore((s) => s.currentMailboxId); const [hidden, setHidden] = React.useState(true); const [state, setState] = React.useState<'idle' | 'busy' | 'done' | 'error'>('idle'); @@ -102,7 +101,8 @@ export function UnsubscribeBanner({ email, list, messageKey, jmapAccountId }: Pr if (!fields) throw new Error('invalid mailto'); const identity = findReceivingIdentity(identities, email) ?? identities[0]; if (!identity) throw new Error(t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available')); - const scoped = mailboxesForSiblingOf(mailboxes, currentMailboxId); + // Sent/Drafts of the account the mail is submitted from: the message's. + const scoped = mailboxesOfAccount(mailboxes, jmapAccountId); const sent = scoped.find((m) => m.role === 'sent'); const drafts = scoped.find((m) => m.role === 'drafts'); if (!sent) throw new Error(t('email_composer.no_sent_folder', 'No Sent folder found')); diff --git a/src/lib/__tests__/mailbox-tree.test.ts b/src/lib/__tests__/mailbox-tree.test.ts index d491c9e4..cf41fa3e 100644 --- a/src/lib/__tests__/mailbox-tree.test.ts +++ b/src/lib/__tests__/mailbox-tree.test.ts @@ -2,7 +2,10 @@ import { describe, it, expect } from 'vitest'; import { buildMailboxTree, flattenVisible, + mailboxAccountId, mailboxesForSiblingOf, + mailboxesOfAccount, + mailboxOfEmail, ownMailboxes, SHARED_ACCOUNT_NODE_PREFIX, } from '../mailbox-tree'; @@ -139,6 +142,29 @@ describe('account scoping helpers', () => { expect(mailboxesForSiblingOf(all, 'gone').map((m) => m.id)).toEqual(['inbox', 'trash']); expect(mailboxesForSiblingOf(all, null).map((m) => m.id)).toEqual(['inbox', 'trash']); }); + + it('names a folder\'s account: the owner when shared, none for the user\'s own', () => { + expect(mailboxAccountId(all, 'grp-1:inbox')).toBe('grp-1'); + expect(mailboxAccountId(all, 'inbox')).toBeUndefined(); + expect(mailboxAccountId(all, null)).toBeUndefined(); + }); + + it('scopes to a message\'s account whatever folder is open (B3)', () => { + expect(mailboxesOfAccount(all, undefined).map((m) => m.id)).toEqual(['inbox', 'trash']); + expect(mailboxesOfAccount(all, 'grp-1').map((m) => m.id)).toEqual(['grp-1:inbox', 'grp-1:trash']); + // An account whose folders are not loaded has none: nothing to guess from. + expect(mailboxesOfAccount(all, 'grp-9')).toEqual([]); + }); + + it('finds the folder a message is filed in, preferring the one it was opened from', () => { + const grp = mailboxesOfAccount(all, 'grp-1'); + // Raw ids: the team's "inbox" is not the user's own inbox. + expect(mailboxOfEmail(grp, { inbox: true })?.id).toBe('grp-1:inbox'); + expect(mailboxOfEmail(grp, { inbox: true, trash: true }, 'grp-1:trash')?.id).toBe('grp-1:trash'); + expect(mailboxOfEmail(grp, { inbox: true }, 'inbox')?.id).toBe('grp-1:inbox'); + expect(mailboxOfEmail(grp, { elsewhere: true })).toBeUndefined(); + expect(mailboxOfEmail(grp, undefined)).toBeUndefined(); + }); }); describe('role-folder deduplication (#771)', () => { diff --git a/src/lib/mailbox-tree.ts b/src/lib/mailbox-tree.ts index bf00cd94..da7b89e8 100644 --- a/src/lib/mailbox-tree.ts +++ b/src/lib/mailbox-tree.ts @@ -32,6 +32,41 @@ export function mailboxesForSiblingOf(mailboxes: Mailbox[], mailboxId: string | return mailboxes.filter((m) => m.isShared && m.accountId === current.accountId); } +/** The JMAP account a folder lives in: its owner when shared, undefined for the user's own. */ +export function mailboxAccountId(mailboxes: Mailbox[], mailboxId: string | null): string | undefined { + const mailbox = mailboxId ? mailboxes.find((m) => m.id === mailboxId) : undefined; + return mailbox?.isShared ? mailbox.accountId : undefined; +} + +/** + * Folders of one JMAP account: the user's own for `undefined`, else that + * shared account's. A message's role folders (trash, archive, junk…) must + * come from the account it lives in, not from whatever folder is open — + * ids are only unique per account. Empty when the account's folders are not + * loaded, so callers refuse instead of guessing. + */ +export function mailboxesOfAccount(mailboxes: Mailbox[], accountId: string | undefined): Mailbox[] { + if (!accountId) return ownMailboxes(mailboxes); + return mailboxes.filter((m) => m.isShared && m.accountId === accountId); +} + +/** + * The folder a message is filed in among one account's `mailboxes`: the + * preferred one (the folder it was opened from) when the message is in it, + * otherwise the first of its folders that is known. `mailboxIds` comes from + * the server unprefixed, so it is matched on each folder's raw id. + */ +export function mailboxOfEmail( + mailboxes: Mailbox[], + mailboxIds: Record | undefined, + preferredId?: string | null, +): Mailbox | undefined { + const holds = (m: Mailbox) => !!mailboxIds?.[m.originalId ?? m.id]; + const preferred = preferredId ? mailboxes.find((m) => m.id === preferredId) : undefined; + if (preferred && holds(preferred)) return preferred; + return mailboxes.find(holds); +} + // Matches `ROLE_PRIORITY` from [lib/utils.ts] in the webmail. const ROLE_PRIORITY: Record = { inbox: 0, diff --git a/src/navigation/types.ts b/src/navigation/types.ts index d2b12da8..3ed53acb 100644 --- a/src/navigation/types.ts +++ b/src/navigation/types.ts @@ -55,6 +55,11 @@ export type RootStackParamList = { emailId: string; threadId: string; subject?: string; + /** + * JMAP account the message lives in; unset for the user's own mail. The + * opener names it: the viewer never derives it from the open folder, + * whose account may hold another message under the same id. + */ jmapAccountId?: string; /** * Ids to page over when the message was opened from a list other than diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index 2f31dee6..51f0941c 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -31,7 +31,7 @@ import { shareEmailEml } from '../lib/email-export'; import { useKeywordsStore, keywordToken, type KeywordDef } from '../stores/keywords-store'; import { useSheetDrag } from '../lib/use-sheet-drag'; import { useLocaleStore } from '../stores/locale-store'; -import { findTrashMailbox, mailboxesForSiblingOf } from '../lib/mailbox-tree'; +import { findTrashMailbox, mailboxAccountId, mailboxesOfAccount, mailboxOfEmail } from '../lib/mailbox-tree'; import { pickEmailBody, plainTextBody } from '../lib/email-body'; import { singleLine } from '../lib/single-line'; import { buildForwardAsAttachmentPayload } from '../lib/forward-as-attachment'; @@ -44,7 +44,11 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const c = useColors(); const styles = React.useMemo(() => makeStyles(c), [c]); const { t } = useLocaleStore(); - const { jmapAccountId } = route.params; + // The JMAP account the message lives in, as the screen that opened it named + // it (undefined = the user's own). Never the open folder's: that may be + // another account holding a different message under the same id (B3). + // Fixed at open, since a later navigate() here only swaps route.params. + const [ownerAccountId] = React.useState(route.params.jmapAccountId); // The displayed email is tracked in local state (not a route param) so that // swiping / Prev-Next can switch messages in place without remounting the // screen — which is what produced the loading flash on every change. @@ -72,6 +76,13 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const resolvedTheme = useResolvedTheme(); React.useEffect(() => { if (identities.length === 0) void fetchIdentities(); }, [identities.length, fetchIdentities]); + // The account whose mail the store's list holds (the open folder's). Its + // rows only stand for this message when that is the message's account. + const listAccountId = React.useMemo( + () => mailboxAccountId(mailboxes, currentMailboxId), + [mailboxes, currentMailboxId], + ); + // The list the pager pages over. A message opened from the active folder // pages over that folder (collapsed to one page per thread when threading // is on, the opened message standing in for its thread); one opened from @@ -81,17 +92,18 @@ export default function EmailThreadScreen({ route, navigation }: Props) { // pager would render `emails[0]` while the toolbar acted on the tapped one. const emails = React.useMemo(() => { const { emailIds, emailId, threadId } = route.params; + const list = listAccountId === ownerAccountId ? storeEmails : []; if (emailIds && emailIds.length > 0) { - const byId = jmapAccountId ? null : new Map(storeEmails.map((e) => [e.id, e])); - return emailIds.map((id) => byId?.get(id) ?? ({ id, threadId } as Email)); + const byId = new Map(list.map((e) => [e.id, e])); + return emailIds.map((id) => byId.get(id) ?? ({ id, threadId } as Email)); } - const opened = jmapAccountId ? undefined : storeEmails.find((e) => e.id === emailId); + const opened = list.find((e) => e.id === emailId); if (opened) { - if (disableThreading) return storeEmails; + if (disableThreading) return list; const openedKey = opened.threadId || opened.id; const seen = new Set(); const out: Email[] = []; - for (const e of storeEmails) { + for (const e of list) { const key = e.threadId || e.id; if (seen.has(key)) continue; seen.add(key); @@ -100,18 +112,8 @@ export default function EmailThreadScreen({ route, navigation }: Props) { return out; } return [{ id: emailId, threadId } as Email]; - }, [storeEmails, route.params, jmapAccountId, disableThreading]); + }, [storeEmails, route.params, listAccountId, ownerAccountId, disableThreading]); - // The JMAP account the open message belongs to: the route param when the - // unified inbox opened a group message, otherwise the account behind the - // folder it was opened from. Undefined means the user's own mail. - const ownerAccountId = React.useMemo(() => { - if (jmapAccountId) return jmapAccountId; - const current = currentMailboxId - ? mailboxes.find((m) => m.id === currentMailboxId) - : undefined; - return current?.isShared ? current.accountId : undefined; - }, [jmapAccountId, mailboxes, currentMailboxId]); const currentMailboxRole = React.useMemo( () => (currentMailboxId ? mailboxes.find((m) => m.id === currentMailboxId)?.role ?? null : null), [mailboxes, currentMailboxId], @@ -250,13 +252,23 @@ export default function EmailThreadScreen({ route, navigation }: Props) { bumpCache(); }, [detailCache, bumpCache]); + // Mark read through the store (its list row and offline queue) only when + // the list holds the message's account; the store would otherwise read the + // id as a row of the open folder's account (B3), so go to the server. + const markSeen = React.useCallback( + (id: string) => (listAccountId === ownerAccountId + ? markRead(id, ownerAccountId) + : patchKeywordsForEmails([id], { $seen: true }, ownerAccountId)), + [listAccountId, ownerAccountId, markRead], + ); + // Mark a message read per the user's delay setting: -1 never, 0 instantly, // >0 after that many milliseconds. Returns a cancel function. const scheduleMarkRead = React.useCallback((target: Email): (() => void) => { if (target.keywords?.$seen || markAsReadDelay === -1) return () => undefined; const apply = () => { updateLocalKeywords(target.id, { ...target.keywords, $seen: true }); - void markRead(target.id, ownerAccountId); + void markSeen(target.id); }; if (markAsReadDelay > 0) { const timer = setTimeout(apply, markAsReadDelay); @@ -264,7 +276,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { } apply(); return () => undefined; - }, [markAsReadDelay, markRead, ownerAccountId, updateLocalKeywords]); + }, [markAsReadDelay, markSeen, updateLocalKeywords]); React.useEffect(() => { let cancelled = false; @@ -290,11 +302,12 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const starred = !!email?.keywords?.$flagged; const unread = !!email && !email.keywords?.$seen; - // Move/archive/spam targets have to live in the same account as the folder - // the message was opened from. + // Move/archive/spam/trash targets have to live in the message's own account, + // whatever folder is open (B3). None are found while that account's folders + // are unknown, and the actions then refuse rather than guess. const scopedMailboxes = React.useMemo( - () => mailboxesForSiblingOf(mailboxes, currentMailboxId), - [mailboxes, currentMailboxId], + () => mailboxesOfAccount(mailboxes, ownerAccountId), + [mailboxes, ownerAccountId], ); const archiveMailbox = React.useMemo( () => scopedMailboxes.find((m) => m.role === 'archive'), @@ -312,7 +325,17 @@ export default function EmailThreadScreen({ route, navigation }: Props) { // folder's raw id rather than the sidebar key. const isInJunk = !!(junkMailbox && email?.mailboxIds?.[junkMailbox.originalId ?? junkMailbox.id]); const trashMailbox = React.useMemo(() => findTrashMailbox(scopedMailboxes), [scopedMailboxes]); - const isInTrash = !!(trashMailbox && currentMailboxId === trashMailbox.id); + // The folder the message is filed in (the one it was opened from, when + // that is one of its own): what delete and move take it out of. + const sourceMailbox = React.useMemo( + () => mailboxOfEmail(scopedMailboxes, email?.mailboxIds, currentMailboxId), + [scopedMailboxes, email, currentMailboxId], + ); + const isInTrash = !!(trashMailbox && sourceMailbox?.id === trashMailbox.id); + // Archive still resolves its folder from the open one, so it is only + // offered for mail of that account. + const canArchive = !!archiveMailbox && listAccountId === ownerAccountId + && sourceMailbox?.id !== archiveMailbox.id; const onToggleKeyword = (token: string) => { if (!email) return; @@ -338,7 +361,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const onToggleUnread = () => { if (!email) return; if (unread) { - void markRead(email.id, ownerAccountId); + void markSeen(email.id); updateLocalKeywords(email.id, { ...email.keywords, $seen: true }); } else { const next = { ...email.keywords }; @@ -354,14 +377,14 @@ export default function EmailThreadScreen({ route, navigation }: Props) { }, [detailCache, bumpCache]); const performDelete = () => { - if (!email || !currentMailboxId || !trashMailbox) return; - void deleteEmail(email.id, trashMailbox.id, currentMailboxId); + if (!email || !sourceMailbox || !trashMailbox) return; + void deleteEmail(email.id, trashMailbox.id, sourceMailbox.id, { email, accountId: ownerAccountId }); navigation.goBack(); }; const onDelete = () => { - if (!email || !currentMailboxId) return; - if (!trashMailbox) { + if (!email) return; + if (!trashMailbox || !sourceMailbox) { Alert.alert( t('email_list.error', 'Error'), t('email_list.no_trash_folder', 'Could not find a Trash folder on the server. Please check your mailbox configuration.'), @@ -387,31 +410,31 @@ export default function EmailThreadScreen({ route, navigation }: Props) { }; const onArchive = () => { - if (!email || !currentMailboxId || !archiveMailbox) return; - if (currentMailboxId === archiveMailbox.id) return; + if (!email || !canArchive) return; void archiveEmailAction(email.id); navigation.goBack(); }; const onToggleSpam = () => { - if (!email || !currentMailboxId) return; + if (!email || !sourceMailbox) return; setMoreMenuOpen(false); + const viewed = { email, accountId: ownerAccountId }; if (isInJunk) { - const target = inboxMailbox ?? scopedMailboxes.find((m) => m.id !== currentMailboxId); + const target = inboxMailbox ?? scopedMailboxes.find((m) => m.id !== sourceMailbox.id); if (!target) return; - void moveToMailbox(email.id, currentMailboxId, target.id); + void moveToMailbox(email.id, sourceMailbox.id, target.id, viewed); } else { if (!junkMailbox) return; - void moveToMailbox(email.id, currentMailboxId, junkMailbox.id); + void moveToMailbox(email.id, sourceMailbox.id, junkMailbox.id, viewed); } navigation.goBack(); }; const onMoveToMailbox = (toId: string) => { - if (!email || !currentMailboxId || toId === currentMailboxId) return; + if (!email || !sourceMailbox || toId === sourceMailbox.id) return; setMoveMenuOpen(false); setMoreMenuOpen(false); - void moveToMailbox(email.id, currentMailboxId, toId); + void moveToMailbox(email.id, sourceMailbox.id, toId, { email, accountId: ownerAccountId }); navigation.goBack(); }; @@ -512,7 +535,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { label: t('email_viewer.archive', 'Archive'), icon: (s, col) => , onPress: onArchive, - available: !!archiveMailbox && currentMailboxId !== archiveMailbox?.id, + available: canArchive, }, markUnread: { label: unread ? t('email_viewer.read', 'Read') : t('email_viewer.unread', 'Unread'), @@ -532,14 +555,14 @@ export default function EmailThreadScreen({ route, navigation }: Props) { label: t('email_viewer.move', 'Move'), icon: (s, col) => , onPress: () => setMoveMenuOpen(true), - available: mailboxes.length > 0, + available: !!sourceMailbox, }, spam: { label: isInJunk ? t('email_viewer.not_spam_short', 'Not spam') : t('email_viewer.spam_short', 'Spam'), icon: (s, col) => isInJunk ? : , onPress: onToggleSpam, - available: !!junkMailbox || isInJunk, + available: !!junkMailbox && !!sourceMailbox, }, tag: { label: t('email_viewer.tag', 'Tag'), @@ -552,7 +575,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const bottomBarHeight = 60 + Math.max(insets.bottom, 4); // Drop optional toolbar buttons on narrow screens. const showMarkUnread = windowWidth >= 340; - const showArchive = windowWidth >= 400 && !!archiveMailbox; + const showArchive = windowWidth >= 400 && canArchive; // Current rendering mode of the active message, for the More sheet label. const activeRenderDark = email @@ -713,10 +736,10 @@ export default function EmailThreadScreen({ route, navigation }: Props) { visible={moreMenuOpen} onClose={() => setMoreMenuOpen(false)} unread={unread} - canArchive={!!archiveMailbox && currentMailboxId !== archiveMailbox?.id} + canArchive={canArchive} canMarkUnread={true} - canMove={mailboxes.length > 0} - showSpam={!!junkMailbox || isInJunk} + canMove={!!sourceMailbox} + showSpam={!!junkMailbox && !!sourceMailbox} isInJunk={isInJunk} canViewSource={!!email?.blobId} canExport={!!email?.blobId} @@ -770,7 +793,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { visible={moveMenuOpen} onClose={() => setMoveMenuOpen(false)} mailboxes={scopedMailboxes} - currentMailboxId={currentMailboxId} + currentMailboxId={sourceMailbox?.id ?? null} onPick={onMoveToMailbox} /> diff --git a/src/stores/__tests__/email-store-viewer-actions.test.ts b/src/stores/__tests__/email-store-viewer-actions.test.ts new file mode 100644 index 00000000..17432f64 --- /dev/null +++ b/src/stores/__tests__/email-store-viewer-actions.test.ts @@ -0,0 +1,190 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Actions the viewer runs on the message it shows, which may live in another +// account than the open folder (unified inbox, notification, deep link). +// Stalwart ids are only unique per account, so the fixtures reuse the same +// message id ("e1") and raw folder ids ("a" = Inbox, "t" = Trash) in the +// user's own account and in the group account "grp-1". + +vi.mock('../../api/email', () => ({ + getMailboxes: vi.fn(), + getMailboxesWithState: vi.fn(async () => ({ list: [], state: 'mb-state-0' })), + getSharedMailboxes: vi.fn(async () => []), + getMailboxesByIds: vi.fn(async () => ({ list: [], state: 'mb-state-0' })), + getMailboxChanges: vi.fn(async () => null), + queryEmails: vi.fn(), + getEmailQueryChanges: vi.fn(async () => null), + getEmails: vi.fn(), + getEmailsWithState: vi.fn(async () => ({ list: [], state: 'em-state-0' })), + getEmailChanges: vi.fn(async () => null), + getFullEmail: vi.fn(), + importEmailBlob: vi.fn(), + patchKeywordsForEmails: vi.fn(), + patchKeywordsPerEmail: vi.fn(), + moveEmail: vi.fn(), + moveEmails: vi.fn(), + archiveEmails: vi.fn(), + restoreEmailMailboxes: vi.fn(), + setEmailMailboxes: vi.fn(), + destroyEmails: vi.fn(), + deleteEmail: vi.fn(), + deleteEmails: vi.fn(), + searchEmails: vi.fn(), + markAsSpam: vi.fn(), + undoSpam: vi.fn(), + unprefixMailboxId: (id: string, accountId?: string) => + (accountId && id.startsWith(`${accountId}:`) ? id.slice(accountId.length + 1) : id), +})); + +vi.mock('../locale-store', () => ({ + t: (_key: string, fallback?: string) => fallback ?? _key, + useLocaleStore: { getState: () => ({ locale: 'en', t: (_k: string, f?: string) => f ?? _k }) }, +})); + +// Online with nothing queued: run the online runner straight away. +vi.mock('../outbox-store', () => { + const applyOrQueueBatch = async (_ops: unknown[], onlineRun?: () => Promise) => { + if (onlineRun) await onlineRun(); + return { queued: false }; + }; + return { + applyOrQueueBatch, + applyOrQueue: async (op: unknown, onlineRun?: () => Promise) => applyOrQueueBatch([op], onlineRun), + useOutboxStore: { + getState: () => ({ entries: [], count: () => 0, setAccount: vi.fn(async () => undefined), flush: vi.fn() }), + }, + }; +}); + +vi.mock('../settings-store', () => { + const settings: Record = { archiveMode: 'single', deleteAction: 'trash' }; + return { + useSettingsStore: { + getState: () => ({ + ...settings, + updateSetting: (key: string, value: unknown) => { settings[key] = value; }, + }), + }, + }; +}); + +vi.mock('../offline-cache-store', () => ({ + useOfflineCacheStore: { + getState: () => ({ + has: () => false, + get: vi.fn(async () => null), + put: vi.fn(async () => undefined), + patch: vi.fn(async () => undefined), + remove: vi.fn(async () => undefined), + }), + }, +})); + +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { + isConnected: true, + accountId: 'acc-1', + username: 'test@example.com', + serverUrl: 'https://mail.example.com', + getMaxObjectsInGet: () => 500, + getMaxCallsInRequest: () => 16, + getSharedMailAccounts: () => [{ id: 'grp-1', name: 'Support' }], + request: vi.fn(), + }, +})); + +import * as emailApi from '../../api/email'; +import { useEmailStore, type ViewedEmail } from '../email-store'; +import type { Email, Mailbox } from '../../api/types'; + +const mockDeleteEmail = emailApi.deleteEmail as ReturnType; +const mockMoveEmail = emailApi.moveEmail as ReturnType; +const mockRestore = emailApi.restoreEmailMailboxes as ReturnType; + +function own(id: string, role: string): Mailbox { + return { id, name: role, role, accountId: 'acc-1', isShared: false } as Mailbox; +} +function team(rawId: string, role: string): Mailbox { + return { id: `grp-1:${rawId}`, originalId: rawId, name: role, role, accountId: 'grp-1', isShared: true } as Mailbox; +} + +const MAILBOXES = [ + own('a', 'inbox'), own('t', 'trash'), own('x', 'archive'), + team('a', 'inbox'), team('t', 'trash'), team('x', 'archive'), +]; +// The user's own "Security audit #18" and the team's message share the id. +const OWN_ROW = { id: 'e1', subject: 'Security audit #18', keywords: {}, mailboxIds: { a: true } } as unknown as Email; +const TEAM_MESSAGE = { id: 'e1', subject: 'Team message', keywords: {}, mailboxIds: { a: true } } as unknown as Email; +const TEAM_VIEWED: ViewedEmail = { email: TEAM_MESSAGE, accountId: 'grp-1' }; + +beforeEach(() => { + vi.clearAllMocks(); + useEmailStore.getState().reset(); +}); + +describe('viewer actions on another account\'s message (B3)', () => { + it('trashes a team message in the team account and leaves the open inbox\'s same-id row alone', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await useEmailStore.getState().deleteEmail('e1', 'grp-1:t', 'grp-1:a', TEAM_VIEWED); + + expect(mockDeleteEmail).toHaveBeenCalledWith('e1', 't', 'a', 'grp-1'); + expect(useEmailStore.getState().emails).toEqual([OWN_ROW]); + const undo = useEmailStore.getState().pendingUndo!; + expect(undo.accountId).toBe('grp-1'); + expect(undo.items[0].email.subject).toBe('Team message'); + + // Undo restores it in the team account, and does not slot it into the + // user's own Inbox although its raw folder id matches. + await useEmailStore.getState().undoLast(); + expect(mockRestore).toHaveBeenCalledWith([{ id: 'e1', mailboxIds: { a: true } }], 'grp-1'); + expect(useEmailStore.getState().emails).toEqual([OWN_ROW]); + }); + + it('trashes an own message in the own account while a team folder is open', async () => { + const teamRow = { ...TEAM_MESSAGE }; + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'grp-1:a', emails: [teamRow] }); + + await useEmailStore.getState().deleteEmail('e1', 't', 'a', { email: OWN_ROW, accountId: undefined }); + + expect(mockDeleteEmail).toHaveBeenCalledWith('e1', 't', 'a', undefined); + expect(useEmailStore.getState().emails).toEqual([teamRow]); + }); + + it('refuses to trash a team message into another account\'s Trash', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await expect(useEmailStore.getState().deleteEmail('e1', 't', 'grp-1:a', TEAM_VIEWED)) + .rejects.toThrow(/same account/); + + expect(mockDeleteEmail).not.toHaveBeenCalled(); + expect(useEmailStore.getState().emails).toEqual([OWN_ROW]); + }); + + it('moves a team message within the team account', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await useEmailStore.getState().moveToMailbox('e1', 'grp-1:a', 'grp-1:x', TEAM_VIEWED); + + expect(mockMoveEmail).toHaveBeenCalledWith('e1', 'a', 'x', 'grp-1'); + expect(useEmailStore.getState().emails).toEqual([OWN_ROW]); + }); + + it('refuses to move a team message into the user\'s own folders', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await expect(useEmailStore.getState().moveToMailbox('e1', 'grp-1:a', 'x', TEAM_VIEWED)) + .rejects.toThrow(/same account/); + + expect(mockMoveEmail).not.toHaveBeenCalled(); + }); + + it('still drops the row when the viewer acts on a message of the open folder\'s account', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await useEmailStore.getState().deleteEmail('e1', 't', 'a', { email: OWN_ROW, accountId: undefined }); + + expect(mockDeleteEmail).toHaveBeenCalledWith('e1', 't', 'a', undefined); + expect(useEmailStore.getState().emails).toEqual([]); + }); +}); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index cb4914b6..0d73b8c4 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -132,6 +132,38 @@ function currentAccountId(state: EmailState): string | undefined { return refFor(state.mailboxes, state.currentMailboxId).accountId; } +/** + * A message acted on from the viewer: the copy the viewer holds and the JMAP + * account it lives in (undefined = the user's own). The loaded list only + * holds the open folder's account and ids are only unique per account, so a + * row with the same id is another message unless the accounts match (B3). + */ +export interface ViewedEmail { + email: Email; + accountId?: string; +} + +// The message an action works on, and whether the loaded list holds it: the +// list's row, but for the viewer only when the list is the message's account; +// otherwise the viewer's copy, and the list is left alone. +function actionTarget( + state: EmailState, + emailId: string, + viewed?: ViewedEmail, +): { email: Email | undefined; listed: boolean } { + const listed = !viewed || viewed.accountId === currentAccountId(state); + const row = listed ? state.emails.find((e) => e.id === emailId) : undefined; + return { email: row ?? viewed?.email, listed }; +} + +// Refuse a viewer action aimed at folders outside the message's account: +// the same id there is another message. +function assertViewedAccount(viewed: ViewedEmail | undefined, ...refs: MailboxRef[]): void { + if (viewed && refs.some((ref) => ref.accountId !== viewed.accountId)) { + throw new Error(t('email_list.move_same_account', 'Messages can only be moved within the same account')); + } +} + // Strip the shared-folder id prefix off a whole list. Server-side folder // matching (archive year/month auto-foldering) compares ids and parent links // against what Mailbox/set returns, which is always unprefixed. @@ -316,9 +348,9 @@ export interface EmailState { markUnread: (emailId: string) => Promise; toggleStar: (emailId: string, starred: boolean) => Promise; togglePin: (emailId: string, pinned: boolean) => Promise; - moveToMailbox: (emailId: string, fromMailboxId: string, toMailboxId: string) => Promise; + moveToMailbox: (emailId: string, fromMailboxId: string, toMailboxId: string, viewed?: ViewedEmail) => Promise; archiveEmail: (emailId: string) => Promise; - deleteEmail: (emailId: string, trashMailboxId: string, currentMailboxId: string) => Promise; + deleteEmail: (emailId: string, trashMailboxId: string, currentMailboxId: string, viewed?: ViewedEmail) => Promise; /** * File messages into the current account's Junk and flip `$junk`/`$notjunk` * (#850); honours the "trash-and-read" delete action by also marking read. @@ -1142,11 +1174,12 @@ export const useEmailStore = create()( for (const e of targets) patchCache(e.id, { mailboxIds: inboxTarget, keywords: keywordPatch }, inbox.accountId); }, - moveToMailbox: async (emailId, fromMailboxId, toMailboxId) => { + moveToMailbox: async (emailId, fromMailboxId, toMailboxId, viewed) => { const state = get(); - const email = state.emails.find((e) => e.id === emailId); + const { email, listed } = actionTarget(state, emailId, viewed); const from = refFor(state.mailboxes, fromMailboxId); const to = refFor(state.mailboxes, toMailboxId); + assertViewedAccount(viewed, from, to); // A single Email/set is scoped to one account: a move between the user's // own folders and a shared account's (or between two shared accounts) is // a copy-then-delete across accounts (webmail 1.7.2), online only. @@ -1169,7 +1202,7 @@ export const useEmailStore = create()( { kind: 'mailboxes', emailId, accountId: from.accountId, mailboxIds: target }, () => moveEmail(emailId, from.id, to.id, from.accountId), ); - set({ emails: get().emails.filter((e) => e.id !== emailId) }); + if (listed) set({ emails: get().emails.filter((e) => e.id !== emailId) }); patchCache(emailId, { mailboxIds: target }, from.accountId); if (email && original) { @@ -1247,13 +1280,14 @@ export const useEmailStore = create()( } }, - deleteEmail: async (emailId, trashMailboxId, currentMailboxId) => { + deleteEmail: async (emailId, trashMailboxId, currentMailboxId, viewed) => { const state = get(); - const email = state.emails.find((e) => e.id === emailId); + const { email, listed } = actionTarget(state, emailId, viewed); const original = email ? { ...email.mailboxIds } : null; const settings = useSettingsStore.getState(); const trash = refFor(state.mailboxes, trashMailboxId); const source = refFor(state.mailboxes, currentMailboxId); + assertViewedAccount(viewed, trash, source); const junkMailbox = mailboxesForSiblingOf(state.mailboxes, currentMailboxId) .find((m) => m.role === 'junk' || m.role === 'spam'); const junkId = junkMailbox ? rawMailboxId(state.mailboxes, junkMailbox.id) : null; @@ -1299,7 +1333,7 @@ export const useEmailStore = create()( patchCache(emailId, { mailboxIds: target }, source.accountId); } } - set({ emails: get().emails.filter((e) => e.id !== emailId) }); + if (listed) set({ emails: get().emails.filter((e) => e.id !== emailId) }); // Permanent destroy can't be undone - skip the snackbar so we don't // promise an undo we can't deliver. @@ -1600,9 +1634,10 @@ export const useEmailStore = create()( // Re-insert each restored email into the visible list if its original // mailboxIds include the current view. Server is the source of truth for - // ordering, but local re-insertion gives the user instant feedback. + // ordering, but local re-insertion gives the user instant feedback. Only + // into a list of the same account: mailbox ids repeat across accounts. const { currentMailboxId, emails, mailboxes } = get(); - if (currentMailboxId) { + if (currentMailboxId && entry.accountId === currentAccountId(get())) { const currentRawId = rawMailboxId(mailboxes, currentMailboxId); const restored = entry.items .filter((it) => it.originalMailboxIds[currentRawId])