fix: report spam from the viewer the way the list does #695

The viewer's Spam and Not spam buttons were a plain move into Junk or
Inbox: they sent no `$junk`/`$notjunk` flip, so the server's classifier
never learned from them, and they ignored the "trash-and-read" delete
action. They now call the store's `markSpam`/`unmarkSpam`, the path the
list and swipe actions use, which take the viewed message and file it
in its own account's Junk or Inbox, also when the list does not hold
it. Not spam is only offered when that account has an Inbox.
This commit is contained in:
Linus Rath
2026-09-24 19:06:55 +02:00
parent 9c31de2e80
commit 5e42c6737e
3 changed files with 83 additions and 25 deletions
+12 -11
View File
@@ -61,6 +61,8 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
const deleteEmail = useEmailStore((s) => s.deleteEmail);
const moveToMailbox = useEmailStore((s) => s.moveToMailbox);
const archiveEmailAction = useEmailStore((s) => s.archiveEmail);
const markSpam = useEmailStore((s) => s.markSpam);
const unmarkSpam = useEmailStore((s) => s.unmarkSpam);
const mailboxes = useEmailStore((s) => s.mailboxes);
const currentMailboxId = useEmailStore((s) => s.currentMailboxId);
const disableThreading = useSettingsStore((s) => s.disableThreading);
@@ -318,6 +320,8 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
// offered for mail of that account.
const canArchive = !!archiveMailbox && listAccountId === ownerAccountId
&& sourceMailbox?.id !== archiveMailbox.id;
// Not spam files back into the Inbox, so it needs one.
const canToggleSpam = isInJunk ? !!inboxMailbox : !!junkMailbox;
const onToggleKeyword = (token: string) => {
if (!email) return;
@@ -397,18 +401,15 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
navigation.goBack();
};
// The same store path as the list and swipe actions: files into the
// message's own Junk or Inbox, flips `$junk`/`$notjunk` and honours
// trash-and-read (#695).
const onToggleSpam = () => {
if (!email || !sourceMailbox) return;
if (!email || !canToggleSpam) return;
setMoreMenuOpen(false);
const viewed = { email, accountId: ownerAccountId };
if (isInJunk) {
const target = inboxMailbox ?? scopedMailboxes.find((m) => m.id !== sourceMailbox.id);
if (!target) return;
void moveToMailbox(email.id, sourceMailbox.id, target.id, viewed);
} else {
if (!junkMailbox) return;
void moveToMailbox(email.id, sourceMailbox.id, junkMailbox.id, viewed);
}
if (isInJunk) void unmarkSpam([email.id], viewed);
else void markSpam([email.id], viewed);
navigation.goBack();
};
@@ -544,7 +545,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
icon: (s, col) =>
isInJunk ? <ShieldCheck size={s} color={c.success} /> : <ShieldAlert size={s} color={col} />,
onPress: onToggleSpam,
available: !!junkMailbox && !!sourceMailbox,
available: canToggleSpam,
},
tag: {
label: t('email_viewer.tag', 'Tag'),
@@ -721,7 +722,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) {
canArchive={canArchive}
canMarkUnread={true}
canMove={!!sourceMailbox}
showSpam={!!junkMailbox && !!sourceMailbox}
showSpam={canToggleSpam}
isInJunk={isInJunk}
canViewSource={!!email?.blobId}
canExport={!!email?.blobId}
@@ -95,11 +95,14 @@ vi.mock('../../api/jmap-client', () => ({
import * as emailApi from '../../api/email';
import { useEmailStore, type ViewedEmail } from '../email-store';
import { useSettingsStore } from '../settings-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>;
const mockMarkAsSpam = emailApi.markAsSpam as ReturnType<typeof vi.fn>;
const mockUndoSpam = emailApi.undoSpam as ReturnType<typeof vi.fn>;
function own(id: string, role: string): Mailbox {
return { id, name: role, role, accountId: 'acc-1', isShared: false } as Mailbox;
@@ -109,8 +112,8 @@ function team(rawId: string, role: string): Mailbox {
}
const MAILBOXES = [
own('a', 'inbox'), own('t', 'trash'), own('x', 'archive'),
team('a', 'inbox'), team('t', 'trash'), team('x', 'archive'),
own('a', 'inbox'), own('t', 'trash'), own('x', 'archive'), own('j', 'junk'),
team('a', 'inbox'), team('t', 'trash'), team('x', 'archive'), team('j', 'junk'),
];
// 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;
@@ -188,3 +191,37 @@ describe('viewer actions on another account\'s message (B3)', () => {
expect(useEmailStore.getState().emails).toEqual([]);
});
});
describe('viewer Spam takes the list and swipe path (#695)', () => {
it('files a team message into the team Junk flipping $junk/$notjunk, and leaves the list alone', async () => {
useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] });
await useEmailStore.getState().markSpam(['e1'], TEAM_VIEWED);
expect(mockMarkAsSpam).toHaveBeenCalledWith(['e1'], 'j', 'grp-1', { markRead: false });
expect(useEmailStore.getState().emails).toEqual([OWN_ROW]);
const undo = useEmailStore.getState().pendingUndo!;
expect(undo).toMatchObject({ kind: 'spam', accountId: 'grp-1', keywordPatch: { $junk: true, $notjunk: null } });
expect(undo.items[0].originalMailboxIds).toEqual({ a: true });
});
it('honours trash-and-read for a message the list does not hold', async () => {
useSettingsStore.getState().updateSetting('deleteAction', 'trash-and-read');
useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [] });
await useEmailStore.getState().markSpam(['e1'], { email: OWN_ROW, accountId: undefined });
expect(mockMarkAsSpam).toHaveBeenCalledWith(['e1'], 'j', undefined, { markRead: true });
useSettingsStore.getState().updateSetting('deleteAction', 'trash');
});
it('files a team message back into the team Inbox with $notjunk', async () => {
useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] });
const junked = { ...TEAM_MESSAGE, keywords: { $junk: true }, mailboxIds: { j: true } } as unknown as Email;
await useEmailStore.getState().unmarkSpam(['e1'], { email: junked, accountId: 'grp-1' });
expect(mockUndoSpam).toHaveBeenCalledWith(['e1'], 'a', 'grp-1');
expect(useEmailStore.getState().emails).toEqual([OWN_ROW]);
});
});
+32 -12
View File
@@ -34,7 +34,7 @@ import {
} from '../api/email';
import { useNetworkStore } from './network-store';
import { JMAPMethodError } from '../api/jmap-result';
import { mailboxesForSiblingOf, findJunkMailbox } from '../lib/mailbox-tree';
import { mailboxesForSiblingOf, mailboxesOfAccount, findJunkMailbox } from '../lib/mailbox-tree';
import { toWildcardQuery } from '../lib/search-utils';
import { orderForMailbox, sanitizeSortLevels, type SortLevel } from '../lib/message-list-order';
import { buildListSort, markKeywordSortUnsupported } from '../lib/keyword-sort-polarity';
@@ -156,6 +156,25 @@ function actionTarget(
return { email: row ?? viewed?.email, listed };
}
// actionTarget for the selection actions: the viewer only acts on its one message.
function actionTargets(
state: EmailState,
emailIds: string[],
viewed?: ViewedEmail,
): { targets: Email[]; listed: boolean } {
if (!viewed) return { targets: state.emails.filter((e) => emailIds.includes(e.id)), listed: true };
const { email, listed } = actionTarget(state, viewed.email.id, viewed);
return { targets: email ? [email] : [], listed };
}
// Where an action looks up Junk, Inbox or Archive: the viewed message's
// account, otherwise the open folder's.
function actionMailboxes(state: EmailState, viewed?: ViewedEmail): Mailbox[] {
return viewed
? mailboxesOfAccount(state.mailboxes, viewed.accountId)
: mailboxesForSiblingOf(state.mailboxes, state.currentMailboxId);
}
// 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 {
@@ -354,11 +373,12 @@ export interface EmailState {
/**
* File messages into the current account's Junk and flip `$junk`/`$notjunk`
* (#850); honours the "trash-and-read" delete action by also marking read.
* Works for one id or a selection; offers undo.
* Works for one id or a selection; offers undo. With `viewed`, the viewer's
* message goes to its own account's Junk (#695).
*/
markSpam: (emailIds: string[]) => Promise<void>;
markSpam: (emailIds: string[], viewed?: ViewedEmail) => Promise<void>;
/** Inverse of markSpam: back to Inbox with `$notjunk`. */
unmarkSpam: (emailIds: string[]) => Promise<void>;
unmarkSpam: (emailIds: string[], viewed?: ViewedEmail) => Promise<void>;
// ── Batch (multi-select) actions ──────────────────────────────
archiveEmailsBatch: (emailIds: string[]) => Promise<void>;
moveEmailsToMailbox: (emailIds: string[], toMailboxId: string) => Promise<void>;
@@ -1088,11 +1108,11 @@ export const useEmailStore = create<EmailState>()(
patchCache(emailId, { keywords: patch }, currentAccountId(state));
},
markSpam: async (emailIds) => {
markSpam: async (emailIds, viewed) => {
const state = get();
const targets = state.emails.filter((e) => emailIds.includes(e.id));
const { targets, listed } = actionTargets(state, emailIds, viewed);
if (targets.length === 0) return;
const scoped = mailboxesForSiblingOf(state.mailboxes, state.currentMailboxId);
const scoped = actionMailboxes(state, viewed);
const junkMailbox = findJunkMailbox(scoped);
if (!junkMailbox) {
set({ error: t('email_list.no_junk_folder', 'Could not find a Spam/Junk folder on the server.') });
@@ -1116,7 +1136,7 @@ export const useEmailStore = create<EmailState>()(
() => apiMarkAsSpam(targets.map((e) => e.id), junk.id, junk.accountId, { markRead }),
);
const removed = new Set(targets.map((e) => e.id));
const removed = new Set(listed ? targets.map((e) => e.id) : []);
set({
emails: get().emails.filter((e) => !removed.has(e.id)),
pendingUndo: {
@@ -1133,11 +1153,11 @@ export const useEmailStore = create<EmailState>()(
for (const e of targets) patchCache(e.id, { mailboxIds: junkTarget, keywords: keywordPatch }, junk.accountId);
},
unmarkSpam: async (emailIds) => {
unmarkSpam: async (emailIds, viewed) => {
const state = get();
const targets = state.emails.filter((e) => emailIds.includes(e.id));
const { targets, listed } = actionTargets(state, emailIds, viewed);
if (targets.length === 0) return;
const scoped = mailboxesForSiblingOf(state.mailboxes, state.currentMailboxId);
const scoped = actionMailboxes(state, viewed);
const inboxMailbox = scoped.find((m) => m.role === 'inbox');
if (!inboxMailbox) return;
const inbox = refFor(state.mailboxes, inboxMailbox.id);
@@ -1157,7 +1177,7 @@ export const useEmailStore = create<EmailState>()(
() => apiUndoSpam(targets.map((e) => e.id), inbox.id, inbox.accountId),
);
const removed = new Set(targets.map((e) => e.id));
const removed = new Set(listed ? targets.map((e) => e.id) : []);
set({
emails: get().emails.filter((e) => !removed.has(e.id)),
pendingUndo: {