From a4e313f73e903072aaa848c939423bf769303a23 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:15:26 +0200 Subject: [PATCH] fix: leave trash and spam out of tag views and their counts #1156 --- lib/__tests__/jmap-request-limits.test.ts | 21 ++++++++++++++++ lib/__tests__/unified-mailbox-tags.test.ts | 23 ++++++++++++++++++ lib/demo/demo-client.ts | 5 ++-- lib/jmap/client-interface.ts | 5 ++-- lib/jmap/client.ts | 10 ++++++-- lib/unified-mailbox.ts | 24 +++++++++++++++---- .../__tests__/email-store-tag-counts.test.ts | 2 +- .../email-store-tag-view-accounts.test.ts | 19 +++++++++++++-- stores/email-store.ts | 5 ++-- 9 files changed, 98 insertions(+), 16 deletions(-) diff --git a/lib/__tests__/jmap-request-limits.test.ts b/lib/__tests__/jmap-request-limits.test.ts index c6743f062..6883600ce 100644 --- a/lib/__tests__/jmap-request-limits.test.ts +++ b/lib/__tests__/jmap-request-limits.test.ts @@ -147,6 +147,27 @@ describe('JMAPClient request limits', () => { expect(sent.map(calls => calls.length)).toEqual([4, 4, 4, 4, 2]); }); + + it('leaves the excluded folders out of both counts (#1156)', async () => { + const client = await connectedClient({ maxCallsInRequest: 16 }); + const sent = recordRequests((methodCalls) => ({ + methodResponses: methodCalls.map(([, , callId]) => ['Email/query', { total: 0 }, callId]), + })); + + await client.getTagCounts(['work'], undefined, ['trash', 'junk']); + + const [total, unread] = sent[0].map(([, args]) => (args as { filter: unknown }).filter); + expect(total).toEqual({ + operator: 'AND', + conditions: [{ hasKeyword: '$label:work' }, { inMailboxOtherThan: ['trash', 'junk'] }], + }); + expect(unread).toEqual({ + operator: 'AND', + conditions: [ + { hasKeyword: '$label:work' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['trash', 'junk'] }, + ], + }); + }); }); describe('getCategoryUnreadCounts', () => { diff --git a/lib/__tests__/unified-mailbox-tags.test.ts b/lib/__tests__/unified-mailbox-tags.test.ts index 2504c32bf..b191adec0 100644 --- a/lib/__tests__/unified-mailbox-tags.test.ts +++ b/lib/__tests__/unified-mailbox-tags.test.ts @@ -93,6 +93,29 @@ describe('fetchTagEmails', () => { expect(get).toHaveBeenNthCalledWith(2, undefined, 'g', 25, 50, keyword, true, undefined, order); }); + it("leaves each account's Trash and Junk out, ANDed with a search filter (#1156)", async () => { + const role = (id: string, r: string, originalId?: string) => ({ ...mb(id, originalId), role: r } as Mailbox); + const ownGet = vi.fn(async () => ({ emails: [], total: 0, hasMore: false })); + const groupGet = vi.fn(async () => ({ emails: [], total: 0, hasMore: false })); + const own = makeAccount({ + accountId: 'login', mailboxes: [mb('inbox'), role('trash', 'trash'), role('junk', 'junk')], + }, ownGet); + const group = makeAccount({ + accountId: 'group', isShared: true, mailboxes: [mb('group:g-inbox', 'g-inbox'), role('group:g-trash', 'trash', 'g-trash')], + }, groupGet); + + await fetchTagEmails([own, group], keyword, 50, 0, [], { text: 'invoice' }); + + expect(ownGet).toHaveBeenCalledWith(undefined, undefined, 50, 0, keyword, true, + { operator: 'AND', conditions: [{ text: 'invoice' }, { inMailboxOtherThan: ['trash', 'junk'] }] }, []); + expect(groupGet).toHaveBeenCalledWith(undefined, 'group', 50, 0, keyword, true, + { operator: 'AND', conditions: [{ text: 'invoice' }, { inMailboxOtherThan: ['g-trash'] }] }, []); + + await fetchTagEmails([own], keyword, 50, 0); + expect(ownGet).toHaveBeenLastCalledWith(undefined, undefined, 50, 0, keyword, true, + { inMailboxOtherThan: ['trash', 'junk'] }, []); + }); + it('keeps the other accounts when one fails and reports the failure', async () => { const own = makeAccount({ accountId: 'login' }, vi.fn(async () => ({ emails: [email('own-1', '2026-09-10T00:00:00Z', 'inbox')], total: 1, hasMore: false }))); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index 4cd970e8b..312d408c8 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -252,10 +252,11 @@ export class DemoJMAPClient implements IJMAPClient { return this.data.emails.find(e => e.id === emailId) ?? null; } - async getTagCounts(tagIds: string[]): Promise> { + async getTagCounts(tagIds: string[], _accountId?: string, excludeMailboxIds?: string[]): Promise> { const result: Record = {}; + const exclusion = excludeMailboxIds?.length ? { inMailboxOtherThan: excludeMailboxIds } : null; for (const tagId of tagIds) { - const tagged = this.data.emails.filter(e => e.keywords[tagId]); + const tagged = this.data.emails.filter(e => e.keywords[tagId] && (!exclusion || this.matchesFilter(e, exclusion))); result[tagId] = { total: tagged.length, unread: tagged.filter(e => !e.keywords.$seen).length, diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 49511495b..41eef9402 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -194,9 +194,10 @@ export interface IJMAPClient { * Total / unread message counts per tag id. `accountId` scopes the count to * a group/shared account reached through this client (defaults to the * client's own account); the store sums it over every account a tag view - * spans (#1038). + * spans (#1038). A message filed only in `excludeMailboxIds` (the account's + * Trash and Junk, which the tag view leaves out, #1156) is not counted. */ - getTagCounts(tagIds: string[], accountId?: string): Promise>; + getTagCounts(tagIds: string[], accountId?: string, excludeMailboxIds?: string[]): Promise>; /** * Enumerate account keywords for extensions. Servers supporting Keyword/get * can return exact counts and provider-label metadata; other servers use the diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index f635ba6f8..63aa729b9 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2078,10 +2078,13 @@ export class JMAPClient implements IJMAPClient { return allEmails; } - async getTagCounts(tagIds: string[], accountId?: string): Promise> { + async getTagCounts(tagIds: string[], accountId?: string, excludeMailboxIds?: string[]): Promise> { if (tagIds.length === 0) return {}; const result: Record = {}; const targetAccountId = accountId || this.accountId; + const exclusion = excludeMailboxIds && excludeMailboxIds.length > 0 + ? [{ inMailboxOtherThan: excludeMailboxIds }] + : []; const CALLS_PER_TAG = 2; const perRequest = itemsPerRequest(this.getMaxCallsInRequest(), CALLS_PER_TAG); @@ -2094,7 +2097,9 @@ export class JMAPClient implements IJMAPClient { // Total count for this tag methodCalls.push(["Email/query", { accountId: targetAccountId, - filter: { hasKeyword: keyword }, + filter: exclusion.length > 0 + ? { operator: "AND", conditions: [{ hasKeyword: keyword }, ...exclusion] } + : { hasKeyword: keyword }, // limit 1, not 0: Stalwart treats 0 as "no limit" and returns every id. limit: 1, calculateTotal: true, @@ -2107,6 +2112,7 @@ export class JMAPClient implements IJMAPClient { conditions: [ { hasKeyword: keyword }, { notKeyword: "$seen" }, + ...exclusion, ], }, // limit 1, not 0: Stalwart treats 0 as "no limit" and returns every id. diff --git a/lib/unified-mailbox.ts b/lib/unified-mailbox.ts index f65303c35..3dc8162d0 100644 --- a/lib/unified-mailbox.ts +++ b/lib/unified-mailbox.ts @@ -93,11 +93,16 @@ export function jmapMailboxIdOf(account: UnifiedAccountClient, mailbox: Mailbox) * search, or null when the account has neither folder. */ export function trashAndJunkExclusion(account: UnifiedAccountClient): Record | null { - const ids = (['trash', 'junk'] as const) + const ids = trashAndJunkIds(account); + return ids.length > 0 ? { inMailboxOtherThan: ids } : null; +} + +/** The JMAP ids of an account's Trash and Junk folders (those it has). */ +export function trashAndJunkIds(account: UnifiedAccountClient): string[] { + return (['trash', 'junk'] as const) .map((role) => findMailboxByRole(account.mailboxes, role)) .filter((mailbox): mailbox is Mailbox => Boolean(mailbox)) .map((mailbox) => jmapMailboxIdOf(account, mailbox)); - return ids.length > 0 ? { inMailboxOtherThan: ids } : null; } /** @@ -545,6 +550,11 @@ export async function advancedSearchCrossViewEmails( * the same page (`limit`/`position`) with pinned-first ordering plus the * configured list order, mirroring `fetchEmails`, and the pages are merged * under that same order. Per-account failures land in `errors`. + * + * Trash and Junk are left out, as in Gmail's label views: a deleted message + * keeps its keywords, so it stayed listed under the tag (and came back on + * every refresh after Delete removed the row), looking no different from + * live mail (#1156). It is still reachable from the Trash folder itself. */ export async function fetchTagEmails( accounts: UnifiedAccountClient[], @@ -556,9 +566,13 @@ export async function fetchTagEmails( ): Promise { return fanOutAccountQuery( accounts, - (account, jmapAccountId) => account.client.getEmails( - undefined, jmapAccountId, limit, positionFor(position, account), keyword, true, extraFilter, order, - ), + (account, jmapAccountId) => { + const filter = andFilters(extraFilter ?? {}, trashAndJunkExclusion(account)); + return account.client.getEmails( + undefined, jmapAccountId, limit, positionFor(position, account), keyword, true, + Object.keys(filter).length > 0 ? filter : undefined, order, + ); + }, compareEmails(order, { pinnedFirst: true }), ); } diff --git a/stores/__tests__/email-store-tag-counts.test.ts b/stores/__tests__/email-store-tag-counts.test.ts index b99ea9fdf..0d2e158cb 100644 --- a/stores/__tests__/email-store-tag-counts.test.ts +++ b/stores/__tests__/email-store-tag-counts.test.ts @@ -215,7 +215,7 @@ describe('email-store tag counts stay in step with read state', () => { // The server bulk-marks emails that are not in `state.emails`, so a local // delta would under-count: it has to refetch. - expect(client.getTagCounts).toHaveBeenCalledWith(['ingsel', 'work'], undefined); + expect(client.getTagCounts).toHaveBeenCalledWith(['ingsel', 'work'], undefined, []); await vi.waitFor(() => { expect(useEmailStore.getState().tagCounts).toEqual({ diff --git a/stores/__tests__/email-store-tag-view-accounts.test.ts b/stores/__tests__/email-store-tag-view-accounts.test.ts index d17faea6b..50c2fd9bf 100644 --- a/stores/__tests__/email-store-tag-view-accounts.test.ts +++ b/stores/__tests__/email-store-tag-view-accounts.test.ts @@ -179,11 +179,26 @@ describe('tag view across the own and group accounts (#1038)', () => { it('sums the sidebar tag badge over every account', async () => { await useEmailStore.getState().fetchTagCounts(client); - expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined); - expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group'); + expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined, []); + expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group', []); expect(useEmailStore.getState().tagCounts[label]).toEqual({ total: 4, unread: 1 }); }); + it("leaves every account's Trash and Junk out of the badge, as the tag view does (#1156)", async () => { + const ownTrash = { id: 'trash', name: 'Trash', role: 'trash', isShared: false } as Mailbox; + const ownJunk = { id: 'junk', name: 'Junk', role: 'junk', isShared: false } as Mailbox; + const groupTrash = { + id: 'group:trash', originalId: 'trash', name: 'Trash', role: 'trash', + isShared: true, accountId: 'group', accountName: 'group@server.tld', + } as Mailbox; + useEmailStore.setState({ mailboxes: [ownInbox, ownTrash, ownJunk, groupInbox, groupTrash] }); + + await useEmailStore.getState().fetchTagCounts(client); + + expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined, ['trash', 'junk']); + expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group', ['trash']); + }); + it('still lists only the selected folder when no tag is selected', async () => { useEmailStore.setState({ selectedMailbox: groupInbox.id }); await useEmailStore.getState().fetchEmails(client); diff --git a/stores/email-store.ts b/stores/email-store.ts index d7986eef2..fe5c6d48f 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -10,7 +10,7 @@ import { emailHooks } from "@/lib/plugin-hooks"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils"; import type { ExternalSearchResult } from "@/lib/plugin-types"; -import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; +import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, trashAndJunkIds, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox"; import { defaultSearchScopeFor, isAllFoldersSearchScope, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders"; import { useAuthStore } from "@/stores/auth-store"; import { pathNamesMailFolder } from "@/lib/deep-links"; @@ -1778,9 +1778,10 @@ export const useEmailStore = create((set, get) => ({ // The badge counts what the tag view lists: messages in the own account // AND in the group/shared accounts this login reaches (#1038). Sum the // per-account counts; an account that fails just contributes nothing. + // Trash and Junk are left out of both, see fetchTagEmails (#1156). const built = buildTagViewAccountClients(client); const perAccount = await Promise.allSettled( - built.map((a) => a.client.getTagCounts(tagIds, a.isShared ? a.accountId : undefined)), + built.map((a) => a.client.getTagCounts(tagIds, a.isShared ? a.accountId : undefined, trashAndJunkIds(a))), ); const counts: Record = {}; for (const outcome of perAccount) {