mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 16:05:39 +00:00
fix: stop viewer actions from hitting another account's message with the same id
Stalwart ids are only unique per account. When its opener named no account, the viewer used the account of the folder open in the list, and it resolved Trash, Junk and move targets from that folder as well. Deleting a team message from the Unified Inbox therefore sent `Email/set` to the user's own account and trashed an unrelated message with the same id, and a message opened from a notification, deep link or contact while a shared folder was open went to the shared account. - the list names the open folder's account when it opens the viewer, and the viewer takes the account from its opener only, fixed at open - Trash, Junk, Inbox and move targets come from the message's account, and delete and move take the message out of its own folder - the store's delete and move take the viewed message: they only touch the list's row when the list shows that account, and refuse folders of any other account - another account's message is marked read on the server directly instead of through the open folder's list - quick reply, unsubscribe and read receipts file into the message's account's folders - undo no longer slots another account's message into the open list - Archive is only offered for mail of the open folder's account, since it still resolves the archive folder from there
This commit is contained in:
@@ -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<RootStackParam
|
||||
<EmailListScreen
|
||||
onComposePress={() => 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),
|
||||
});
|
||||
}}
|
||||
/>
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
@@ -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<string>();
|
||||
@@ -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'));
|
||||
|
||||
@@ -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)', () => {
|
||||
|
||||
@@ -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<string, boolean> | 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<string, number> = {
|
||||
inbox: 0,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<Email[]>(() => {
|
||||
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<string>();
|
||||
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) => <Archive size={s} color={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) => <FolderInput size={s} color={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 ? <ShieldCheck size={s} color={c.success} /> : <ShieldAlert size={s} color={col} />,
|
||||
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}
|
||||
/>
|
||||
|
||||
|
||||
@@ -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<void>) => {
|
||||
if (onlineRun) await onlineRun();
|
||||
return { queued: false };
|
||||
};
|
||||
return {
|
||||
applyOrQueueBatch,
|
||||
applyOrQueue: async (op: unknown, onlineRun?: () => Promise<void>) => applyOrQueueBatch([op], onlineRun),
|
||||
useOutboxStore: {
|
||||
getState: () => ({ entries: [], count: () => 0, setAccount: vi.fn(async () => undefined), flush: vi.fn() }),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../settings-store', () => {
|
||||
const settings: Record<string, unknown> = { 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<typeof vi.fn>;
|
||||
const mockMoveEmail = emailApi.moveEmail as ReturnType<typeof vi.fn>;
|
||||
const mockRestore = emailApi.restoreEmailMailboxes as ReturnType<typeof vi.fn>;
|
||||
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
+45
-10
@@ -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<void>;
|
||||
toggleStar: (emailId: string, starred: boolean) => Promise<void>;
|
||||
togglePin: (emailId: string, pinned: boolean) => Promise<void>;
|
||||
moveToMailbox: (emailId: string, fromMailboxId: string, toMailboxId: string) => Promise<void>;
|
||||
moveToMailbox: (emailId: string, fromMailboxId: string, toMailboxId: string, viewed?: ViewedEmail) => Promise<void>;
|
||||
archiveEmail: (emailId: string) => Promise<void>;
|
||||
deleteEmail: (emailId: string, trashMailboxId: string, currentMailboxId: string) => Promise<void>;
|
||||
deleteEmail: (emailId: string, trashMailboxId: string, currentMailboxId: string, viewed?: ViewedEmail) => Promise<void>;
|
||||
/**
|
||||
* 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<EmailState>()(
|
||||
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<EmailState>()(
|
||||
{ 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<EmailState>()(
|
||||
}
|
||||
},
|
||||
|
||||
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<EmailState>()(
|
||||
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<EmailState>()(
|
||||
|
||||
// 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])
|
||||
|
||||
Reference in New Issue
Block a user