diff --git a/src/components/SidebarDrawer.tsx b/src/components/SidebarDrawer.tsx index dfa9d656..528550bc 100644 --- a/src/components/SidebarDrawer.tsx +++ b/src/components/SidebarDrawer.tsx @@ -32,7 +32,7 @@ import { jmapClient } from '../api/jmap-client'; import { markMailboxAsRead, emptyMailbox, createMailbox, updateMailbox, deleteMailbox, } from '../api/email'; -import { fetchTagCounts, type TagCount } from '../api/tag-counts'; +import { useTagCountsStore } from '../stores/tag-counts-store'; import { useNavigation } from '@react-navigation/native'; import type { NativeStackNavigationProp } from '@react-navigation/native-stack'; import type { RootStackParamList } from '../navigation/types'; @@ -338,7 +338,9 @@ export default function SidebarDrawer({ visible, onClose }: SidebarDrawerProps) title: string; message?: string; initial?: string; confirmLabel: string; onSubmit: (v: string) => void; } | null>(null); const [busy, setBusy] = React.useState(false); - const [tagCounts, setTagCounts] = React.useState>(new Map()); + const tagCounts = useTagCountsStore((s) => s.counts); + const tagCountsGeneration = useTagCountsStore((s) => s.generation); + const ensureTagCounts = useTagCountsStore((s) => s.ensure); React.useEffect(() => { if (!keywordsHydrated) void hydrateKeywords(); }, [keywordsHydrated, hydrateKeywords]); @@ -429,23 +431,17 @@ export default function SidebarDrawer({ visible, onClose }: SidebarDrawerProps) onClose(); }, [setFilters, onClose]); - // 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). + // Tag counts: one query pair per tag and account. A tag view lists the + // tagged mail of the own and the shared accounts, so its badge counts all + // of them (#1038). They are cached until an Email change is reported + // (PF6): opening the drawer costs no request when nothing changed, and + // while it is open a change refreshes them. const tagAccountsKey = React.useMemo(() => spannedAccounts(mailboxes).join('|'), [mailboxes]); React.useEffect(() => { - if (!visible || !tagsExpanded || keywordDefs.length === 0 || !jmapClient.isConnected) return; - let cancelled = false; + if (!visible || !tagsExpanded || keywordDefs.length === 0 || !activeAccountId || !jmapClient.isConnected) return; 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, tagAccountsKey]); + void ensureTagCounts(activeAccountId, keywordDefs.map((k) => k.id), accounts); + }, [visible, tagsExpanded, keywordDefs, tagAccountsKey, activeAccountId, tagCountsGeneration, ensureTagCounts]); // ── Folder actions (long-press) ─────────────────────────────────────── const refFor = (mb: Mailbox) => ({ @@ -1101,7 +1097,7 @@ export default function SidebarDrawer({ visible, onClose }: SidebarDrawerProps) {t('sidebar.tags', 'Tags')} {tagsExpanded && keywordDefs.map((kw) => { - const counts = tagCounts.get(kw.id); + const counts = tagCounts[kw.id]; const isSelected = filters.keyword === keywordToken(kw.id); const dot = c.tags[kw.color]?.dot ?? c.textMuted; return ( diff --git a/src/stores/__tests__/email-store-all-folders.test.ts b/src/stores/__tests__/email-store-all-folders.test.ts index 77803dfa..5abecd7a 100644 --- a/src/stores/__tests__/email-store-all-folders.test.ts +++ b/src/stores/__tests__/email-store-all-folders.test.ts @@ -93,6 +93,7 @@ import * as settingsModule from '../settings-store'; import { useEmailStore, viewerParamsForRow, deleteDestroysAcrossAccounts, accountIdOfRow, } from '../email-store'; +import { useTagCountsStore } from '../tag-counts-store'; import type { Email, Mailbox } from '../../api/types'; const settings = (settingsModule as unknown as { __settings: Record }).__settings; @@ -507,3 +508,19 @@ describe('tag view across the own and the team account (#1038)', () => { expect(queries[0][1]).toMatchObject({ accountId: 'team', filter: { inMailbox: 't-inbox' } }); }); }); + +describe('tag badges (PF6)', () => { + it('go stale on an Email change in any account, not on a Mailbox change', async () => { + const before = useTagCountsStore.getState().generation; + + await useEmailStore.getState().handleStateChange({ + '@type': 'StateChange', changed: { team: { Mailbox: 'm2' } }, + } as never); + expect(useTagCountsStore.getState().generation).toBe(before); + + await useEmailStore.getState().handleStateChange({ + '@type': 'StateChange', changed: { team: { Email: 's2' } }, + } as never); + expect(useTagCountsStore.getState().generation).toBe(before + 1); + }); +}); diff --git a/src/stores/__tests__/tag-counts-store.test.ts b/src/stores/__tests__/tag-counts-store.test.ts new file mode 100644 index 00000000..ac0f637f --- /dev/null +++ b/src/stores/__tests__/tag-counts-store.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// PF6: every drawer open sent 14 Email/query calls for the tag badges, even +// when nothing had changed. The counts are now cached and fetched again only +// after an Email change was reported. + +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { + accountId: 'c', + getMaxCallsInRequest: () => 16, + request: vi.fn(async (calls: Array<[string, Record, string]>) => ({ + methodResponses: calls.map(([name, args, callId]) => [ + name, + { ids: [], total: 'operator' in (args.filter as object) ? 1 : 3 }, + callId, + ]), + })), + }, +})); + +import { jmapClient } from '../../api/jmap-client'; +import { useTagCountsStore } from '../tag-counts-store'; + +const mockRequest = jmapClient.request as ReturnType; +const TAGS = ['red', 'blue', 'green', 'work', 'home', 'todo', 'later']; + +beforeEach(() => { + mockRequest.mockClear(); + useTagCountsStore.setState({ counts: {}, login: null, key: null, generation: 0, fetchedAt: -1 }); +}); + +describe('tag counts cache (PF6)', () => { + it('fetches the 7 tags in one request, then serves the cache', async () => { + const { ensure } = useTagCountsStore.getState(); + await ensure('me', TAGS, [undefined]); + + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls[0][0]).toHaveLength(14); + expect(useTagCountsStore.getState().counts.red).toEqual({ id: 'red', total: 3, unread: 1 }); + + // The drawer opens again: nothing changed, nothing is sent. + await ensure('me', TAGS, [undefined]); + await ensure('me', TAGS, [undefined]); + expect(mockRequest).toHaveBeenCalledTimes(1); + }); + + it('fetches again after an Email change', async () => { + const { ensure, invalidate } = useTagCountsStore.getState(); + await ensure('me', TAGS, [undefined]); + invalidate(); + await ensure('me', TAGS, [undefined]); + expect(mockRequest).toHaveBeenCalledTimes(2); + }); + + it('shares one request between overlapping opens', async () => { + const { ensure } = useTagCountsStore.getState(); + await Promise.all([ensure('me', TAGS, [undefined]), ensure('me', TAGS, [undefined])]); + expect(mockRequest).toHaveBeenCalledTimes(1); + }); + + it('stays stale when a change lands while the counts are in flight', async () => { + const { ensure, invalidate } = useTagCountsStore.getState(); + const first = ensure('me', TAGS, [undefined]); + invalidate(); + await first; + await ensure('me', TAGS, [undefined]); + expect(mockRequest).toHaveBeenCalledTimes(2); + }); + + it('fetches for other tags, other accounts or another login', async () => { + const { ensure } = useTagCountsStore.getState(); + await ensure('me', TAGS, [undefined]); + await ensure('me', [...TAGS, 'new'], [undefined]); + await ensure('me', [...TAGS, 'new'], [undefined, 'team']); + expect(mockRequest).toHaveBeenCalledTimes(1 + 1 + 2); + + // Another login's badges are dropped at once, not shown meanwhile. + const other = ensure('someone-else', TAGS, [undefined]); + expect(useTagCountsStore.getState().counts).toEqual({}); + await other; + expect(useTagCountsStore.getState().counts.red).toBeDefined(); + }); + + it('keeps the old counts when the fetch fails and retries next time', async () => { + const { ensure, invalidate } = useTagCountsStore.getState(); + await ensure('me', TAGS, [undefined]); + invalidate(); + mockRequest.mockRejectedValueOnce(new Error('offline')); + await ensure('me', TAGS, [undefined]); + expect(useTagCountsStore.getState().counts.red).toEqual({ id: 'red', total: 3, unread: 1 }); + + await ensure('me', TAGS, [undefined]); + expect(mockRequest).toHaveBeenCalledTimes(3); + }); +}); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index c5863ef0..4726918b 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -48,6 +48,7 @@ import { t } from './locale-store'; import { useSettingsStore } from './settings-store'; import { useOfflineCacheStore } from './offline-cache-store'; import { useOutboxStore, applyOrQueue, applyOrQueueBatch, type OutboxOp } from './outbox-store'; +import { useTagCountsStore } from './tag-counts-store'; // ── Refresh coalescing ───────────────────────────────────────────────── // Push events, mount effects and post-action follow-ups all call @@ -1159,12 +1160,16 @@ export const useEmailStore = create()( let mailboxChanged = false; let emailChanged = false; + let tagCountsChanged = false; for (const [accountId, accountChanges] of Object.entries(change.changed ?? {})) { if (!known.has(accountId) || !accountChanges) continue; if ('Mailbox' in accountChanges) mailboxChanged = true; + // The sidebar tag badges count every account (PF6, #1038). + if ('Email' in accountChanges) tagCountsChanged = true; if (accountId !== currentAccountId && !spanning) continue; if ('Email' in accountChanges || 'EmailDelivery' in accountChanges) emailChanged = true; } + if (tagCountsChanged) useTagCountsStore.getState().invalidate(); if (!mailboxChanged && !emailChanged) return; if (mailboxChanged) { diff --git a/src/stores/tag-counts-store.ts b/src/stores/tag-counts-store.ts new file mode 100644 index 00000000..4121108d --- /dev/null +++ b/src/stores/tag-counts-store.ts @@ -0,0 +1,62 @@ +import { create } from 'zustand'; +import { fetchTagCounts, type TagCount } from '../api/tag-counts'; + +// Sidebar tag badges (PF6). Every drawer open used to send the full set of +// Email/query pairs again, one pair per tag and account, although nothing +// had changed. The counts are now kept for the login, the accounts and the +// tags they were fetched for, and fetched again only once an Email change +// was reported for one of those accounts (push, or the poll fallback), or +// when the login, the accounts or the tags differ. + +interface TagCountsState { + /** Tag id → counts, summed over the accounts they were fetched for. */ + counts: Record; + /** Login the counts belong to; another login's are dropped, not shown. */ + login: string | null; + /** The login, accounts and tags the counts were fetched for. */ + key: string | null; + /** Bumped by every reported Email change. */ + generation: number; + /** `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; + /** An Email change arrived: the next `ensure` fetches again. */ + invalidate: () => void; +} + +let inflight: { key: string; generation: number; seq: number; promise: Promise } | null = null; +let lastSeq = 0; + +export const useTagCountsStore = create((set, get) => ({ + counts: {}, + login: null, + key: null, + generation: 0, + fetchedAt: -1, + + ensure: (login, tagIds, accountIds) => { + const key = `${login}|${accountIds.map((id) => id ?? '').join(',')}|${tagIds.join(',')}`; + if (get().login !== login) set({ counts: {}, login, key: null }); + const { generation } = get(); + if (get().key === key && get().fetchedAt === generation) return Promise.resolve(); + if (inflight && inflight.key === key && inflight.generation === generation) return inflight.promise; + const seq = ++lastSeq; + const promise = (async () => { + try { + const list = await fetchTagCounts(tagIds, accountIds); + // 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 }); + } catch { + // Counts are decoration: keep the old ones and try again next time. + } finally { + if (inflight?.seq === seq) inflight = null; + } + })(); + inflight = { key, generation, seq, promise }; + return promise; + }, + + invalidate: () => set({ generation: get().generation + 1 }), +}));