mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 00:07:01 +00:00
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.
This commit is contained in:
@@ -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<typeof vi.fn>).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' }];
|
||||
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
+81
-13
@@ -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<string, unknown>;
|
||||
/** 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<string, unknown>;
|
||||
/** Owning JMAP account when the mailbox belongs to a shared account. */
|
||||
accountId?: string;
|
||||
collapseThreads?: boolean;
|
||||
}
|
||||
|
||||
function emailQueryArgs(mailboxId: string | undefined, options?: EmailQueryOptions): Record<string, unknown> {
|
||||
const accountId = options?.accountId ?? jmapClient.accountId;
|
||||
const filter = buildMailboxQueryFilter(mailboxId, options?.filter);
|
||||
const args: Record<string, unknown> = {
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Map<string, number>>(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]);
|
||||
|
||||
|
||||
@@ -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<typeof vi.fn>;
|
||||
const mockGetSharedMailboxes = emailApi.getSharedMailboxes as ReturnType<typeof vi.fn>;
|
||||
const mockQueryEmails = emailApi.queryEmails as ReturnType<typeof vi.fn>;
|
||||
const mockQueryEmailPage = emailApi.queryEmailPage as ReturnType<typeof vi.fn>;
|
||||
const mockGetThreads = emailApi.getThreads as ReturnType<typeof vi.fn>;
|
||||
const mockGetEmailQueryChanges = emailApi.getEmailQueryChanges as ReturnType<typeof vi.fn>;
|
||||
const mockGetEmails = emailApi.getEmails as ReturnType<typeof vi.fn>;
|
||||
const mockGetEmailsWithState = emailApi.getEmailsWithState as ReturnType<typeof vi.fn>;
|
||||
@@ -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<typeof vi.fn>;
|
||||
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] });
|
||||
|
||||
+65
-35
@@ -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<string, number>;
|
||||
|
||||
// ── 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<Email[]> {
|
||||
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<EmailState>()(
|
||||
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<EmailState>()(
|
||||
filters: {},
|
||||
pendingUndo: null,
|
||||
retainedIds: [],
|
||||
threadCounts: {},
|
||||
error: null,
|
||||
loading: false,
|
||||
});
|
||||
@@ -750,6 +743,7 @@ export const useEmailStore = create<EmailState>()(
|
||||
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<EmailState>()(
|
||||
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<EmailState> = { emails: merged, totalEmails: total, loading: false };
|
||||
const updates: Partial<EmailState> = {
|
||||
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<EmailState>()(
|
||||
searchQuery: '',
|
||||
filters: {},
|
||||
retainedIds: [],
|
||||
threadCounts: {},
|
||||
}),
|
||||
}),
|
||||
{
|
||||
@@ -1894,6 +1893,34 @@ async function fetchMailboxesImpl(activeAccountId: string): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
// Record Thread/get results as conversation sizes for the list's badges.
|
||||
function withThreadCounts(counts: Record<string, number>, threads: Thread[]): Record<string, number> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
const get = useEmailStore.getState;
|
||||
const set = useEmailStore.setState;
|
||||
@@ -2053,6 +2080,7 @@ async function refreshEmailsImpl(): Promise<void> {
|
||||
},
|
||||
},
|
||||
});
|
||||
void fillThreadCounts(trimmed, ref.accountId);
|
||||
return;
|
||||
}
|
||||
// queryChanges === null → cannotCalculateChanges. Drop our queryState
|
||||
@@ -2062,41 +2090,43 @@ async function refreshEmailsImpl(): Promise<void> {
|
||||
// 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<ReturnType<typeof queryEmails>>;
|
||||
// One request carries the query, its messages and (for the
|
||||
// conversation badges) their threads.
|
||||
const threads = !useSettingsStore.getState().disableThreading;
|
||||
let queryRes: Awaited<ReturnType<typeof queryEmailPage>>;
|
||||
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<EmailState> = {
|
||||
// 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,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user