mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 08:13:50 +00:00
Merge pull request #1103 from guisea/fix/folder-view-unified-blend
fix(email-store): stop late unified/cross-view fetches from stomping folder selection
This commit is contained in:
@@ -0,0 +1,112 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/**
|
||||
* Deleting a message in a real folder (e.g. one built from a Sieve `fileinto`
|
||||
* rule) could show "Inbox" badges and stale rows blended in, because a
|
||||
* still-in-flight fetchUnifiedEmails/fetchCrossView call (kicked off before
|
||||
* the user navigated to that folder) resolved AFTER selectMailbox and
|
||||
* unconditionally stomped isUnifiedView back to true. The next
|
||||
* refreshCurrentMailbox (triggered by the delete's push echo) then took the
|
||||
* unified branch instead of refreshing the real folder.
|
||||
*
|
||||
* viewToken is bumped by every view-changing action; fetchUnifiedEmails and
|
||||
* fetchCrossView capture it before their async fetch and bail if it no
|
||||
* longer matches on return, so a late response can't clobber a newer view.
|
||||
*/
|
||||
|
||||
const { fetchCrossViewMock, fetchUnifiedMock } = vi.hoisted(() => ({
|
||||
fetchCrossViewMock: vi.fn(),
|
||||
fetchUnifiedMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/unified-mailbox', async (importActual) => {
|
||||
const actual = await importActual<typeof import('@/lib/unified-mailbox')>();
|
||||
return {
|
||||
...actual,
|
||||
fetchCrossViewEmails: fetchCrossViewMock,
|
||||
fetchUnifiedEmails: fetchUnifiedMock,
|
||||
};
|
||||
});
|
||||
|
||||
import { useEmailStore } from '../email-store';
|
||||
import { useSettingsStore } from '../settings-store';
|
||||
import type { Email } from '@/lib/jmap/types';
|
||||
import type { UnifiedAccountClient } from '@/lib/unified-mailbox';
|
||||
|
||||
const makeEmail = (id: string): 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-13T10:00:00Z',
|
||||
preview: '',
|
||||
hasAttachment: false,
|
||||
size: 1,
|
||||
}) as unknown as Email;
|
||||
|
||||
describe('viewToken guards against late-resolving unified fetches (#delete-folder-blend)', () => {
|
||||
let resolveUnified!: (value: unknown) => void;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
useSettingsStore.setState({ emailsPerPage: 25 });
|
||||
useEmailStore.setState({
|
||||
emails: [],
|
||||
totalEmails: 0,
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
selectedMailbox: '',
|
||||
selectedKeyword: null,
|
||||
});
|
||||
fetchUnifiedMock.mockImplementation(
|
||||
() => new Promise((resolve) => { resolveUnified = resolve; }),
|
||||
);
|
||||
});
|
||||
|
||||
it('a late-resolving fetchUnifiedEmails does not stomp a folder selected afterward', async () => {
|
||||
const accounts: UnifiedAccountClient[] = [];
|
||||
|
||||
// Kick off a unified fetch and don't await it yet - simulates it being
|
||||
// in flight when the user clicks away.
|
||||
const pending = useEmailStore.getState().fetchUnifiedEmails(accounts, 'inbox');
|
||||
expect(useEmailStore.getState().isUnifiedView).toBe(true);
|
||||
|
||||
// User navigates to a real folder (e.g. "Later") before the fetch settles.
|
||||
useEmailStore.getState().selectMailbox('later-folder-id');
|
||||
expect(useEmailStore.getState().isUnifiedView).toBe(false);
|
||||
expect(useEmailStore.getState().selectedMailbox).toBe('later-folder-id');
|
||||
|
||||
// The stale unified fetch now resolves.
|
||||
resolveUnified({
|
||||
emails: [makeEmail('unified-a'), makeEmail('unified-b')],
|
||||
hasMore: false,
|
||||
total: 2,
|
||||
errors: new Map(),
|
||||
});
|
||||
await pending;
|
||||
|
||||
// It must not have re-entered unified view or overwritten the folder's state.
|
||||
expect(useEmailStore.getState().isUnifiedView).toBe(false);
|
||||
expect(useEmailStore.getState().selectedMailbox).toBe('later-folder-id');
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).not.toContain('unified-a');
|
||||
});
|
||||
|
||||
it('a fresh fetchUnifiedEmails call still applies its own results normally', async () => {
|
||||
fetchUnifiedMock.mockResolvedValueOnce({
|
||||
emails: [makeEmail('a')],
|
||||
hasMore: false,
|
||||
total: 1,
|
||||
errors: new Map(),
|
||||
});
|
||||
|
||||
await useEmailStore.getState().fetchUnifiedEmails([], 'inbox');
|
||||
|
||||
expect(useEmailStore.getState().isUnifiedView).toBe(true);
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['a']);
|
||||
});
|
||||
});
|
||||
+33
-6
@@ -121,6 +121,11 @@ interface EmailStore {
|
||||
externalSearchResults: ExternalSearchResult[];
|
||||
|
||||
// Unified mailbox state
|
||||
// Bumped by any action that changes which mailbox/keyword/unified view is
|
||||
// active. fetchUnifiedEmails/fetchCrossView capture it before their async
|
||||
// fetch and check it on return, so a late-resolving fetch from a view the
|
||||
// user already navigated away from can't stomp the newer view's state (#delete-folder-blend).
|
||||
viewToken: number;
|
||||
isUnifiedView: boolean;
|
||||
unifiedRole: UnifiedMailboxRole | null;
|
||||
// Cross-account view ('unread' | 'starred' | 'all') when active; null for the
|
||||
@@ -1432,6 +1437,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
externalSearchResults: [],
|
||||
|
||||
// Unified mailbox state
|
||||
viewToken: 0,
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
@@ -1516,7 +1522,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
emailHooks.onEmailOpen.emitSync(email);
|
||||
}
|
||||
},
|
||||
selectKeyword: (keyword) => set({
|
||||
selectKeyword: (keyword) => set(state => ({
|
||||
selectedKeyword: keyword,
|
||||
isLoadingMore: false,
|
||||
selectedEmail: null,
|
||||
@@ -1524,7 +1530,11 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
expandedThreadIds: new Set(),
|
||||
threadEmailsCache: new Map(),
|
||||
threadEmailCounts: new Map(),
|
||||
}),
|
||||
viewToken: state.viewToken + 1,
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
})),
|
||||
fetchTagCounts: (client) => coalesceRefresh(client, 'tagCounts', async () => {
|
||||
try {
|
||||
const keywords = useSettingsStore.getState().emailKeywords;
|
||||
@@ -1553,7 +1563,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
console.error('Failed to fetch tag counts:', error);
|
||||
}
|
||||
}),
|
||||
selectMailbox: (mailboxId) => set({
|
||||
selectMailbox: (mailboxId) => set(state => ({
|
||||
selectedMailbox: mailboxId,
|
||||
isLoadingMore: false,
|
||||
selectedEmail: null,
|
||||
@@ -1563,7 +1573,11 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
threadEmailsCache: new Map(),
|
||||
threadEmailCounts: new Map(),
|
||||
isLoadingThread: null,
|
||||
}),
|
||||
viewToken: state.viewToken + 1,
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
})),
|
||||
setLoading: (loading) => set({ isLoading: loading }),
|
||||
setLoadingEmail: (loading) => set({ isLoadingEmail: loading }),
|
||||
setError: (error) => set({ error }),
|
||||
@@ -4777,6 +4791,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
|
||||
// Unified mailbox operations
|
||||
fetchUnifiedEmails: async (accounts, role) => {
|
||||
const token = get().viewToken + 1;
|
||||
set({
|
||||
isLoading: true,
|
||||
error: null,
|
||||
@@ -4785,11 +4800,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
crossView: null,
|
||||
selectedKeyword: null,
|
||||
retainedInViewIds: new Set(),
|
||||
viewToken: token,
|
||||
});
|
||||
try {
|
||||
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
|
||||
const order = getMessageListOrderFor(role);
|
||||
const result = await fetchUnifiedEmails(accounts, role, emailsPerPage, 0, order);
|
||||
// The user may have navigated to a different mailbox/keyword/unified view
|
||||
// while this was in flight — don't let a stale response overwrite it.
|
||||
if (get().viewToken !== token) return;
|
||||
set({
|
||||
emails: result.emails,
|
||||
hasMoreEmails: result.hasMore,
|
||||
@@ -4799,6 +4818,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
unifiedErrors: result.errors,
|
||||
});
|
||||
} catch (error) {
|
||||
if (get().viewToken !== token) return;
|
||||
console.error('Failed to fetch unified emails:', error);
|
||||
set({
|
||||
error: error instanceof Error ? error.message : "Failed to fetch unified emails",
|
||||
@@ -4855,6 +4875,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
},
|
||||
|
||||
fetchCrossView: async (accounts, view) => {
|
||||
const token = get().viewToken + 1;
|
||||
set({
|
||||
isLoading: true,
|
||||
error: null,
|
||||
@@ -4863,10 +4884,14 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
crossView: view,
|
||||
selectedKeyword: null,
|
||||
retainedInViewIds: new Set(),
|
||||
viewToken: token,
|
||||
});
|
||||
try {
|
||||
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
|
||||
const result = await fetchCrossViewEmails(accounts, view, emailsPerPage, 0);
|
||||
// The user may have navigated to a different mailbox/keyword/unified view
|
||||
// while this was in flight — don't let a stale response overwrite it.
|
||||
if (get().viewToken !== token) return;
|
||||
set({
|
||||
emails: result.emails,
|
||||
hasMoreEmails: result.hasMore,
|
||||
@@ -4875,6 +4900,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
unifiedErrors: result.errors,
|
||||
});
|
||||
} catch (error) {
|
||||
if (get().viewToken !== token) return;
|
||||
console.error('Failed to fetch cross-account view:', error);
|
||||
set({
|
||||
error: error instanceof Error ? error.message : "Failed to fetch cross-account view",
|
||||
@@ -4898,13 +4924,14 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
},
|
||||
|
||||
exitUnifiedView: () => {
|
||||
set({
|
||||
set(state => ({
|
||||
isUnifiedView: false,
|
||||
unifiedRole: null,
|
||||
crossView: null,
|
||||
unifiedErrors: new Map(),
|
||||
retainedInViewIds: new Set(),
|
||||
});
|
||||
viewToken: state.viewToken + 1,
|
||||
}));
|
||||
},
|
||||
|
||||
setScheduledView: (isScheduledView, accountScope = null) => set(state => {
|
||||
|
||||
Reference in New Issue
Block a user