diff --git a/src/api/__tests__/email.test.ts b/src/api/__tests__/email.test.ts index 5100df45..e15c4555 100644 --- a/src/api/__tests__/email.test.ts +++ b/src/api/__tests__/email.test.ts @@ -21,7 +21,8 @@ import { getEmails, getFullEmail, getThread, - setEmailKeywords, + patchKeywordsForEmails, + patchKeywordsPerEmail, moveEmail, deleteEmail, searchEmails, @@ -257,14 +258,40 @@ describe('email operations', () => { }); }); - describe('setEmailKeywords', () => { - it('should update email keywords', async () => { - mockRequest.mockResolvedValue({ methodResponses: [['Email/set', { updated: {} }, '0']] }); + describe('keyword patches', () => { + // A whole `keywords` map on update replaces it and erased stars and tags + // the local copy didn't have; only per-keyword pointers may go out. + it('patches only the named keywords, clearing with null', async () => { + mockRequest.mockResolvedValue({ methodResponses: [['Email/set', { updated: { e1: null, e2: null } }, '0']] }); - await setEmailKeywords('e1', { $seen: true, $flagged: true }); + await patchKeywordsForEmails(['e1', 'e2'], { $seen: true, $flagged: false, '$label:work/clients': null }, 'grp-1'); - const call = mockRequest.mock.calls[0][0][0]; - expect(call[1].update).toEqual({ e1: { keywords: { $seen: true, $flagged: true } } }); + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls[0][0]).toEqual([['Email/set', { + accountId: 'grp-1', + update: { + e1: { 'keywords/$seen': true, 'keywords/$flagged': null, 'keywords/$label:work~1clients': null }, + e2: { 'keywords/$seen': true, 'keywords/$flagged': null, 'keywords/$label:work~1clients': null }, + }, + }, '0']]); + }); + + it('applies a separate patch to each message in one Email/set', async () => { + mockRequest.mockResolvedValue({ methodResponses: [['Email/set', { updated: { e1: null, e2: null } }, '0']] }); + + await patchKeywordsPerEmail([ + { id: 'e1', patch: { $junk: null, $notjunk: true } }, + { id: 'e2', patch: { $junk: null, $notjunk: null } }, + ]); + + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest.mock.calls[0][0]).toEqual([['Email/set', { + accountId: 'acc-1', + update: { + e1: { 'keywords/$junk': null, 'keywords/$notjunk': true }, + e2: { 'keywords/$junk': null, 'keywords/$notjunk': null }, + }, + }, '0']]); }); }); diff --git a/src/api/email.ts b/src/api/email.ts index a3d55b2c..f1991501 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -599,13 +599,16 @@ export async function getThreadEmails(threadId: string, accountIdOverride?: stri return emails.sort((a, b) => (order.get(a.id) ?? 0) - (order.get(b.id) ?? 0)); } -export async function setEmailKeywords( - emailId: string, - keywords: Record, - accountIdOverride?: string, -): Promise { - const accountId = accountIdOverride ?? jmapClient.accountId; - await emailSetBatched(accountId, { update: { [emailId]: { keywords } } }); +// One `keywords/` pointer per changed keyword. Never send the whole +// `keywords` map on update: the server replaces it, erasing every keyword the +// local copy didn't have (a star or tag set elsewhere, or all of them on a +// message that was never loaded). +function keywordPointers(patch: Record): Record { + const pointers: Record = {}; + for (const [keyword, value] of Object.entries(patch)) { + pointers[keywordPointer(keyword)] = value === false ? null : value; + } + return pointers; } /** Patch individual keywords (`{ $seen: true, $junk: null }`) on many messages. */ @@ -616,10 +619,7 @@ export async function patchKeywordsForEmails( ): Promise { if (ids.length === 0) return; const accountId = accountIdOverride ?? jmapClient.accountId; - const pointerPatch: Record = {}; - for (const [keyword, value] of Object.entries(patch)) { - pointerPatch[keywordPointer(keyword)] = value === false ? null : value; - } + const pointerPatch = keywordPointers(patch); const update: Record> = {}; for (const id of ids) update[id] = { ...pointerPatch }; await emailSetBatched(accountId, { update }); @@ -713,15 +713,16 @@ export async function deleteEmails( } } -// Apply keyword maps to several emails in one round-trip (chunked). -export async function setKeywordsForEmails( - updates: Array<{ id: string; keywords: Record }>, +// Apply a different keyword patch to each of several emails in one +// round-trip (chunked). Undo uses it to put back each message's own keywords. +export async function patchKeywordsPerEmail( + updates: Array<{ id: string; patch: Record }>, accountIdOverride?: string, ): Promise { if (updates.length === 0) return; const accountId = accountIdOverride ?? jmapClient.accountId; - const update: Record }> = {}; - for (const u of updates) update[u.id] = { keywords: u.keywords }; + const update: Record> = {}; + for (const u of updates) update[u.id] = keywordPointers(u.patch); await emailSetBatched(accountId, { update }); } diff --git a/src/lib/__tests__/keyword-patch.test.ts b/src/lib/__tests__/keyword-patch.test.ts new file mode 100644 index 00000000..3a198f33 --- /dev/null +++ b/src/lib/__tests__/keyword-patch.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from 'vitest'; +import { applyKeywordPatch, revertKeywordPatch } from '../keyword-patch'; + +describe('applyKeywordPatch', () => { + it('sets and clears only the keywords the patch names', () => { + const before = { $seen: true, $flagged: true, '$label:work': true }; + expect(applyKeywordPatch(before, { $seen: null, $pinned: true, $flagged: false })).toEqual({ + '$label:work': true, + $pinned: true, + }); + expect(before).toEqual({ $seen: true, $flagged: true, '$label:work': true }); + }); + + it('starts from nothing when the message has no keywords yet', () => { + expect(applyKeywordPatch(undefined, { $seen: true })).toEqual({ $seen: true }); + }); +}); + +describe('revertKeywordPatch', () => { + it('restores each touched keyword to its original value and nothing else', () => { + expect(revertKeywordPatch( + { $junk: true, $notjunk: null, $seen: true }, + { $notjunk: true, $flagged: true }, + )).toEqual({ $junk: null, $notjunk: true, $seen: null }); + }); +}); diff --git a/src/lib/keyword-patch.ts b/src/lib/keyword-patch.ts new file mode 100644 index 00000000..e2293a3b --- /dev/null +++ b/src/lib/keyword-patch.ts @@ -0,0 +1,39 @@ +/** + * Keyword changes as an `Email/set` update sends them: one + * `keywords/` pointer per keyword, `true` to set it and `null` (or + * `false`) to clear it. Keywords the patch does not name are left alone. + * + * Sending the whole `keywords` map instead replaces it on the server, which + * erases whatever the local copy lacked: a star or tag another client set + * since the list loaded, or everything on a message that was never loaded. + */ +export type KeywordPatch = Record; + +/** `keywords` with `patch` applied: the local mirror of the server update. */ +export function applyKeywordPatch( + keywords: Record | undefined, + patch: KeywordPatch, +): Record { + const next = { ...keywords }; + for (const [keyword, value] of Object.entries(patch)) { + if (value) next[keyword] = true; + else delete next[keyword]; + } + return next; +} + +/** + * The patch that undoes `patch` on a message whose keywords were `original` + * before it: each keyword the patch touched goes back to what it was, and + * nothing else changes. + */ +export function revertKeywordPatch( + patch: KeywordPatch, + original: Record | undefined, +): KeywordPatch { + const revert: KeywordPatch = {}; + for (const keyword of Object.keys(patch)) { + revert[keyword] = original?.[keyword] ? true : null; + } + return revert; +} diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index 0e3a032d..8ca6440c 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -26,7 +26,7 @@ import { REPLY_QUICK_ACTIONS, type QuickAction, } from '../stores/settings-store'; -import { setEmailKeywords, getThreadEmails } from '../api/email'; +import { patchKeywordsForEmails, getThreadEmails } from '../api/email'; import { shareEmailEml } from '../lib/email-export'; import { useKeywordsStore, keywordToken, type KeywordDef } from '../stores/keywords-store'; import { useSheetDrag } from '../lib/use-sheet-drag'; @@ -319,7 +319,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { if (next[token]) delete next[token]; else next[token] = true; updateLocalKeywords(email.id, next); - void setEmailKeywords(email.id, next, ownerAccountId); + void patchKeywordsForEmails([email.id], { [token]: next[token] ?? null }, ownerAccountId); }; // Toggle the star on a specific message — used both by the toolbar (current @@ -329,7 +329,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { if (next.$flagged) delete next.$flagged; else next.$flagged = true; updateLocalKeywords(target.id, next); - void setEmailKeywords(target.id, next, ownerAccountId); + void patchKeywordsForEmails([target.id], { $flagged: next.$flagged ?? null }, ownerAccountId); }, [updateLocalKeywords, ownerAccountId]); const onToggleStar = () => { if (email) toggleStarFor(email); }; @@ -343,7 +343,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const next = { ...email.keywords }; delete next.$seen; updateLocalKeywords(email.id, next); - void setEmailKeywords(email.id, next, ownerAccountId); + void patchKeywordsForEmails([email.id], { $seen: null }, ownerAccountId); } }; diff --git a/src/stores/__tests__/email-store.test.ts b/src/stores/__tests__/email-store.test.ts index 2887d16a..35f227c9 100644 --- a/src/stores/__tests__/email-store.test.ts +++ b/src/stores/__tests__/email-store.test.ts @@ -15,8 +15,8 @@ vi.mock('../../api/email', () => ({ getEmailChanges: vi.fn(async () => null), getFullEmail: vi.fn(), importEmailBlob: vi.fn(async () => 'imported-1'), - setEmailKeywords: vi.fn(), - setKeywordsForEmails: vi.fn(), + patchKeywordsForEmails: vi.fn(), + patchKeywordsPerEmail: vi.fn(), moveEmail: vi.fn(), moveEmails: vi.fn(), archiveEmails: vi.fn(), @@ -49,8 +49,8 @@ vi.mock('../locale-store', () => ({ // without pulling in network-store / NetInfo. vi.mock('../outbox-store', async () => { const api = await import('../../api/email') as unknown as Record Promise>; - const runOp = async (op: { kind: string; emailId: string; accountId?: string; keywords?: unknown; mailboxIds?: unknown }) => { - if (op.kind === 'keywords') return api.setEmailKeywords(op.emailId, op.keywords, op.accountId); + const runOp = async (op: { kind: string; emailId: string; accountId?: string; patch?: unknown; mailboxIds?: unknown }) => { + if (op.kind === 'keywords') return api.patchKeywordsForEmails([op.emailId], op.patch, op.accountId); if (op.kind === 'mailboxes') return api.setEmailMailboxes(op.emailId, op.mailboxIds, op.accountId); if (op.kind === 'destroy') return api.destroyEmails([op.emailId], op.accountId); }; @@ -155,7 +155,7 @@ const mockGetEmailQueryChanges = emailApi.getEmailQueryChanges as ReturnType; const mockGetEmailsWithState = emailApi.getEmailsWithState as ReturnType; const mockGetFullEmail = emailApi.getFullEmail as ReturnType; -const mockSetKeywords = emailApi.setEmailKeywords as ReturnType; +const mockPatchKeywords = emailApi.patchKeywordsForEmails as ReturnType; const mockMoveEmail = emailApi.moveEmail as ReturnType; const mockDeleteEmail = emailApi.deleteEmail as ReturnType; const mockSearchEmails = emailApi.searchEmails as ReturnType; @@ -261,11 +261,11 @@ describe('email-store', () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: {} } as any], }); - mockSetKeywords.mockResolvedValue(undefined); + mockPatchKeywords.mockResolvedValue(undefined); await useEmailStore.getState().markRead('e1'); - expect(mockSetKeywords).toHaveBeenCalledWith('e1', { $seen: true }, undefined); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $seen: true }, undefined); expect(useEmailStore.getState().emails[0].keywords.$seen).toBe(true); }); }); @@ -275,12 +275,12 @@ describe('email-store', () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: { $seen: true, $flagged: true } } as any], }); - mockSetKeywords.mockResolvedValue(undefined); + mockPatchKeywords.mockResolvedValue(undefined); await useEmailStore.getState().markUnread('e1'); - expect(mockSetKeywords).toHaveBeenCalledWith('e1', { $flagged: true }, undefined); - expect(useEmailStore.getState().emails[0].keywords.$seen).toBeUndefined(); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $seen: null }, undefined); + expect(useEmailStore.getState().emails[0].keywords).toEqual({ $flagged: true }); }); }); @@ -289,10 +289,11 @@ describe('email-store', () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: { $seen: true } } as any], }); - mockSetKeywords.mockResolvedValue(undefined); + mockPatchKeywords.mockResolvedValue(undefined); await useEmailStore.getState().toggleStar('e1', true); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $flagged: true }, undefined); expect(useEmailStore.getState().emails[0].keywords.$flagged).toBe(true); }); @@ -300,10 +301,11 @@ describe('email-store', () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: { $seen: true, $flagged: true } } as any], }); - mockSetKeywords.mockResolvedValue(undefined); + mockPatchKeywords.mockResolvedValue(undefined); await useEmailStore.getState().toggleStar('e1', false); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $flagged: null }, undefined); expect(useEmailStore.getState().emails[0].keywords.$flagged).toBeUndefined(); }); }); @@ -736,13 +738,86 @@ describe('email-store', () => { }); }); + // A keyword change must reach the server as `keywords/` pointers: + // sending the list's keyword map replaced the server's and erased stars and + // tags the list didn't know about (audit B4). + describe('keyword changes send only the keywords they change', () => { + it('marks a message outside the loaded list read without touching its other keywords', async () => { + useEmailStore.setState({ emails: [] }); + + await useEmailStore.getState().markRead('e9', 'grp-1'); + await useEmailStore.getState().markRead('e8'); + + expect(mockPatchKeywords).toHaveBeenNthCalledWith(1, ['e9'], { $seen: true }, 'grp-1'); + expect(mockPatchKeywords).toHaveBeenNthCalledWith(2, ['e8'], { $seen: true }, undefined); + }); + + it('stars from a stale list row by sending $flagged alone, keeping keywords that changed meanwhile', async () => { + useEmailStore.setState({ emails: [{ id: 'e1', keywords: { $notjunk: true, $seen: true } } as any] }); + // The list refreshes while the request is in flight and picks up a tag + // another client added. + mockPatchKeywords.mockImplementationOnce(async () => { + useEmailStore.setState({ emails: [{ id: 'e1', keywords: { $notjunk: true, $seen: true, '$label:work': true } } as any] }); + }); + + await useEmailStore.getState().toggleStar('e1', true); + + expect(mockPatchKeywords).toHaveBeenCalledTimes(1); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $flagged: true }, undefined); + expect(useEmailStore.getState().emails[0].keywords).toEqual({ + $notjunk: true, $seen: true, '$label:work': true, $flagged: true, + }); + }); + + it('tags and untags a selection with one patch naming only that tag', async () => { + useEmailStore.setState({ + emails: [ + { id: 'e1', keywords: { $seen: true } } as any, + { id: 'e2', keywords: { $flagged: true, '$label:work': true } } as any, + ], + }); + + await useEmailStore.getState().setKeywordForEmails(['e1', 'e2'], '$label:work', true); + expect(mockPatchKeywords).toHaveBeenLastCalledWith(['e1', 'e2'], { '$label:work': true }, undefined); + expect(useEmailStore.getState().emails.map((e) => e.keywords)).toEqual([ + { $seen: true, '$label:work': true }, + { $flagged: true, '$label:work': true }, + ]); + + await useEmailStore.getState().setKeywordForEmails(['e2'], '$label:work', false); + expect(mockPatchKeywords).toHaveBeenLastCalledWith(['e2'], { '$label:work': null }, undefined); + expect(useEmailStore.getState().emails[1].keywords).toEqual({ $flagged: true }); + }); + + it('batch delete with trash-and-read marks only the unread messages read with a $seen patch', async () => { + useSettingsStore.getState().updateSetting('deleteAction', 'trash-and-read'); + useEmailStore.setState({ + mailboxes: [ + { id: 'mb-1', name: 'Inbox', role: 'inbox', isShared: false } as any, + { id: 'mb-trash', name: 'Trash', role: 'trash', isShared: false } as any, + ], + currentMailboxId: 'mb-1', + emails: [ + { id: 'e1', keywords: { $flagged: true }, mailboxIds: { 'mb-1': true } } as any, + { id: 'e2', keywords: { $seen: true }, mailboxIds: { 'mb-1': true } } as any, + ], + }); + + await useEmailStore.getState().deleteEmailsBatch(['e1', 'e2'], 'mb-trash', 'mb-1'); + + expect(mockPatchKeywords).toHaveBeenCalledTimes(1); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $seen: true }, undefined); + useSettingsStore.getState().updateSetting('deleteAction', 'trash'); + }); + }); + describe('pin and spam keywords', () => { it('togglePin writes $pinned, not $important', async () => { useEmailStore.setState({ emails: [{ id: 'e1', keywords: {} } as any] }); await useEmailStore.getState().togglePin('e1', true); - expect(mockSetKeywords).toHaveBeenCalledWith('e1', { $pinned: true }, undefined); + expect(mockPatchKeywords).toHaveBeenCalledWith(['e1'], { $pinned: true }, undefined); await useEmailStore.getState().togglePin('e1', false); - expect(mockSetKeywords).toHaveBeenLastCalledWith('e1', {}, undefined); + expect(mockPatchKeywords).toHaveBeenLastCalledWith(['e1'], { $pinned: null }, undefined); }); const RIGHTS = {} as any; @@ -752,7 +827,7 @@ describe('email-store', () => { it('markSpam files into Junk flipping $junk/$notjunk and offers an undo that restores keywords', async () => { const mockMarkAsSpam = emailApi.markAsSpam as ReturnType; const mockRestore = emailApi.restoreEmailMailboxes as ReturnType; - const mockSetKeywordsForEmails = emailApi.setKeywordsForEmails as ReturnType; + const mockPatchPerEmail = emailApi.patchKeywordsPerEmail as ReturnType; useSettingsStore.getState().updateSetting('deleteAction', 'trash-and-read'); useEmailStore.setState({ mailboxes: [inbox, junk], @@ -770,7 +845,12 @@ describe('email-store', () => { await useEmailStore.getState().undoLast(); expect(mockRestore).toHaveBeenCalledWith([{ id: 'e1', mailboxIds: { 'mb-1': true } }], undefined); - expect(mockSetKeywordsForEmails).toHaveBeenCalledWith([{ id: 'e1', keywords: { $notjunk: true } }], undefined); + // Only the keywords the spam action touched go back; `$seen` was set by + // trash-and-read on a message that was unread before. + expect(mockPatchPerEmail).toHaveBeenCalledWith( + [{ id: 'e1', patch: { $junk: null, $notjunk: true, $seen: null } }], + undefined, + ); expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['e1']); useSettingsStore.getState().updateSetting('deleteAction', 'trash'); }); diff --git a/src/stores/__tests__/offline-cache-store.test.ts b/src/stores/__tests__/offline-cache-store.test.ts new file mode 100644 index 00000000..8e896c20 --- /dev/null +++ b/src/stores/__tests__/offline-cache-store.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { useOfflineCacheStore } from '../offline-cache-store'; + +const ACCOUNT = 'acc-1'; + +beforeEach(async () => { + await useOfflineCacheStore.getState().setAccount(null); + await useOfflineCacheStore.getState().setAccount(ACCOUNT); + await useOfflineCacheStore.getState().clearAll(); +}); + +describe('offline cache patch', () => { + it('applies a keyword patch to the cached copy instead of replacing its keywords', async () => { + const cache = useOfflineCacheStore.getState(); + await cache.put({ + id: 'e1', + receivedAt: '2026-01-01T00:00:00Z', + keywords: { $flagged: true, '$label:work': true }, + mailboxIds: { inbox: true }, + } as any, 100); + + await useOfflineCacheStore.getState().patch('e1', { keywords: { $seen: true, $flagged: null } }); + + const cached = await useOfflineCacheStore.getState().get('e1'); + expect(cached?.keywords).toEqual({ '$label:work': true, $seen: true }); + expect(cached?.mailboxIds).toEqual({ inbox: true }); + }); + + it('replaces mailboxIds wholesale', async () => { + await useOfflineCacheStore.getState().put({ + id: 'e1', receivedAt: '2026-01-01T00:00:00Z', keywords: { $seen: true }, mailboxIds: { inbox: true }, + } as any, 100); + + await useOfflineCacheStore.getState().patch('e1', { mailboxIds: { trash: true } }); + + const cached = await useOfflineCacheStore.getState().get('e1'); + expect(cached?.mailboxIds).toEqual({ trash: true }); + expect(cached?.keywords).toEqual({ $seen: true }); + }); +}); diff --git a/src/stores/__tests__/outbox-store.test.ts b/src/stores/__tests__/outbox-store.test.ts index 2add515e..babc2363 100644 --- a/src/stores/__tests__/outbox-store.test.ts +++ b/src/stores/__tests__/outbox-store.test.ts @@ -13,15 +13,16 @@ vi.mock('../../api/jmap-client', () => ({ jmapClient: { isConnected: true }, })); -const setEmailKeywords = vi.fn(async (..._a: any[]) => undefined); +const patchKeywordsForEmails = vi.fn(async (..._a: any[]) => undefined); const setEmailMailboxes = vi.fn(async (..._a: any[]) => undefined); const destroyEmails = vi.fn(async (..._a: any[]) => undefined); vi.mock('../../api/email', () => ({ - setEmailKeywords: (...a: any[]) => setEmailKeywords(...a), + patchKeywordsForEmails: (...a: any[]) => patchKeywordsForEmails(...a), setEmailMailboxes: (...a: any[]) => setEmailMailboxes(...a), destroyEmails: (...a: any[]) => destroyEmails(...a), })); +import AsyncStorage from '@react-native-async-storage/async-storage'; import { useOutboxStore, applyOrQueue } from '../outbox-store'; import { useNetworkStore } from '../network-store'; @@ -38,26 +39,27 @@ beforeEach(async () => { }); describe('outbox enqueue + coalescing', () => { - it('coalesces repeated keyword ops for the same email (last-write-wins)', () => { + it('merges repeated keyword patches for the same email (last write wins per keyword)', () => { const store = useOutboxStore.getState(); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true, $flagged: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $flagged: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: null } }); const entries = useOutboxStore.getState().entries; expect(entries).toHaveLength(1); - expect(entries[0].op).toEqual({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true, $flagged: true } }); + expect(entries[0].op).toEqual({ kind: 'keywords', emailId: 'e1', patch: { $seen: null, $flagged: true } }); }); it('keeps keyword and mailbox ops for the same email separate', () => { const store = useOutboxStore.getState(); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); store.enqueue({ kind: 'mailboxes', emailId: 'e1', mailboxIds: { archive: true } }); expect(useOutboxStore.getState().entries).toHaveLength(2); }); it('destroy supersedes pending edits for that email', () => { const store = useOutboxStore.getState(); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); store.enqueue({ kind: 'mailboxes', emailId: 'e1', mailboxIds: { archive: true } }); store.enqueue({ kind: 'destroy', emailId: 'e1' }); @@ -69,7 +71,7 @@ describe('outbox enqueue + coalescing', () => { it('ignores further edits once a destroy is queued', () => { const store = useOutboxStore.getState(); store.enqueue({ kind: 'destroy', emailId: 'e1' }); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); expect(useOutboxStore.getState().entries).toHaveLength(1); expect(useOutboxStore.getState().entries[0].op.kind).toBe('destroy'); }); @@ -77,9 +79,9 @@ describe('outbox enqueue + coalescing', () => { describe('applyOrQueue', () => { it('runs immediately when online with an empty queue', async () => { - const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); expect(result.queued).toBe(false); - expect(setEmailKeywords).toHaveBeenCalledWith('e1', { $seen: true }, undefined); + expect(patchKeywordsForEmails).toHaveBeenCalledWith(['e1'], { $seen: true }, undefined); expect(useOutboxStore.getState().entries).toHaveLength(0); }); @@ -92,9 +94,9 @@ describe('applyOrQueue', () => { it('queues instead of running when offline', async () => { useNetworkStore.setState({ online: false }); - const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); expect(result.queued).toBe(true); - expect(setEmailKeywords).not.toHaveBeenCalled(); + expect(patchKeywordsForEmails).not.toHaveBeenCalled(); expect(useOutboxStore.getState().entries).toHaveLength(1); }); @@ -103,9 +105,9 @@ describe('applyOrQueue', () => { await applyOrQueue({ kind: 'mailboxes', emailId: 'e1', mailboxIds: { a: true } }); useNetworkStore.setState({ online: true }); // Now online, but an op is already queued for e1 → must queue, not run. - const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); expect(result.queued).toBe(true); - expect(setEmailKeywords).not.toHaveBeenCalled(); + expect(patchKeywordsForEmails).not.toHaveBeenCalled(); }); it('surfaces a non-transient (server) error to the caller', async () => { @@ -120,7 +122,7 @@ describe('applyOrQueue', () => { const err = new Error('Network request failed'); err.name = 'NetworkError'; const onlineRun = vi.fn(async () => { throw err; }); - const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }, onlineRun); + const result = await applyOrQueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }, onlineRun); expect(result.queued).toBe(true); expect(useOutboxStore.getState().entries).toHaveLength(1); }); @@ -130,14 +132,14 @@ describe('flush', () => { it('replays queued ops in order and clears them', async () => { useNetworkStore.setState({ online: false }); const store = useOutboxStore.getState(); - store.enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + store.enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); store.enqueue({ kind: 'mailboxes', emailId: 'e2', mailboxIds: { trash: true } }); store.enqueue({ kind: 'destroy', emailId: 'e3' }); useNetworkStore.setState({ online: true }); await useOutboxStore.getState().flush(); - expect(setEmailKeywords).toHaveBeenCalledWith('e1', { $seen: true }, undefined); + expect(patchKeywordsForEmails).toHaveBeenCalledWith(['e1'], { $seen: true }, undefined); expect(setEmailMailboxes).toHaveBeenCalledWith('e2', { trash: true }, undefined); expect(destroyEmails).toHaveBeenCalledWith(['e3'], undefined); expect(useOutboxStore.getState().entries).toHaveLength(0); @@ -145,20 +147,20 @@ describe('flush', () => { it('does nothing while offline', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); await useOutboxStore.getState().flush(); - expect(setEmailKeywords).not.toHaveBeenCalled(); + expect(patchKeywordsForEmails).not.toHaveBeenCalled(); expect(useOutboxStore.getState().entries).toHaveLength(1); }); it('stops and retains the op on a transient failure', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); useNetworkStore.setState({ online: true }); const err = new Error('Network request failed'); err.name = 'NetworkError'; - setEmailKeywords.mockRejectedValueOnce(err); + patchKeywordsForEmails.mockRejectedValueOnce(err); await useOutboxStore.getState().flush(); expect(useOutboxStore.getState().entries).toHaveLength(1); @@ -167,10 +169,10 @@ describe('flush', () => { it('drops a poison op after repeated server rejections', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); useNetworkStore.setState({ online: true }); - setEmailKeywords.mockRejectedValue(new Error('JMAP request failed: 400')); + patchKeywordsForEmails.mockRejectedValue(new Error('JMAP request failed: 400')); // MAX_ATTEMPTS is 5; flush bumps one attempt per run. for (let i = 0; i < 5; i++) await useOutboxStore.getState().flush(); @@ -181,27 +183,27 @@ describe('flush', () => { describe('failed ops and archive replay', () => { it('parks a poison op in `failed` instead of dropping it, and retryFailed re-queues it', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); useNetworkStore.setState({ online: true }); - setEmailKeywords.mockRejectedValue(new Error('JMAP request failed: 400')); + patchKeywordsForEmails.mockRejectedValue(new Error('JMAP request failed: 400')); for (let i = 0; i < 5; i++) await useOutboxStore.getState().flush(); expect(useOutboxStore.getState().entries).toHaveLength(0); expect(useOutboxStore.getState().failed).toHaveLength(1); expect(useOutboxStore.getState().failed[0].lastError).toContain('400'); - setEmailKeywords.mockResolvedValue(undefined); + patchKeywordsForEmails.mockResolvedValue(undefined); await useOutboxStore.getState().retryFailed(); expect(useOutboxStore.getState().failed).toHaveLength(0); expect(useOutboxStore.getState().entries).toHaveLength(0); - expect(setEmailKeywords).toHaveBeenLastCalledWith('e1', { $seen: true }, undefined); + expect(patchKeywordsForEmails).toHaveBeenLastCalledWith(['e1'], { $seen: true }, undefined); }); it('discardFailed forgets parked ops', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); useNetworkStore.setState({ online: true }); - setEmailKeywords.mockRejectedValue(new Error('JMAP request failed: 400')); + patchKeywordsForEmails.mockRejectedValue(new Error('JMAP request failed: 400')); for (let i = 0; i < 5; i++) await useOutboxStore.getState().flush(); expect(useOutboxStore.getState().failed).toHaveLength(1); useOutboxStore.getState().discardFailed(); @@ -210,11 +212,11 @@ describe('failed ops and archive replay', () => { it('pauses the queue on an authentication failure without counting an attempt', async () => { useNetworkStore.setState({ online: false }); - useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', keywords: { $seen: true } }); + useOutboxStore.getState().enqueue({ kind: 'keywords', emailId: 'e1', patch: { $seen: true } }); useNetworkStore.setState({ online: true }); const err = new Error('Session expired'); err.name = 'AuthenticationError'; - setEmailKeywords.mockRejectedValueOnce(err); + patchKeywordsForEmails.mockRejectedValueOnce(err); await useOutboxStore.getState().flush(); expect(useOutboxStore.getState().paused).toBe(true); @@ -222,9 +224,9 @@ describe('failed ops and archive replay', () => { expect(useOutboxStore.getState().entries[0].attempts).toBe(0); // A second flush is a no-op while paused; re-attaching the account lifts it. - setEmailKeywords.mockClear(); + patchKeywordsForEmails.mockClear(); await useOutboxStore.getState().flush(); - expect(setEmailKeywords).not.toHaveBeenCalled(); + expect(patchKeywordsForEmails).not.toHaveBeenCalled(); await useOutboxStore.getState().setAccount(ACCOUNT); expect(useOutboxStore.getState().paused).toBe(false); }); @@ -239,3 +241,47 @@ describe('failed ops and archive replay', () => { expect(entries[0].op.kind).toBe('archive'); }); }); + +describe('keyword ops queued by earlier builds', () => { + beforeEach(async () => { + patchKeywordsForEmails.mockResolvedValue(undefined); + await AsyncStorage.removeItem('webmail:outbox:v1:acc-legacy'); + await AsyncStorage.removeItem('webmail:outbox:v1:acc-legacy:failed'); + }); + + it('replays a whole keyword map as a patch that sets its keywords and clears nothing else', async () => { + // Earlier builds queued the message's full keyword map, which replaced + // the server's: replaying it as-is would erase a star or tag set since. + const legacy = [ + { id: 'a', createdAt: 1, attempts: 0, op: { kind: 'keywords', emailId: 'e1', keywords: { $seen: true, '$label:work': true, $answered: false } } }, + { id: 'b', createdAt: 2, attempts: 0, op: { kind: 'keywords', emailId: 'e2', accountId: 'grp-1', keywords: { $junk: true } } }, + { id: 'c', createdAt: 3, attempts: 0, op: { kind: 'mailboxes', emailId: 'e3', mailboxIds: { trash: true } } }, + ]; + await AsyncStorage.setItem('webmail:outbox:v1:acc-legacy', JSON.stringify(legacy)); + await useOutboxStore.getState().setAccount('acc-legacy'); + + expect(useOutboxStore.getState().entries[0].op).toEqual({ + kind: 'keywords', emailId: 'e1', accountId: undefined, patch: { $seen: true, '$label:work': true }, + }); + await useOutboxStore.getState().flush(); + + expect(patchKeywordsForEmails).toHaveBeenNthCalledWith(1, ['e1'], { $seen: true, '$label:work': true }, undefined); + // `$junk` and `$notjunk` exclude each other, so setting one clears the other. + expect(patchKeywordsForEmails).toHaveBeenNthCalledWith(2, ['e2'], { $junk: true, $notjunk: null }, 'grp-1'); + expect(setEmailMailboxes).toHaveBeenCalledWith('e3', { trash: true }, undefined); + expect(useOutboxStore.getState().entries).toHaveLength(0); + }); + + it('upgrades parked failed ops too, so a retry sends a patch', async () => { + const legacy = [ + { id: 'a', createdAt: 1, attempts: 5, op: { kind: 'keywords', emailId: 'e1', keywords: { $notjunk: true, $flagged: true } } }, + ]; + await AsyncStorage.setItem('webmail:outbox:v1:acc-legacy:failed', JSON.stringify(legacy)); + await useOutboxStore.getState().setAccount('acc-legacy'); + + await useOutboxStore.getState().retryFailed(); + + expect(patchKeywordsForEmails).toHaveBeenCalledWith(['e1'], { $notjunk: true, $flagged: true, $junk: null }, undefined); + expect(useOutboxStore.getState().failed).toHaveLength(0); + }); +}); diff --git a/src/stores/email-store.ts b/src/stores/email-store.ts index 21b6cee6..fe5cfaeb 100644 --- a/src/stores/email-store.ts +++ b/src/stores/email-store.ts @@ -16,8 +16,8 @@ import { getEmailChanges, getFullEmail, importEmailBlob, - setEmailKeywords, - setKeywordsForEmails, + patchKeywordsForEmails, + patchKeywordsPerEmail, moveEmail, moveEmails as apiMoveEmails, archiveEmails as apiArchiveEmails, @@ -37,6 +37,7 @@ import { toWildcardQuery } from '../lib/search-utils'; import { orderForMailbox, sanitizeSortLevels, type SortLevel } from '../lib/message-list-order'; import { buildListSort, markKeywordSortUnsupported } from '../lib/keyword-sort-polarity'; import { generateAccountId } from '../lib/account-utils'; +import { applyKeywordPatch, revertKeywordPatch, type KeywordPatch } from '../lib/keyword-patch'; import { t } from './locale-store'; import { useSettingsStore } from './settings-store'; import { useOfflineCacheStore } from './offline-cache-store'; @@ -78,7 +79,7 @@ const provisionRetried = new Set(); // re-opening a message while offline shows the change. Fire-and-forget. function patchCache( id: string, - changes: { keywords?: Record; mailboxIds?: Record }, + changes: { keywords?: KeywordPatch; mailboxIds?: Record }, accountId?: string, ): void { void useOfflineCacheStore.getState().patch(id, changes, accountId); @@ -222,6 +223,12 @@ export interface UndoEntry { createdAt: number; /** JMAP account the messages live under; unset for the user's own mail. */ accountId?: string; + /** + * Keywords the action also changed (spam / not spam flip + * `$junk`/`$notjunk`). Undo puts just these back to each item's + * `originalKeywords`. + */ + keywordPatch?: KeywordPatch; /** * Each item is one email's pre-action mailboxIds, used to restore it. * `originalKeywords` is set when the action also changed keywords @@ -963,70 +970,67 @@ export const useEmailStore = create()( markRead: async (emailId, accountId) => { const state = get(); const email = state.emails.find((e) => e.id === emailId); - const nextKeywords = { ...(email?.keywords ?? {}), $seen: true }; + // Only `$seen` goes to the server: the message may not be in the list at + // all, and a whole keyword map would erase its stars and tags. + const patch = { $seen: true }; // A group/shared message opened from the unified inbox lives under another // JMAP account and isn't in the active list/cache or the (account-scoped) // offline queue — mark it read directly against its owning account. if (accountId && !email) { - await setEmailKeywords(emailId, nextKeywords, accountId); + await patchKeywordsForEmails([emailId], patch, accountId); return; } const owner = accountId ?? currentAccountId(state); - await applyOrQueue({ kind: 'keywords', emailId, accountId: owner, keywords: nextKeywords }); + await applyOrQueue({ kind: 'keywords', emailId, accountId: owner, patch }); set({ emails: get().emails.map((e) => - e.id === emailId ? { ...e, keywords: nextKeywords } : e, + e.id === emailId ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), ...(state.filters.isUnread === true ? { retainedIds: retain(get().retainedIds, [emailId]) } : {}), }); - patchCache(emailId, { keywords: nextKeywords }, owner); + patchCache(emailId, { keywords: patch }, owner); }, markUnread: async (emailId) => { const state = get(); const email = state.emails.find((e) => e.id === emailId); if (!email) return; - const { $seen, ...rest } = email.keywords; + const patch = { $seen: null }; await applyOrQueue({ kind: 'keywords', emailId, accountId: currentAccountId(state), - keywords: rest, + patch, }); set({ emails: get().emails.map((e) => - e.id === emailId ? { ...e, keywords: rest } : e, + e.id === emailId ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), ...(state.filters.isUnread === false ? { retainedIds: retain(get().retainedIds, [emailId]) } : {}), }); - patchCache(emailId, { keywords: rest }, currentAccountId(state)); + patchCache(emailId, { keywords: patch }, currentAccountId(state)); }, toggleStar: async (emailId, starred) => { const state = get(); const email = state.emails.find((e) => e.id === emailId); if (!email) return; - const keywords = { ...email.keywords }; - if (starred) { - keywords.$flagged = true; - } else { - delete keywords.$flagged; - } + const patch = { $flagged: starred ? true : null }; await applyOrQueue({ kind: 'keywords', emailId, accountId: currentAccountId(state), - keywords, + patch, }); set({ emails: get().emails.map((e) => - e.id === emailId ? { ...e, keywords } : e, + e.id === emailId ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), ...(state.filters.isStarred !== undefined && state.filters.isStarred !== starred ? { retainedIds: retain(get().retainedIds, [emailId]) } : {}), }); - patchCache(emailId, { keywords }, currentAccountId(state)); + patchCache(emailId, { keywords: patch }, currentAccountId(state)); }, togglePin: async (emailId, pinned) => { @@ -1035,24 +1039,19 @@ export const useEmailStore = create()( if (!email) return; // `$pinned` is what the webmail reads and writes; a pin set as // `$important` was invisible to it (and vice versa). - const keywords = { ...email.keywords }; - if (pinned) { - keywords.$pinned = true; - } else { - delete keywords.$pinned; - } + const patch = { $pinned: pinned ? true : null }; await applyOrQueue({ kind: 'keywords', emailId, accountId: currentAccountId(state), - keywords, + patch, }); set({ emails: get().emails.map((e) => - e.id === emailId ? { ...e, keywords } : e, + e.id === emailId ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), }); - patchCache(emailId, { keywords }, currentAccountId(state)); + patchCache(emailId, { keywords: patch }, currentAccountId(state)); }, markSpam: async (emailIds) => { @@ -1068,10 +1067,7 @@ export const useEmailStore = create()( const junk = refFor(state.mailboxes, junkMailbox.id); const markRead = useSettingsStore.getState().deleteAction === 'trash-and-read'; const junkTarget = { [junk.id]: true }; - const nextKeywords = new Map(targets.map((e) => { - const { $notjunk: _drop, ...rest } = e.keywords ?? {}; - return [e.id, { ...rest, $junk: true, ...(markRead ? { $seen: true } : {}) }]; - })); + const keywordPatch: KeywordPatch = { $junk: true, $notjunk: null, ...(markRead ? { $seen: true } : {}) }; const items = targets.map((e) => ({ email: e, originalMailboxIds: { ...e.mailboxIds }, @@ -1081,7 +1077,7 @@ export const useEmailStore = create()( await applyOrQueueBatch( targets.flatMap((e): OutboxOp[] => [ { kind: 'mailboxes', emailId: e.id, accountId: junk.accountId, mailboxIds: junkTarget }, - { kind: 'keywords', emailId: e.id, accountId: junk.accountId, keywords: nextKeywords.get(e.id)! }, + { kind: 'keywords', emailId: e.id, accountId: junk.accountId, patch: keywordPatch }, ]), () => apiMarkAsSpam(targets.map((e) => e.id), junk.id, junk.accountId, { markRead }), ); @@ -1096,10 +1092,11 @@ export const useEmailStore = create()( : t('email_list.marked_as_spam_count', `${targets.length} emails marked as spam`, { count: targets.length }), createdAt: Date.now(), accountId: junk.accountId, + keywordPatch, items, }, }); - for (const e of targets) patchCache(e.id, { mailboxIds: junkTarget, keywords: nextKeywords.get(e.id) }, junk.accountId); + for (const e of targets) patchCache(e.id, { mailboxIds: junkTarget, keywords: keywordPatch }, junk.accountId); }, unmarkSpam: async (emailIds) => { @@ -1111,10 +1108,7 @@ export const useEmailStore = create()( if (!inboxMailbox) return; const inbox = refFor(state.mailboxes, inboxMailbox.id); const inboxTarget = { [inbox.id]: true }; - const nextKeywords = new Map(targets.map((e) => { - const { $junk: _drop, ...rest } = e.keywords ?? {}; - return [e.id, { ...rest, $notjunk: true }]; - })); + const keywordPatch: KeywordPatch = { $junk: null, $notjunk: true }; const items = targets.map((e) => ({ email: e, originalMailboxIds: { ...e.mailboxIds }, @@ -1124,7 +1118,7 @@ export const useEmailStore = create()( await applyOrQueueBatch( targets.flatMap((e): OutboxOp[] => [ { kind: 'mailboxes', emailId: e.id, accountId: inbox.accountId, mailboxIds: inboxTarget }, - { kind: 'keywords', emailId: e.id, accountId: inbox.accountId, keywords: nextKeywords.get(e.id)! }, + { kind: 'keywords', emailId: e.id, accountId: inbox.accountId, patch: keywordPatch }, ]), () => apiUndoSpam(targets.map((e) => e.id), inbox.id, inbox.accountId), ); @@ -1139,10 +1133,11 @@ export const useEmailStore = create()( : t('email_list.marked_not_spam_count', `${targets.length} emails marked as not spam`, { count: targets.length }), createdAt: Date.now(), accountId: inbox.accountId, + keywordPatch, items, }, }); - for (const e of targets) patchCache(e.id, { mailboxIds: inboxTarget, keywords: nextKeywords.get(e.id) }, inbox.accountId); + for (const e of targets) patchCache(e.id, { mailboxIds: inboxTarget, keywords: keywordPatch }, inbox.accountId); }, moveToMailbox: async (emailId, fromMailboxId, toMailboxId) => { @@ -1290,14 +1285,14 @@ export const useEmailStore = create()( // "Move to Trash and mark as read" (#323): when the user picked that // delete action, also clear unread state for messages moved to trash. if (settings.deleteAction === 'trash-and-read' && email && !email.keywords?.$seen) { - const nextKeywords = { ...email.keywords, $seen: true }; + const patch = { $seen: true }; await applyOrQueue({ kind: 'keywords', emailId, accountId: source.accountId, - keywords: nextKeywords, + patch, }); - patchCache(emailId, { mailboxIds: target, keywords: nextKeywords }, source.accountId); + patchCache(emailId, { mailboxIds: target, keywords: patch }, source.accountId); } else { patchCache(emailId, { mailboxIds: target }, source.accountId); } @@ -1462,9 +1457,8 @@ export const useEmailStore = create()( settings.deleteAction === 'trash-and-read' ? toTrash.filter((e) => !e.keywords?.$seen) : []; - const markReadKeywords = new Map( - toMarkRead.map((e) => [e.id, { ...e.keywords, $seen: true }]), - ); + const markReadIds = new Set(toMarkRead.map((e) => e.id)); + const markReadPatch = { $seen: true }; const ops: OutboxOp[] = [ ...toDestroy.map((e): OutboxOp => ({ @@ -1482,7 +1476,7 @@ export const useEmailStore = create()( kind: 'keywords', emailId: e.id, accountId: source.accountId, - keywords: markReadKeywords.get(e.id)!, + patch: markReadPatch, })), ]; await applyOrQueueBatch(ops, async () => { @@ -1493,10 +1487,7 @@ export const useEmailStore = create()( await apiMoveEmails(toTrash.map((e) => e.id), source.id, trash.id, source.accountId); } if (toMarkRead.length > 0) { - await setKeywordsForEmails( - toMarkRead.map((e) => ({ id: e.id, keywords: markReadKeywords.get(e.id)! })), - source.accountId, - ); + await patchKeywordsForEmails([...markReadIds], markReadPatch, source.accountId); } }); @@ -1504,7 +1495,7 @@ export const useEmailStore = create()( for (const e of toTrash) { patchCache(e.id, { mailboxIds: mailboxesAfterMove(e.mailboxIds, source.id, trash.id), - ...(markReadKeywords.has(e.id) ? { keywords: markReadKeywords.get(e.id) } : {}), + ...(markReadIds.has(e.id) ? { keywords: markReadPatch } : {}), }, source.accountId); } @@ -1532,32 +1523,28 @@ export const useEmailStore = create()( const targets = state.emails.filter((e) => emailIds.includes(e.id)); if (targets.length === 0) return; const owner = currentAccountId(state); - const updates = targets.map((e) => { - const keywords = { ...e.keywords }; - if (on) keywords[token] = true; - else delete keywords[token]; - return { id: e.id, keywords }; - }); + const ids = targets.map((e) => e.id); + const patch = { [token]: on ? true : null }; await applyOrQueueBatch( - updates.map((u): OutboxOp => ({ + ids.map((id): OutboxOp => ({ kind: 'keywords', - emailId: u.id, + emailId: id, accountId: owner, - keywords: u.keywords, + patch, })), - () => setKeywordsForEmails(updates, owner), + () => patchKeywordsForEmails(ids, patch, owner), ); - const byId = new Map(updates.map((u) => [u.id, u.keywords])); + const touched = new Set(ids); set({ emails: get().emails.map((e) => - byId.has(e.id) ? { ...e, keywords: byId.get(e.id)! } : e, + touched.has(e.id) ? { ...e, keywords: applyKeywordPatch(e.keywords, patch) } : e, ), // Untagging inside that tag's view keeps the rows until it's re-opened. ...(!on && state.filters.keyword === token - ? { retainedIds: retain(get().retainedIds, updates.map((u) => u.id)) } + ? { retainedIds: retain(get().retainedIds, ids) } : {}), }); - for (const u of updates) patchCache(u.id, { keywords: u.keywords }, owner); + for (const id of ids) patchCache(id, { keywords: patch }, owner); }, undoLast: async () => { @@ -1579,26 +1566,28 @@ export const useEmailStore = create()( ), ); // Spam / not-spam also flipped `$junk`/`$notjunk` (and maybe `$seen`): - // put the keywords back as they were. - const withKeywords = entry.items.filter((it) => it.originalKeywords); - if (withKeywords.length > 0) { + // put those keywords back as they were, leaving the rest alone. + const keywordPatch = entry.keywordPatch; + const keywordUndo = keywordPatch + ? entry.items.map((it) => ({ id: it.email.id, patch: revertKeywordPatch(keywordPatch, it.originalKeywords) })) + : []; + if (keywordUndo.length > 0) { await applyOrQueueBatch( - withKeywords.map((it): OutboxOp => ({ + keywordUndo.map((u): OutboxOp => ({ kind: 'keywords', - emailId: it.email.id, + emailId: u.id, accountId: entry.accountId, - keywords: it.originalKeywords!, + patch: u.patch, })), - () => setKeywordsForEmails( - withKeywords.map((it) => ({ id: it.email.id, keywords: it.originalKeywords! })), - entry.accountId, - ), + () => patchKeywordsPerEmail(keywordUndo, entry.accountId), ); } + const undoById = new Map(keywordUndo.map((u) => [u.id, u.patch])); for (const it of entry.items) { + const patch = undoById.get(it.email.id); patchCache(it.email.id, { mailboxIds: it.originalMailboxIds, - ...(it.originalKeywords ? { keywords: it.originalKeywords } : {}), + ...(patch ? { keywords: patch } : {}), }, entry.accountId); } } catch (err) { diff --git a/src/stores/offline-cache-store.ts b/src/stores/offline-cache-store.ts index db71c062..e15a0955 100644 --- a/src/stores/offline-cache-store.ts +++ b/src/stores/offline-cache-store.ts @@ -12,6 +12,7 @@ import { create } from 'zustand'; import AsyncStorage from '@react-native-async-storage/async-storage'; import type { Email } from '../api/types'; +import { applyKeywordPatch, type KeywordPatch } from '../lib/keyword-patch'; const INDEX_KEY_PREFIX = 'webmail:offline-cache:index:v2:'; const ENTRY_KEY_PREFIX = 'webmail:offline-cache:entry:v2:'; @@ -78,10 +79,15 @@ interface OfflineCacheState { has: (id: string, jmapAccountId?: string) => boolean; get: (id: string, jmapAccountId?: string) => Promise; put: (email: Email, approxBytes: number, jmapAccountId?: string) => Promise; - // Shallow-merge a change into a cached email (no-op when not cached). Used to - // keep the cached body consistent with an optimistic/queued mutation so a - // re-open while offline reflects the new keywords / mailboxIds. - patch: (id: string, changes: Partial>, jmapAccountId?: string) => Promise; + // Apply a change to a cached email (no-op when not cached). Used to keep the + // cached body consistent with an optimistic/queued mutation so a re-open + // while offline reflects the new keywords / mailboxIds. `keywords` is a + // patch (only the keywords it names change); `mailboxIds` replaces the map. + patch: ( + id: string, + changes: { keywords?: KeywordPatch; mailboxIds?: Record }, + jmapAccountId?: string, + ) => Promise; remove: (ids: string[], jmapAccountId?: string) => Promise; // Evict oldest-received entries until the cache fits within maxBytes. Keeps // the persisted cache from growing without bound on a noisy account. @@ -227,7 +233,11 @@ export const useOfflineCacheStore = create((set, get) => ({ const raw = await AsyncStorage.getItem(entryKey(accountId, key)); if (!raw) return; const email = JSON.parse(raw) as Email; - const updated: Email = { ...email, ...changes }; + const updated: Email = { + ...email, + ...(changes.mailboxIds ? { mailboxIds: changes.mailboxIds } : {}), + ...(changes.keywords ? { keywords: applyKeywordPatch(email.keywords, changes.keywords) } : {}), + }; if (get().activeAccountId !== accountId) return; await AsyncStorage.setItem(entryKey(accountId, key), JSON.stringify(updated)); } catch (err) { diff --git a/src/stores/outbox-store.ts b/src/stores/outbox-store.ts index 2acc4b57..7b19a11f 100644 --- a/src/stores/outbox-store.ts +++ b/src/stores/outbox-store.ts @@ -2,15 +2,16 @@ // // Every mutation the app performs on a message reduces to one of four // idempotent primitives: -// - keywords: replace the full keyword map (read/unread, flag, pin, …) +// - keywords: set or clear individual keywords (read/unread, flag, pin, …) // - mailboxes: replace the full mailboxIds map (move, trash) // - archive: file into Archive with the user's year/month auto-foldering // - destroy: permanently delete the message // -// Because each primitive assigns the *whole* target state rather than a delta, -// replaying it is safe regardless of the server's current state, so we can -// coalesce repeated edits to the same message (last-write-wins) and retry on a -// flaky connection without corrupting anything. +// Because each primitive assigns a target state rather than a delta, replaying +// it is safe regardless of the server's current state, so we can coalesce +// repeated edits to the same message (last-write-wins) and retry on a flaky +// connection without corrupting anything. Keyword ops name only the keywords +// the user changed, so replay never touches ones another client set meanwhile. // // When the device is online and nothing is already queued for a message, the // op runs immediately (preserving today's behaviour, including surfacing real @@ -25,8 +26,9 @@ import { isTransientNetworkError, isAuthError } from '../lib/network-error'; import { useNetworkStore } from './network-store'; import { jmapClient } from '../api/jmap-client'; import { - setEmailKeywords, setEmailMailboxes, destroyEmails, archiveEmails, unprefixMailboxId, + patchKeywordsForEmails, setEmailMailboxes, destroyEmails, archiveEmails, unprefixMailboxId, } from '../api/email'; +import type { KeywordPatch } from '../lib/keyword-patch'; import type { ArchiveMode } from './settings-store'; const KEY_PREFIX = 'webmail:outbox:v1:'; @@ -47,7 +49,7 @@ function storageKey(accountId: string): string { // user's own mail (the client's primary account); set when the message belongs // to a shared/group account, so replay after a reconnect still targets it. export type OutboxOp = - | { kind: 'keywords'; emailId: string; accountId?: string; keywords: Record } + | { kind: 'keywords'; emailId: string; accountId?: string; patch: KeywordPatch } | { kind: 'mailboxes'; emailId: string; accountId?: string; mailboxIds: Record } | { /** @@ -111,12 +113,30 @@ function persistFailed(accountId: string, failed: OutboxEntry[]): void { }); } +// Keyword ops queued by earlier builds carried the message's whole keyword +// map, which the server took as a replacement. That map can't tell which +// keyword the user changed, so it replays as a patch that only sets the +// keywords it holds (clearing the other half of the exclusive `$junk` / +// `$notjunk` pair) and clears nothing else: an unread or unstar queued back +// then is lost, but no star or tag another client set gets erased. +type LegacyKeywordsOp = { kind: 'keywords'; emailId: string; accountId?: string; keywords: Record }; + +function upgradeEntry(entry: OutboxEntry): OutboxEntry { + const op = entry.op as OutboxOp | LegacyKeywordsOp; + if (op.kind !== 'keywords' || !('keywords' in op)) return entry; + const patch: KeywordPatch = {}; + for (const [keyword, on] of Object.entries(op.keywords ?? {})) if (on) patch[keyword] = true; + if (patch.$junk && !patch.$notjunk) patch.$notjunk = null; + if (patch.$notjunk && !patch.$junk) patch.$junk = null; + return { ...entry, op: { kind: 'keywords', emailId: op.emailId, accountId: op.accountId, patch } }; +} + async function load(accountId: string, suffix = ''): Promise { try { const raw = await AsyncStorage.getItem(storageKey(accountId) + suffix); if (raw) { const parsed = JSON.parse(raw); - if (Array.isArray(parsed)) return parsed as OutboxEntry[]; + if (Array.isArray(parsed)) return (parsed as OutboxEntry[]).map(upgradeEntry); } } catch (err) { console.warn('[outbox] hydrate failed', err); @@ -127,7 +147,7 @@ async function load(accountId: string, suffix = ''): Promise { async function runOp(op: OutboxOp): Promise { switch (op.kind) { case 'keywords': - return setEmailKeywords(op.emailId, op.keywords, op.accountId); + return patchKeywordsForEmails([op.emailId], op.patch, op.accountId); case 'mailboxes': return setEmailMailboxes(op.emailId, op.mailboxIds, op.accountId); case 'archive': { @@ -251,11 +271,16 @@ export const useOutboxStore = create((set, get) => ({ // A queued destroy wins; further edits to a doomed message are pointless. if (entries.some((e) => sameMessage(e.op) && e.op.kind === 'destroy')) return; // Coalesce: replace any pending op of the same family for this message - // (full-state replace makes the latest one authoritative). Keep its + // (full-state replace makes the latest one authoritative; keyword + // patches merge, the later value winning per keyword). Keep its // position so creation order is preserved for replay. const idx = entries.findIndex((e) => sameMessage(e.op) && opFamily(e.op) === opFamily(op)); if (idx >= 0) { - entries[idx] = { ...entries[idx], op, attempts: 0, lastError: undefined }; + const prev = entries[idx].op; + const merged: OutboxOp = prev.kind === 'keywords' && op.kind === 'keywords' + ? { ...op, patch: { ...prev.patch, ...op.patch } } + : op; + entries[idx] = { ...entries[idx], op: merged, attempts: 0, lastError: undefined }; } else { entries.push({ id: generateUUID(), op, createdAt: Date.now(), attempts: 0 }); }