From ddff2f7b66ae19d57b38e60e30684d959c76d61d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:43:31 +0200 Subject: [PATCH] fix: queue star, tag and unread from the viewer while offline The viewer's star, tag and mark-unread buttons called the API directly, so offline they failed and the list row behind the viewer kept its old state. They now go through the email store's keyword action like the list's selection actions: offline the change waits in the outbox, and the list row follows. The store action takes the viewed message and its account, so it only touches the list when the list holds that account and never a same-id row of another one. --- src/screens/EmailThreadScreen.tsx | 19 +++++-- .../email-store-viewer-actions.test.ts | 51 ++++++++++++++++++- src/stores/email-store.ts | 18 ++++--- 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index fcaffb22..56202568 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -68,6 +68,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const { width: windowWidth } = useWindowDimensions(); const getEmailDetail = useEmailStore((s) => s.getEmailDetail); const markRead = useEmailStore((s) => s.markRead); + const setKeywordForEmails = useEmailStore((s) => s.setKeywordForEmails); const deleteEmail = useEmailStore((s) => s.deleteEmail); const moveToMailbox = useEmailStore((s) => s.moveToMailbox); const archiveEmailAction = useEmailStore((s) => s.archiveEmail); @@ -256,6 +257,16 @@ export default function EmailThreadScreen({ route, navigation }: Props) { [listAccountId, ownerAccountId, markRead], ); + // Star, tag and unread go through the store as well, which knows the + // message's account from `viewed`: offline they wait in the outbox, and the + // list row follows when the list holds that account (never a same-id row + // of another one). + const setKeyword = React.useCallback( + (target: Email, token: string, on: boolean) => + setKeywordForEmails([target.id], token, on, { email: target, accountId: ownerAccountId }), + [setKeywordForEmails, ownerAccountId], + ); + // Mark a message read per the user's delay setting: -1 never, 0 instantly, // >0 after that many milliseconds. Returns a cancel function. const scheduleMarkRead = React.useCallback((target: Email): (() => void) => { @@ -336,7 +347,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { if (next[token]) delete next[token]; else next[token] = true; updateLocalKeywords(email.id, next); - patchKeywordsForEmails([email.id], { [token]: next[token] ?? null }, ownerAccountId).catch((err) => { + setKeyword(email, token, !!next[token]).catch((err) => { updateLocalKeywords(email.id, email.keywords); toastFailure(t('notifications.tag_failed', 'Tagging failed'), err); }); @@ -349,11 +360,11 @@ export default function EmailThreadScreen({ route, navigation }: Props) { if (next.$flagged) delete next.$flagged; else next.$flagged = true; updateLocalKeywords(target.id, next); - patchKeywordsForEmails([target.id], { $flagged: next.$flagged ?? null }, ownerAccountId).catch((err) => { + setKeyword(target, '$flagged', !!next.$flagged).catch((err) => { updateLocalKeywords(target.id, target.keywords); toastFailure(t('notifications.error_updating', 'Failed to update email'), err); }); - }, [updateLocalKeywords, ownerAccountId, t]); + }, [updateLocalKeywords, setKeyword, t]); const onToggleStar = () => { if (email) toggleStarFor(email); }; @@ -370,7 +381,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const next = { ...email.keywords }; delete next.$seen; updateLocalKeywords(email.id, next); - patchKeywordsForEmails([email.id], { $seen: null }, ownerAccountId).catch(failed); + setKeyword(email, '$seen', false).catch(failed); } }; diff --git a/src/stores/__tests__/email-store-viewer-actions.test.ts b/src/stores/__tests__/email-store-viewer-actions.test.ts index f53734b0..ee6ec8ea 100644 --- a/src/stores/__tests__/email-store-viewer-actions.test.ts +++ b/src/stores/__tests__/email-store-viewer-actions.test.ts @@ -43,10 +43,10 @@ vi.mock('../locale-store', () => ({ // Online with nothing queued: run the online runner straight away. vi.mock('../outbox-store', () => { - const applyOrQueueBatch = async (_ops: unknown[], onlineRun?: () => Promise) => { + const applyOrQueueBatch = vi.fn(async (_ops: unknown[], onlineRun?: () => Promise) => { if (onlineRun) await onlineRun(); return { queued: false }; - }; + }); return { applyOrQueueBatch, applyOrQueue: async (op: unknown, onlineRun?: () => Promise) => applyOrQueueBatch([op], onlineRun), @@ -94,6 +94,7 @@ vi.mock('../../api/jmap-client', () => ({ })); import * as emailApi from '../../api/email'; +import { applyOrQueueBatch } from '../outbox-store'; import { useEmailStore, type ViewedEmail } from '../email-store'; import { useSettingsStore } from '../settings-store'; import type { Email, Mailbox } from '../../api/types'; @@ -104,6 +105,8 @@ const mockRestore = emailApi.restoreEmailMailboxes as ReturnType; const mockMarkAsSpam = emailApi.markAsSpam as ReturnType; const mockUndoSpam = emailApi.undoSpam as ReturnType; const mockArchive = emailApi.archiveEmails as ReturnType; +const mockPatchKeywords = emailApi.patchKeywordsForEmails as ReturnType; +const mockApplyOrQueueBatch = applyOrQueueBatch as unknown as ReturnType; function own(id: string, role: string): Mailbox { return { id, name: role, role, accountId: 'acc-1', isShared: false } as Mailbox; @@ -254,3 +257,47 @@ describe('viewer Archive of a message the list does not hold', () => { expect(useEmailStore.getState().pendingUndo?.accountId).toBe('grp-1'); }); }); + +describe('viewer star, tag and unread go through the store', () => { + it("stars a team message in the team account and leaves the open inbox's same-id row alone", async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await useEmailStore.getState().setKeywordForEmails(['e1'], '$flagged', true, TEAM_VIEWED); + + // Queued for the team account when offline, sent to it when online. + expect(mockApplyOrQueueBatch.mock.calls[0][0]).toEqual([ + { kind: 'keywords', emailId: 'e1', accountId: 'grp-1', patch: { $flagged: true } }, + ]); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $flagged: true }, 'grp-1'); + expect(useEmailStore.getState().emails).toEqual([OWN_ROW]); + }); + + it("updates the list row when the list holds the message's account", async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [OWN_ROW] }); + + await useEmailStore.getState().setKeywordForEmails(['e1'], '$label1', true, { email: OWN_ROW, accountId: undefined }); + + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $label1: true }, undefined); + expect(useEmailStore.getState().emails[0].keywords).toEqual({ $label1: true }); + }); + + it('keeps an unstarred row in the Starred view like the list does', async () => { + const starred = { ...OWN_ROW, keywords: { $flagged: true } } as Email; + useEmailStore.setState({ + mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [starred], filters: { isStarred: true }, + }); + + await useEmailStore.getState().setKeywordForEmails(['e1'], '$flagged', false, { email: starred, accountId: undefined }); + + expect(useEmailStore.getState().retainedIds).toEqual(['e1']); + }); + + it('still sends the change for a message the list does not show', async () => { + useEmailStore.setState({ mailboxes: MAILBOXES, currentMailboxId: 'a', emails: [] }); + + await useEmailStore.getState().setKeywordForEmails(['e1'], '$seen', false, { email: OWN_ROW, accountId: undefined }); + + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $seen: null }, undefined); + expect(useEmailStore.getState().emails).toEqual([]); + }); +}); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index 61aebd32..32a2e58c 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -383,8 +383,12 @@ export interface EmailState { archiveEmailsBatch: (emailIds: string[]) => Promise; moveEmailsToMailbox: (emailIds: string[], toMailboxId: string) => Promise; deleteEmailsBatch: (emailIds: string[], trashMailboxId: string, currentMailboxId: string) => Promise; - /** Set or clear one keyword (tag, `$seen`, `$flagged`) on a selection in one `Email/set`. */ - setKeywordForEmails: (emailIds: string[], token: string, on: boolean) => Promise; + /** + * Set or clear one keyword (tag, `$seen`, `$flagged`) on a selection in one + * `Email/set`. With `viewed`, on the viewer's message in its own account; + * the list row follows only when the list holds that account. + */ + setKeywordForEmails: (emailIds: string[], token: string, on: boolean, viewed?: ViewedEmail) => Promise; undoLast: () => Promise; clearUndo: () => void; searchEmails: (query: string) => Promise; @@ -1578,11 +1582,11 @@ export const useEmailStore = create()( } }, - setKeywordForEmails: async (emailIds, token, on) => { + setKeywordForEmails: async (emailIds, token, on, viewed) => { const state = get(); - const targets = state.emails.filter((e) => emailIds.includes(e.id)); + const { targets, listed } = actionTargets(state, emailIds, viewed); if (targets.length === 0) return; - const owner = currentAccountId(state); + const owner = viewed ? viewed.accountId : currentAccountId(state); const ids = targets.map((e) => e.id); const patch = { [token]: on ? true : null }; await applyOrQueueBatch( @@ -1594,14 +1598,14 @@ export const useEmailStore = create()( })), () => patchKeywordsForEmails(ids, patch, owner), ); - const touched = new Set(ids); + const touched = new Set(listed ? ids : []); set({ emails: get().emails.map((e) => touched.has(e.id) ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), // Reading inside Unread, unstarring inside Starred or untagging inside // that tag's view keeps the rows until it's re-opened. - ...(leavesView(state.filters, token, on) + ...(listed && leavesView(state.filters, token, on) ? { retainedIds: retain(get().retainedIds, ids) } : {}), });