mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: list and count tagged mail from every account in the tag view #1038
A tag sits on messages in the user's own account and in the shared (group) accounts alike, but the tag view only asked the account of the open folder, and the sidebar badge only counted the own account: with 2 own and 2 team messages tagged Red, the Red view showed 2, and which 2 depended on the folder it was opened from. The tag view now fans out like the "All folders" search (#1082), which it is: every folder of the own account and of each shared account with folders in the sidebar, in one request, with rows stamped with their account so opening, untagging and every other action reach the right one. A search inside the tag view narrows it in every account. The tag badges sum the counts of the same accounts; the query pairs for all of them go out packed into as few requests as `maxCallsInRequest` allows, and an account that can't be reached counts nothing instead of failing the badges.
This commit is contained in:
@@ -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<typeof vi.fn>;
|
||||
const mockMaxCalls = jmapClient.getMaxCallsInRequest as ReturnType<typeof vi.fn>;
|
||||
|
||||
// Answers every Email/query with `totals[accountId][callId prefix]`, or an
|
||||
// error for an account listed in `failing`.
|
||||
function answer(totals: Record<string, { total: number; unread: number }>, failing: string[] = []) {
|
||||
mockRequest.mockImplementation(async (calls: Array<[string, Record<string, unknown>, 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 }]);
|
||||
});
|
||||
});
|
||||
+36
-30
@@ -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<TagCount[]> {
|
||||
export async function fetchTagCounts(
|
||||
tagIds: string[],
|
||||
accountIds: Array<string | undefined> = [undefined],
|
||||
): Promise<TagCount[]> {
|
||||
if (tagIds.length === 0) return [];
|
||||
const accountId = accountIdOverride ?? jmapClient.accountId;
|
||||
const perRequest = Math.max(2, jmapClient.getMaxCallsInRequest());
|
||||
const counts = new Map<string, TagCount>(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<string, { id: string; kind: 'total' | 'unread' }>();
|
||||
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)!);
|
||||
|
||||
+2
-2
@@ -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;
|
||||
|
||||
@@ -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) => ({
|
||||
|
||||
@@ -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' } });
|
||||
});
|
||||
});
|
||||
|
||||
+18
-11
@@ -360,8 +360,8 @@ export interface EmailState {
|
||||
*/
|
||||
threadCounts: Record<string, number>;
|
||||
/**
|
||||
* 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<string, string>;
|
||||
|
||||
@@ -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<EmailState, 'searchQuery' | 'filters'>): 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<string | undefined> {
|
||||
/**
|
||||
* 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<string | undefined> {
|
||||
const out: Array<string | undefined> = [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<EmailState>()(
|
||||
? 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;
|
||||
|
||||
Reference in New Issue
Block a user