mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-03 08:15:38 +00:00
fix(email): stop All-Mail views emptying on delete/star/read
refreshCurrentMailbox runs on every Email state push (delete, star, and
mark-read all raise one). It only handled real single mailboxes: in a
unified ("All Mail") or cross-account view, selectedMailbox is a virtual
id (e.g. __cross_all__ / __unified_inbox__) that no folder matches, so it
was sent to getEmails as an inMailbox filter, returned an empty page, and
the first-page merge wiped the whole aggregated list. The empty query was
previously masked by the length-based merge cutoff; af2b00dd (#592)
changed the cutoff to the page size and unmasked it.
Make refreshCurrentMailbox view-aware, mirroring loadMoreEmails /
performSearch: fan out page 0 via fetchCrossViewEmails / fetchUnifiedEmails
(search-aware) when in those views, then run the existing merge so loaded
pages are preserved. Surface cross-view fetch errors via unifiedErrors.
Test: email-store-refresh-unified asserts a push refresh in the cross-All
and unified-inbox views keeps the list and never queries the virtual id
(fails without the fix: the list wipes to []).
This commit is contained in:
@@ -0,0 +1,130 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* refreshCurrentMailbox runs on every Email state push - which a delete, star,
|
||||
* or mark-read action all raise. In a unified ("All Mail") or cross-account
|
||||
* view, selectedMailbox is a VIRTUAL id (e.g. __cross_all__) that no real
|
||||
* folder matches. The single-mailbox path would send it to getEmails as an
|
||||
* inMailbox filter, get an empty page back, and the merge would wipe the whole
|
||||
* aggregated list. This test pins that a refresh in those views fans out via
|
||||
* the unified loaders and keeps the list intact.
|
||||
*/
|
||||
|
||||
const { buildMock, fetchCrossViewMock, fetchUnifiedMock } = vi.hoisted(() => ({
|
||||
buildMock: vi.fn(async () => [] as unknown[]),
|
||||
fetchCrossViewMock: vi.fn(),
|
||||
fetchUnifiedMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/unified-mailbox', async (importActual) => {
|
||||
const actual = await importActual<typeof import('@/lib/unified-mailbox')>();
|
||||
return {
|
||||
...actual,
|
||||
buildUnifiedAccountClients: buildMock,
|
||||
fetchCrossViewEmails: fetchCrossViewMock,
|
||||
fetchUnifiedEmails: fetchUnifiedMock,
|
||||
};
|
||||
});
|
||||
|
||||
import { useEmailStore } from '../email-store';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
import { CROSS_ALL, UNIFIED_INBOX } from '@/lib/jmap/types';
|
||||
import type { Email } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
|
||||
const makeEmail = (id: string, keywords: Record<string, boolean> = {}): Email =>
|
||||
({
|
||||
id,
|
||||
threadId: `t-${id}`,
|
||||
mailboxIds: { inbox: true },
|
||||
keywords,
|
||||
from: [{ email: 'a@example.com' }],
|
||||
to: [{ email: 'b@example.com' }],
|
||||
subject: `mail ${id}`,
|
||||
receivedAt: '2026-08-03T10:00:00Z',
|
||||
preview: '',
|
||||
hasAttachment: false,
|
||||
size: 1,
|
||||
}) as unknown as Email;
|
||||
|
||||
// A single-account client whose getEmails would return an empty page for the
|
||||
// virtual id - the pre-fix bug. If refreshCurrentMailbox ever calls it in a
|
||||
// unified view, the list wipes and the assertions below fail.
|
||||
const makeVirtualIdClient = () =>
|
||||
({
|
||||
getEmails: vi.fn(async () => ({ emails: [], hasMore: false, total: 0 })),
|
||||
}) as unknown as IJMAPClient & { getEmails: ReturnType<typeof vi.fn> };
|
||||
|
||||
describe('refreshCurrentMailbox in unified / cross-account views', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
buildMock.mockResolvedValue([]);
|
||||
useSettingsStore.setState({ emailsPerPage: 25 });
|
||||
useEmailStore.setState({
|
||||
emails: [],
|
||||
totalEmails: 0,
|
||||
searchQuery: '',
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('cross-account "All Mail" refresh keeps the list instead of wiping it after an action', async () => {
|
||||
const a = makeEmail('a');
|
||||
const b = makeEmail('b', { $seen: true }); // e.g. just marked read
|
||||
// The aggregated fan-out still returns both messages (only b's keywords changed).
|
||||
fetchCrossViewMock.mockResolvedValue({
|
||||
emails: [a, b],
|
||||
hasMore: false,
|
||||
total: 2,
|
||||
errors: new Map(),
|
||||
});
|
||||
|
||||
useEmailStore.setState({
|
||||
selectedMailbox: CROSS_ALL,
|
||||
isUnifiedView: true,
|
||||
crossView: 'all',
|
||||
emails: [makeEmail('a'), makeEmail('b')],
|
||||
totalEmails: 2,
|
||||
});
|
||||
|
||||
const client = makeVirtualIdClient();
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
// List survived and reflects the fanned-out page (b now read).
|
||||
const emails = useEmailStore.getState().emails;
|
||||
expect(emails.map((e) => e.id)).toEqual(['a', 'b']);
|
||||
expect(emails.find((e) => e.id === 'b')?.keywords?.$seen).toBe(true);
|
||||
// Fanned out via the cross-view loader; never queried the virtual id directly.
|
||||
expect(fetchCrossViewMock).toHaveBeenCalledTimes(1);
|
||||
expect(client.getEmails).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('unified-role view refresh fans out via the unified loader, not the virtual id', async () => {
|
||||
const a = makeEmail('a');
|
||||
const b = makeEmail('b');
|
||||
fetchUnifiedMock.mockResolvedValue({
|
||||
emails: [a, b],
|
||||
hasMore: false,
|
||||
total: 2,
|
||||
errors: new Map(),
|
||||
});
|
||||
|
||||
useEmailStore.setState({
|
||||
selectedMailbox: UNIFIED_INBOX,
|
||||
isUnifiedView: true,
|
||||
unifiedRole: 'inbox',
|
||||
crossView: null,
|
||||
emails: [makeEmail('a'), makeEmail('b')],
|
||||
totalEmails: 2,
|
||||
});
|
||||
|
||||
const client = makeVirtualIdClient();
|
||||
await useEmailStore.getState().refreshCurrentMailbox(client);
|
||||
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['a', 'b']);
|
||||
expect(fetchUnifiedMock).toHaveBeenCalledTimes(1);
|
||||
expect(client.getEmails).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
+44
-13
@@ -2934,28 +2934,58 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
}
|
||||
|
||||
try {
|
||||
// Fetch emails for the current mailbox without clearing the list first
|
||||
// This provides a smoother update experience
|
||||
const mailboxes = resolveActionMailboxes();
|
||||
const effectiveClient = resolveActionClient(client);
|
||||
const mailbox = mailboxes.find(mb => mb.id === selectedMailbox);
|
||||
const accountId = mailbox?.isShared ? mailbox.accountId : undefined;
|
||||
const jmapMailboxId = mailbox?.originalId || selectedMailbox;
|
||||
|
||||
// Get emails per page from settings
|
||||
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
|
||||
|
||||
// Respect active search filters / query so that a push-triggered refresh
|
||||
// does not silently replace a filtered list with an unfiltered one.
|
||||
const { searchQuery, searchFilters } = get();
|
||||
const { searchQuery, searchFilters, isUnifiedView, unifiedRole, crossView } = get();
|
||||
const hasFilters = !isFilterEmpty(searchFilters);
|
||||
|
||||
// The unified ("All Mail") and cross-account views hold a VIRTUAL
|
||||
// selectedMailbox id (e.g. __cross_all__ / __unified_inbox__) that no real
|
||||
// folder matches. Feeding it to getEmails as an inMailbox filter returns an
|
||||
// empty page, and the merge below would then wipe the whole aggregated list
|
||||
// on every push - and any delete/star/read raises an Email state change.
|
||||
// Fan out via the same loaders the initial load and loadMore use so the
|
||||
// aggregated list survives a refresh. `mailbox` stays undefined for these
|
||||
// views; it only gates the inbox new-mail notification, which the unified
|
||||
// views don't raise.
|
||||
let result;
|
||||
if (hasFilters || searchQuery) {
|
||||
const filter = buildJMAPFilter(searchQuery, searchFilters, jmapMailboxId);
|
||||
result = await effectiveClient.advancedSearchEmails(filter, accountId, emailsPerPage, 0);
|
||||
let mailbox;
|
||||
let unifiedErrors: Map<string, string> | undefined;
|
||||
if (isUnifiedView && crossView) {
|
||||
const includeGroup = useSettingsStore.getState().includeGroupInUnified;
|
||||
const built = await buildUnifiedAccountClients({ includeGroup });
|
||||
result = hasFilters
|
||||
? await advancedSearchCrossViewEmails(built, crossView, buildJMAPFilter(searchQuery, searchFilters, undefined), emailsPerPage, 0)
|
||||
: searchQuery
|
||||
? await searchCrossViewEmails(built, crossView, searchQuery, emailsPerPage, 0)
|
||||
: await fetchCrossViewEmails(built, crossView, emailsPerPage, 0);
|
||||
unifiedErrors = result.errors;
|
||||
} else if (isUnifiedView && unifiedRole) {
|
||||
const includeGroup = useSettingsStore.getState().includeGroupInUnified;
|
||||
const built = await buildUnifiedAccountClients({ includeGroup });
|
||||
result = hasFilters
|
||||
? await advancedSearchUnifiedEmails(built, unifiedRole, (mailboxId) => buildJMAPFilter(searchQuery, searchFilters, mailboxId), emailsPerPage, 0)
|
||||
: searchQuery
|
||||
? await searchUnifiedEmails(built, unifiedRole, searchQuery, emailsPerPage, 0)
|
||||
: await fetchUnifiedEmails(built, unifiedRole, emailsPerPage, 0);
|
||||
unifiedErrors = result.errors;
|
||||
} else {
|
||||
result = await effectiveClient.getEmails(jmapMailboxId, accountId, emailsPerPage, 0, undefined, true);
|
||||
// Single real mailbox (own or shared): query by its JMAP id. Refresh the
|
||||
// list without clearing it first for a smoother update experience.
|
||||
const mailboxes = resolveActionMailboxes();
|
||||
const effectiveClient = resolveActionClient(client);
|
||||
mailbox = mailboxes.find(mb => mb.id === selectedMailbox);
|
||||
const accountId = mailbox?.isShared ? mailbox.accountId : undefined;
|
||||
const jmapMailboxId = mailbox?.originalId || selectedMailbox;
|
||||
if (hasFilters || searchQuery) {
|
||||
const filter = buildJMAPFilter(searchQuery, searchFilters, jmapMailboxId);
|
||||
result = await effectiveClient.advancedSearchEmails(filter, accountId, emailsPerPage, 0);
|
||||
} else {
|
||||
result = await effectiveClient.getEmails(jmapMailboxId, accountId, emailsPerPage, 0, undefined, true);
|
||||
}
|
||||
}
|
||||
|
||||
const currentEmails = get().emails;
|
||||
@@ -3066,6 +3096,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
hasMoreEmails: hasMore,
|
||||
totalEmails: result.total,
|
||||
threadEmailsCache: newCache,
|
||||
...(unifiedErrors !== undefined ? { unifiedErrors } : {}),
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user