From eb6bf6d38e3e20bf14f36a223b00fe3add9f6c1e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 04:37:42 +0100 Subject: [PATCH] fix: leave trash and spam out of tag views and tag counts, and count tags with Keyword/get where the server can (bulwarkmail/webmail#1156) --- src/api/__tests__/live-stalwart-mail.test.ts | 24 +++++++ src/api/__tests__/tag-counts.test.ts | 58 ++++++++++++++- src/api/tag-counts.ts | 70 +++++++++++++++++-- src/components/SidebarDrawer.tsx | 8 ++- .../__tests__/email-store-all-folders.test.ts | 13 ++-- src/stores/email-store.ts | 7 +- src/stores/tag-counts-store.ts | 14 ++-- 7 files changed, 171 insertions(+), 23 deletions(-) diff --git a/src/api/__tests__/live-stalwart-mail.test.ts b/src/api/__tests__/live-stalwart-mail.test.ts index ea415837..88f46e8a 100644 --- a/src/api/__tests__/live-stalwart-mail.test.ts +++ b/src/api/__tests__/live-stalwart-mail.test.ts @@ -41,6 +41,7 @@ import { } from '../email'; import { senderWarningOf } from '../../lib/email-headers'; import { scheduledEmailIdsInThread } from '../thread-scheduled'; +import { fetchTagCounts, KEYWORDS_CAPABILITY } from '../tag-counts'; import { uploadBytes } from '../blob'; import { getIdentities, updateIdentity } from '../identity'; import { isJMAPSetError } from '../jmap-result'; @@ -466,5 +467,28 @@ live('live Stalwart: mail, contacts, search (0.16 and 1.0)', () => { const [filed] = await getFullEmails([result.emailId!]); expect((await scheduledEmailIdsInThread(filed.threadId)).size).toBe(0); }, 90_000); + + it('leaves Trash out of the tag counts (bulwarkmail/webmail#1156)', async (ctx) => { + const boxes = await getMailboxes(); + const trash = boxes.find((b) => b.role === 'trash')!.id; + const tag = 'readerpass'; + const keyword = `$label:${tag}`; + const live = await importMessage(scratch, `tag live ${mailTag}`, 'a'); + const gone = await importMessage(trash, `tag deleted ${mailTag}`, 'b', { $seen: true }); + // Set afterwards: 0.16 drops the keywords Email/import is given. + await patchKeywordsForEmails([live, gone], { [keyword]: true }); + const flags = await getEmailFlags([live]); + if (!flags.list[0]?.keywords?.[keyword]) { + // Seen on the 0.16.27 test server: Email/set answers "updated" but + // keeps only system keywords for this account. Nothing to count. + ctx.skip('the server did not keep the tag keyword'); + return; + } + const all = await eventually(() => fetchTagCounts([tag]), ([c]) => c.total === 2); + expect(all).toEqual([{ id: tag, total: 2, unread: 1 }]); + expect(await fetchTagCounts([tag], [undefined], { '': [trash] })).toEqual([{ id: tag, total: 1, unread: 1 }]); + // Stalwart answers Keyword/get for no keyword counters (no capability). + expect(client.currentSession?.capabilities?.[KEYWORDS_CAPABILITY]).toBeUndefined(); + }, 60_000); }); }); diff --git a/src/api/__tests__/tag-counts.test.ts b/src/api/__tests__/tag-counts.test.ts index ea758a48..288eb4e9 100644 --- a/src/api/__tests__/tag-counts.test.ts +++ b/src/api/__tests__/tag-counts.test.ts @@ -5,11 +5,13 @@ vi.mock('../jmap-client', () => ({ accountId: 'c', request: vi.fn(), getMaxCallsInRequest: vi.fn(() => 16), + currentSession: { capabilities: {} as Record }, + hasAccountCapability: vi.fn(() => true), }, })); import { jmapClient } from '../jmap-client'; -import { fetchTagCounts } from '../tag-counts'; +import { fetchTagCounts, KEYWORDS_CAPABILITY } from '../tag-counts'; const mockRequest = jmapClient.request as ReturnType; const mockMaxCalls = jmapClient.getMaxCallsInRequest as ReturnType; @@ -30,6 +32,7 @@ function answer(totals: Record, faili beforeEach(() => { vi.clearAllMocks(); mockMaxCalls.mockReturnValue(16); + jmapClient.currentSession!.capabilities = {}; }); describe('fetchTagCounts', () => { @@ -39,11 +42,11 @@ describe('fetchTagCounts', () => { expect(await fetchTagCounts(['red'])).toEqual([{ id: 'red', total: 2, unread: 1 }]); const calls = mockRequest.mock.calls[0][0]; expect(calls).toEqual([ - ['Email/query', { accountId: 'c', filter: { hasKeyword: '$label:red' }, limit: 0, calculateTotal: true }, 't0:red'], + ['Email/query', { accountId: 'c', filter: { hasKeyword: '$label:red' }, limit: 1, calculateTotal: true }, 't0:red'], ['Email/query', { accountId: 'c', filter: { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }] }, - limit: 0, + limit: 1, calculateTotal: true, }, 'u0:red'], ]); @@ -77,4 +80,53 @@ describe('fetchTagCounts', () => { expect(await fetchTagCounts(['red'], [undefined, 'team'])).toEqual([{ id: 'red', total: 2, unread: 1 }]); }); + + // The tag view leaves Trash and Junk out; its badge counted them (#1156). + it("leaves each account's Trash and Junk out of the counts", async () => { + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => ({ + methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 1 }, callId]), + })); + + await fetchTagCounts(['red'], [undefined, 'team'], { '': ['trash', 'junk'], team: ['t-trash'] }); + + const filters = mockRequest.mock.calls[0][0].map((c: unknown[]) => (c[1] as { filter: unknown }).filter); + expect(filters).toEqual([ + { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { inMailboxOtherThan: ['trash', 'junk'] }] }, + { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['trash', 'junk'] }] }, + { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { inMailboxOtherThan: ['t-trash'] }] }, + { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['t-trash'] }] }, + ]); + }); + + // One Keyword/get where the server counts keywords (#1399), else the queries. + it('asks Keyword/get when the server counts keywords, and falls back to queries when it fails', async () => { + jmapClient.currentSession!.capabilities = { [KEYWORDS_CAPABILITY]: { supportsCounts: true } }; + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => { + if (calls[0][0] === 'Keyword/get') { + return { methodResponses: [['Keyword/get', { list: [{ id: '$LABEL:Red', total: 7, unread: 3 }] }, '0']] }; + } + return { methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 1 }, callId]) }; + }); + + expect(await fetchTagCounts(['red', 'blue'])).toEqual([ + { id: 'red', total: 7, unread: 3 }, + { id: 'blue', total: 0, unread: 0 }, + ]); + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls[0][0][0]).toEqual(['Keyword/get', { accountId: 'c', ids: ['$label:red', '$label:blue'] }, '0']); + + mockRequest.mockClear(); + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => ( + calls[0][0] === 'Keyword/get' + ? { methodResponses: [['error', { type: 'unknownMethod' }, '0']] } + : { methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 2 }, callId]) } + )); + expect(await fetchTagCounts(['red'])).toEqual([{ id: 'red', total: 2, unread: 2 }]); + + // Without supportsCounts nothing asks Keyword/get. + jmapClient.currentSession!.capabilities = { [KEYWORDS_CAPABILITY]: {} }; + mockRequest.mockClear(); + await fetchTagCounts(['red']); + expect(mockRequest.mock.calls[0][0][0][0]).toBe('Email/query'); + }); }); diff --git a/src/api/tag-counts.ts b/src/api/tag-counts.ts index 8b4aa009..65065a1e 100644 --- a/src/api/tag-counts.ts +++ b/src/api/tag-counts.ts @@ -1,5 +1,8 @@ import { jmapClient } from './jmap-client'; -import type { JMAPMethodCall } from './types'; +import { CAPABILITIES, type JMAPMethodCall } from './types'; + +/** Bulwark's keyword counters (webmail lib/jmap/client.ts KEYWORDS_CAPABILITY). */ +export const KEYWORDS_CAPABILITY = 'https://bulwarkmail.com/ns/jmap/keywords'; export interface TagCount { /** Tag id (the part after `$label:`). */ @@ -17,33 +20,61 @@ export interface TagCount { * allows. Port of the webmail's `fetchTagCounts` (stores/email-store.ts). A * call that fails, such as for an account that can't be reached, counts 0 * rather than sinking the rest. + * + * `excluded` names each account's Trash and Junk (key '' = the own account): + * the counts leave them out like the tag view does (webmail #1156). An + * account whose server counts keywords itself (Bulwark's keywords capability + * with `supportsCounts`) is asked once with Keyword/get instead (#1399); its + * counters cover the whole account, as the webmail's do. */ export async function fetchTagCounts( tagIds: string[], accountIds: Array = [undefined], + excluded: Record = {}, ): Promise { if (tagIds.length === 0) return []; const perRequest = Math.max(2, jmapClient.getMaxCallsInRequest()); const counts = new Map(tagIds.map((id) => [id, { id, total: 0, unread: 0 }])); + const queried: Array = []; + for (const override of accountIds) { + const counted = countsKeywords(override ?? jmapClient.accountId) + ? await tagCountsFromKeywords(tagIds, override ?? jmapClient.accountId) + : null; + if (!counted) { + queried.push(override); + continue; + } + for (const c of counted) { + const entry = counts.get(c.id)!; + entry.total += c.total; + entry.unread += c.unread; + } + } + const calls: JMAPMethodCall[] = []; const targets = new Map(); - accountIds.forEach((override, index) => { + queried.forEach((override, index) => { const accountId = override ?? jmapClient.accountId; + const skip = excluded[override ?? ''] ?? []; + const exclusion = skip.length > 0 ? [{ inMailboxOtherThan: skip }] : []; for (const id of tagIds) { const keyword = `$label:${id}`; targets.set(`t${index}:${id}`, { id, kind: 'total' }); calls.push(['Email/query', { accountId, - filter: { hasKeyword: keyword }, - limit: 0, + filter: exclusion.length > 0 + ? { operator: 'AND', conditions: [{ hasKeyword: keyword }, ...exclusion] } + : { hasKeyword: keyword }, + // 1, not 0: Stalwart takes 0 as "no limit" and returns every id. + limit: 1, calculateTotal: true, }, `t${index}:${id}`]); targets.set(`u${index}:${id}`, { id, kind: 'unread' }); calls.push(['Email/query', { accountId, - filter: { operator: 'AND', conditions: [{ hasKeyword: keyword }, { notKeyword: '$seen' }] }, - limit: 0, + filter: { operator: 'AND', conditions: [{ hasKeyword: keyword }, { notKeyword: '$seen' }, ...exclusion] }, + limit: 1, calculateTotal: true, }, `u${index}:${id}`]); } @@ -63,3 +94,30 @@ export async function fetchTagCounts( } return tagIds.map((id) => counts.get(id)!); } + +/** Whether the server reports every keyword's total and unread in Keyword/get. */ +function countsKeywords(accountId: string): boolean { + const capability = jmapClient.currentSession?.capabilities?.[KEYWORDS_CAPABILITY] as { supportsCounts?: unknown } | undefined; + return capability?.supportsCounts === true && jmapClient.hasAccountCapability(KEYWORDS_CAPABILITY, accountId); +} + +/** Counts from the server's keyword counters, or null so the caller queries instead. */ +async function tagCountsFromKeywords(tagIds: string[], accountId: string): Promise { + try { + const res = await jmapClient.request( + [['Keyword/get', { accountId, ids: tagIds.map((id) => `$label:${id}`) }, '0']], + [CAPABILITIES.CORE, KEYWORDS_CAPABILITY], + ); + const [name, body] = res.methodResponses?.[0] ?? []; + if (name !== 'Keyword/get' || !Array.isArray(body?.list)) return null; + // Keywords are case-insensitive (RFC 8621 4.1.1). + const byId = new Map((body.list as Array<{ id: string; total?: number; unread?: number }>) + .map((k) => [k.id.toLowerCase(), k])); + return tagIds.map((id) => { + const k = byId.get(`$label:${id}`.toLowerCase()); + return { id, total: k?.total ?? 0, unread: k?.unread ?? 0 }; + }); + } catch { + return null; + } +} diff --git a/src/components/SidebarDrawer.tsx b/src/components/SidebarDrawer.tsx index f0bc4f10..82326d9c 100644 --- a/src/components/SidebarDrawer.tsx +++ b/src/components/SidebarDrawer.tsx @@ -15,7 +15,7 @@ import { import { spacing, radius, typography, type ThemePalette } from '../theme/tokens'; import { useColors } from '../theme/colors'; import { useAnimDuration } from '../theme/dynamic'; -import { useEmailStore, spannedAccounts } from '../stores/email-store'; +import { useEmailStore, spannedAccounts, trashAndJunkIds } from '../stores/email-store'; import { useAuthStore } from '../stores/auth-store'; import { useAccountStore } from '../stores/account-store'; import { useSettingsStore } from '../stores/settings-store'; @@ -462,8 +462,10 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side React.useEffect(() => { if (!visible || !tagsExpanded || keywordDefs.length === 0 || !activeAccountId || !jmapClient.isConnected) return; const accounts = tagAccountsKey.split('|').map((id) => id || undefined); - void ensureTagCounts(activeAccountId, keywordDefs.map((k) => k.id), accounts); - }, [visible, tagsExpanded, keywordDefs, tagAccountsKey, activeAccountId, tagCountsGeneration, ensureTagCounts]); + // Trash and Junk stay out of the counts as they do of the tag view (#1156). + const excluded = Object.fromEntries(accounts.map((id) => [id ?? '', trashAndJunkIds(mailboxes, id)])); + void ensureTagCounts(activeAccountId, keywordDefs.map((k) => k.id), accounts, excluded); + }, [visible, tagsExpanded, keywordDefs, tagAccountsKey, mailboxes, activeAccountId, tagCountsGeneration, ensureTagCounts]); // ── Folder actions (long-press) ─────────────────────────────────────── const refFor = (mb: Mailbox) => ({ diff --git a/src/stores/__tests__/email-store-all-folders.test.ts b/src/stores/__tests__/email-store-all-folders.test.ts index 7f4cb3cd..4a382792 100644 --- a/src/stores/__tests__/email-store-all-folders.test.ts +++ b/src/stores/__tests__/email-store-all-folders.test.ts @@ -626,6 +626,8 @@ describe('tag view across the own and the team account (#1038)', () => { mail('o1', 'inbox', 20, 'own red 1', { keywords: { [RED]: true } }), mail('o2', 'archive', 10, 'own red 2', { keywords: { [RED]: true, $seen: true } }), mail('o3', 'inbox', 21, 'own plain'), + // Deleted, but it keeps its keywords: not listed under the tag (#1156). + mail('o4', 'trash', 24, 'own red deleted', { keywords: { [RED]: true } }), ], team: [ mail('m1', 't-inbox', 22, 'team red 1', { keywords: { [RED]: true } }), @@ -647,9 +649,10 @@ describe('tag view across the own and the team account (#1038)', () => { expect(ids()).toEqual(['m1', 'o1', 'm2', 'o2']); expect(server.current!.requests).toHaveLength(1); const queries = server.current!.callsOf('Email/query'); + // Each account leaves its own Trash and Junk out (webmail #1156). expect(queries.map(([, a]) => [a.accountId, a.filter])).toEqual([ - ['c', { hasKeyword: RED }], - ['team', { hasKeyword: RED }], + ['c', { operator: 'AND', conditions: [{ hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }] }], + ['team', { operator: 'AND', conditions: [{ hasKeyword: RED }, { inMailboxOtherThan: ['t-trash', 't-junk'] }] }], ]); }); @@ -669,8 +672,8 @@ describe('tag view across the own and the team account (#1038)', () => { const last = server.current!.requests.at(-1)!.filter(([n]) => n === 'Email/query'); expect(last.map(([, a]) => a.filter)).toEqual([ - { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }] }, - { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }] }, + { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }] }, + { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }, { inMailboxOtherThan: ['t-trash', 't-junk'] }] }, ]); }); @@ -689,7 +692,7 @@ describe('tag view across the own and the team account (#1038)', () => { const last = server.current!.requests.at(-1)!.filter(([n]) => n === 'Email/query'); expect(last[0][1].filter).toEqual({ operator: 'AND', - conditions: [{ text: '"red 2"' }, { text: 'notes' }, { hasKeyword: RED }], + conditions: [{ text: '"red 2"' }, { text: 'notes' }, { hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }], }); }); await vi.waitFor(() => expect(useEmailStore.getState().loading).toBe(false)); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index 4b8ec214..b4a01730 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -303,9 +303,12 @@ export function searchSkipsTrashAndJunk( 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; + // A tag view leaves Trash and Junk out, as Gmail's label views do: a + // deleted message keeps its keywords and stayed listed under the tag + // (webmail #1156). It is still in the Trash folder itself. + if (filters.keyword) return true; return effectiveFolderScope(searchQuery, filters, openRole) === 'all'; } @@ -626,7 +629,7 @@ export function spansAccounts( } // The raw JMAP ids of one account's Trash and Junk (undefined = the user's own). -function trashAndJunkIds(mailboxes: Mailbox[], accountId: string | undefined): string[] { +export function trashAndJunkIds(mailboxes: Mailbox[], accountId: string | undefined): string[] { return accountMailboxes(mailboxes, accountId) .filter((m) => isTrashOrJunkRole(m.role)) .map((m) => m.originalId ?? m.id); diff --git a/src/stores/tag-counts-store.ts b/src/stores/tag-counts-store.ts index 4121108d..4dbef392 100644 --- a/src/stores/tag-counts-store.ts +++ b/src/stores/tag-counts-store.ts @@ -20,7 +20,13 @@ interface TagCountsState { /** `generation` the counts were fetched at. */ fetchedAt: number; /** Fetch the counts unless the cached ones are current. */ - ensure: (login: string, tagIds: string[], accountIds: Array) => Promise; + ensure: ( + login: string, + tagIds: string[], + accountIds: Array, + /** Each account's Trash and Junk ids (key '' = the own account), left out of the counts. */ + excluded?: Record, + ) => Promise; /** An Email change arrived: the next `ensure` fetches again. */ invalidate: () => void; } @@ -35,8 +41,8 @@ export const useTagCountsStore = create((set, get) => ({ generation: 0, fetchedAt: -1, - ensure: (login, tagIds, accountIds) => { - const key = `${login}|${accountIds.map((id) => id ?? '').join(',')}|${tagIds.join(',')}`; + ensure: (login, tagIds, accountIds, excluded = {}) => { + const key = `${login}|${accountIds.map((id) => id ?? '').join(',')}|${tagIds.join(',')}|${JSON.stringify(excluded)}`; if (get().login !== login) set({ counts: {}, login, key: null }); const { generation } = get(); if (get().key === key && get().fetchedAt === generation) return Promise.resolve(); @@ -44,7 +50,7 @@ export const useTagCountsStore = create((set, get) => ({ const seq = ++lastSeq; const promise = (async () => { try { - const list = await fetchTagCounts(tagIds, accountIds); + const list = await fetchTagCounts(tagIds, accountIds, excluded); // A later ensure (another login, other tags) supersedes this one. if (inflight?.seq !== seq || get().login !== login) return; set({ counts: Object.fromEntries(list.map((c) => [c.id, c])), key, fetchedAt: generation });