diff --git a/src/api/__tests__/tag-counts.test.ts b/src/api/__tests__/tag-counts.test.ts new file mode 100644 index 00000000..ea758a48 --- /dev/null +++ b/src/api/__tests__/tag-counts.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../jmap-client', () => ({ + jmapClient: { + accountId: 'c', + request: vi.fn(), + getMaxCallsInRequest: vi.fn(() => 16), + }, +})); + +import { jmapClient } from '../jmap-client'; +import { fetchTagCounts } from '../tag-counts'; + +const mockRequest = jmapClient.request as ReturnType; +const mockMaxCalls = jmapClient.getMaxCallsInRequest as ReturnType; + +// Answers every Email/query with `totals[accountId][callId prefix]`, or an +// error for an account listed in `failing`. +function answer(totals: Record, failing: string[] = []) { + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => ({ + methodResponses: calls.map(([, args, callId]) => { + const accountId = args.accountId as string; + if (failing.includes(accountId)) return ['error', { type: 'forbidden' }, callId]; + const unread = 'operator' in (args.filter as object); + return ['Email/query', { ids: [], total: unread ? totals[accountId].unread : totals[accountId].total }, callId]; + }), + })); +} + +beforeEach(() => { + vi.clearAllMocks(); + mockMaxCalls.mockReturnValue(16); +}); + +describe('fetchTagCounts', () => { + it('counts the own account alone by default', async () => { + answer({ c: { total: 2, unread: 1 } }); + + 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: { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }] }, + limit: 0, + calculateTotal: true, + }, 'u0:red'], + ]); + }); + + it('sums a tag over the own and the team account in one request (#1038)', async () => { + answer({ c: { total: 2, unread: 1 }, team: { total: 2, unread: 2 } }); + + const counts = await fetchTagCounts(['red', 'blue'], [undefined, 'team']); + + expect(counts).toEqual([ + { id: 'red', total: 4, unread: 3 }, + { id: 'blue', total: 4, unread: 3 }, + ]); + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls[0][0].map((c: unknown[]) => (c[1] as { accountId: string }).accountId)) + .toEqual(['c', 'c', 'c', 'c', 'team', 'team', 'team', 'team']); + }); + + it('packs the calls into requests of maxCallsInRequest', async () => { + mockMaxCalls.mockReturnValue(4); + answer({ c: { total: 1, unread: 0 }, team: { total: 1, unread: 1 } }); + + await fetchTagCounts(['red', 'blue', 'green'], [undefined, 'team']); + + expect(mockRequest.mock.calls.map((c) => c[0].length)).toEqual([4, 4, 4]); + }); + + it('lets an account that fails count nothing', async () => { + answer({ c: { total: 2, unread: 1 } }, ['team']); + + expect(await fetchTagCounts(['red'], [undefined, 'team'])).toEqual([{ id: 'red', total: 2, unread: 1 }]); + }); +}); diff --git a/src/api/tag-counts.ts b/src/api/tag-counts.ts index afb94fdc..8b4aa009 100644 --- a/src/api/tag-counts.ts +++ b/src/api/tag-counts.ts @@ -9,50 +9,56 @@ export interface TagCount { } /** - * Unread/total counts per tag across every folder of one account — two - * `Email/query` calls (`calculateTotal`, `limit: 0`) per tag, batched to the - * server's maxCallsInRequest. Port of the webmail's `fetchTagCounts` - * (stores/email-store.ts). A tag whose queries fail is reported as 0/0 - * rather than sinking the whole batch. + * Unread/total counts per tag across every folder of the given JMAP accounts + * (undefined = the user's own), summed: a tag sits on messages in the own and + * in the shared (group) accounts alike, and its view lists all of them + * (#1038). Two `Email/query` calls (`calculateTotal`, `limit: 0`) per tag and + * account, packed into as few requests as the server's maxCallsInRequest + * 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. */ -export async function fetchTagCounts(tagIds: string[], accountIdOverride?: string): Promise { +export async function fetchTagCounts( + tagIds: string[], + accountIds: Array = [undefined], +): Promise { if (tagIds.length === 0) return []; - const accountId = accountIdOverride ?? jmapClient.accountId; const perRequest = Math.max(2, jmapClient.getMaxCallsInRequest()); const counts = new Map(tagIds.map((id) => [id, { id, total: 0, unread: 0 }])); - const calls: Array<{ id: string; kind: 'total' | 'unread'; call: JMAPMethodCall }> = []; - for (const id of tagIds) { - const keyword = `$label:${id}`; - calls.push({ - id, - kind: 'total', - call: ['Email/query', { accountId, filter: { hasKeyword: keyword }, limit: 0, calculateTotal: true }, `t:${id}`], - }); - calls.push({ - id, - kind: 'unread', - call: ['Email/query', { + const calls: JMAPMethodCall[] = []; + const targets = new Map(); + accountIds.forEach((override, index) => { + const accountId = override ?? jmapClient.accountId; + 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, + 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, calculateTotal: true, - }, `u:${id}`], - }); - } + }, `u${index}:${id}`]); + } + }); for (let i = 0; i < calls.length; i += perRequest) { - const chunk = calls.slice(i, i + perRequest); - const res = await jmapClient.request(chunk.map((c) => c.call)); + const res = await jmapClient.request(calls.slice(i, i + perRequest)); for (const [name, body, callId] of res.methodResponses) { if (name !== 'Email/query' || typeof callId !== 'string') continue; - const [kind, ...rest] = callId.split(':'); - const id = rest.join(':'); - const entry = counts.get(id); - if (!entry) continue; + const target = targets.get(callId); + const entry = target ? counts.get(target.id) : undefined; + if (!target || !entry) continue; const total = typeof body.total === 'number' ? body.total : 0; - if (kind === 't') entry.total = total; - else if (kind === 'u') entry.unread = total; + if (target.kind === 'total') entry.total += total; + else entry.unread += total; } } return tagIds.map((id) => counts.get(id)!); diff --git a/src/api/types.ts b/src/api/types.ts index 829cabe4..bd73eb99 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -94,8 +94,8 @@ export interface Email { bodyStructure?: BodyPart; /** * Client-side stamp, never sent by the server: the JMAP account a row of a - * list that spans accounts (an "All folders" search) was fetched from, the - * user's own account included. Rows of a single folder carry none; they + * list that spans accounts (an "All folders" search, a tag view) was + * fetched from, the user's own account included. Rows of a single folder carry none; they * belong to the folder's account. */ jmapAccountId?: string; diff --git a/src/components/SidebarDrawer.tsx b/src/components/SidebarDrawer.tsx index caeb679f..dfa9d656 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 } from '../stores/email-store'; +import { useEmailStore, spannedAccounts } from '../stores/email-store'; import { useAuthStore } from '../stores/auth-store'; import { useAccountStore } from '../stores/account-store'; import { useSettingsStore } from '../stores/settings-store'; @@ -429,19 +429,23 @@ export default function SidebarDrawer({ visible, onClose }: SidebarDrawerProps) onClose(); }, [setFilters, onClose]); - // Tag counts: one query pair per tag, refreshed each time the drawer opens - // with the section expanded. + // Tag counts: one query pair per tag and account, refreshed each time the + // drawer opens with the section expanded. A tag view lists the tagged mail + // of the own and the shared accounts, so its badge counts all of them + // (#1038). + const tagAccountsKey = React.useMemo(() => spannedAccounts(mailboxes).join('|'), [mailboxes]); React.useEffect(() => { if (!visible || !tagsExpanded || keywordDefs.length === 0 || !jmapClient.isConnected) return; let cancelled = false; - fetchTagCounts(keywordDefs.map((k) => k.id)) + const accounts = tagAccountsKey.split('|').map((id) => id || undefined); + fetchTagCounts(keywordDefs.map((k) => k.id), accounts) .then((counts) => { if (cancelled) return; setTagCounts(new Map(counts.map((tc) => [tc.id, tc]))); }) .catch(() => { /* counts are decoration */ }); return () => { cancelled = true; }; - }, [visible, tagsExpanded, keywordDefs]); + }, [visible, tagsExpanded, keywordDefs, tagAccountsKey]); // ── 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 5a03d676..77803dfa 100644 --- a/src/stores/__tests__/email-store-all-folders.test.ts +++ b/src/stores/__tests__/email-store-all-folders.test.ts @@ -405,3 +405,105 @@ describe('opening and acting on "All folders" rows (#1082)', () => { expect(deleteDestroysAcrossAccounts(['o4'])).toBeNull(); }); }); + +// #1038: a tag is set on messages in the own and in the team account alike, +// but the tag view only asked the account of the open folder: 2 own and 2 +// team messages tagged Red showed 2 or the other 2, depending on the folder. +describe('tag view across the own and the team account (#1038)', () => { + const RED = '$label:red'; + + beforeEach(() => { + server.current = createFakeJmap({ + c: [ + 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'), + ], + team: [ + mail('m1', 't-inbox', 22, 'team red 1', { keywords: { [RED]: true } }), + mail('m2', 't-inbox', 15, 'team red 2', { keywords: { [RED]: true } }), + mail('m3', 't-inbox', 23, 'team plain'), + ], + }); + }); + + async function openTag(folder: string) { + useEmailStore.setState({ currentMailboxId: folder }); + useEmailStore.getState().setFilters({ keyword: RED }); + await vi.waitFor(() => expect(useEmailStore.getState().loading).toBe(false)); + } + + it('lists the tagged mail of both accounts while a team folder is open', async () => { + await openTag('team:t-inbox'); + + expect(ids()).toEqual(['m1', 'o1', 'm2', 'o2']); + expect(server.current!.requests).toHaveLength(1); + const queries = server.current!.callsOf('Email/query'); + expect(queries.map(([, a]) => [a.accountId, a.filter])).toEqual([ + ['c', { hasKeyword: RED }], + ['team', { hasKeyword: RED }], + ]); + }); + + it('lists the same set while an own folder is open', async () => { + await openTag('inbox'); + + expect(ids()).toEqual(['m1', 'o1', 'm2', 'o2']); + expect(useEmailStore.getState().emails.map((e) => e.jmapAccountId)).toEqual(['team', 'c', 'team', 'c']); + }); + + it('narrows the tag with a search across both accounts', async () => { + await openTag('inbox'); + useEmailStore.getState().setSearchQuery('red 2'); + await vi.waitFor(() => expect(ids()).toEqual(['m2', 'o2'])); + + 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 }] }, + ]); + }); + + it('untags a team row in the team account and keeps it in view', async () => { + await openTag('inbox'); + await useEmailStore.getState().setKeywordForEmails(['m2'], RED, false); + + expect(server.current!.callsOf('Email/set')[0][1]).toEqual({ + accountId: 'team', update: { m2: { 'keywords/$label:red': null } }, + }); + expect(ids()).toContain('m2'); + expect(useEmailStore.getState().retainedIds).toEqual(['m2']); + }); + + it('pages each account on from its own rows', async () => { + settings.emailsPerPage = 1; + await openTag('inbox'); + expect(ids()).toEqual(['m1', 'o1']); + + await useEmailStore.getState().loadMoreEmails(); + + const page2 = server.current!.requests[1].filter(([n]) => n === 'Email/query'); + expect(page2.map(([, a]) => [a.accountId, a.position])).toEqual([['c', 1], ['team', 1]]); + expect(ids()).toEqual(['m1', 'o1', 'm2', 'o2']); + }); + + it('refreshes when only the team account reports a change', async () => { + await openTag('inbox'); + server.current!.accounts.team[2].keywords = { [RED]: true }; + + await useEmailStore.getState().handleStateChange({ + '@type': 'StateChange', changed: { team: { Email: 's2' } }, + } as never); + + expect(ids()).toEqual(['m3', 'm1', 'o1', 'm2', 'o2']); + }); + + it('still lists only the open folder when no tag is selected', async () => { + useEmailStore.setState({ currentMailboxId: 'team:t-inbox' }); + await useEmailStore.getState().refreshEmails(); + + const queries = server.current!.callsOf('Email/query'); + expect(queries).toHaveLength(1); + expect(queries[0][1]).toMatchObject({ accountId: 'team', filter: { inMailbox: 't-inbox' } }); + }); +}); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index cf331704..c5863ef0 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -360,8 +360,8 @@ export interface EmailState { */ threadCounts: Record; /** - * Accounts an "All folders" list could not reach, by JMAP account id → the - * error. The other accounts' messages still show. + * Accounts an "All folders" list or a tag view could not reach, by JMAP + * account id → the error. The other accounts' messages still show. */ accountErrors: Record; @@ -521,18 +521,24 @@ function isUnsupportedSort(err: unknown): boolean { // ── Lists that span accounts ─────────────────────────────────────────── // "All folders" covers every folder of the user's own account AND of the // shared (group) accounts whose folders are in the sidebar (#1082, webmail -// `searchAcrossAccounts`). Each account is asked for its own page, the pages -// are merged under the list order and every row is stamped with its JMAP -// account (`Email.jmapAccountId`), so opening and acting on a row reach the -// account it lives in. A search scoped to one folder stays in its account. +// `searchAcrossAccounts`), and so does a tag view: the same tag keyword sits +// on messages in all of them (#1038, webmail `fetchTagEmails`). Each account +// is asked for its own page, the pages are merged under the list order and +// every row is stamped with its JMAP account (`Email.jmapAccountId`), so +// opening and acting on a row reach the account it lives in. A search scoped +// 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' && !state.filters.keyword; + return effectiveFolderScope(state.searchQuery, state.filters) === 'all'; } -// The JMAP accounts such a list covers; undefined = the user's own. -function listAccounts(mailboxes: Mailbox[]): Array { +/** + * The JMAP accounts a list spanning accounts covers, and a tag badge counts: + * the user's own (undefined) and every shared account with folders in the + * sidebar. + */ +export function spannedAccounts(mailboxes: Mailbox[]): Array { const out: Array = [undefined]; for (const m of mailboxes) { if (m.isShared && m.accountId && m.accountId !== jmapClient.accountId && !out.includes(m.accountId)) { @@ -616,7 +622,7 @@ async function fetchSpanningPage( }))), { limit, filter, threads }, ); - let pages = await run(listAccounts(state.mailboxes)); + let pages = await run(spannedAccounts(state.mailboxes)); const refused = pages.filter((p) => !p.ok && isUnsupportedSort(p.error)); if (refused.length > 0) { for (const p of refused) markKeywordSortUnsupported(p.accountId ?? primary); @@ -1147,7 +1153,8 @@ export const useEmailStore = create()( ? refFor(state.mailboxes, state.currentMailboxId).accountId ?? primaryId : primaryId; const known = new Set([primaryId, ...jmapClient.getSharedMailAccounts().map((a) => a.id)]); - // An "All folders" list shows every account's mail (#1082). + // An "All folders" list or a tag view shows every account's mail + // (#1082, #1038). const spanning = spansAccounts(state); let mailboxChanged = false;