From a412b5163acff9ecc0da8576753c2e3215d71f27 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:28:34 +0100 Subject: [PATCH] feat: leave Spam and Trash out of search unless All folders is picked --- src/screens/EmailListScreen.tsx | 26 ++++- .../__tests__/email-store-all-folders.test.ts | 38 ++++++++ src/stores/__tests__/fake-jmap-server.ts | 1 + src/stores/email-store.ts | 96 ++++++++++++++++--- 4 files changed, 145 insertions(+), 16 deletions(-) diff --git a/src/screens/EmailListScreen.tsx b/src/screens/EmailListScreen.tsx index 75e3578f..454642d3 100644 --- a/src/screens/EmailListScreen.tsx +++ b/src/screens/EmailListScreen.tsx @@ -28,6 +28,7 @@ import type { LoadListAttachments } from '../lib/list-attachments'; import { useNetworkStore } from '../stores/network-store'; import { useEmailStore, effectiveFolderScope, spansAccounts, accountIdOfRow, deleteDestroysAcrossAccounts, + searchSkipsTrashAndJunk, isTrashOrJunkRole, SCOPE_EXCEPT_TRASH_JUNK, type EmailFilters, } from '../stores/email-store'; import { useSettingsStore, type SwipeAction, type SwipeMode } from '../stores/settings-store'; @@ -1003,9 +1004,12 @@ export default function EmailListScreen({ } }, [backStep, clearSelection, clearSearchAndFilters]); useBackWhileFocused(backStep !== 'leave', handleBack); - const folderScope = effectiveFolderScope(storeSearchQuery, filters); + const openRole = currentMailbox?.role; + const folderScope = effectiveFolderScope(storeSearchQuery, filters, openRole); + // The default search scope leaves Spam and Trash out; "All folders" doesn't. + const skipsTrashAndJunk = searchSkipsTrashAndJunk(storeSearchQuery, filters, openRole); // Accounts an "All folders" list could not reach (#1082). - const unreachedAccounts = spansAccounts({ searchQuery: storeSearchQuery, filters }) + const unreachedAccounts = spansAccounts({ searchQuery: storeSearchQuery, filters, mailboxes, currentMailboxId }) ? Object.values(accountErrors) : []; const scopeFolderName = React.useMemo(() => { @@ -1020,7 +1024,12 @@ export default function EmailListScreen({ }, [filters.keyword, keywordDefs]); const setFolderScope = (scope: string) => { const updated: EmailFilters = { ...filters }; - if (scope === 'current') delete updated.folder; + // The scope a search gets by default stays unset, so it keeps following + // the open folder; any other pick is kept as it is. + const isDefault = scope === SCOPE_EXCEPT_TRASH_JUNK + ? !isTrashOrJunkRole(openRole) + : scope === 'current' && isTrashOrJunkRole(openRole); + if (isDefault) delete updated.folder; else updated.folder = scope; setFilters(updated); }; @@ -1402,7 +1411,9 @@ export default function EmailListScreen({ } label={ - folderScope === 'all' + skipsTrashAndJunk + ? t('advanced_search.all_folders_except_spam_trash', 'All folders except Spam and Trash') + : folderScope === 'all' ? t('email_list.scope_all_folders', 'All folders') : folderScope === 'current' ? t('email_list.scope_this_folder', 'This folder') @@ -1719,9 +1730,14 @@ export default function EmailListScreen({ {t('advanced_search.folder', 'Folder')} + setFolderScope(SCOPE_EXCEPT_TRASH_JUNK)} + /> setFolderScope('all')} /> useEmailStore.getState().emails.map((e) => e.id); +// "All folders" picked explicitly, unless a test picked a folder: the default +// scope also leaves Trash and Junk out (see "the default search scope"). async function search(query: string) { + const { filters } = useEmailStore.getState(); + if (!filters.folder) useEmailStore.setState({ filters: { ...filters, folder: 'all' } }); + await searchDefault(query); +} + +async function searchDefault(query: string) { useEmailStore.getState().setSearchQuery(query); await vi.waitFor(() => expect(useEmailStore.getState().loading).toBe(false)); } @@ -182,6 +190,36 @@ beforeEach(() => { }); }); +// Webmail 763732fdc / 3904af2cb: a search leaves Spam and Trash out unless +// "All folders" is picked, and one started inside them searches them. +describe('the default search scope', () => { + it("leaves every account's Trash and Junk out", async () => { + await searchDefault('zephyr'); + + expect(ids()).toEqual(['m1', 'o1', 'm2', 'm3', 'o2', 'm4', 'o3']); + const queries = server.current!.callsOf('Email/query'); + expect(queries.map(([, args]) => [args.accountId, args.filter])).toEqual([ + ['c', { operator: 'AND', conditions: [{ text: 'zephyr' }, { inMailboxOtherThan: ['trash', 'junk'] }] }], + ['team', { operator: 'AND', conditions: [{ text: 'zephyr' }, { inMailboxOtherThan: ['t-trash', 't-junk'] }] }], + ]); + }); + + it('searches Trash itself when the search starts there', async () => { + useEmailStore.setState({ currentMailboxId: 'team:t-trash' }); + await searchDefault('zephyr'); + + expect(ids()).toEqual(['m5']); + const queries = server.current!.callsOf('Email/query'); + expect(queries.map(([, args]) => args.accountId)).toEqual(['team']); + }); + + it('keeps Trash and Junk in when "All folders" is picked', async () => { + useEmailStore.setState({ filters: { folder: 'all' } }); + await searchDefault('zephyr'); + expect(ids()).toContain('m5'); + }); +}); + describe('"All folders" search across the own and the team account (#1082)', () => { it('finds the team inbox matches too, in one request', async () => { await search('zephyr'); diff --git a/src/stores/__tests__/fake-jmap-server.ts b/src/stores/__tests__/fake-jmap-server.ts index 58cee7eb..5f212f8d 100644 --- a/src/stores/__tests__/fake-jmap-server.ts +++ b/src/stores/__tests__/fake-jmap-server.ts @@ -20,6 +20,7 @@ function matches(email: FakeEmail, filter: Filter | undefined, textModel: Stalwa if (!filter) return true; return textFilterMatches(textModel, filter, { subject: email.subject ?? '', body: email.preview ?? '' }, (key, value) => { if (key === 'inMailbox') return !!email.mailboxIds[value as string]; + if (key === 'inMailboxOtherThan') return !(value as string[]).some((id) => email.mailboxIds[id]); if (key === 'hasKeyword') return !!email.keywords[value as string]; if (key === 'notKeyword') return !email.keywords[value as string]; throw new Error(`fake server: unsupported filter ${key}`); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index e7145537..cdc932ed 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -264,11 +264,55 @@ export interface EmailFilters { keyword?: string; } -/** The folder scope a query runs in, resolving the unset default. */ -export function effectiveFolderScope(searchQuery: string, filters: EmailFilters): SearchFolderScope { +/** + * The `folder` value of the "All folders except Spam and Trash" scope when + * it is picked explicitly (from inside Spam or Trash, where it is not the + * default). It spans the folders like "All folders". + */ +export const SCOPE_EXCEPT_TRASH_JUNK = 'all-except-trash-junk'; + +/** + * The folder scope a query runs in, resolving the unset default. A search + * from inside Spam or Trash looks there; anywhere else it spans every + * folder (and leaves Spam and Trash out, see `searchSkipsTrashAndJunk`). + * Webmail 763732fdc, 3904af2cb. + */ +export function effectiveFolderScope( + searchQuery: string, + filters: EmailFilters, + openRole?: string | null, +): SearchFolderScope { if (filters.keyword) return 'all'; + if (filters.folder === SCOPE_EXCEPT_TRASH_JUNK) return 'all'; if (filters.folder) return filters.folder; - return searchQuery.trim() ? 'all' : 'current'; + if (!searchQuery.trim()) return 'current'; + return isTrashOrJunkRole(openRole) ? 'current' : 'all'; +} + +/** Trash, or Junk under either role name servers use. */ +export function isTrashOrJunkRole(role: string | null | undefined): boolean { + return role === 'trash' || role === 'junk' || role === 'spam'; +} + +/** + * Whether a search across folders leaves every account's Trash and Junk + * out: the default scope does, "All folders" picked explicitly doesn't. + */ +export function searchSkipsTrashAndJunk( + searchQuery: string, + filters: EmailFilters, + openRole?: string | null, +): boolean { + if (filters.keyword) return false; + if (filters.folder === SCOPE_EXCEPT_TRASH_JUNK) return true; + if (filters.folder) return false; + return effectiveFolderScope(searchQuery, filters, openRole) === 'all'; +} + +// The role of the folder on screen. +function openRoleOf(state: Partial>): string | undefined { + if (!state.currentMailboxId || !state.mailboxes) return undefined; + return state.mailboxes.find((m) => m.id === state.currentMailboxId)?.role ?? undefined; } // Snapshot of an action that can still be reversed via the undo snackbar. @@ -487,7 +531,7 @@ function buildJmapFilter( // The raw mailbox id an Email/query is scoped to: the open folder, an // explicitly picked folder, or undefined for "all folders" (#788). function queryScope(state: EmailState, current: MailboxRef): { mailboxId: string | undefined; accountId?: string } { - const scope = effectiveFolderScope(state.searchQuery, state.filters); + const scope = effectiveFolderScope(state.searchQuery, state.filters, openRoleOf(state)); if (scope === 'current') return { mailboxId: current.id, accountId: current.accountId }; if (scope === 'all') return { mailboxId: undefined, accountId: current.accountId }; const ref = refFor(state.mailboxes, scope); @@ -552,8 +596,29 @@ function isUnsupportedSort(err: unknown): boolean { // to one folder stays in its account. /** Whether the list on screen spans the own and the shared accounts. */ -export function spansAccounts(state: Pick): boolean { - return effectiveFolderScope(state.searchQuery, state.filters) === 'all'; +export function spansAccounts( + state: Pick & Partial>, +): boolean { + return effectiveFolderScope(state.searchQuery, state.filters, openRoleOf(state)) === 'all'; +} + +// The raw JMAP ids of one account's Trash and Junk (undefined = the user's own). +function trashAndJunkIds(mailboxes: Mailbox[], accountId: string | undefined): string[] { + return accountMailboxes(mailboxes, accountId) + .filter((m) => isTrashOrJunkRole(m.role)) + .map((m) => m.originalId ?? m.id); +} + +// `filter` AND `extra`, flattened into an existing top-level AND. +function andFilter( + filter: Record | undefined, + extra: Record, +): Record { + if (!filter || Object.keys(filter).length === 0) return extra; + if (filter.operator === 'AND' && Array.isArray(filter.conditions)) { + return { operator: 'AND', conditions: [...(filter.conditions as unknown[]), extra] }; + } + return { operator: 'AND', conditions: [filter, extra] }; } /** @@ -675,14 +740,23 @@ async function fetchSpanningPage( ): Promise { const primary = jmapClient.accountId; const threads = !useSettingsStore.getState().disableThreading; - const run = async (accounts: Array) => queryEmailPagesAcrossAccounts( - await Promise.all(accounts.map(async (accountId) => ({ + const skipTrashAndJunk = searchSkipsTrashAndJunk(state.searchQuery, state.filters, openRoleOf(state)); + const run = async (accounts: Array) => { + const targets = await Promise.all(accounts.map(async (accountId) => ({ accountId, position: loaded[accountId ?? primary] ?? 0, sort: await resolveSort(state, accountId), - }))), - { limit, filter, threads }, - ); + }))); + if (!skipTrashAndJunk) return queryEmailPagesAcrossAccounts(targets, { limit, filter, threads }); + // Each account leaves its own Trash and Junk out, so each gets its own + // filter (and request). + const pages = await Promise.all(targets.map((target) => { + const ids = trashAndJunkIds(state.mailboxes, target.accountId); + const scoped = ids.length > 0 ? andFilter(filter, { inMailboxOtherThan: ids }) : filter; + return queryEmailPagesAcrossAccounts([target], { limit, filter: scoped, threads }); + })); + return pages.map((p) => p[0]); + }; let pages = await run(spannedAccounts(state.mailboxes)); const refused = pages.filter((p) => !p.ok && isUnsupportedSort(p.error)); if (refused.length > 0) {