mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: leave trash and spam out of tag views and their counts #1156
This commit is contained in:
@@ -147,6 +147,27 @@ describe('JMAPClient request limits', () => {
|
||||
|
||||
expect(sent.map(calls => calls.length)).toEqual([4, 4, 4, 4, 2]);
|
||||
});
|
||||
|
||||
it('leaves the excluded folders out of both counts (#1156)', async () => {
|
||||
const client = await connectedClient({ maxCallsInRequest: 16 });
|
||||
const sent = recordRequests((methodCalls) => ({
|
||||
methodResponses: methodCalls.map(([, , callId]) => ['Email/query', { total: 0 }, callId]),
|
||||
}));
|
||||
|
||||
await client.getTagCounts(['work'], undefined, ['trash', 'junk']);
|
||||
|
||||
const [total, unread] = sent[0].map(([, args]) => (args as { filter: unknown }).filter);
|
||||
expect(total).toEqual({
|
||||
operator: 'AND',
|
||||
conditions: [{ hasKeyword: '$label:work' }, { inMailboxOtherThan: ['trash', 'junk'] }],
|
||||
});
|
||||
expect(unread).toEqual({
|
||||
operator: 'AND',
|
||||
conditions: [
|
||||
{ hasKeyword: '$label:work' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['trash', 'junk'] },
|
||||
],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCategoryUnreadCounts', () => {
|
||||
|
||||
@@ -93,6 +93,29 @@ describe('fetchTagEmails', () => {
|
||||
expect(get).toHaveBeenNthCalledWith(2, undefined, 'g', 25, 50, keyword, true, undefined, order);
|
||||
});
|
||||
|
||||
it("leaves each account's Trash and Junk out, ANDed with a search filter (#1156)", async () => {
|
||||
const role = (id: string, r: string, originalId?: string) => ({ ...mb(id, originalId), role: r } as Mailbox);
|
||||
const ownGet = vi.fn(async () => ({ emails: [], total: 0, hasMore: false }));
|
||||
const groupGet = vi.fn(async () => ({ emails: [], total: 0, hasMore: false }));
|
||||
const own = makeAccount({
|
||||
accountId: 'login', mailboxes: [mb('inbox'), role('trash', 'trash'), role('junk', 'junk')],
|
||||
}, ownGet);
|
||||
const group = makeAccount({
|
||||
accountId: 'group', isShared: true, mailboxes: [mb('group:g-inbox', 'g-inbox'), role('group:g-trash', 'trash', 'g-trash')],
|
||||
}, groupGet);
|
||||
|
||||
await fetchTagEmails([own, group], keyword, 50, 0, [], { text: 'invoice' });
|
||||
|
||||
expect(ownGet).toHaveBeenCalledWith(undefined, undefined, 50, 0, keyword, true,
|
||||
{ operator: 'AND', conditions: [{ text: 'invoice' }, { inMailboxOtherThan: ['trash', 'junk'] }] }, []);
|
||||
expect(groupGet).toHaveBeenCalledWith(undefined, 'group', 50, 0, keyword, true,
|
||||
{ operator: 'AND', conditions: [{ text: 'invoice' }, { inMailboxOtherThan: ['g-trash'] }] }, []);
|
||||
|
||||
await fetchTagEmails([own], keyword, 50, 0);
|
||||
expect(ownGet).toHaveBeenLastCalledWith(undefined, undefined, 50, 0, keyword, true,
|
||||
{ inMailboxOtherThan: ['trash', 'junk'] }, []);
|
||||
});
|
||||
|
||||
it('keeps the other accounts when one fails and reports the failure', async () => {
|
||||
const own = makeAccount({ accountId: 'login' },
|
||||
vi.fn(async () => ({ emails: [email('own-1', '2026-09-10T00:00:00Z', 'inbox')], total: 1, hasMore: false })));
|
||||
|
||||
@@ -252,10 +252,11 @@ export class DemoJMAPClient implements IJMAPClient {
|
||||
return this.data.emails.find(e => e.id === emailId) ?? null;
|
||||
}
|
||||
|
||||
async getTagCounts(tagIds: string[]): Promise<Record<string, { total: number; unread: number }>> {
|
||||
async getTagCounts(tagIds: string[], _accountId?: string, excludeMailboxIds?: string[]): Promise<Record<string, { total: number; unread: number }>> {
|
||||
const result: Record<string, { total: number; unread: number }> = {};
|
||||
const exclusion = excludeMailboxIds?.length ? { inMailboxOtherThan: excludeMailboxIds } : null;
|
||||
for (const tagId of tagIds) {
|
||||
const tagged = this.data.emails.filter(e => e.keywords[tagId]);
|
||||
const tagged = this.data.emails.filter(e => e.keywords[tagId] && (!exclusion || this.matchesFilter(e, exclusion)));
|
||||
result[tagId] = {
|
||||
total: tagged.length,
|
||||
unread: tagged.filter(e => !e.keywords.$seen).length,
|
||||
|
||||
@@ -194,9 +194,10 @@ export interface IJMAPClient {
|
||||
* Total / unread message counts per tag id. `accountId` scopes the count to
|
||||
* a group/shared account reached through this client (defaults to the
|
||||
* client's own account); the store sums it over every account a tag view
|
||||
* spans (#1038).
|
||||
* spans (#1038). A message filed only in `excludeMailboxIds` (the account's
|
||||
* Trash and Junk, which the tag view leaves out, #1156) is not counted.
|
||||
*/
|
||||
getTagCounts(tagIds: string[], accountId?: string): Promise<Record<string, { total: number; unread: number }>>;
|
||||
getTagCounts(tagIds: string[], accountId?: string, excludeMailboxIds?: string[]): Promise<Record<string, { total: number; unread: number }>>;
|
||||
/**
|
||||
* Enumerate account keywords for extensions. Servers supporting Keyword/get
|
||||
* can return exact counts and provider-label metadata; other servers use the
|
||||
|
||||
+8
-2
@@ -2078,10 +2078,13 @@ export class JMAPClient implements IJMAPClient {
|
||||
return allEmails;
|
||||
}
|
||||
|
||||
async getTagCounts(tagIds: string[], accountId?: string): Promise<Record<string, { total: number; unread: number }>> {
|
||||
async getTagCounts(tagIds: string[], accountId?: string, excludeMailboxIds?: string[]): Promise<Record<string, { total: number; unread: number }>> {
|
||||
if (tagIds.length === 0) return {};
|
||||
const result: Record<string, { total: number; unread: number }> = {};
|
||||
const targetAccountId = accountId || this.accountId;
|
||||
const exclusion = excludeMailboxIds && excludeMailboxIds.length > 0
|
||||
? [{ inMailboxOtherThan: excludeMailboxIds }]
|
||||
: [];
|
||||
|
||||
const CALLS_PER_TAG = 2;
|
||||
const perRequest = itemsPerRequest(this.getMaxCallsInRequest(), CALLS_PER_TAG);
|
||||
@@ -2094,7 +2097,9 @@ export class JMAPClient implements IJMAPClient {
|
||||
// Total count for this tag
|
||||
methodCalls.push(["Email/query", {
|
||||
accountId: targetAccountId,
|
||||
filter: { hasKeyword: keyword },
|
||||
filter: exclusion.length > 0
|
||||
? { operator: "AND", conditions: [{ hasKeyword: keyword }, ...exclusion] }
|
||||
: { hasKeyword: keyword },
|
||||
// limit 1, not 0: Stalwart treats 0 as "no limit" and returns every id.
|
||||
limit: 1,
|
||||
calculateTotal: true,
|
||||
@@ -2107,6 +2112,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
conditions: [
|
||||
{ hasKeyword: keyword },
|
||||
{ notKeyword: "$seen" },
|
||||
...exclusion,
|
||||
],
|
||||
},
|
||||
// limit 1, not 0: Stalwart treats 0 as "no limit" and returns every id.
|
||||
|
||||
+19
-5
@@ -93,11 +93,16 @@ export function jmapMailboxIdOf(account: UnifiedAccountClient, mailbox: Mailbox)
|
||||
* search, or null when the account has neither folder.
|
||||
*/
|
||||
export function trashAndJunkExclusion(account: UnifiedAccountClient): Record<string, unknown> | null {
|
||||
const ids = (['trash', 'junk'] as const)
|
||||
const ids = trashAndJunkIds(account);
|
||||
return ids.length > 0 ? { inMailboxOtherThan: ids } : null;
|
||||
}
|
||||
|
||||
/** The JMAP ids of an account's Trash and Junk folders (those it has). */
|
||||
export function trashAndJunkIds(account: UnifiedAccountClient): string[] {
|
||||
return (['trash', 'junk'] as const)
|
||||
.map((role) => findMailboxByRole(account.mailboxes, role))
|
||||
.filter((mailbox): mailbox is Mailbox => Boolean(mailbox))
|
||||
.map((mailbox) => jmapMailboxIdOf(account, mailbox));
|
||||
return ids.length > 0 ? { inMailboxOtherThan: ids } : null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -545,6 +550,11 @@ export async function advancedSearchCrossViewEmails(
|
||||
* the same page (`limit`/`position`) with pinned-first ordering plus the
|
||||
* configured list order, mirroring `fetchEmails`, and the pages are merged
|
||||
* under that same order. Per-account failures land in `errors`.
|
||||
*
|
||||
* Trash and Junk are left out, as in Gmail's label views: a deleted message
|
||||
* keeps its keywords, so it stayed listed under the tag (and came back on
|
||||
* every refresh after Delete removed the row), looking no different from
|
||||
* live mail (#1156). It is still reachable from the Trash folder itself.
|
||||
*/
|
||||
export async function fetchTagEmails(
|
||||
accounts: UnifiedAccountClient[],
|
||||
@@ -556,9 +566,13 @@ export async function fetchTagEmails(
|
||||
): Promise<UnifiedFetchResult> {
|
||||
return fanOutAccountQuery(
|
||||
accounts,
|
||||
(account, jmapAccountId) => account.client.getEmails(
|
||||
undefined, jmapAccountId, limit, positionFor(position, account), keyword, true, extraFilter, order,
|
||||
),
|
||||
(account, jmapAccountId) => {
|
||||
const filter = andFilters(extraFilter ?? {}, trashAndJunkExclusion(account));
|
||||
return account.client.getEmails(
|
||||
undefined, jmapAccountId, limit, positionFor(position, account), keyword, true,
|
||||
Object.keys(filter).length > 0 ? filter : undefined, order,
|
||||
);
|
||||
},
|
||||
compareEmails(order, { pinnedFirst: true }),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -215,7 +215,7 @@ describe('email-store tag counts stay in step with read state', () => {
|
||||
|
||||
// The server bulk-marks emails that are not in `state.emails`, so a local
|
||||
// delta would under-count: it has to refetch.
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith(['ingsel', 'work'], undefined);
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith(['ingsel', 'work'], undefined, []);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||
|
||||
@@ -179,11 +179,26 @@ describe('tag view across the own and group accounts (#1038)', () => {
|
||||
it('sums the sidebar tag badge over every account', async () => {
|
||||
await useEmailStore.getState().fetchTagCounts(client);
|
||||
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined);
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group');
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined, []);
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group', []);
|
||||
expect(useEmailStore.getState().tagCounts[label]).toEqual({ total: 4, unread: 1 });
|
||||
});
|
||||
|
||||
it("leaves every account's Trash and Junk out of the badge, as the tag view does (#1156)", async () => {
|
||||
const ownTrash = { id: 'trash', name: 'Trash', role: 'trash', isShared: false } as Mailbox;
|
||||
const ownJunk = { id: 'junk', name: 'Junk', role: 'junk', isShared: false } as Mailbox;
|
||||
const groupTrash = {
|
||||
id: 'group:trash', originalId: 'trash', name: 'Trash', role: 'trash',
|
||||
isShared: true, accountId: 'group', accountName: 'group@server.tld',
|
||||
} as Mailbox;
|
||||
useEmailStore.setState({ mailboxes: [ownInbox, ownTrash, ownJunk, groupInbox, groupTrash] });
|
||||
|
||||
await useEmailStore.getState().fetchTagCounts(client);
|
||||
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], undefined, ['trash', 'junk']);
|
||||
expect(client.getTagCounts).toHaveBeenCalledWith([label], 'group', ['trash']);
|
||||
});
|
||||
|
||||
it('still lists only the selected folder when no tag is selected', async () => {
|
||||
useEmailStore.setState({ selectedMailbox: groupInbox.id });
|
||||
await useEmailStore.getState().fetchEmails(client);
|
||||
|
||||
@@ -10,7 +10,7 @@ import { emailHooks } from "@/lib/plugin-hooks";
|
||||
import { resolveThreadRoute } from "@/lib/thread-routing";
|
||||
import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils";
|
||||
import type { ExternalSearchResult } from "@/lib/plugin-types";
|
||||
import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox";
|
||||
import { positionsByAccount, fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, trashAndJunkIds, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type AcrossAccountsSearchOptions, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox";
|
||||
import { defaultSearchScopeFor, isAllFoldersSearchScope, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders";
|
||||
import { useAuthStore } from "@/stores/auth-store";
|
||||
import { pathNamesMailFolder } from "@/lib/deep-links";
|
||||
@@ -1778,9 +1778,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
// The badge counts what the tag view lists: messages in the own account
|
||||
// AND in the group/shared accounts this login reaches (#1038). Sum the
|
||||
// per-account counts; an account that fails just contributes nothing.
|
||||
// Trash and Junk are left out of both, see fetchTagEmails (#1156).
|
||||
const built = buildTagViewAccountClients(client);
|
||||
const perAccount = await Promise.allSettled(
|
||||
built.map((a) => a.client.getTagCounts(tagIds, a.isShared ? a.accountId : undefined)),
|
||||
built.map((a) => a.client.getTagCounts(tagIds, a.isShared ? a.accountId : undefined, trashAndJunkIds(a))),
|
||||
);
|
||||
const counts: Record<string, { total: number; unread: number }> = {};
|
||||
for (const outcome of perAccount) {
|
||||
|
||||
Reference in New Issue
Block a user