fix: keep sidebar tag counts in step with read/unread changes
Marking mail as read left the sidebar's tag unread counts untouched — the folder counts cleared, but a tag went on showing "47 unread" in bold until the page was reloaded. tagCounts is fetched from the server (Email/query per $label keyword) rather than derived from state, and no read/unread mutation refreshed or adjusted it. The per-mailbox unreadEmails counters were kept current by a local delta; tags simply had no equivalent. Add applyTagCountReadDelta alongside the existing mailbox-counter helpers and apply it wherever the affected emails are known locally: markAsRead, batchMarkAsRead, and setEmailKeywordsLocal. Only a genuine $seen flip moves a count, so re-marking a read email as read cannot drift it, and unread is clamped at zero. A tag's total is never touched by a read-state change. markMailboxAsRead is the exception and refetches instead: it is a server-side bulk operation over an entire mailbox, so it also marks emails that were never loaded into state.emails, and a local delta would leave the counts high.
This commit is contained in:
@@ -0,0 +1,257 @@
|
|||||||
|
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import { useEmailStore } from '../email-store';
|
||||||
|
import { useAuthStore } from '../auth-store';
|
||||||
|
import { useSettingsStore } from '../settings-store';
|
||||||
|
import type { Email, Mailbox } from '@/lib/jmap/types';
|
||||||
|
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||||
|
|
||||||
|
// Sidebar tag badges render from `tagCounts`, which is fetched from the server
|
||||||
|
// (`fetchTagCounts` -> `client.getTagCounts`) rather than derived from
|
||||||
|
// `state.emails`. Read/unread mutations therefore have to keep it in step the
|
||||||
|
// same way they keep `mailboxes[].unreadEmails` in step, or the tag unread
|
||||||
|
// count (and the bold tag name) stays stale until a full page reload.
|
||||||
|
|
||||||
|
function makeMailbox(overrides: Partial<Mailbox> = {}): Mailbox {
|
||||||
|
return {
|
||||||
|
id: 'inbox',
|
||||||
|
name: 'Inbox',
|
||||||
|
role: 'inbox',
|
||||||
|
sortOrder: 0,
|
||||||
|
totalEmails: 10,
|
||||||
|
unreadEmails: 5,
|
||||||
|
totalThreads: 10,
|
||||||
|
unreadThreads: 5,
|
||||||
|
myRights: {
|
||||||
|
mayReadItems: true,
|
||||||
|
mayAddItems: true,
|
||||||
|
mayRemoveItems: true,
|
||||||
|
maySetSeen: true,
|
||||||
|
maySetKeywords: true,
|
||||||
|
mayCreateChild: true,
|
||||||
|
mayRename: true,
|
||||||
|
mayDelete: true,
|
||||||
|
maySubmit: true,
|
||||||
|
},
|
||||||
|
isSubscribed: true,
|
||||||
|
isShared: false,
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeEmail(overrides: Partial<Email> = {}): Email {
|
||||||
|
return {
|
||||||
|
id: 'email-1',
|
||||||
|
threadId: 'thread-1',
|
||||||
|
subject: 'Hi',
|
||||||
|
receivedAt: new Date().toISOString(),
|
||||||
|
keywords: {},
|
||||||
|
mailboxIds: { inbox: true },
|
||||||
|
...overrides,
|
||||||
|
} as Email;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeClient() {
|
||||||
|
return {
|
||||||
|
markAsRead: vi.fn().mockResolvedValue(undefined),
|
||||||
|
batchMarkAsRead: vi.fn().mockResolvedValue(undefined),
|
||||||
|
markMailboxAsRead: vi.fn().mockResolvedValue(3),
|
||||||
|
getTagCounts: vi.fn().mockResolvedValue({}),
|
||||||
|
getAccountId: vi.fn().mockReturnValue('account-a'),
|
||||||
|
} as unknown as IJMAPClient;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('email-store tag counts stay in step with read state', () => {
|
||||||
|
let client: IJMAPClient;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
client = makeClient();
|
||||||
|
|
||||||
|
useAuthStore.setState({
|
||||||
|
activeAccountId: 'account-a',
|
||||||
|
getClientForAccount: (() => client) as never,
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
useSettingsStore.setState({
|
||||||
|
emailKeywords: [
|
||||||
|
{ id: 'ingsel', label: 'Ingsel', color: 'red' },
|
||||||
|
{ id: 'work', label: 'Work', color: 'blue' },
|
||||||
|
],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
useEmailStore.setState({
|
||||||
|
isUnifiedView: false,
|
||||||
|
viewingAccountId: null,
|
||||||
|
selectedMailbox: 'inbox',
|
||||||
|
mailboxes: [makeMailbox()],
|
||||||
|
accountMailboxes: {},
|
||||||
|
emails: [],
|
||||||
|
selectedEmail: null,
|
||||||
|
selectedEmailIds: new Set(),
|
||||||
|
processingReadStatus: new Set(),
|
||||||
|
threadEmailsCache: new Map(),
|
||||||
|
tagCounts: {
|
||||||
|
ingsel: { total: 1658, unread: 47 },
|
||||||
|
work: { total: 200, unread: 9 },
|
||||||
|
},
|
||||||
|
} as never);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('decrements only the matching tag when a tagged email is marked read', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||||
|
ingsel: { total: 1658, unread: 46 },
|
||||||
|
work: { total: 200, unread: 9 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('increments the tag again when the email is marked unread', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true, $seen: true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', false);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel).toEqual({ total: 1658, unread: 48 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('updates both tags when an email carries two tags', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true, '$label:work': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||||
|
ingsel: { total: 1658, unread: 46 },
|
||||||
|
work: { total: 200, unread: 8 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves tag counts alone for an untagged email', async () => {
|
||||||
|
useEmailStore.setState({ emails: [makeEmail({ keywords: {} })] } as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||||
|
ingsel: { total: 1658, unread: 47 },
|
||||||
|
work: { total: 200, unread: 9 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not double-decrement when an already-read email is marked read', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true, $seen: true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel).toEqual({ total: 1658, unread: 47 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never drives a tag unread count negative', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
tagCounts: { ingsel: { total: 3, unread: 0 } },
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel).toEqual({ total: 3, unread: 0 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never alters `total` on a read-state change', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', true);
|
||||||
|
await useEmailStore.getState().markAsRead(client, 'email-1', false);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel.total).toBe(1658);
|
||||||
|
expect(useEmailStore.getState().tagCounts.work.total).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('batchMarkAsRead', () => {
|
||||||
|
it('applies the delta once per tag per changed email', async () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [
|
||||||
|
makeEmail({ id: 'e1', keywords: { '$label:ingsel': true } }),
|
||||||
|
makeEmail({ id: 'e2', keywords: { '$label:ingsel': true, '$label:work': true } }),
|
||||||
|
// Already read: must not contribute a delta.
|
||||||
|
makeEmail({ id: 'e3', keywords: { '$label:work': true, $seen: true } }),
|
||||||
|
// Untagged: must not contribute a delta.
|
||||||
|
makeEmail({ id: 'e4', keywords: {} }),
|
||||||
|
],
|
||||||
|
selectedEmailIds: new Set(['e1', 'e2', 'e3', 'e4']),
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
await useEmailStore.getState().batchMarkAsRead(client, true);
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||||
|
ingsel: { total: 1658, unread: 45 },
|
||||||
|
work: { total: 200, unread: 8 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('markMailboxAsRead', () => {
|
||||||
|
it('refetches tag counts from the server rather than applying a local delta', async () => {
|
||||||
|
(client.getTagCounts as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
ingsel: { total: 1658, unread: 0 },
|
||||||
|
work: { total: 200, unread: 4 },
|
||||||
|
});
|
||||||
|
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
const count = await useEmailStore.getState().markMailboxAsRead(client, 'inbox');
|
||||||
|
expect(count).toBe(3);
|
||||||
|
|
||||||
|
// 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']);
|
||||||
|
|
||||||
|
await vi.waitFor(() => {
|
||||||
|
expect(useEmailStore.getState().tagCounts).toEqual({
|
||||||
|
ingsel: { total: 1658, unread: 0 },
|
||||||
|
work: { total: 200, unread: 4 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('setEmailKeywordsLocal', () => {
|
||||||
|
it('adjusts tag unread counts when the local keyword patch flips $seen', () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
useEmailStore.getState().setEmailKeywordsLocal('email-1', {
|
||||||
|
'$label:ingsel': true,
|
||||||
|
$seen: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel).toEqual({ total: 1658, unread: 46 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves tag unread counts alone when $seen is unchanged', () => {
|
||||||
|
useEmailStore.setState({
|
||||||
|
emails: [makeEmail({ keywords: { '$label:ingsel': true } })],
|
||||||
|
} as never);
|
||||||
|
|
||||||
|
// Pin toggle: labels/pin change, read state does not.
|
||||||
|
useEmailStore.getState().setEmailKeywordsLocal('email-1', {
|
||||||
|
'$label:ingsel': true,
|
||||||
|
$pinned: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(useEmailStore.getState().tagCounts.ingsel).toEqual({ total: 1658, unread: 47 });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
+72
-8
@@ -584,6 +584,37 @@ function applyBatchMailboxCounterUpdate(
|
|||||||
return { mailboxes, accountMailboxes };
|
return { mailboxes, accountMailboxes };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Sidebar tag badges render from `tagCounts`, which is *fetched from the server*
|
||||||
|
// (`fetchTagCounts` -> `getTagCounts`) rather than derived from `state.emails`.
|
||||||
|
// So a read/unread mutation has to keep it in step locally, exactly as it does
|
||||||
|
// for `mailboxes[].unreadEmails` - otherwise the tag unread count (and the bold
|
||||||
|
// tag name) stays stale until a full page reload.
|
||||||
|
//
|
||||||
|
// `changes` carries one entry per email whose read state *actually changed*
|
||||||
|
// (callers already compute that), with delta -1 when it became read and +1 when
|
||||||
|
// it became unread. Only `unread` moves: read state never changes tag
|
||||||
|
// membership, so `total` is left alone.
|
||||||
|
function applyTagCountReadDelta(
|
||||||
|
tagCounts: Record<string, { total: number; unread: number }>,
|
||||||
|
changes: Array<{ keywords?: Record<string, boolean>; delta: number }>,
|
||||||
|
): Record<string, { total: number; unread: number }> {
|
||||||
|
const keywordIds = useSettingsStore.getState().emailKeywords.map(k => k.id);
|
||||||
|
if (keywordIds.length === 0) return tagCounts;
|
||||||
|
|
||||||
|
let next: Record<string, { total: number; unread: number }> | null = null;
|
||||||
|
for (const { keywords, delta } of changes) {
|
||||||
|
if (!keywords || delta === 0) continue;
|
||||||
|
for (const id of keywordIds) {
|
||||||
|
if (!keywords[`$label:${id}`]) continue;
|
||||||
|
const current = (next ?? tagCounts)[id];
|
||||||
|
if (!current) continue; // Tag not in the fetched counts yet; nothing to adjust.
|
||||||
|
next = next ?? { ...tagCounts };
|
||||||
|
next[id] = { total: current.total, unread: Math.max(0, current.unread + delta) };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return next ?? tagCounts;
|
||||||
|
}
|
||||||
|
|
||||||
// Per-mailbox counter map (for applyBatchMailboxCounterUpdate) for removing a
|
// Per-mailbox counter map (for applyBatchMailboxCounterUpdate) for removing a
|
||||||
// group of emails from a folder: decrement total (and unread for unseen) for
|
// group of emails from a folder: decrement total (and unread for unseen) for
|
||||||
// each group email that lives in the mailbox.
|
// each group email that lives in the mailbox.
|
||||||
@@ -1421,6 +1452,11 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
|
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
|
||||||
: state.selectedEmail,
|
: state.selectedEmail,
|
||||||
...mailboxPatch,
|
...mailboxPatch,
|
||||||
|
// Same delta, applied to every tag this email carries, so the sidebar
|
||||||
|
// tag badges track the folder counters instead of going stale.
|
||||||
|
tagCounts: applyTagCountReadDelta(state.tagCounts, [
|
||||||
|
{ keywords: emailInState.keywords, delta },
|
||||||
|
]),
|
||||||
processingReadStatus: newProcessingSet,
|
processingReadStatus: newProcessingSet,
|
||||||
// Also update threadEmailsCache so expanded dropdowns reflect the change
|
// Also update threadEmailsCache so expanded dropdowns reflect the change
|
||||||
threadEmailsCache: (() => {
|
threadEmailsCache: (() => {
|
||||||
@@ -1976,14 +2012,24 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
},
|
},
|
||||||
|
|
||||||
setEmailKeywordsLocal: (emailId, keywords) => {
|
setEmailKeywordsLocal: (emailId, keywords) => {
|
||||||
set((state) => ({
|
set((state) => {
|
||||||
emails: state.emails.map(e =>
|
// This patch replaces the whole keyword map, so it can flip $seen as well
|
||||||
e.id === emailId ? { ...e, keywords: { ...keywords } } : e
|
// as labels. Only a genuine read-state change moves the tag unread counts.
|
||||||
),
|
const previous = state.emails.find(e => e.id === emailId) ?? state.selectedEmail;
|
||||||
selectedEmail: state.selectedEmail?.id === emailId
|
const wasRead = previous?.keywords?.$seen ?? false;
|
||||||
? { ...state.selectedEmail, keywords: { ...keywords } }
|
const isRead = keywords.$seen ?? false;
|
||||||
: state.selectedEmail,
|
const delta = wasRead === isRead ? 0 : (isRead ? -1 : 1);
|
||||||
}));
|
|
||||||
|
return {
|
||||||
|
emails: state.emails.map(e =>
|
||||||
|
e.id === emailId ? { ...e, keywords: { ...keywords } } : e
|
||||||
|
),
|
||||||
|
selectedEmail: state.selectedEmail?.id === emailId
|
||||||
|
? { ...state.selectedEmail, keywords: { ...keywords } }
|
||||||
|
: state.selectedEmail,
|
||||||
|
tagCounts: applyTagCountReadDelta(state.tagCounts, [{ keywords, delta }]),
|
||||||
|
};
|
||||||
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
// Batch operations
|
// Batch operations
|
||||||
@@ -2041,9 +2087,19 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Tag badges follow the same delta as the folder counters, counting only
|
||||||
|
// the emails whose read state actually changed.
|
||||||
|
const tagCounts = applyTagCountReadDelta(
|
||||||
|
get().tagCounts,
|
||||||
|
affectedEmails
|
||||||
|
.filter(email => (email.keywords?.$seen ?? false) !== read)
|
||||||
|
.map(email => ({ keywords: email.keywords, delta: read ? -1 : 1 })),
|
||||||
|
);
|
||||||
|
|
||||||
set({
|
set({
|
||||||
emails: updatedEmails,
|
emails: updatedEmails,
|
||||||
...mailboxPatch,
|
...mailboxPatch,
|
||||||
|
tagCounts,
|
||||||
selectedEmailIds: new Set(),
|
selectedEmailIds: new Set(),
|
||||||
isLoading: false
|
isLoading: false
|
||||||
});
|
});
|
||||||
@@ -3158,6 +3214,14 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
),
|
),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// Tag counts are refetched here rather than adjusted with a local delta
|
||||||
|
// (as markAsRead/batchMarkAsRead do). This is a server-side bulk operation
|
||||||
|
// over the *whole* mailbox, so it also marks emails that were never loaded
|
||||||
|
// into `state.emails` - a local delta would only see the loaded page and
|
||||||
|
// would leave the tag counts drifting high. Fire-and-forget: the folder
|
||||||
|
// counters above already update instantly.
|
||||||
|
void get().fetchTagCounts(client);
|
||||||
|
|
||||||
return count;
|
return count;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
set({ error: error instanceof Error ? error.message : 'Failed to mark folder as read' });
|
set({ error: error instanceof Error ? error.message : 'Failed to mark folder as read' });
|
||||||
|
|||||||
Reference in New Issue
Block a user