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.
This commit is contained in:
Linus Rath
2026-09-24 19:45:07 +02:00
parent d8db4a5597
commit ddff2f7b66
3 changed files with 75 additions and 13 deletions
+15 -4
View File
@@ -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);
}
};
@@ -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<void>) => {
const applyOrQueueBatch = vi.fn(async (_ops: unknown[], onlineRun?: () => Promise<void>) => {
if (onlineRun) await onlineRun();
return { queued: false };
};
});
return {
applyOrQueueBatch,
applyOrQueue: async (op: unknown, onlineRun?: () => Promise<void>) => 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<typeof vi.fn>;
const mockMarkAsSpam = emailApi.markAsSpam as ReturnType<typeof vi.fn>;
const mockUndoSpam = emailApi.undoSpam as ReturnType<typeof vi.fn>;
const mockArchive = emailApi.archiveEmails as ReturnType<typeof vi.fn>;
const mockPatchKeywords = emailApi.patchKeywordsForEmails as ReturnType<typeof vi.fn>;
const mockApplyOrQueueBatch = applyOrQueueBatch as unknown as ReturnType<typeof vi.fn>;
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([]);
});
});
+11 -7
View File
@@ -383,8 +383,12 @@ export interface EmailState {
archiveEmailsBatch: (emailIds: string[]) => Promise<void>;
moveEmailsToMailbox: (emailIds: string[], toMailboxId: string) => Promise<void>;
deleteEmailsBatch: (emailIds: string[], trashMailboxId: string, currentMailboxId: string) => Promise<void>;
/** Set or clear one keyword (tag, `$seen`, `$flagged`) on a selection in one `Email/set`. */
setKeywordForEmails: (emailIds: string[], token: string, on: boolean) => Promise<void>;
/**
* 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<void>;
undoLast: () => Promise<void>;
clearUndo: () => void;
searchEmails: (query: string) => Promise<Email[]>;
@@ -1578,11 +1582,11 @@ export const useEmailStore = create<EmailState>()(
}
},
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<EmailState>()(
})),
() => 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) }
: {}),
});