fix: route unread-counter update to the email's own account in aggregate views
In a cross-account view, marking a second account's email read/unread updated the *active* account's folder counter instead of the email's. Two causes: the optimistic counter update only touched `state.mailboxes` (the active account), and JMAP mailbox ids can collide across accounts so the id match hit the wrong folder. Add applyMailboxCounterUpdate(): route the counter delta to the list that holds the email's folders — the active account's `mailboxes` (incl. its shared folders) for active-account/shared emails, otherwise that account's `accountMailboxes[sourceClientAccountId]` entry. Use it in markAsRead. Regression test: a 2nd-account email with a colliding inbox id decrements that account's counter and leaves the active account's untouched.
This commit is contained in:
@@ -68,6 +68,7 @@ describe('unified-view single-email action routing (#281)', () => {
|
|||||||
// account-a is the active login; account-b is a second direct login; the
|
// account-a is the active login; account-b is a second direct login; the
|
||||||
// active login (account-a) also delegates access to the shared owner 'owner-x'.
|
// active login (account-a) also delegates access to the shared owner 'owner-x'.
|
||||||
useAuthStore.setState({
|
useAuthStore.setState({
|
||||||
|
activeAccountId: 'account-a',
|
||||||
getClientForAccount: (id: string) =>
|
getClientForAccount: (id: string) =>
|
||||||
(id === 'account-b' ? accountBClient : id === 'account-a' ? activeClient : undefined) as never,
|
(id === 'account-b' ? accountBClient : id === 'account-a' ? activeClient : undefined) as never,
|
||||||
} as never);
|
} as never);
|
||||||
@@ -124,6 +125,27 @@ describe('unified-view single-email action routing (#281)', () => {
|
|||||||
expect(activeClient.moveEmail).not.toHaveBeenCalled();
|
expect(activeClient.moveEmail).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('updates the unread counter on the email’s own account, not the active one (id collision)', async () => {
|
||||||
|
// Real per-account JMAP ids can collide; here both inboxes use the same id.
|
||||||
|
useEmailStore.setState({
|
||||||
|
mailboxes: [makeMailbox({ id: 'inbox', role: 'inbox', unreadEmails: 5 })],
|
||||||
|
accountMailboxes: {
|
||||||
|
'account-a': [makeMailbox({ id: 'inbox', role: 'inbox', unreadEmails: 5 })],
|
||||||
|
'account-b': [makeMailbox({ id: 'inbox', role: 'inbox', unreadEmails: 3 })],
|
||||||
|
},
|
||||||
|
emails: [
|
||||||
|
makeEmail({ id: 'b1', sourceClientAccountId: 'account-b', sourceAccountId: 'account-b', keywords: {}, mailboxIds: { inbox: true } }),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
|
||||||
|
await useEmailStore.getState().markAsRead(activeClient, 'b1', true);
|
||||||
|
|
||||||
|
const s = useEmailStore.getState();
|
||||||
|
expect(s.accountMailboxes['account-b'][0].unreadEmails).toBe(2); // account-b decremented
|
||||||
|
expect(s.mailboxes[0].unreadEmails).toBe(5); // active account untouched
|
||||||
|
expect(s.accountMailboxes['account-a'][0].unreadEmails).toBe(5); // active list untouched
|
||||||
|
});
|
||||||
|
|
||||||
it('routes a shared/group email through the delegating login client + owner accountId', async () => {
|
it('routes a shared/group email through the delegating login client + owner accountId', async () => {
|
||||||
await useEmailStore.getState().markAsRead(activeClient, 'email-shared', true);
|
await useEmailStore.getState().markAsRead(activeClient, 'email-shared', true);
|
||||||
// Reached via account-a's client (the active one), targeting the owner account.
|
// Reached via account-a's client (the active one), targeting the owner account.
|
||||||
|
|||||||
+41
-16
@@ -522,6 +522,34 @@ function emailInMailbox(
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Apply a per-mailbox counter adjustment to the mailbox list that actually holds
|
||||||
|
// the email's folders, and return the matching `set()` partial.
|
||||||
|
//
|
||||||
|
// In aggregate views an email may belong to another logged-in account. The
|
||||||
|
// sidebar shows the active account's folders from `mailboxes` (its
|
||||||
|
// getAllMailboxes, incl. its delegated shared folders) and every *other*
|
||||||
|
// account's folders from `accountMailboxes[<AccountEntry.id>]`. Updating the
|
||||||
|
// wrong list silently corrupts counters - and because JMAP mailbox ids can
|
||||||
|
// collide across accounts, blindly matching `mailboxes` would even decrement the
|
||||||
|
// *active* account's folder for a different account's email. So route by the
|
||||||
|
// email's source login: a different account → its `accountMailboxes` entry,
|
||||||
|
// otherwise (active account, its shared folders, or a non-aggregate view) →
|
||||||
|
// `mailboxes`. (#281)
|
||||||
|
function applyMailboxCounterUpdate(
|
||||||
|
state: { mailboxes: Mailbox[]; accountMailboxes: Record<string, Mailbox[]> },
|
||||||
|
email: { sourceClientAccountId?: string },
|
||||||
|
adjust: (mb: Mailbox) => Mailbox,
|
||||||
|
): { mailboxes?: Mailbox[]; accountMailboxes?: Record<string, Mailbox[]> } {
|
||||||
|
const srcClient = email.sourceClientAccountId;
|
||||||
|
const activeId = useAuthStore.getState().activeAccountId;
|
||||||
|
if (srcClient && srcClient !== activeId) {
|
||||||
|
const list = state.accountMailboxes[srcClient];
|
||||||
|
if (!list) return {};
|
||||||
|
return { accountMailboxes: { ...state.accountMailboxes, [srcClient]: list.map(adjust) } };
|
||||||
|
}
|
||||||
|
return { mailboxes: state.mailboxes.map(adjust) };
|
||||||
|
}
|
||||||
|
|
||||||
// Find the trash mailbox for a given account scope. Prefers JMAP role, but
|
// Find the trash mailbox for a given account scope. Prefers JMAP role, but
|
||||||
// falls back to name matching ("trash" / "deleted") so users with custom or
|
// falls back to name matching ("trash" / "deleted") so users with custom or
|
||||||
// pre-existing folders (e.g. "Deleted Items") aren't silently destroyed.
|
// pre-existing folders (e.g. "Deleted Items") aren't silently destroyed.
|
||||||
@@ -1327,21 +1355,18 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
return { processingReadStatus: newProcessingSet }; // State unchanged, skip counter update
|
return { processingReadStatus: newProcessingSet }; // State unchanged, skip counter update
|
||||||
}
|
}
|
||||||
|
|
||||||
const updatedMailboxes = state.mailboxes.map(mailbox => {
|
// Adjust the unread counter on the folder(s) holding this email, in the
|
||||||
// Check if this email belongs to this mailbox. Shared mailboxes are
|
// email's *own* account's mailbox list (#281). -1 marking read, +1 unread.
|
||||||
// stored under a namespaced id but the email's mailboxIds are keyed by
|
const delta = read ? -1 : 1;
|
||||||
// the owner-side JMAP id (originalId), so match on originalId first.
|
const mailboxPatch = applyMailboxCounterUpdate(state, emailInState, (mailbox) =>
|
||||||
if (emailInMailbox(emailInState, mailbox)) {
|
emailInMailbox(emailInState, mailbox)
|
||||||
// Adjust unread counter: -1 if marking as read, +1 if marking as unread
|
? {
|
||||||
const delta = read ? -1 : 1;
|
...mailbox,
|
||||||
return {
|
unreadEmails: Math.max(0, mailbox.unreadEmails + delta),
|
||||||
...mailbox,
|
unreadThreads: Math.max(0, mailbox.unreadThreads + delta),
|
||||||
unreadEmails: Math.max(0, mailbox.unreadEmails + delta),
|
}
|
||||||
unreadThreads: Math.max(0, mailbox.unreadThreads + delta)
|
: mailbox,
|
||||||
};
|
);
|
||||||
}
|
|
||||||
return mailbox;
|
|
||||||
});
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
emails: state.emails.map(e =>
|
emails: state.emails.map(e =>
|
||||||
@@ -1350,7 +1375,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
|||||||
selectedEmail: state.selectedEmail?.id === emailId
|
selectedEmail: state.selectedEmail?.id === emailId
|
||||||
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
|
? { ...state.selectedEmail, keywords: { ...state.selectedEmail.keywords, $seen: read } }
|
||||||
: state.selectedEmail,
|
: state.selectedEmail,
|
||||||
mailboxes: updatedMailboxes,
|
...mailboxPatch,
|
||||||
processingReadStatus: newProcessingSet,
|
processingReadStatus: newProcessingSet,
|
||||||
// Also update threadEmailsCache so expanded dropdowns reflect the change
|
// Also update threadEmailsCache so expanded dropdowns reflect the change
|
||||||
threadEmailsCache: (() => {
|
threadEmailsCache: (() => {
|
||||||
|
|||||||
Reference in New Issue
Block a user