fix(email): keep just-read/unstarred mail in the open unread/starred view

In the unread ('unread') and starred ('starred') cross views, marking a mail
read or removing its star flips the very keyword the view filters on
(notKeyword:$seen / hasKeyword:$flagged). The push-triggered refresh re-runs
that filtered query, so the mail was absent from the fresh page and the merge
dropped it - the row vanished from under the cursor on the user's own action.

Keep such rows in place: only their status + the counters update (both already
applied by markAsRead/toggleStar/batchMarkAsRead), they stay visible until the
view is re-opened. A new retainedInViewIds set records the ids the user just
read/unstarred while in these views; refreshCurrentMailbox re-inserts those rows
at their prior position via mergeRetainedRows. The set is scoped to the two
keyword cross views, cleared on navigation (fetchEmails/fetchUnifiedEmails/
fetchCrossView/exitUnifiedView), and dropped per-id when the action is reversed
(mark unread / re-star). totalEmails counts the retained rows so hasMore stays
correct.

Genuinely removed rows (deleted/moved -> absent from the local list) are not
resurrected, and a change the user did NOT make in this view (e.g. read on
another device, retain set empty) still drops as before.

Tests: stores/__tests__/email-store-retained-in-view.test.ts - read keeps the
row (unread view), unstar keeps the row (starred view), reversing/ navigation
clear retention, external read still drops, 'all' view does not retain. The
row-keeping cases fail without the fix.
This commit is contained in:
Stefan Hildebrandt
2026-08-09 22:17:50 +02:00
parent 2a1585a4d3
commit 7a3a262c87
2 changed files with 331 additions and 5 deletions
@@ -0,0 +1,204 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
/**
* When an unread or starred cross view is open, reading a mail (unread view) or
* removing its star (starred view) must NOT drop it out of the list on the next
* push refresh - the row stays put with its status + counters updated, until the
* view is re-opened. The row only drops if it genuinely left (deleted/moved) or
* left the filter due to a change the user did NOT make in this view (e.g. read
* on another device).
*/
const { buildMock, fetchCrossViewMock } = vi.hoisted(() => ({
buildMock: vi.fn(async () => [] as unknown[]),
fetchCrossViewMock: vi.fn(),
}));
vi.mock('@/lib/unified-mailbox', async (importActual) => {
const actual = await importActual<typeof import('@/lib/unified-mailbox')>();
return {
...actual,
buildUnifiedAccountClients: buildMock,
fetchCrossViewEmails: fetchCrossViewMock,
};
});
import { useEmailStore } from '../email-store';
import { useAuthStore } from '../auth-store';
import { useSettingsStore } from '../settings-store';
import { CROSS_UNREAD, CROSS_STARRED } from '@/lib/jmap/types';
import type { Email, Mailbox } from '@/lib/jmap/types';
import type { IJMAPClient } from '@/lib/jmap/client-interface';
function makeMailbox(): Mailbox {
return {
id: 'inbox', name: 'Inbox', sortOrder: 0, role: 'inbox',
totalEmails: 0, unreadEmails: 5, totalThreads: 0, unreadThreads: 5,
isSubscribed: true, isShared: false,
} as unknown as Mailbox;
}
const makeEmail = (id: string, keywords: Record<string, boolean>): Email =>
({
id,
threadId: `t-${id}`,
subject: `mail ${id}`,
receivedAt: '2026-08-07T10:00:00Z',
keywords,
mailboxIds: { inbox: true },
accountId: 'account-a',
sourceClientAccountId: 'account-a',
sourceAccountId: 'account-a',
}) as unknown as Email;
function makeClient() {
return {
markAsRead: vi.fn().mockResolvedValue(undefined),
toggleStar: vi.fn().mockResolvedValue(undefined),
} as unknown as IJMAPClient;
}
// A single-account client whose getEmails would return an empty page for the
// virtual cross id - it must never be consulted for a cross view.
const client = makeClient();
function seedView(id: typeof CROSS_UNREAD | typeof CROSS_STARRED, view: 'unread' | 'starred', emails: Email[]) {
useSettingsStore.setState({ emailsPerPage: 25 });
useAuthStore.setState({
activeAccountId: 'account-a',
getClientForAccount: (a: string) => (a === 'account-a' ? client : undefined) as never,
} as never);
useEmailStore.setState({
isUnifiedView: true,
unifiedRole: null,
crossView: view,
selectedMailbox: id,
selectedKeyword: null,
searchQuery: '',
mailboxes: [makeMailbox()],
accountMailboxes: { 'account-a': [makeMailbox()] },
processingReadStatus: new Set(),
selectedEmailIds: new Set(),
retainedInViewIds: new Set(),
emails,
totalEmails: emails.length,
});
}
describe('retained-in-view: unread cross view', () => {
beforeEach(() => {
vi.clearAllMocks();
buildMock.mockResolvedValue([]);
});
it('keeps a mail the user just read; the fresh unread page no longer lists it', async () => {
const a = makeEmail('a', {});
const b = makeEmail('b', {});
seedView(CROSS_UNREAD, 'unread', [a, b]);
await useEmailStore.getState().markAsRead(client, 'a', true);
expect(useEmailStore.getState().retainedInViewIds.has('a')).toBe(true);
// Push refresh: the unread filter drops 'a' (now $seen); only 'b' comes back.
fetchCrossViewMock.mockResolvedValue({ emails: [makeEmail('b', {})], hasMore: false, total: 1, errors: new Map() });
await useEmailStore.getState().refreshCurrentMailbox(client);
const emails = useEmailStore.getState().emails;
expect(emails.map(e => e.id)).toEqual(['a', 'b']); // 'a' retained in place
expect(emails.find(e => e.id === 'a')?.keywords?.$seen).toBe(true);
// Counter reflects the two visible rows, not just the server's unread total.
expect(useEmailStore.getState().totalEmails).toBe(2);
});
it('does NOT retain a mail read elsewhere (retain set empty): it drops on refresh', async () => {
const a = makeEmail('a', {});
const b = makeEmail('b', {});
seedView(CROSS_UNREAD, 'unread', [a, b]);
// No local action -> retain set stays empty. 'a' left the unread filter
// (e.g. read on another device); the refresh should drop it.
fetchCrossViewMock.mockResolvedValue({ emails: [makeEmail('b', {})], hasMore: false, total: 1, errors: new Map() });
await useEmailStore.getState().refreshCurrentMailbox(client);
expect(useEmailStore.getState().emails.map(e => e.id)).toEqual(['b']);
});
it('marking the mail unread again drops the retention', async () => {
const a = makeEmail('a', { $seen: true });
seedView(CROSS_UNREAD, 'unread', [a]);
useEmailStore.setState({ retainedInViewIds: new Set(['a']) });
await useEmailStore.getState().markAsRead(client, 'a', false);
expect(useEmailStore.getState().retainedInViewIds.has('a')).toBe(false);
});
it('clears retention when navigating away to a real mailbox', async () => {
const a = makeEmail('a', {});
seedView(CROSS_UNREAD, 'unread', [a]);
await useEmailStore.getState().markAsRead(client, 'a', true);
expect(useEmailStore.getState().retainedInViewIds.size).toBe(1);
// Navigating to a real folder is a fresh load; retention must reset.
await useEmailStore.getState().fetchEmails(client, 'inbox').catch(() => {});
expect(useEmailStore.getState().retainedInViewIds.size).toBe(0);
});
});
describe('retained-in-view: starred cross view', () => {
beforeEach(() => {
vi.clearAllMocks();
buildMock.mockResolvedValue([]);
});
it('keeps a mail the user just un-starred; the fresh starred page omits it', async () => {
const a = makeEmail('a', { $flagged: true });
const b = makeEmail('b', { $flagged: true });
seedView(CROSS_STARRED, 'starred', [a, b]);
await useEmailStore.getState().toggleStar(client, 'a'); // removes the star
expect(useEmailStore.getState().retainedInViewIds.has('a')).toBe(true);
fetchCrossViewMock.mockResolvedValue({ emails: [makeEmail('b', { $flagged: true })], hasMore: false, total: 1, errors: new Map() });
await useEmailStore.getState().refreshCurrentMailbox(client);
const emails = useEmailStore.getState().emails;
expect(emails.map(e => e.id)).toEqual(['a', 'b']);
expect(emails.find(e => e.id === 'a')?.keywords?.$flagged).toBe(false);
});
it('re-starring drops the retention', async () => {
const a = makeEmail('a', {}); // currently unflagged
seedView(CROSS_STARRED, 'starred', [a]);
useEmailStore.setState({ retainedInViewIds: new Set(['a']) });
await useEmailStore.getState().toggleStar(client, 'a'); // adds the star back
expect(useEmailStore.getState().retainedInViewIds.has('a')).toBe(false);
});
});
describe('retained-in-view: scope', () => {
beforeEach(() => {
vi.clearAllMocks();
buildMock.mockResolvedValue([]);
});
it('does not retain on read in the "all" cross view (no keyword filter to fall out of)', async () => {
const a = makeEmail('a', {});
// 'all' view: reading never removes the row, so nothing to retain.
useSettingsStore.setState({ emailsPerPage: 25 });
useAuthStore.setState({
activeAccountId: 'account-a',
getClientForAccount: (acc: string) => (acc === 'account-a' ? client : undefined) as never,
} as never);
useEmailStore.setState({
isUnifiedView: true, unifiedRole: null, crossView: 'all', selectedMailbox: '__cross_all__',
selectedKeyword: null, searchQuery: '', mailboxes: [makeMailbox()],
accountMailboxes: { 'account-a': [makeMailbox()] },
processingReadStatus: new Set(), selectedEmailIds: new Set(), retainedInViewIds: new Set(),
emails: [a], totalEmails: 1,
});
await useEmailStore.getState().markAsRead(client, 'a', true);
expect(useEmailStore.getState().retainedInViewIds.size).toBe(0);
});
});
+127 -5
View File
@@ -60,6 +60,10 @@ interface EmailStore {
quota: { used: number; total: number } | null;
processingReadStatus: Set<string>; // Track emails being marked as read/unread
selectedEmailIds: Set<string>; // Track selected emails for batch operations
// Emails the user just read (unread view) or unstarred (starred view) that
// should stay visible in that self-filtering cross view until it is re-opened,
// instead of vanishing on the next push refresh. Cleared on navigation.
retainedInViewIds: Set<string>;
hasMoreEmails: boolean; // Track if more emails are available to load
totalEmails: number; // Total number of emails in the current mailbox/query
isPushConnected: boolean; // Track if push notifications are connected
@@ -804,6 +808,72 @@ if (typeof window !== 'undefined') {
});
}
/**
* The two cross views whose filter is the very keyword the read/star actions
* flip: 'unread' (notKeyword $seen) and 'starred' (hasKeyword $flagged). In
* those views a just-read / just-unstarred mail no longer matches the query, so
* a naive push refresh drops it. We keep such rows (see retainedInViewIds).
* The 'all' view has no keyword filter and needs no retention.
*/
function retainingCrossView(state: { isUnifiedView: boolean; crossView: CrossView | null }): 'unread' | 'starred' | null {
if (!state.isUnifiedView) return null;
return state.crossView === 'unread' || state.crossView === 'starred' ? state.crossView : null;
}
/**
* Immutably add/remove an id from the retain set. Returns the same set instance
* when nothing changes (op 'none', or a no-op add/remove) to avoid needless
* re-renders.
*/
function updateRetainedForView(current: Set<string>, op: 'add' | 'remove' | 'none', id: string): Set<string> {
if (op === 'none') return current;
if (op === 'add') {
if (current.has(id)) return current;
return new Set(current).add(id);
}
if (!current.has(id)) return current;
const next = new Set(current);
next.delete(id);
return next;
}
/**
* Re-insert rows the user just read/unstarred in an unread/starred cross view
* that fell out of the fresh keyword-filtered page, keeping them at their prior
* position so the row updates its status + counter instead of vanishing from
* under the cursor. Rows genuinely gone (deleted/moved, hence absent from
* `previous`) are NOT resurrected - only ids still present locally are kept.
* Returns the original `merged` untouched when nothing needs retaining.
*/
function mergeRetainedRows(previous: Email[], merged: Email[], retainedIds: Set<string>): Email[] {
if (retainedIds.size === 0) return merged;
const mergedById = new Map(merged.map(e => [e.id, e] as const));
const toRetain = previous.filter(e => retainedIds.has(e.id) && !mergedById.has(e.id));
if (toRetain.length === 0) return merged;
const previousIds = new Set(previous.map(e => e.id));
const out: Email[] = [];
const emitted = new Set<string>();
// Genuinely new arrivals (not previously loaded) keep their fresh top slot.
for (const e of merged) {
if (!previousIds.has(e.id)) { out.push(e); emitted.add(e.id); }
}
// Then previous order: fresh copy if the row still matches, retained local
// copy otherwise. Rows absent from both `merged` and the retain set are the
// truly-removed ones and get dropped here.
for (const e of previous) {
if (emitted.has(e.id)) continue;
const fresh = mergedById.get(e.id);
if (fresh) { out.push(fresh); emitted.add(e.id); }
else if (retainedIds.has(e.id)) { out.push(e); emitted.add(e.id); }
}
// Any deeper-page rows merged in but not covered above, in merged order.
for (const e of merged) {
if (!emitted.has(e.id)) { out.push(e); emitted.add(e.id); }
}
return out;
}
export const useEmailStore = create<EmailStore>((set, get) => ({
emails: [],
mailboxes: [],
@@ -819,6 +889,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
quota: null,
processingReadStatus: new Set(),
selectedEmailIds: new Set(),
retainedInViewIds: new Set(),
lastSelectedEmailId: null,
hasMoreEmails: false,
totalEmails: 0,
@@ -1074,7 +1145,9 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// repopulates the list without showing the loading overlay, so switching to
// an already-visited account doesn't flash a spinner over the visible mail.
const background = opts?.background ?? false;
set(background ? { error: null } : { isLoading: true, error: null }); // Keep previous emails visible during transition
// Loading a real mailbox is a fresh navigation (leaving any cross view), so
// drop the unread/starred retain set.
set(background ? { error: null, retainedInViewIds: new Set() } : { isLoading: true, error: null, retainedInViewIds: new Set() }); // Keep previous emails visible during transition
try {
const targetMailboxId = mailboxId || get().selectedMailbox;
if (targetMailboxId === VIRTUAL_SCHEDULED_MAILBOX_ID) {
@@ -1592,6 +1665,16 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
: mailbox,
);
// In the unread cross view, a mail just marked read no longer matches the
// filter. Retain it so the row stays put (status + counter update only)
// rather than vanishing on the next push refresh; marking it unread again
// makes it match, so drop the retention.
const retainedInViewIds = updateRetainedForView(
state.retainedInViewIds,
retainingCrossView(state) === 'unread' ? (read ? 'add' : 'remove') : 'none',
emailId,
);
return {
emails: state.emails.map(e =>
e.id === emailId ? { ...e, keywords: { ...e.keywords, $seen: read } } : e
@@ -1599,6 +1682,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
selectedEmail: state.selectedEmail?.id === emailId
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
: state.selectedEmail,
retainedInViewIds,
...mailboxPatch,
// Same delta, applied to every tag this email carries, so the sidebar
// tag badges track the folder counters instead of going stale.
@@ -2212,7 +2296,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
),
selectedEmail: state.selectedEmail?.id === emailId
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $flagged: !isFlagged } }
: state.selectedEmail
: state.selectedEmail,
// In the starred cross view, un-starring drops the mail out of the
// hasKeyword:$flagged filter. Retain the row so it stays visible until the
// view is re-opened; re-starring makes it match again, so drop retention.
retainedInViewIds: updateRetainedForView(
state.retainedInViewIds,
retainingCrossView(state) === 'starred' ? (isFlagged ? 'add' : 'remove') : 'none',
emailId,
),
}));
} catch (error) {
set({
@@ -2309,10 +2401,21 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
.map(email => ({ keywords: email.keywords, delta: read ? -1 : 1 })),
);
// In the unread cross view, retain every mail we just marked read so the
// rows stay put (status + counter update only); marking unread drops them.
let retainedInViewIds = get().retainedInViewIds;
if (retainingCrossView(get()) === 'unread') {
const op = read ? 'add' : 'remove';
for (const e of affectedEmails) {
retainedInViewIds = updateRetainedForView(retainedInViewIds, op, e.id);
}
}
set({
emails: updatedEmails,
...mailboxPatch,
tagCounts,
retainedInViewIds,
selectedEmailIds: new Set(),
isLoading: false
});
@@ -3016,7 +3119,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// append the whole previous list: drafts are saved as destroy+create, so
// the old draft can disappear from the refreshed first page and must not
// be reintroduced from stale local state.
const merged: Email[] = [...refreshedEmails];
let merged: Email[] = [...refreshedEmails];
const mergedIds = new Set(refreshedEmails.map((e: Email) => e.id));
const insertedCount = Math.max((result.total || 0) - previousTotal, 0);
// Derive the cutoff from the page size, not from the refreshed list's
@@ -3035,6 +3138,18 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
}
}
// Keep rows the user just read/unstarred in the unread/starred cross view:
// the fresh keyword-filtered page dropped them, but they should stay put
// (their status + the counters already updated in the action) until the
// view is re-opened. Only applies to those two views; the retain set is
// empty otherwise.
let retainedAddedCount = 0;
if (retainingCrossView({ isUnifiedView, crossView })) {
const beforeLen = merged.length;
merged = mergeRetainedRows(currentEmails, merged, get().retainedInViewIds);
retainedAddedCount = merged.length - beforeLen;
}
// Check if anything actually changed to avoid unnecessary re-renders
const hasChanged =
currentEmails.length !== merged.length ||
@@ -3049,9 +3164,13 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
});
if (hasChanged) {
// Retained rows sit beyond the server's keyword-filtered count, so add
// them to the total; otherwise the list would claim fewer items than it
// shows and mis-compute hasMore.
const effectiveTotal = (result.total || 0) + retainedAddedCount;
// hasMore should reflect whether there are still more emails beyond
// what we have loaded, using the fresh total from the server.
const hasMore = merged.length < (result.total || 0);
const hasMore = merged.length < effectiveTotal;
// Invalidate thread email caches for threads whose composition changed
// so expanded threads pick up new/removed emails.
@@ -3094,7 +3213,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
return {
emails: merged,
hasMoreEmails: hasMore,
totalEmails: result.total,
totalEmails: effectiveTotal,
threadEmailsCache: newCache,
...(unifiedErrors !== undefined ? { unifiedErrors } : {}),
};
@@ -3555,6 +3674,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
unifiedRole: role,
crossView: null,
selectedKeyword: null,
retainedInViewIds: new Set(),
});
try {
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
@@ -3630,6 +3750,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
unifiedRole: null,
crossView: view,
selectedKeyword: null,
retainedInViewIds: new Set(),
});
try {
const emailsPerPage = useSettingsStore.getState().emailsPerPage;
@@ -3670,6 +3791,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
unifiedRole: null,
crossView: null,
unifiedErrors: new Map(),
retainedInViewIds: new Set(),
});
},