mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: a search inside a tag view narrows the tag instead of replacing it
Selecting a tag does not clear an active search: `handleTagSelect` leaves
`searchQuery`/`searchFilters` alone and `selectKeyword` leaves
`selectedMailbox`, so a tag view and a search are active together. The
paths then disagreed about which one wins:
fetchEmails / refreshCurrentMailbox → `selectedKeyword` first, so the
tag fan-out ran and the query was dropped;
searchEmails / advancedSearch / loadMoreEmails → the search first, so
since #1082 the all-folders fan-out ran and the tag was dropped.
Both orders are defensible on their own; having both is not. A
push-triggered refresh silently replaced a search result list with the
whole tag, and "load more" appended rows the tag does not contain — the
same class of inconsistency #1082 set out to remove between a search and
its own next page.
Resolve it the way the codebase already documents ("a tag view takes
precedence over the selected folder"), and keep the query: a search inside
a tag view now narrows the tag, in all five paths. `fetchTagEmails` grows
an optional `extraFilter`, which `getEmails` already accepts alongside
`hasKeyword` — the same argument the message-list category tabs use — so
no new query plumbing is needed.
Searching within the tag is also the more useful reading: a tag view is a
list the user deliberately opened, and a search box above it reads as
"within this", not "forget this".
This commit is contained in:
@@ -512,11 +512,12 @@ export async function fetchTagEmails(
|
||||
limit: number,
|
||||
position: number,
|
||||
order: SortLevel[] = [],
|
||||
extraFilter?: Record<string, unknown>,
|
||||
): Promise<UnifiedFetchResult> {
|
||||
return fanOutAccountQuery(
|
||||
accounts,
|
||||
(account, jmapAccountId) => account.client.getEmails(
|
||||
undefined, jmapAccountId, limit, position, keyword, true, undefined, order,
|
||||
undefined, jmapAccountId, limit, position, keyword, true, extraFilter, order,
|
||||
),
|
||||
compareEmails(order, { pinnedFirst: true }),
|
||||
);
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useEmailStore } from '../email-store';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
import { useAuthStore } from '../auth-store';
|
||||
import { useMessageListTabsStore } from '../message-list-tabs-store';
|
||||
import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
|
||||
import type { Email, Mailbox } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
|
||||
// Selecting a tag does not clear an active search (`handleTagSelect` leaves
|
||||
// searchQuery/searchFilters alone, and `selectKeyword` leaves selectedMailbox),
|
||||
// so a tag view and a search can be active together. The paths disagreed about
|
||||
// which one wins: fetchEmails and refreshCurrentMailbox tested `selectedKeyword`
|
||||
// first (tag fan-out, query dropped), while searchEmails / advancedSearch /
|
||||
// loadMoreEmails tested the search first (all-folders fan-out, tag dropped).
|
||||
// A push-triggered refresh therefore silently replaced a search result list
|
||||
// with the whole tag, and page 2 appended rows the tag did not contain.
|
||||
//
|
||||
// A search inside a tag view now NARROWS the tag everywhere.
|
||||
|
||||
const ownInbox = { id: 'inbox', name: 'Inbox', role: 'inbox', isShared: false } as Mailbox;
|
||||
const groupInbox = {
|
||||
id: 'group:inbox', originalId: 'inbox', name: 'Team', role: 'inbox',
|
||||
isShared: true, accountId: 'group', accountName: 'team@example.org',
|
||||
} as Mailbox;
|
||||
|
||||
function email(id: string, mailbox: string, receivedAt: string, subject: string): Email {
|
||||
return {
|
||||
id, threadId: `thread-${id}`, mailboxIds: { [mailbox]: true },
|
||||
keywords: { $seen: true, '$label:red': true },
|
||||
subject, preview: subject, receivedAt,
|
||||
from: [{ email: 'sender@example.com' }], to: [], size: 1, hasAttachment: false,
|
||||
} as Email;
|
||||
}
|
||||
|
||||
const tagged: Record<string, Email[]> = {
|
||||
me: [email('own-1', 'inbox', '2026-09-14T00:00:00Z', 'invoice red')],
|
||||
group: [email('grp-1', 'group:inbox', '2026-09-13T00:00:00Z', 'invoice team')],
|
||||
};
|
||||
|
||||
function makeClient() {
|
||||
// getEmails is the tag-view call: (mailboxId, accountId, limit, position,
|
||||
// hasKeyword, pinnedFirst, extraFilter, order).
|
||||
const getEmails = vi.fn<IJMAPClient['getEmails']>(async (
|
||||
_mailboxId?: string, accountId?: string, limit = 50, position = 0,
|
||||
hasKeyword?: string, _pinnedFirst?: boolean, extraFilter?: Record<string, unknown>,
|
||||
) => {
|
||||
let rows = hasKeyword ? (tagged[accountId ?? 'me'] ?? []) : [];
|
||||
if (extraFilter) {
|
||||
// Match either a free-text condition or a `subject` field filter.
|
||||
const blob = JSON.stringify(extraFilter);
|
||||
const term = (blob.match(/"text":"([^"*]+)/) ?? blob.match(/"subject":"([^"*]+)/))?.[1] ?? '';
|
||||
if (term) rows = rows.filter(r => (r.subject ?? '').includes(term));
|
||||
}
|
||||
return {
|
||||
emails: rows.slice(position, position + limit),
|
||||
total: rows.length,
|
||||
hasMore: position + limit < rows.length,
|
||||
};
|
||||
});
|
||||
return {
|
||||
getEmails,
|
||||
searchEmails: vi.fn(async () => ({ emails: [], total: 0, hasMore: false })),
|
||||
advancedSearchEmails: vi.fn(async () => ({ emails: [], total: 0, hasMore: false })),
|
||||
getSomeEmails: vi.fn(async () => []),
|
||||
getThreads: vi.fn(async () => []),
|
||||
getAccountId: () => 'me',
|
||||
} as unknown as IJMAPClient & { getEmails: typeof getEmails };
|
||||
}
|
||||
|
||||
/** The `extraFilter` of every tag-view getEmails call. */
|
||||
const filtersOf = (c: ReturnType<typeof makeClient>) =>
|
||||
c.getEmails.mock.calls.filter(args => !!args[4]).map(args => args[6]);
|
||||
|
||||
describe('searching inside a tag view narrows the tag', () => {
|
||||
let client: ReturnType<typeof makeClient>;
|
||||
|
||||
beforeEach(() => {
|
||||
client = makeClient();
|
||||
useAuthStore.setState({
|
||||
activeAccountId: 'login',
|
||||
getClientForAccount: ((id: string) => (id === 'login' ? client : undefined)) as never,
|
||||
getAllConnectedClients: (() => new Map([['login', client]])) as never,
|
||||
} as never);
|
||||
useMessageListTabsStore.setState(useMessageListTabsStore.getInitialState());
|
||||
useSettingsStore.setState({
|
||||
emailsPerPage: 50, messageListOrder: [], messageListOrderScope: 'inbox', emailKeywords: [],
|
||||
} as never);
|
||||
useEmailStore.setState({
|
||||
...useEmailStore.getInitialState(),
|
||||
selectedMailbox: ownInbox.id,
|
||||
selectedKeyword: 'red',
|
||||
searchMailboxId: '',
|
||||
searchFilters: { ...DEFAULT_SEARCH_FILTERS },
|
||||
mailboxes: [ownInbox, groupInbox],
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the keyword and applies the query, instead of searching all folders', async () => {
|
||||
await useEmailStore.getState().searchEmails(client, 'team');
|
||||
|
||||
expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['grp-1']);
|
||||
// The tag fan-out ran (one keyword query per account), not a folder search.
|
||||
expect(client.searchEmails).not.toHaveBeenCalled();
|
||||
expect(client.getEmails).toHaveBeenCalled();
|
||||
for (const f of filtersOf(client)) expect(JSON.stringify(f)).toContain('team');
|
||||
});
|
||||
|
||||
it('carries the query into a field-filter search too', async () => {
|
||||
useEmailStore.setState({
|
||||
searchQuery: '',
|
||||
searchFilters: { ...DEFAULT_SEARCH_FILTERS, subject: 'team' },
|
||||
});
|
||||
await useEmailStore.getState().advancedSearch(client);
|
||||
|
||||
expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['grp-1']);
|
||||
expect(client.advancedSearchEmails).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('carries the query into a refresh, so a push does not widen the list', async () => {
|
||||
useEmailStore.setState({ searchQuery: 'team' });
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['grp-1']);
|
||||
const applied = filtersOf(client);
|
||||
expect(applied.length).toBeGreaterThan(0);
|
||||
for (const f of applied) expect(JSON.stringify(f)).toContain('team');
|
||||
});
|
||||
|
||||
it('still lists the whole tag when no search is active', async () => {
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
expect(useEmailStore.getState().emails.map(e => e.id).sort()).toEqual(['grp-1', 'own-1']);
|
||||
for (const f of filtersOf(client)) expect(f).toBeUndefined();
|
||||
});
|
||||
});
|
||||
+56
-5
@@ -5,7 +5,7 @@ import type { IJMAPClient } from "@/lib/jmap/client-interface";
|
||||
import { useSettingsStore, getMessageListOrderFor } from "@/stores/settings-store";
|
||||
import { useCalendarStore } from "@/stores/calendar-store";
|
||||
import type { SortLevel } from "@/lib/message-list-order";
|
||||
import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty } from "@/lib/jmap/search-utils";
|
||||
import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty, toWildcardQuery } from "@/lib/jmap/search-utils";
|
||||
import { emailHooks } from "@/lib/plugin-hooks";
|
||||
import { resolveThreadRoute } from "@/lib/thread-routing";
|
||||
import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils";
|
||||
@@ -1743,7 +1743,17 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
if (selectedKeyword) {
|
||||
const order = getMessageListOrderFor(null);
|
||||
const built = buildTagViewAccountClients(client);
|
||||
const result = await fetchTagEmails(built, `$label:${selectedKeyword}`, emailsPerPage, 0, order);
|
||||
// Selecting a tag keeps an active search (`handleTagSelect` does not
|
||||
// clear it), and the search paths narrow the tag rather than replace
|
||||
// it — so honour the query here too, or the list would flip between
|
||||
// narrowed and whole depending on which path last ran.
|
||||
const { searchQuery: tagQuery, searchFilters: tagFilters } = get();
|
||||
const tagFilter = !isFilterEmpty(tagFilters)
|
||||
? buildJMAPFilter(tagQuery, tagFilters, undefined)
|
||||
: tagQuery
|
||||
? { text: toWildcardQuery(tagQuery) }
|
||||
: undefined;
|
||||
const result = await fetchTagEmails(built, `$label:${selectedKeyword}`, emailsPerPage, 0, order, tagFilter);
|
||||
const enrichedEmails = await emailHooks.onEmailsFetched.transform(result.emails);
|
||||
if (!isCurrentView()) return;
|
||||
set({
|
||||
@@ -1947,7 +1957,19 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
const { searchFilters } = get();
|
||||
const hasFilters = !isFilterEmpty(searchFilters);
|
||||
|
||||
if (searchQuery || hasFilters) {
|
||||
if ((searchQuery || hasFilters) && selectedKeyword) {
|
||||
// Searching inside a tag view narrows the tag (see searchEmails), so
|
||||
// page 2 must carry the same keyword AND the same query — otherwise it
|
||||
// appends unrelated all-folders hits to a tag list.
|
||||
const built = buildTagViewAccountClients(client);
|
||||
result = await fetchTagEmails(
|
||||
built, `$label:${selectedKeyword}`, emailsPerPage, position, getMessageListOrderFor(null),
|
||||
hasFilters
|
||||
? buildJMAPFilter(searchQuery, searchFilters, undefined)
|
||||
: { text: toWildcardQuery(searchQuery) },
|
||||
);
|
||||
set({ unifiedErrors: result.errors });
|
||||
} else if (searchQuery || hasFilters) {
|
||||
// Paginate with the same scope the search itself ran under, not the
|
||||
// folder that happens to be open in the list.
|
||||
const { searchMailboxId } = get();
|
||||
@@ -2748,7 +2770,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
searchAbortController: controller,
|
||||
}); // Clear emails for loading state
|
||||
try {
|
||||
const { isUnifiedView, unifiedRole, crossView, searchMailboxId, searchFilters } = get();
|
||||
const { isUnifiedView, unifiedRole, crossView, searchMailboxId, searchFilters, selectedKeyword } = get();
|
||||
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
|
||||
|
||||
let result;
|
||||
@@ -2767,6 +2789,19 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
result = await searchUnifiedEmails(built, unifiedRole, query, emailsPerPage, 0);
|
||||
unifiedErrors = result.errors;
|
||||
|
||||
} else if (selectedKeyword) {
|
||||
// A tag view takes precedence over the folder, as it does in
|
||||
// fetchEmails and refreshCurrentMailbox — so searching inside one
|
||||
// NARROWS the tag rather than silently replacing the list with an
|
||||
// unrelated all-folders result that the next push refresh would throw
|
||||
// away again.
|
||||
const built = buildTagViewAccountClients(client);
|
||||
result = await fetchTagEmails(
|
||||
built, `$label:${selectedKeyword}`, emailsPerPage, 0, getMessageListOrderFor(null),
|
||||
{ text: toWildcardQuery(query) },
|
||||
);
|
||||
unifiedErrors = result.errors;
|
||||
|
||||
} else if (searchMailboxId === '') {
|
||||
// "All folders" (the default): every folder of the own account AND of
|
||||
// every group/shared account this login reaches - the same fan-out the
|
||||
@@ -2836,7 +2871,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
},
|
||||
|
||||
advancedSearch: async (client) => {
|
||||
const { searchQuery, searchFilters, searchMailboxId, searchAbortController, isUnifiedView, unifiedRole, crossView } = get();
|
||||
const { searchQuery, searchFilters, searchMailboxId, searchAbortController, isUnifiedView, unifiedRole, crossView, selectedKeyword } = get();
|
||||
const mailboxes = resolveActionMailboxes();
|
||||
|
||||
if (searchAbortController) {
|
||||
@@ -2881,6 +2916,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
);
|
||||
unifiedErrors = result.errors;
|
||||
|
||||
} else if (selectedKeyword) {
|
||||
// Narrow the tag rather than replace it — see searchEmails.
|
||||
const built = buildTagViewAccountClients(client);
|
||||
result = await fetchTagEmails(
|
||||
built, `$label:${selectedKeyword}`, emailsPerPage, 0, getMessageListOrderFor(null),
|
||||
buildJMAPFilter(searchQuery, searchFilters, undefined),
|
||||
);
|
||||
unifiedErrors = result.errors;
|
||||
|
||||
} else if (searchMailboxId === '') {
|
||||
// "All folders": fan out over the own + group/shared accounts with a
|
||||
// filter that carries no inMailbox clause (#1082).
|
||||
@@ -3905,9 +3949,16 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
// Match fetchEmails: tag views take precedence and query the label
|
||||
// across all folders and across the own + group accounts (#1038).
|
||||
// They cannot establish a folder delta baseline.
|
||||
// An active search narrows the tag, so carry it or the refresh
|
||||
// would silently widen the list back to the whole tag.
|
||||
const built = buildTagViewAccountClients(client);
|
||||
result = await fetchTagEmails(
|
||||
built, `$label:${selectedKeyword}`, emailsPerPage, 0, getMessageListOrderFor(null),
|
||||
hasFilters
|
||||
? buildJMAPFilter(searchQuery, searchFilters, undefined)
|
||||
: searchQuery
|
||||
? { text: toWildcardQuery(searchQuery) }
|
||||
: undefined,
|
||||
);
|
||||
unifiedErrors = result.errors;
|
||||
} else if (hasFilters || searchQuery) {
|
||||
|
||||
Reference in New Issue
Block a user