From d98793003467d7bc341c644f02c25fec1d3b5421 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:59:08 +0200 Subject: [PATCH] fix: load each list page and search in one request Opening a folder, searching or loading more took three sequential round trips: `Email/query`, then `Email/get` of its ids, then a `Thread/get` the list screen fired after rendering to size the conversation badges. - `queryEmailPage` sends all three in one request, chaining `Email/get` to the query's `/ids` and `Thread/get` to the fetched messages' `/list/*/threadId` (left out while threading is off). The page is capped at `maxObjectsInGet` so the chained get can't be refused, and a failed `Thread/get` keeps the page. - The full refresh, search and load-more use it; load-more no longer needs its own chunked `Email/get`. - The thread sizes now live in the email store (`threadCounts`, cleared on folder or account change) and the list screen reads them instead of fetching them itself. Rows restored from a snapshot or added by an incremental refresh get theirs with one `Thread/get` after the refresh. - The chained `Email/get` also reports the Email state for an empty folder, so that list gets its own `Email/changes` baseline. --- src/api/__tests__/email.test.ts | 96 ++++++++++++++ src/api/__tests__/live-stalwart.test.ts | 14 +++ src/api/email.ts | 94 ++++++++++++-- src/screens/EmailListScreen.tsx | 32 +---- src/stores/__tests__/email-store.test.ts | 152 ++++++++++++++++++++--- src/stores/email-store.ts | 100 +++++++++------ 6 files changed, 398 insertions(+), 90 deletions(-) diff --git a/src/api/__tests__/email.test.ts b/src/api/__tests__/email.test.ts index e15c4555..5a64b4d0 100644 --- a/src/api/__tests__/email.test.ts +++ b/src/api/__tests__/email.test.ts @@ -18,6 +18,7 @@ import { getMailboxes, getSharedMailboxes, queryEmails, + queryEmailPage, getEmails, getFullEmail, getThread, @@ -211,6 +212,101 @@ describe('email operations', () => { }); }); + describe('queryEmailPage', () => { + it('chains Email/query, Email/get and Thread/get in one request', async () => { + mockRequest.mockResolvedValue({ + methodResponses: [ + ['Email/query', { ids: ['e2', 'e1'], total: 7, queryState: 'q-1' }, '0'], + // Email/get may answer in another order than the query's. + ['Email/get', { list: [{ id: 'e1', threadId: 't1' }, { id: 'e2', threadId: 't1' }], state: 's-1' }, '1'], + ['Thread/get', { list: [{ id: 't1', emailIds: ['e1', 'e2', 'e3'] }] }, '2'], + ], + }); + + const result = await queryEmailPage('mb-1', { limit: 25, position: 50, threads: true }); + + expect(mockRequest).toHaveBeenCalledTimes(1); + const calls = mockRequest.mock.calls[0][0]; + expect(calls).toEqual([ + ['Email/query', expect.objectContaining({ + accountId: 'acc-1', filter: { inMailbox: 'mb-1' }, position: 50, limit: 25, calculateTotal: true, + }), '0'], + ['Email/get', expect.objectContaining({ + accountId: 'acc-1', + '#ids': { resultOf: '0', name: 'Email/query', path: '/ids' }, + }), '1'], + ['Thread/get', { + accountId: 'acc-1', + '#ids': { resultOf: '1', name: 'Email/get', path: '/list/*/threadId' }, + }, '2'], + ]); + expect(calls[1][1].properties).toContain('preview'); + expect(calls[1][1].properties).not.toContain('bodyStructure'); + expect(result).toEqual({ + ids: ['e2', 'e1'], + total: 7, + queryState: 'q-1', + list: [{ id: 'e2', threadId: 't1' }, { id: 'e1', threadId: 't1' }], + state: 's-1', + threads: [{ id: 't1', emailIds: ['e1', 'e2', 'e3'] }], + }); + }); + + it('leaves Thread/get out unless asked, and targets a shared account', async () => { + mockRequest.mockResolvedValue({ + methodResponses: [ + ['Email/query', { ids: [], total: 0, queryState: 'q-1' }, '0'], + ['Email/get', { list: [], state: 's-1' }, '1'], + ], + }); + + const result = await queryEmailPage('mb-1', { accountId: 'grp-1' }); + + const calls = mockRequest.mock.calls[0][0]; + expect(calls.map((c: unknown[]) => c[0])).toEqual(['Email/query', 'Email/get']); + expect(calls[0][1].accountId).toBe('grp-1'); + expect(calls[1][1].accountId).toBe('grp-1'); + // An empty page still reports the Email state. + expect(result).toMatchObject({ list: [], state: 's-1', threads: [] }); + }); + + it('keeps the page when Thread/get fails, and fails with the query', async () => { + mockRequest.mockResolvedValueOnce({ + methodResponses: [ + ['Email/query', { ids: ['e1'], total: 1 }, '0'], + ['Email/get', { list: [{ id: 'e1', threadId: 't1' }], state: 's-1' }, '1'], + ['error', { type: 'serverFail' }, '2'], + ], + }); + const result = await queryEmailPage('mb-1', { threads: true }); + expect(result.list).toEqual([{ id: 'e1', threadId: 't1' }]); + expect(result.threads).toEqual([]); + + mockRequest.mockResolvedValueOnce({ + methodResponses: [ + ['error', { type: 'unsupportedSort' }, '0'], + ['error', { type: 'invalidResultReference' }, '1'], + ['error', { type: 'invalidResultReference' }, '2'], + ], + }); + await expect(queryEmailPage('mb-1', { threads: true })).rejects.toMatchObject({ type: 'unsupportedSort' }); + }); + + it('caps the page at the server\'s maxObjectsInGet', async () => { + (jmapClient.getMaxObjectsInGet as ReturnType).mockReturnValueOnce(100); + mockRequest.mockResolvedValue({ + methodResponses: [ + ['Email/query', { ids: [], total: 0 }, '0'], + ['Email/get', { list: [], state: 's-1' }, '1'], + ], + }); + + await queryEmailPage('mb-1', { limit: 250 }); + + expect(mockRequest.mock.calls[0][0][0][1].limit).toBe(100); + }); + }); + describe('getEmails', () => { it('should fetch emails by id with list properties', async () => { const emails = [{ id: 'e1', subject: 'Test' }]; diff --git a/src/api/__tests__/live-stalwart.test.ts b/src/api/__tests__/live-stalwart.test.ts index f37f5c4a..14bc435c 100644 --- a/src/api/__tests__/live-stalwart.test.ts +++ b/src/api/__tests__/live-stalwart.test.ts @@ -18,6 +18,7 @@ import { JMAPClient } from '../jmap-client'; import { getMailboxes, queryEmails, + queryEmailPage, getFullEmails, getThreadEmails, createDraft, @@ -157,6 +158,19 @@ live('live Stalwart', () => { expect([...times].sort((a, b) => a - b)).toEqual(times); }, 60_000); + it('loads a list page with its threads in one chained request (PF7)', async () => { + const page = await queryEmailPage(inbox, { limit: 20, threads: true }); + const { ids, total } = await queryEmails(inbox, { limit: 20 }); + expect(page.ids).toEqual(ids); + expect(page.total).toBe(total); + expect(page.list.map((e) => e.id)).toEqual(ids); + expect(page.state).toBeTruthy(); + const threadIds = new Set(page.list.map((e) => e.threadId)); + expect(new Set(page.threads.map((t) => t.id))).toEqual(threadIds); + const reply = page.list.find((e) => (e.subject ?? '').startsWith('Re: '))!; + expect(page.threads.find((t) => t.id === reply.threadId)!.emailIds.length).toBeGreaterThanOrEqual(2); + }, 60_000); + it('saves a draft into Drafts with $draft', async () => { const id = await createDraft( { diff --git a/src/api/email.ts b/src/api/email.ts index e3998caf..158a56b8 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -345,18 +345,17 @@ function buildMailboxQueryFilter( return { ...inMailbox, ...userFilter }; } -export async function queryEmails( - mailboxId: string | undefined, - options?: { - position?: number; - limit?: number; - sort?: Array<{ property: string; isAscending: boolean; keyword?: string }>; - filter?: Record; - /** Owning JMAP account when the mailbox belongs to a shared account. */ - accountId?: string; - collapseThreads?: boolean; - }, -): Promise<{ ids: string[]; total: number; queryState?: string }> { +interface EmailQueryOptions { + position?: number; + limit?: number; + sort?: Array<{ property: string; isAscending: boolean; keyword?: string }>; + filter?: Record; + /** Owning JMAP account when the mailbox belongs to a shared account. */ + accountId?: string; + collapseThreads?: boolean; +} + +function emailQueryArgs(mailboxId: string | undefined, options?: EmailQueryOptions): Record { const accountId = options?.accountId ?? jmapClient.accountId; const filter = buildMailboxQueryFilter(mailboxId, options?.filter); const args: Record = { @@ -368,7 +367,14 @@ export async function queryEmails( calculateTotal: true, }; if (options?.collapseThreads) args.collapseThreads = true; - const res = await jmapClient.request([['Email/query', args, '0']]); + return args; +} + +export async function queryEmails( + mailboxId: string | undefined, + options?: EmailQueryOptions, +): Promise<{ ids: string[]; total: number; queryState?: string }> { + const res = await jmapClient.request([['Email/query', emailQueryArgs(mailboxId, options), '0']]); const body = requireMethodResult(res, '0', 'Email/query'); return { ids: (body.ids as string[]) ?? [], @@ -377,6 +383,68 @@ export async function queryEmails( }; } +/** + * One page of a message list in a single request: Email/query, Email/get of + * the ids it returns and, with `threads`, Thread/get of those messages' + * threads (for the conversation-size badges), chained with result references + * instead of three round trips. `state` is the Email state the rows were + * read at. + */ +export async function queryEmailPage( + mailboxId: string | undefined, + options?: EmailQueryOptions & { threads?: boolean }, +): Promise<{ + ids: string[]; + total: number; + queryState?: string; + list: Email[]; + state?: string; + threads: Thread[]; +}> { + // The chained Email/get takes every id the query returns, so a page can't + // be larger than the server lets one /get fetch. + const args = emailQueryArgs(mailboxId, { + ...options, + limit: Math.min(options?.limit ?? 50, maxInGet()), + }); + const accountId = args.accountId; + const calls: JMAPMethodCall[] = [ + ['Email/query', args, '0'], + ['Email/get', { + accountId, + '#ids': { resultOf: '0', name: 'Email/query', path: '/ids' }, + properties: EMAIL_LIST_PROPERTIES, + }, '1'], + ]; + if (options?.threads) { + calls.push(['Thread/get', { + accountId, + '#ids': { resultOf: '1', name: 'Email/get', path: '/list/*/threadId' }, + }, '2']); + } + const res = await jmapClient.request(calls); + const query = requireMethodResult(res, '0', 'Email/query'); + const got = requireMethodResult(res, '1', 'Email/get'); + const ids = (query.ids as string[]) ?? []; + let threads: Thread[] = []; + if (options?.threads) { + // Thread sizes only decorate the rows: a failed Thread/get keeps the page. + try { + threads = (requireMethodResult(res, '2', 'Thread/get').list as Thread[]) ?? []; + } catch { /* the list falls back to the in-page count */ } + } + // Email/get may answer in any order; keep the query's. + const byId = new Map(((got.list as Email[]) ?? []).map((e) => [e.id, e])); + return { + ids, + total: (query.total as number) ?? 0, + queryState: query.queryState as string | undefined, + list: ids.flatMap((id) => byId.get(id) ?? []), + state: got.state as string | undefined, + threads, + }; +} + export interface EmailQueryChangesResult { oldQueryState: string; newQueryState: string; diff --git a/src/screens/EmailListScreen.tsx b/src/screens/EmailListScreen.tsx index b3b8aedd..abb7bd36 100644 --- a/src/screens/EmailListScreen.tsx +++ b/src/screens/EmailListScreen.tsx @@ -41,7 +41,7 @@ import { } from '../lib/thread-utils'; import { isPermanentDelete, confirmPermanentDelete } from '../lib/delete-confirm'; import { draftContextFromEmail, isDraftEmail } from '../lib/draft-context'; -import { getThreads, getFullEmail, emptyMailbox as apiEmptyMailbox } from '../api/email'; +import { getFullEmail, emptyMailbox as apiEmptyMailbox } from '../api/email'; import type { RootStackParamList } from '../navigation/types'; import type { Email } from '../api/types'; @@ -334,35 +334,15 @@ export default function EmailListScreen({ onEmailPress, onComposePress }: EmailL ); // Real conversation sizes from Thread/get (a thread's other messages may - // live in other folders); the loaded-page count is the fallback until the - // response lands and for messages whose thread the server no longer knows. - const [serverThreadCounts, setServerThreadCounts] = React.useState>(new Map()); - React.useEffect(() => { - if (disableThreading || visibleEmails.length === 0) return; - const ids = Array.from(new Set(visibleEmails.map((e) => e.threadId).filter(Boolean))); - const missing = ids.filter((id) => !serverThreadCounts.has(id)); - if (missing.length === 0) return; - let cancelled = false; - getThreads(missing, currentOwnerAccountId) - .then((threads) => { - if (cancelled) return; - setServerThreadCounts((prev) => { - const next = new Map(prev); - for (const th of threads) next.set(th.id, th.emailIds.length); - return next; - }); - }) - .catch(() => { /* fall back to the in-page count */ }); - return () => { cancelled = true; }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [visibleEmails, disableThreading, currentOwnerAccountId]); - // Counts are per account and per snapshot; drop them on folder switch. - React.useEffect(() => { setServerThreadCounts(new Map()); }, [currentMailboxId, disableThreading]); + // live in other folders), fetched by the store together with each list + // page; the loaded-page count is the fallback until they land and for + // messages whose thread the server no longer knows. + const serverThreadCounts = useEmailStore((s) => s.threadCounts); const threadCountFor = React.useCallback((e: Email): number => { if (disableThreading) return 1; const local = threadGroups.get(threadKeyOf(e, false))?.length ?? 1; - const server = e.threadId ? serverThreadCounts.get(e.threadId) : undefined; + const server = e.threadId ? serverThreadCounts[e.threadId] : undefined; return Math.max(local, server ?? 1); }, [disableThreading, threadGroups, serverThreadCounts]); diff --git a/src/stores/__tests__/email-store.test.ts b/src/stores/__tests__/email-store.test.ts index c1b11193..236077e4 100644 --- a/src/stores/__tests__/email-store.test.ts +++ b/src/stores/__tests__/email-store.test.ts @@ -9,10 +9,14 @@ vi.mock('../../api/email', () => ({ getMailboxesByIds: vi.fn(async () => ({ list: [], state: 'mb-state-0' })), getMailboxChanges: vi.fn(async () => null), queryEmails: vi.fn(), + // A list page: Email/query, Email/get and Thread/get in one request. + // beforeEach scripts it from the queryEmails / getEmailsWithState mocks. + queryEmailPage: vi.fn(), getEmailQueryChanges: vi.fn(async () => null), getEmails: vi.fn(), getEmailsWithState: vi.fn(async () => ({ list: [], state: 'em-state-0' })), getEmailChanges: vi.fn(async () => null), + getThreads: vi.fn(async () => []), getFullEmail: vi.fn(), importEmailBlob: vi.fn(async () => 'imported-1'), patchKeywordsForEmails: vi.fn(), @@ -151,6 +155,8 @@ import { useSettingsStore } from '../settings-store'; const mockGetMailboxesWithState = emailApi.getMailboxesWithState as ReturnType; const mockGetSharedMailboxes = emailApi.getSharedMailboxes as ReturnType; const mockQueryEmails = emailApi.queryEmails as ReturnType; +const mockQueryEmailPage = emailApi.queryEmailPage as ReturnType; +const mockGetThreads = emailApi.getThreads as ReturnType; const mockGetEmailQueryChanges = emailApi.getEmailQueryChanges as ReturnType; const mockGetEmails = emailApi.getEmails as ReturnType; const mockGetEmailsWithState = emailApi.getEmailsWithState as ReturnType; @@ -168,6 +174,15 @@ beforeEach(() => { // doesn't short-circuit the tests. useEmailStore.setState({ activeAccountId: TEST_ACCOUNT_ID }); useSettingsStore.getState().updateSetting('mailSortAscending', false); + // Most tests describe a list page as its query and its Email/get; the + // chained-request tests below script queryEmailPage directly. + mockQueryEmailPage.mockImplementation(async (mailboxId: string | undefined, opts?: { accountId?: string }) => { + const query = await emailApi.queryEmails(mailboxId, opts); + const got = query.ids.length > 0 + ? await emailApi.getEmailsWithState(query.ids, opts?.accountId) + : { list: [], state: 'em-state-0' }; + return { ...query, list: got.list, state: got.state, threads: [] }; + }); }); describe('email-store', () => { @@ -217,9 +232,8 @@ describe('email-store', () => { await useEmailStore.getState().selectMailbox('mb-empty'); expect(useEmailStore.getState().emails).toEqual([]); - // First-time load with no ids hits getEmailsWithState only to prime - // emailState (with an empty ids array). It should NOT call legacy - // getEmails — that path is reserved for pagination/search. + // The page's chained Email/get still reports the folder's Email state. + expect(useEmailStore.getState().emailStates).toEqual({ 'mb-empty': 'em-state-0' }); expect(mockGetEmails).not.toHaveBeenCalled(); }); }); @@ -234,8 +248,9 @@ describe('email-store', () => { loading: false, }); - mockQueryEmails.mockResolvedValue({ ids: ['e2'], total: 2 }); - mockGetEmails.mockResolvedValue([{ id: 'e2', subject: 'Email 2' }]); + mockQueryEmailPage.mockResolvedValue({ + ids: ['e2'], total: 2, list: [{ id: 'e2', subject: 'Email 2' }], threads: [], + }); await useEmailStore.getState().loadMoreEmails(); @@ -267,12 +282,13 @@ describe('email-store', () => { totalEmails: 10, loading: false, }); - mockQueryEmails.mockResolvedValue({ ids: ['e4', 'e5'], total: 9 }); - mockGetEmails.mockResolvedValue([{ id: 'e4' }, { id: 'e5' }]); + mockQueryEmailPage.mockResolvedValue({ + ids: ['e4', 'e5'], total: 9, list: [{ id: 'e4' }, { id: 'e5' }], threads: [], + }); await useEmailStore.getState().loadMoreEmails(); - expect(mockQueryEmails).toHaveBeenCalledWith('mb-1', expect.objectContaining({ + expect(mockQueryEmailPage).toHaveBeenCalledWith('mb-1', expect.objectContaining({ position: 2, filter: { notKeyword: '$seen' }, })); @@ -292,12 +308,11 @@ describe('email-store', () => { totalEmails: 4, loading: false, }); - mockQueryEmails.mockResolvedValue({ ids: ['e5'], total: 4 }); - mockGetEmails.mockResolvedValue([{ id: 'e5' }]); + mockQueryEmailPage.mockResolvedValue({ ids: ['e5'], total: 4, list: [{ id: 'e5' }], threads: [] }); await useEmailStore.getState().loadMoreEmails(); - expect(mockQueryEmails).toHaveBeenCalledWith('mb-1', expect.objectContaining({ position: 3 })); + expect(mockQueryEmailPage).toHaveBeenCalledWith('mb-1', expect.objectContaining({ position: 3 })); expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e1', 'e2', 'e3', 'e4', 'e5']); }); }); @@ -709,12 +724,12 @@ describe('email-store', () => { emails: [{ id: 'e1' } as any, { id: 'e2' } as any], totalEmails: 10, }); - mockQueryEmails.mockResolvedValue({ ids: ['e2', 'e3'], total: 10 }); - mockGetEmails.mockResolvedValue([{ id: 'e3' }]); + mockQueryEmailPage.mockResolvedValue({ + ids: ['e2', 'e3'], total: 10, list: [{ id: 'e2' }, { id: 'e3' }], threads: [], + }); await useEmailStore.getState().loadMoreEmails(); - expect(mockGetEmails).toHaveBeenCalledWith(['e3'], undefined); expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e1', 'e2', 'e3']); }); @@ -984,11 +999,11 @@ describe('email-store', () => { it('an empty folder keeps both its own and the other lists\' Email state', async () => { openInbox({ emailStates: { 'mb-1': 'em-1', 'mb-empty': 'em-0' } }); - mockQueryEmails.mockResolvedValue({ ids: [], total: 0, queryState: 'q-empty' }); + // An empty result from a server that leaves `state` out of Email/get. + mockQueryEmailPage.mockResolvedValueOnce({ ids: [], total: 0, queryState: 'q-empty', list: [], threads: [] }); await useEmailStore.getState().selectMailbox('mb-empty'); - expect(mockGetEmailsWithState).not.toHaveBeenCalled(); expect(useEmailStore.getState().emailStates).toEqual({ 'mb-1': 'em-1', 'mb-empty': 'em-0' }); // Back in the Inbox, the refresh still runs Email/changes from the @@ -1051,6 +1066,111 @@ describe('email-store', () => { }); }); + // PF7: a list page or search used to be Email/query, then Email/get, then a + // Thread/get the list screen fired after rendering. + describe('list page requests', () => { + const page = { + ids: ['e1', 'e2'], total: 2, queryState: 'q-1', state: 'em-1', + list: [{ id: 'e1', threadId: 't1', keywords: {} }, { id: 'e2', threadId: 't2', keywords: {} }], + threads: [{ id: 't1', emailIds: ['e1', 'x1', 'x2'] }, { id: 't2', emailIds: ['e2'] }], + }; + + it('loads a folder page with its messages and thread sizes in one request', async () => { + useEmailStore.setState({ currentMailboxId: 'mb-1' }); + mockQueryEmailPage.mockResolvedValueOnce(page); + + await useEmailStore.getState().refreshEmails(); + + expect(mockQueryEmailPage).toHaveBeenCalledTimes(1); + expect(mockQueryEmailPage).toHaveBeenCalledWith('mb-1', expect.objectContaining({ limit: 25, threads: true })); + expect(mockQueryEmails).not.toHaveBeenCalled(); + expect(mockGetEmailsWithState).not.toHaveBeenCalled(); + expect(mockGetThreads).not.toHaveBeenCalled(); + const state = useEmailStore.getState(); + expect(state.emails.map((e) => e.id)).toEqual(['e1', 'e2']); + expect(state.threadCounts).toEqual({ t1: 3, t2: 1 }); + expect(state.emailStates).toEqual({ 'mb-1': 'em-1' }); + }); + + it('runs a search as the same single request', async () => { + useEmailStore.setState({ currentMailboxId: 'mb-1' }); + mockQueryEmailPage.mockResolvedValue(page); + + useEmailStore.getState().setSearchQuery('invoice'); + await vi.waitFor(() => expect(useEmailStore.getState().threadCounts).toEqual({ t1: 3, t2: 1 })); + + expect(mockQueryEmailPage).toHaveBeenCalledTimes(1); + expect(mockQueryEmailPage.mock.calls[0][1]).toMatchObject({ filter: { text: 'invoice*' }, threads: true }); + expect(mockGetThreads).not.toHaveBeenCalled(); + }); + + it('leaves Thread/get out when threading is off', async () => { + useSettingsStore.getState().updateSetting('disableThreading', true); + try { + useEmailStore.setState({ currentMailboxId: 'mb-1' }); + mockQueryEmailPage.mockResolvedValueOnce({ ...page, threads: [] }); + + await useEmailStore.getState().refreshEmails(); + + expect(mockQueryEmailPage).toHaveBeenCalledWith('mb-1', expect.objectContaining({ threads: false })); + } finally { + useSettingsStore.getState().updateSetting('disableThreading', false); + } + }); + + it('load-more adds the next page\'s thread sizes', async () => { + useEmailStore.setState({ + currentMailboxId: 'mb-1', + emails: [{ id: 'e0', threadId: 't0' } as any], + totalEmails: 3, + threadCounts: { t0: 2 }, + }); + mockQueryEmailPage.mockResolvedValueOnce({ ...page, total: 3 }); + + await useEmailStore.getState().loadMoreEmails(); + + expect(mockQueryEmailPage).toHaveBeenCalledWith('mb-1', expect.objectContaining({ position: 1, threads: true })); + expect(mockGetThreads).not.toHaveBeenCalled(); + expect(useEmailStore.getState().threadCounts).toEqual({ t0: 2, t1: 3, t2: 1 }); + }); + + it('fetches the thread sizes an incremental refresh has none for', async () => { + const mockGetEmailChanges = emailApi.getEmailChanges as ReturnType; + const rows = page.list as any[]; + useEmailStore.setState({ + currentMailboxId: 'mb-1', + emails: rows, + totalEmails: 2, + queryState: 'q-1', + emailStates: { 'mb-1': 'em-1' }, + threadCounts: { t1: 3 }, + mailboxSnapshots: { 'mb-1': { emails: rows, total: 2, queryState: 'q-1' } }, + }); + mockGetEmailQueryChanges.mockResolvedValue({ + oldQueryState: 'q-1', newQueryState: 'q-2', total: 2, removed: [], added: [], + }); + mockGetEmailChanges.mockResolvedValue({ + oldState: 'em-1', newState: 'em-2', hasMoreChanges: false, created: [], updated: [], destroyed: [], + }); + mockGetThreads.mockResolvedValueOnce([{ id: 't2', emailIds: ['e2', 'x3'] }]); + + await useEmailStore.getState().refreshEmails(); + + await vi.waitFor(() => expect(useEmailStore.getState().threadCounts).toEqual({ t1: 3, t2: 2 })); + expect(mockGetThreads).toHaveBeenCalledWith(['t2'], undefined); + expect(mockQueryEmailPage).not.toHaveBeenCalled(); + }); + + it('drops the thread sizes when another folder opens', async () => { + useEmailStore.setState({ currentMailboxId: 'mb-1', threadCounts: { t1: 3 } }); + mockQueryEmailPage.mockResolvedValueOnce({ ids: [], total: 0, list: [], threads: [] }); + + await useEmailStore.getState().selectMailbox('mb-2'); + + expect(useEmailStore.getState().threadCounts).toEqual({}); + }); + }); + describe('pin and spam keywords', () => { it('togglePin writes $pinned, not $important', async () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: {} } as any] }); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index 1e0872a6..cb4914b6 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -2,7 +2,7 @@ import { create } from 'zustand'; import { persist } from 'zustand/middleware'; import { createPersistStorage, memoizeSlice } from './persist-storage'; import { boundEmailCache, type PersistedEmailCache } from './email-cache-persist'; -import type { Email, Mailbox, StateChange } from '../api/types'; +import type { Email, Mailbox, StateChange, Thread } from '../api/types'; import { jmapClient } from '../api/jmap-client'; import { getMailboxes as fetchMailboxes, @@ -10,11 +10,12 @@ import { getSharedMailboxes, getMailboxesByIds, getMailboxChanges, - queryEmails, + queryEmailPage, getEmailQueryChanges, getEmails as fetchEmails, getEmailsWithState, getEmailChanges, + getThreads, getFullEmail, importEmailBlob, patchKeywordsForEmails, @@ -290,6 +291,12 @@ export interface EmailState { * `retainedInViewIds`). */ retainedIds: string[]; + /** + * Conversation sizes (Thread/get `emailIds.length`) for the loaded rows' + * threads, keyed by thread id: a thread's other messages may live in other + * folders. Cleared when the folder or account changes. + */ + threadCounts: Record; // ── Actions ──────────────────────────────────────────────────── setActiveAccount: (accountId: string | null) => void; @@ -524,22 +531,6 @@ function viewFromSnapshot(snap: AccountSnapshot | null): { }; } -// JMAP's maxObjectsInGet bounds how many ids we can pull in one Email/get. -// Chunk to that ceiling (with a small safety fallback) so large change sets -// don't trip 429/413 responses. -async function fetchEmailsChunked(ids: string[], accountId?: string): Promise { - if (ids.length === 0) return []; - const cap = Math.max(1, jmapClient.getMaxObjectsInGet()); - const chunk = Math.min(cap, 200); - if (ids.length <= chunk) return fetchEmails(ids, accountId); - const out: Email[] = []; - for (let i = 0; i < ids.length; i += chunk) { - const slice = await fetchEmails(ids.slice(i, i + chunk), accountId); - out.push(...slice); - } - return out; -} - // Merge a fresh batch of emails into an existing list keyed by id. New entries // replace stale ones (keywords/mailboxIds may have changed); destroyed ids are // dropped. Order is preserved according to the supplied id order — pass the @@ -583,6 +574,7 @@ export const useEmailStore = create()( filters: {}, pendingUndo: null, retainedIds: [], + threadCounts: {}, // Swap which account's data is currently visible. The previous account's // view is tucked into accountSnapshots so a return-trip can restore it @@ -610,6 +602,7 @@ export const useEmailStore = create()( filters: {}, pendingUndo: null, retainedIds: [], + threadCounts: {}, error: null, loading: false, }); @@ -750,6 +743,7 @@ export const useEmailStore = create()( error: null, pendingUndo: null, retainedIds: [], + threadCounts: {}, }); // Stop here if there's no live session OR jmapClient is mid-transition @@ -780,25 +774,29 @@ export const useEmailStore = create()( const scope = queryScope(state, refFor(state.mailboxes, currentMailboxId)); const filter = buildJmapFilter(searchQuery, filters); const limit = useSettingsStore.getState().emailsPerPage; - const { ids, total } = await queryEmails(scope.mailboxId, { + const { list, total, threads } = await queryEmailPage(scope.mailboxId, { position, limit, sort: await resolveSort(state, scope.accountId), filter, accountId: scope.accountId, + threads: !useSettingsStore.getState().disableThreading, }); if (get().activeAccountId !== activeAccountId || get().currentMailboxId !== currentMailboxId) return; // A message that arrived between pages shifts positions and would come // back a second time — drop ids we already show (duplicate keys). const existingIds = new Set(get().emails.map((e) => e.id)); - const fresh = ids.filter((id) => !existingIds.has(id)); - const newEmails = fresh.length > 0 ? await fetchEmailsChunked(fresh, scope.accountId) : []; - if (get().activeAccountId !== activeAccountId || get().currentMailboxId !== currentMailboxId) return; + const newEmails = list.filter((e) => !existingIds.has(e.id)); const merged = [...get().emails, ...newEmails]; // The server's current count: a read in the Unread view has shrunk it // since the list was loaded, and a stale total keeps load-more asking // for a page that isn't there. - const updates: Partial = { emails: merged, totalEmails: total, loading: false }; + const updates: Partial = { + emails: merged, + totalEmails: total, + threadCounts: withThreadCounts(get().threadCounts, threads), + loading: false, + }; if (isBaseView(searchQuery, filters)) { updates.mailboxSnapshots = { ...get().mailboxSnapshots, @@ -1649,6 +1647,7 @@ export const useEmailStore = create()( searchQuery: '', filters: {}, retainedIds: [], + threadCounts: {}, }), }), { @@ -1894,6 +1893,34 @@ async function fetchMailboxesImpl(activeAccountId: string): Promise { } } +// Record Thread/get results as conversation sizes for the list's badges. +function withThreadCounts(counts: Record, threads: Thread[]): Record { + if (threads.length === 0) return counts; + const next = { ...counts }; + for (const th of threads) next[th.id] = th.emailIds.length; + return next; +} + +// Conversation sizes for rows no list page brought any for: restored from a +// snapshot, or added by Email/queryChanges. Fire-and-forget; the loaded-page +// count stands in until they land (and if the request fails). +async function fillThreadCounts(emails: Email[], accountId: string | undefined): Promise { + if (useSettingsStore.getState().disableThreading) return; + const { threadCounts, currentMailboxId, activeAccountId } = useEmailStore.getState(); + const missing = Array.from(new Set( + emails.map((e) => e.threadId).filter((id) => id && threadCounts[id] === undefined), + )); + if (missing.length === 0) return; + try { + const threads = await getThreads(missing, accountId); + const now = useEmailStore.getState(); + if (now.currentMailboxId !== currentMailboxId || now.activeAccountId !== activeAccountId) return; + useEmailStore.setState({ threadCounts: withThreadCounts(now.threadCounts, threads) }); + } catch { + /* keep the loaded-page count */ + } +} + async function refreshEmailsImpl(): Promise { const get = useEmailStore.getState; const set = useEmailStore.setState; @@ -2053,6 +2080,7 @@ async function refreshEmailsImpl(): Promise { }, }, }); + void fillThreadCounts(trimmed, ref.accountId); return; } // queryChanges === null → cannotCalculateChanges. Drop our queryState @@ -2062,41 +2090,43 @@ async function refreshEmailsImpl(): Promise { // Full re-query path. Used when there's no prior queryState, when the // user has search/filters active (queryState only tracks the base // query), or when the server returned cannotCalculateChanges above. - let queryRes: Awaited>; + // One request carries the query, its messages and (for the + // conversation badges) their threads. + const threads = !useSettingsStore.getState().disableThreading; + let queryRes: Awaited>; try { - queryRes = await queryEmails(scope.mailboxId, { limit, sort, filter, accountId: scope.accountId }); + queryRes = await queryEmailPage(scope.mailboxId, { limit, sort, filter, accountId: scope.accountId, threads }); } catch (err) { // The server refused a hasKeyword comparator (unsupportedSort): drop // the keyword levels for this account and re-run with the rest. if (!isUnsupportedSort(err)) throw err; markKeywordSortUnsupported(scope.accountId ?? jmapClient.accountId); sort = await resolveSort(state, scope.accountId); - queryRes = await queryEmails(scope.mailboxId, { limit, sort, filter, accountId: scope.accountId }); + queryRes = await queryEmailPage(scope.mailboxId, { limit, sort, filter, accountId: scope.accountId, threads }); } - const fetched = queryRes.ids.length > 0 - ? await getEmailsWithState(queryRes.ids, scope.accountId) - : { list: [], state: undefined as string | undefined }; if (viewChanged()) return; const updates: Partial = { // Rows the user just read/unstarred in this filtered view stay put // until the view is re-opened, instead of vanishing under them. - emails: baseView ? fetched.list : mergeRetainedRows(get().emails, fetched.list, get().retainedIds), + emails: baseView ? queryRes.list : mergeRetainedRows(get().emails, queryRes.list, get().retainedIds), totalEmails: queryRes.total, + threadCounts: withThreadCounts(get().threadCounts, queryRes.threads), loading: false, }; if (baseView) { updates.queryState = queryRes.queryState; - // An empty result ran no Email/get, so it has no state to record: - // keep the list's previous one rather than dropping it. - if (fetched.state) { - updates.emailStates = withEmailState(get().emailStates, currentMailboxId, fetched.state); + // Email/get reports a state even for an empty result; should a + // server leave it out, keep the list's previous one rather than + // dropping it. + if (queryRes.state) { + updates.emailStates = withEmailState(get().emailStates, currentMailboxId, queryRes.state); } updates.mailboxSnapshots = { ...get().mailboxSnapshots, [currentMailboxId]: { - emails: fetched.list, + emails: queryRes.list, total: queryRes.total, queryState: queryRes.queryState, },