From b7608d0a9af825f8122707fd8e68584ab22b176c Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 09:58:49 -0400 Subject: [PATCH 1/4] Queue Supply run toggles made offline and replay them in order (Feature 17) A toggle made while the server cannot be reached (the browser is offline, or its PATCH fails with a network error) now stays pending in the existing serial toggle mutation, laid over the list with a "will sync" marker, and is sent once the network status says the server is reachable again. Every toggle is saved in order in localStorage until it settles, and SupplyRunSync replays the saved ones after a reload. Any answer from the server settles a toggle: a 409 archived discards it, puts the count back and shows the refusal; other refusals surface as before and are never queued. The Supply run and the household schedule are saved on the device too, so the Supply run screen opens offline from the saved list. Co-Authored-By: Claude Opus 5.5 --- web/src/App.tsx | 2 + web/src/shopping/SupplyRunPage.module.css | 15 ++ web/src/shopping/SupplyRunPage.tsx | 47 +++- web/src/shopping/api.ts | 240 ++++++++++++++----- web/src/shopping/offline.ts | 136 +++++++++++ web/src/shopping/offlineQueue.test.tsx | 279 ++++++++++++++++++++++ 6 files changed, 655 insertions(+), 64 deletions(-) create mode 100644 web/src/shopping/offline.ts create mode 100644 web/src/shopping/offlineQueue.test.tsx diff --git a/web/src/App.tsx b/web/src/App.tsx index 17c0a7c..0472284 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -16,6 +16,7 @@ import { HouseholdSettingsPage } from './settings/HouseholdSettingsPage.tsx' import { RegularsSettingsPage } from './settings/RegularsSettingsPage.tsx' import { SettingsPage } from './settings/SettingsPage.tsx' import { AppShell } from './shell/AppShell.tsx' +import { SupplyRunSync } from './shopping/api.ts' import { SupplyRunPage } from './shopping/SupplyRunPage.tsx' // Every route the app serves, with the heading its placeholder page shows. @@ -88,6 +89,7 @@ export default function App({ api, serviceWorker }: AppProps) { + diff --git a/web/src/shopping/SupplyRunPage.module.css b/web/src/shopping/SupplyRunPage.module.css index 94c0671..b682c0c 100644 --- a/web/src/shopping/SupplyRunPage.module.css +++ b/web/src/shopping/SupplyRunPage.module.css @@ -200,6 +200,21 @@ color: var(--color-neutral-700); } +/* Toggled while the server cannot be reached: the change is saved here and + sent once it can be. */ +.willSync { + display: inline-flex; + align-items: center; + gap: 4px; + margin-left: 8px; + font-family: var(--font-body); + font-size: 12px; + font-weight: 700; + color: var(--color-neutral-700); + text-decoration: none; + vertical-align: middle; +} + .qty { flex: none; font-family: var(--font-body); diff --git a/web/src/shopping/SupplyRunPage.tsx b/web/src/shopping/SupplyRunPage.tsx index 4799fa6..6bc4bf5 100644 --- a/web/src/shopping/SupplyRunPage.tsx +++ b/web/src/shopping/SupplyRunPage.tsx @@ -1,4 +1,4 @@ -import { Archive, Check, MessageSquare } from 'lucide-react' +import { Archive, Check, CloudUpload, MessageSquare } from 'lucide-react' import { Link, useSearchParams } from 'react-router' import { Unavailable } from '../plan/PlanPage.tsx' import { defaultWeek, localNow, weekdayKicker, weekLabel, weekStart as weekOf } from '../plan/week.ts' @@ -20,6 +20,7 @@ import { type SupplyRunItem, type Toggles, } from './api.ts' +import { keep, savedSchedule, useSavedProblem } from './offline.ts' import styles from './SupplyRunPage.module.css' // SUPPLY_ARCHIVE is the Supply archive screen. @@ -39,9 +40,20 @@ export function sentAt(at: Date, timezone: string): string { // ItemRow is one ingredient: on a current Supply run a checkbox, checked // (grape) while it is to buy; unchecked, the name is struck through and // "have it" stands in for the quantity. An archived Supply run shows the -// same marks, read-only. +// same marks, read-only. An item toggled while the server cannot be reached +// says it will sync. function ItemRow({ item, toggles }: { item: SupplyRunItem; toggles: Toggles | null }) { - const name = {item.name} + const name = ( + <> + {item.name} + {toggles?.willSync.has(item.id) && ( + + + will sync + + )} + + ) return (
  • {toggles === null ? ( @@ -194,7 +206,9 @@ function WeekSupplyRun({ weekStart, timezone, layout }: { weekStart: string; tim const run = useSupplyRun(weekStart) const generate = useGenerateSupplyRun(weekStart) - if (run.isError) { + // Offline, the Supply run saved on this device stays shown; the offline + // pill says it is saved data. + if (run.isError && !(run.data !== undefined && run.error instanceof TypeError)) { return void run.refetch()}>Couldn't load the shopping list. {run.error.message} } if (run.isPending || generate.isPending) return @@ -216,8 +230,21 @@ function WeekSupplyRun({ weekStart, timezone, layout }: { weekStart: string; tim // SupplyRunPage is the Supply run (mock 1l on mobile, 2b on tablet, 2f on // desktop) of the week the Plan opens on — next week once this week's // plan_draft time has come, otherwise this week — or of the week chosen by -// ?week=, such as a week in the Supply archive. +// ?week=, such as a week in the Supply archive. Offline, where the settings +// cannot be read, it chooses the week by the schedule saved on this device. export function SupplyRunPage() { + const problem = useSavedProblem() + return ( + <> + {problem !== null && ( + Changes to the shopping list can't be kept on this device while offline. {problem.message} + )} + + + ) +} + +function ChosenSupplyRun() { const layout = useBreakpoint() const settings = useHouseholdSettings() const [params] = useSearchParams() @@ -226,12 +253,14 @@ export function SupplyRunPage() { if (chosen !== null && !isMonday(chosen)) { return {`“${chosen}” is not the Monday of a week, as YYYY-MM-DD.`} } - if (settings.isError) { + // fetch rejects with a TypeError only when the network fails. + const schedule = settings.data ?? (settings.error instanceof TypeError ? keep(savedSchedule) : null) + if (settings.isError && !schedule) { return void settings.refetch()}>Couldn't load the household settings. {settings.error.message} } - if (!settings.data) return - const week = chosen ?? defaultWeek(settings.data, new Date()) - return + if (!schedule) return + const week = chosen ?? defaultWeek(schedule, new Date()) + return } // isMonday says whether date is a real YYYY-MM-DD Monday. diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index 56f8b96..343b4f8 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -1,8 +1,20 @@ -import { useMutation, useMutationState, useQuery, useQueryClient } from '@tanstack/react-query' -import { useReducer } from 'react' -import { ApiError, useApi } from '../api/index.ts' +import { + MutationObserver, + useMutation, + useMutationState, + useQuery, + useQueryClient, + type MutationObserverOptions, + type QueryClient, +} from '@tanstack/react-query' +import { useEffect, useReducer, useSyncExternalStore } from 'react' +import { ApiError, useApi, type ApiClient } from '../api/index.ts' import { isRecord } from '../api/errors.ts' +import { meQueryKey } from '../AuthBootstrap.tsx' import { oneOf } from '../dinners/api.ts' +import type { NetworkStatus } from '../pwa/networkStatus.ts' +import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts' +import { dequeue, enqueue, keep, queued, SAVED_KEY, saveRun, savedRun, saveSchedule } from './offline.ts' export const SHOPPING_PATH = '/api/v1/shopping' @@ -131,12 +143,22 @@ export function parseSupplyRun(where: string, weekStart: string, body: unknown): } } +// savedSupplyRun is the week's Supply run saved on this device, or undefined. +function savedSupplyRun(weekStart: string): SupplyRun | undefined { + const saved = savedRun(weekStart) + return saved === undefined ? undefined : parseSupplyRun(`Saved ${SAVED_KEY}`, weekStart, saved) +} + // useSupplyRun is the Supply run of the week starting on Monday weekStart // (GET /api/v1/shopping/{weekStart}), or null when the week has none (404). +// It starts from the Supply run saved on this device, if there is one, so +// the list opens offline; the saved one is always read again. export function useSupplyRun(weekStart: string) { const { apiRead } = useApi() return useQuery({ queryKey: supplyRunKey(weekStart), + initialData: () => keep(() => savedSupplyRun(weekStart)), + initialDataUpdatedAt: 0, queryFn: async (): Promise => { const path = supplyRunPath(weekStart) let body: unknown @@ -169,75 +191,85 @@ export function useGenerateSupplyRun(weekStart: string) { // Toggle is one item marked to buy (needed) or "have it" (not needed). export type Toggle = { id: string; needed: boolean } +// KeyedToggle is a toggle as sent: key tells it apart from any other toggle +// of the same item, here and in the queue saved on this device. +type KeyedToggle = Toggle & { key: string } + // toggleKey keys the toggles of the week starting on weekStart. function toggleKey(weekStart: string) { return ['shopping', weekStart, 'toggle'] as const } // Toggles is the toggles of one week's Supply run: shown, the Supply run -// with those still pending laid over it, the latest failed toggle no later -// toggle of its item has superseded, and toggle to make another. +// with those still pending laid over it, the items whose toggles will sync +// once the server can be reached, the latest failed toggle no later toggle +// of its item has superseded, and toggle to make another. export type Toggles = { shown: SupplyRun + willSync: ReadonlySet failed: { toggle: Toggle; error: Error } | null toggle: (toggle: Toggle) => void } -// useToggles marks items of the week's Supply run, already read, to buy or -// "have it" (PATCH /api/v1/shopping/items/{id}). The Supply run cached is -// the server's: a toggle's answer writes the item and the counts it answers -// with there. Until then the toggle is pending and is laid over the Supply -// run shown, so the item and the needed count change at once; a refused -// toggle just stops being pending, which puts both back. -// -// The Supply run and the toggles are read straight from the query and -// mutation caches as each render begins, never from the caches' batched -// notices, which can arrive apart: a toggle is in the mutation cache as -// pending as soon as it is made, and its answer is in the query cache -// before it stops being pending. Making a toggle renders again at once, so -// the checkbox clicked never shows its old state; useSupplyRun and -// useMutationState render again on every later change. +// reachable resolves once network says the server can be reached: at once +// when it says so now. +function reachable(network: NetworkStatus): Promise { + if (!network.offline()) return Promise.resolve() + return new Promise((resolve) => { + const stop = network.subscribe(() => { + if (network.offline()) return + stop() + resolve() + }) + }) +} + +// toggleOptions is the mutation every toggle of the week's Supply run is, +// made on the Supply run screen (useToggles) or replayed from the queue saved +// on this device (useSupplyRunSync). // // The week's toggles are sent one at a time, in the order made (one // mutation scope), so the counts each is answered with include every toggle -// before it and none after. Each is sent once, and not held back while the -// device is offline (networkMode always): an item never shows a state for -// longer than it takes the server to refuse it. +// before it and none after. While the server cannot be reached — the device +// is offline, or the toggle's PATCH failed with a network error (fetch +// rejected with a TypeError) — a toggle waits, pending, and is sent once it +// can be: those after it wait behind it. Any answer from the server settles +// it: a refusal (a 400, 404, 409 or any other error response) or a response +// that is not the item as toggled fails it, and it is not sent again. Before +// a toggle is sent, GET /api/v1/me has answered, so it carries the session's +// CSRF token even when the app opened offline. // -// A failed toggle stays failed, and shown, until a later toggle of its item -// supersedes it, such as its Retry: a toggle of another item succeeding -// does not hide it. So that a failure is never collected unseen, toggles are -// kept (gcTime Infinity), and a settled toggle removes the earlier toggles -// of its item it supersedes. A toggle refused because the week was frozen -// into the Supply archive (409 archived) reads the Supply run again, which -// then renders read-only. -export function useToggles(weekStart: string): Toggles { - const { apiFetch } = useApi() - const queryClient = useQueryClient() +// Every toggle is in the queue saved on this device (enqueue, by useToggles) +// until it settles, so a reload replays those the server never answered. +function toggleOptions( + weekStart: string, + api: ApiClient, + queryClient: QueryClient, +): MutationObserverOptions { const key = supplyRunKey(weekStart) const mutationKey = toggleKey(weekStart) - useSupplyRun(weekStart) - useMutationState({ filters: { mutationKey } }) - const [, renderAgain] = useReducer((n: number) => n + 1, 0) - const run = queryClient.getQueryData(key) - if (!run) throw new Error(`The week of ${weekStart} has no Supply run read to toggle items of.`) - const toggles = queryClient - .getMutationCache() - .findAll({ mutationKey }) - .map((mutation) => ({ status: mutation.state.status, toggle: mutation.state.variables as Toggle, error: mutation.state.error })) - const failed = toggles - .filter((t, i) => t.status === 'error' && !toggles.slice(i + 1).some((later) => later.toggle.id === t.toggle.id)) - .at(-1) - const { mutate } = useMutation({ + return { mutationKey, scope: { id: mutationKey.join(' ') }, networkMode: 'always', retry: false, gcTime: Infinity, - mutationFn: async ({ id, needed }: Toggle): Promise => { + mutationFn: async ({ id, needed }: KeyedToggle): Promise => { const path = `${SHOPPING_PATH}/items/${encodeURIComponent(id)}` const where = `PATCH ${path}` - const body = await apiFetch(path, { method: 'PATCH', body: { needed } }) + let body: unknown + for (;;) { + await reachable(api.network) + try { + await queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) + body = await api.apiFetch(path, { method: 'PATCH', body: { needed } }) + break + } catch (error) { + // The API client marks the network offline as fetch rejects; a + // TypeError it did not mark is not a network failure. + if (!(error instanceof TypeError) || !api.network.offline()) throw error + } + } if (!isRecord(body) || body.id !== id || body.needed !== needed) throw new Error(`${where}: response is not item ${id} as toggled`) return parseCounts(where, body) }, @@ -262,24 +294,122 @@ export function useToggles(weekStart: string): Toggles { if (error instanceof ApiError && error.code === 'archived') void queryClient.invalidateQueries({ queryKey: key }) }, // This toggle is still pending as it settles, and those after it wait - // for it: the settled toggles of its item are the earlier ones. - onSettled: (_counts, _error, { id }) => { + // for it: the settled toggles of its item are the earlier ones. Settled, + // it leaves the saved queue. + onSettled: (_counts, _error, { key: settled, id }) => { const cache = queryClient.getMutationCache() for (const earlier of cache.findAll({ mutationKey, predicate: (m) => m.state.status !== 'pending' })) { - if ((earlier.state.variables as Toggle).id === id) cache.remove(earlier) + if ((earlier.state.variables as KeyedToggle).id === id) cache.remove(earlier) } + keep(() => dequeue(settled)) }, - }) + } +} + +// useToggles marks items of the week's Supply run, already read, to buy or +// "have it" (PATCH /api/v1/shopping/items/{id}; see toggleOptions). The +// Supply run cached is the server's: a toggle's answer writes the item and +// the counts it answers with there. Until then the toggle is pending and is +// laid over the Supply run shown, so the item and the needed count change at +// once; a refused toggle just stops being pending, which puts both back. +// While the server cannot be reached, the items with pending toggles will +// sync. +// +// The Supply run and the toggles are read straight from the query and +// mutation caches as each render begins, never from the caches' batched +// notices, which can arrive apart: a toggle is in the mutation cache as +// pending as soon as it is made, and its answer is in the query cache +// before it stops being pending. Making a toggle renders again at once, so +// the checkbox clicked never shows its old state; useSupplyRun and +// useMutationState render again on every later change. +// +// A failed toggle stays failed, and shown, until a later toggle of its item +// supersedes it, such as its Retry: a toggle of another item succeeding +// does not hide it. So that a failure is never collected unseen, toggles are +// kept (gcTime Infinity), and a settled toggle removes the earlier toggles +// of its item it supersedes. A toggle refused because the week was frozen +// into the Supply archive (409 archived), even one replayed after the device +// was offline, reads the Supply run again, which then renders read-only. +export function useToggles(weekStart: string): Toggles { + const api = useApi() + const queryClient = useQueryClient() + const key = supplyRunKey(weekStart) + const mutationKey = toggleKey(weekStart) + useSupplyRun(weekStart) + useMutationState({ filters: { mutationKey } }) + const offline = useSyncExternalStore(api.network.subscribe, api.network.offline) + const [, renderAgain] = useReducer((n: number) => n + 1, 0) + const run = queryClient.getQueryData(key) + if (!run) throw new Error(`The week of ${weekStart} has no Supply run read to toggle items of.`) + const toggles = queryClient + .getMutationCache() + .findAll({ mutationKey }) + .map((mutation) => ({ status: mutation.state.status, toggle: mutation.state.variables as KeyedToggle, error: mutation.state.error })) + const pending = toggles.filter((t) => t.status === 'pending').map((t) => t.toggle) + const failed = toggles + .filter((t, i) => t.status === 'error' && !toggles.slice(i + 1).some((later) => later.toggle.id === t.toggle.id)) + .at(-1) + const { mutate } = useMutation(toggleOptions(weekStart, api, queryClient)) return { - shown: shown(run, toggles.filter((t) => t.status === 'pending').map((t) => t.toggle)), - failed: failed === undefined ? null : { toggle: failed.toggle, error: failed.error! }, - toggle: (toggle) => { - mutate(toggle) + shown: shown(run, pending), + willSync: new Set(offline ? pending.map((toggle) => toggle.id) : []), + failed: failed === undefined ? null : { toggle: { id: failed.toggle.id, needed: failed.toggle.needed }, error: failed.error! }, + toggle: ({ id, needed }) => { + const queuedToggle = { key: crypto.randomUUID(), weekStart, id, needed } + keep(() => enqueue(queuedToggle)) + mutate({ key: queuedToggle.key, id, needed }) renderAgain() }, } } +// useSupplyRunSync keeps, on this device, what the Supply run screen needs +// to open offline — the household's schedule and each Supply run the server +// sends (saveRun) — and replays, in the order made, the toggles saved in the +// queue that are not already pending here: those of a page reloaded before +// the server answered them. A replayed toggle's answer is written over the +// Supply run saved, so the week's Supply run is cached from it first; it +// is still read again (updatedAt 0) when the Supply run screen opens. +export function useSupplyRunSync() { + const api = useApi() + const queryClient = useQueryClient() + useEffect(() => { + const stop = queryClient.getQueryCache().subscribe((event) => { + if (event.type !== 'updated' || event.action.type !== 'success') return + const [scope, week] = event.query.queryKey + const data: unknown = event.query.state.data + if (scope === 'shopping' && typeof week === 'string' && event.query.queryKey.length === 2) { + keep(() => saveRun(week, data as SupplyRun | null)) + } else if (scope === HOUSEHOLD_SETTINGS_KEY[0] && week === HOUSEHOLD_SETTINGS_KEY[1]) { + keep(() => saveSchedule(data as HouseholdSettings)) + } + }) + const pending = new Set( + queryClient + .getMutationCache() + .findAll({ mutationKey: ['shopping'], predicate: (m) => m.state.status === 'pending' }) + .map((m) => (m.state.variables as KeyedToggle).key), + ) + for (const { key, weekStart, id, needed } of keep(queued) ?? []) { + if (pending.has(key)) continue + const runKey = supplyRunKey(weekStart) + if (queryClient.getQueryData(runKey) === undefined) { + const saved = keep(() => savedSupplyRun(weekStart)) + if (saved !== undefined) queryClient.setQueryData(runKey, saved, { updatedAt: 0 }) + } + // A refusal is the mutation's state, which the Supply run screen shows. + new MutationObserver(queryClient, toggleOptions(weekStart, api, queryClient)).mutate({ key, id, needed }).catch(() => {}) + } + return stop + }, [api, queryClient]) +} + +// SupplyRunSync runs useSupplyRunSync for the whole app. +export function SupplyRunSync(): null { + useSupplyRunSync() + return null +} + // shown is run as the household sees it with the pending toggles laid over // it: each item as its latest pending toggle marks it, and the needed count // moved to match. diff --git a/web/src/shopping/offline.ts b/web/src/shopping/offline.ts new file mode 100644 index 0000000..6b1c323 --- /dev/null +++ b/web/src/shopping/offline.ts @@ -0,0 +1,136 @@ +import { useSyncExternalStore } from 'react' +import { isRecord } from '../api/errors.ts' + +// SAVED_KEY is the localStorage key holding, on this device, what the Supply +// run screen needs to open offline — the household's schedule and the +// Supply runs last read — and the toggles made that the server has not +// answered yet, in the order they were made. +export const SAVED_KEY = 'mealquest.supply-run' + +// SAVED_WEEKS is how many weeks' Supply runs are kept: this week's and next +// week's are the only ones still current. +const SAVED_WEEKS = 2 + +// Schedule is the part of the household settings the Supply run screen +// needs to choose the week and say times. The service worker never caches +// the settings, so it is kept here for the screen to open offline. +export type Schedule = { timezone: string; plan_draft_dow: number; plan_draft_time: string } + +// QueuedToggle is a toggle the server has not answered yet: key tells it +// apart from any other toggle of the same item. +export type QueuedToggle = { key: string; weekStart: string; id: string; needed: boolean } + +// Saved is the stored value: runs holds each week's Supply run as the server +// last sent it, by its Monday. +type Saved = { schedule: Schedule | null; runs: Record; queue: QueuedToggle[] } + +function isSchedule(value: unknown): value is Schedule { + return ( + isRecord(value) && + typeof value.timezone === 'string' && + Number.isInteger(value.plan_draft_dow) && + typeof value.plan_draft_time === 'string' + ) +} + +function isQueuedToggle(value: unknown): value is QueuedToggle { + return ( + isRecord(value) && + typeof value.key === 'string' && + typeof value.weekStart === 'string' && + typeof value.id === 'string' && + typeof value.needed === 'boolean' + ) +} + +// read is the stored value, empty when nothing is stored. A value that is not +// one MealQuest stored is an error. +function read(): Saved { + const text = window.localStorage.getItem(SAVED_KEY) + if (text === null) return { schedule: null, runs: {}, queue: [] } + const saved: unknown = JSON.parse(text) + if ( + !isRecord(saved) || + (saved.schedule !== null && !isSchedule(saved.schedule)) || + !isRecord(saved.runs) || + !Array.isArray(saved.queue) || + !saved.queue.every(isQueuedToggle) + ) { + throw new Error(`The Supply run saved on this device (${SAVED_KEY}) is not one MealQuest saved.`) + } + return { schedule: saved.schedule as Schedule | null, runs: saved.runs, queue: saved.queue } +} + +function write(change: (saved: Saved) => Saved) { + window.localStorage.setItem(SAVED_KEY, JSON.stringify(change(read()))) +} + +// savedSchedule is the household's schedule as last read, or null. +export function savedSchedule(): Schedule | null { + return read().schedule +} + +export function saveSchedule({ timezone, plan_draft_dow, plan_draft_time }: Schedule) { + write((saved) => ({ ...saved, schedule: { timezone, plan_draft_dow, plan_draft_time } })) +} + +// savedRun is the week's Supply run as the server last sent it, unparsed, or +// undefined when none is saved. +export function savedRun(weekStart: string): unknown { + return read().runs[weekStart] +} + +// saveRun keeps the week's Supply run as the server sent it, with the latest +// weeks'. A week with none (null) or one frozen into the Supply archive is +// not kept: nothing on it can change. +export function saveRun(weekStart: string, run: { status: string } | null) { + write((saved) => { + const runs = { ...saved.runs } + delete runs[weekStart] + if (run !== null && run.status !== 'archived') runs[weekStart] = run + const latest = Object.keys(runs).sort().slice(-SAVED_WEEKS) + return { ...saved, runs: Object.fromEntries(latest.map((week) => [week, runs[week]])) } + }) +} + +// queued is the toggles the server has not answered, in the order made. +export function queued(): QueuedToggle[] { + return read().queue +} + +export function enqueue(toggle: QueuedToggle) { + write((saved) => ({ ...saved, queue: [...saved.queue, toggle] })) +} + +export function dequeue(key: string) { + write((saved) => ({ ...saved, queue: saved.queue.filter((toggle) => toggle.key !== key) })) +} + +// The latest failure to read or write the saved Supply run, which the Supply +// run screen shows: a toggle that cannot be kept may be lost on a reload. +let problem: Error | null = null +const listeners = new Set<() => void>() + +// keep runs one read or write of the saved Supply run, and returns its +// result, or undefined when it failed: the failure is reported to the Supply +// run screen. Listeners hear of it after the current render. +export function keep(action: () => T): T | undefined { + try { + return action() + } catch (error) { + problem = error instanceof Error ? error : new Error(String(error)) + queueMicrotask(() => listeners.forEach((listener) => listener())) + return undefined + } +} + +function subscribe(listener: () => void) { + listeners.add(listener) + return () => listeners.delete(listener) +} + +// useSavedProblem is the latest failure to read or write the saved Supply +// run, or null. +export function useSavedProblem(): Error | null { + return useSyncExternalStore(subscribe, () => problem) +} diff --git a/web/src/shopping/offlineQueue.test.tsx b/web/src/shopping/offlineQueue.test.tsx new file mode 100644 index 0000000..8ec514c --- /dev/null +++ b/web/src/shopping/offlineQueue.test.tsx @@ -0,0 +1,279 @@ +import { QueryClientProvider } from '@tanstack/react-query' +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { MemoryRouter } from 'react-router' +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' +import { createNetworkStatus, type NetworkWindow } from '../pwa/networkStatus.ts' +import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx' +import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' +import { mockViewport } from '../test/matchMedia.ts' +import { SupplyRunSync } from './api.ts' +import { SAVED_KEY } from './offline.ts' +import { SupplyRunPage } from './SupplyRunPage.tsx' + +const THIS_WEEK = '/api/v1/shopping/2026-10-05' +const ITEM = (id: string) => `/api/v1/shopping/items/${id}` + +// Monday's tacos: two items to buy and one the household has. +const ITEMS = [ + { id: 'i1', name: 'Ground beef', qty: '1 lb', needed: true }, + { id: 'i2', name: 'Corn tortillas', qty: '12', needed: true }, + { id: 'i3', name: 'Taco seasoning', qty: '1 packet', needed: false }, +] + +// The browser's online flag; goOffline and goOnline change it and send the +// page's window the event a browser sends. +let onLine: boolean +// The server can be reached: false, every request fails as fetch does when +// the network fails, whatever onLine says. +let reachable: boolean +// The window of the page now open: a reload opens a new one, and the old +// page hears nothing more. +let win: NetworkWindow & EventTarget + +function openWindow() { + win = Object.assign(new EventTarget(), { + navigator: { + get onLine() { + return onLine + }, + }, + }) as unknown as NetworkWindow & EventTarget + return win +} + +function goOffline() { + onLine = false + act(() => void win.dispatchEvent(new Event('offline'))) +} + +function goOnline() { + onLine = true + reachable = true + act(() => void win.dispatchEvent(new Event('online'))) +} + +// stubServer is the server's Supply run of the week: a PATCH marks its item +// and answers with its counts, unless refuse answers it instead. It counts +// as archived once archive is called. +function stubServer(refuse: (id: string) => Response | undefined = () => undefined) { + const needed = new Map(ITEMS.map((item) => [item.id, item.needed])) + let status = 'current' + const counts = () => ({ needed_count: [...needed.values()].filter(Boolean).length, total_count: needed.size }) + const run = () => ({ + weekStart: '2026-10-05', + label: 'Week 41 · Oct 5 – 11', + title: null, + status, + sent_to_chat_at: null, + ...counts(), + groups: [ + { + dinner: { id: 'd0', name: 'Beef tacos' }, + date: '2026-10-05', + items: ITEMS.map((item) => ({ ...item, needed: needed.get(item.id)! })), + }, + ], + }) + const fetchMock = vi.fn((input, init) => { + if (!onLine || !reachable) return Promise.reject(new TypeError('Failed to fetch')) + const key = `${init?.method ?? 'GET'} ${String(input)}` + if (key === 'GET /api/v1/me') return Promise.resolve(jsonResponse(200, ME)) + if (key === 'GET /api/v1/settings/household') return Promise.resolve(jsonResponse(200, HOUSEHOLD_SETTINGS)) + if (key === `GET ${THIS_WEEK}`) return Promise.resolve(jsonResponse(200, run())) + const item = ITEMS.find((i) => key === `PATCH ${ITEM(i.id)}`) + if (item === undefined) throw new Error(`unexpected fetch: ${key}`) + const refusal = refuse(item.id) + if (refusal !== undefined) return Promise.resolve(refusal) + const body = JSON.parse(String(init?.body)) as { needed: boolean } + needed.set(item.id, body.needed) + return Promise.resolve(jsonResponse(200, { id: item.id, needed: body.needed, ...counts() })) + }) + vi.stubGlobal('fetch', fetchMock) + return { + fetchMock, + archive: () => { + status = 'archived' + }, + } +} + +// patches lists the PATCHes sent, as "path body". +function patches(fetchMock: Mock) { + return fetchMock.mock.calls.filter(([, init]) => init?.method === 'PATCH').map(([input, init]) => `${String(input)} ${String(init?.body)}`) +} + +// queue is the toggles saved on this device, as "id needed". +function queue(): string[] { + const saved = localStorage.getItem(SAVED_KEY) + if (saved === null) return [] + return (JSON.parse(saved) as { queue: { id: string; needed: boolean }[] }).queue.map((t) => `${t.id} ${t.needed}`) +} + +// openPage opens the Supply run screen in a new window, as the app does: +// with SupplyRunSync, and with /me loaded unless the server cannot be +// reached. +async function openPage() { + const api = createApiClient(fakeLocation('/plan/shopping'), createNetworkStatus(openWindow()), createVerifiedSession(localStorage)) + if (onLine && reachable) await api.loadMe() + return render( + + + + + + + + , + ) +} + +function checkbox(name: string) { + return screen.getByRole('checkbox', { name: new RegExp(`^${name}`) }) +} + +function row(name: string) { + return screen.getByText(name).closest('li')! +} + +beforeEach(() => { + onLine = true + reachable = true + mockViewport(390) + vi.useFakeTimers({ toFake: ['Date'] }) + vi.setSystemTime(new Date('2026-10-07T16:00:00Z')) +}) + +afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + Reflect.deleteProperty(window, 'matchMedia') +}) + +describe('offline toggle queue', () => { + it('applies toggles made offline at once, saves them in order and sends them in that order once online', async () => { + const { fetchMock } = stubServer() + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + goOffline() + fireEvent.click(checkbox('Taco seasoning')) + fireEvent.click(checkbox('Ground beef')) + fireEvent.click(checkbox('Taco seasoning')) + + // Applied here, counted, marked to sync and saved in order; nothing sent. + expect(checkbox('Taco seasoning')).not.toBeChecked() + expect(checkbox('Ground beef')).not.toBeChecked() + expect(screen.getByText('1 / 3')).toBeInTheDocument() + expect(within(row('Taco seasoning')).getByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(within(row('Corn tortillas')).queryByText('will sync')).not.toBeInTheDocument() + expect(queue()).toEqual(['i3 true', 'i1 false', 'i3 false']) + expect(patches(fetchMock)).toEqual([]) + + goOnline() + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(fetchMock)).toEqual([ + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i1')} {"needed":false}`, + `${ITEM('i3')} {"needed":false}`, + ]) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + expect(checkbox('Taco seasoning')).not.toBeChecked() + expect(checkbox('Ground beef')).not.toBeChecked() + expect(screen.queryByText('will sync')).not.toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('queues a toggle whose PATCH fails with a network error and sends it again once back online', async () => { + const { fetchMock } = stubServer() + await openPage() + fireEvent.click(await screen.findByRole('checkbox', { name: 'Taco seasoning' })) + await waitFor(() => expect(queue()).toEqual([])) + expect(screen.getByText('3 / 3')).toBeInTheDocument() + + // The browser still says online, but the server cannot be reached. + reachable = false + fireEvent.click(checkbox('Corn tortillas')) + expect(await within(row('Corn tortillas')).findByText('will sync')).toBeInTheDocument() + expect(patches(fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`, `${ITEM('i2')} {"needed":false}`]) + expect(checkbox('Corn tortillas')).not.toBeChecked() + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(queue()).toEqual(['i2 false']) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + + goOnline() + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`, `${ITEM('i2')} {"needed":false}`, `${ITEM('i2')} {"needed":false}`]) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByText('will sync')).not.toBeInTheDocument() + }) + + it('opens the saved list after a reload offline and replays the saved toggles in order once online', async () => { + const { fetchMock } = stubServer() + const first = await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + goOffline() + fireEvent.click(checkbox('Taco seasoning')) + fireEvent.click(checkbox('Ground beef')) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + // Reload while offline: the settings, /me and the list cannot be read. + first.unmount() + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + expect(checkbox('Taco seasoning')).toBeChecked() + expect(checkbox('Ground beef')).not.toBeChecked() + expect(within(row('Taco seasoning')).getByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(patches(fetchMock)).toEqual([]) + + goOnline() + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`, `${ITEM('i1')} {"needed":false}`]) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(checkbox('Taco seasoning')).toBeChecked() + expect(checkbox('Ground beef')).not.toBeChecked() + }) + + it('discards a queued toggle the server refuses because the list was archived, puts the count back and says so', async () => { + const server = stubServer(() => errorResponse(409, 'archived')) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + goOffline() + fireEvent.click(checkbox('Taco seasoning')) + expect(screen.getByText('3 / 3')).toBeInTheDocument() + + server.archive() + goOnline() + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't mark Taco seasoning to buy. archived message") + expect(await screen.findByText('Archived')).toBeInTheDocument() + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(within(row('Taco seasoning')).getByText('have it')).toBeInTheDocument() + expect(screen.queryByText('will sync')).not.toBeInTheDocument() + expect(queue()).toEqual([]) + // Sent once, never again. + goOnline() + await waitFor(() => expect(patches(server.fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`])) + }) + + it('does not queue a toggle the server refuses: it is put back and says why', async () => { + const server = stubServer(() => errorResponse(400, 'invalid')) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + fireEvent.click(checkbox('Ground beef')) + expect(await screen.findByRole('alert')).toHaveTextContent("Couldn't mark Ground beef “have it”. invalid message") + expect(checkbox('Ground beef')).toBeChecked() + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByText('will sync')).not.toBeInTheDocument() + expect(queue()).toEqual([]) + + goOffline() + goOnline() + await waitFor(() => expect(patches(server.fetchMock)).toEqual([`${ITEM('i1')} {"needed":false}`])) + }) +}) -- 2.55.0 From 4ba647107077f5730f490aa00bf2e1bc261ad6ee Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 10:20:23 -0400 Subject: [PATCH 2/4] Serialize Supply run toggle replay and keep "will sync" until it settles (Feature 17) Every Supply run toggle, made here or replayed from the saved queue, now runs in one mutation scope whatever its week, so a replay spanning weeks is sent strictly in the stored order and never overlaps a live toggle. The "will sync" marker latches on a toggle that was pending while the server could not be reached and stays until that toggle settles, rather than vanishing as soon as the network returns while its replay is still in flight. The Supply run screen reads the saved schedule once per mount instead of on every render, so a corrupt saved store offline reports its problem once instead of looping. Replay is documented as at-least-once: a toggle sets needed to a fixed value, so a duplicate sent after a reload leaves the item and the counts unchanged. Tests cover a two-week queue replayed in order one PATCH at a time, a duplicate replay, corrupt storage offline and a delayed replay answer. Co-Authored-By: Claude Opus 5.5 --- web/src/shopping/SupplyRunPage.tsx | 6 +- web/src/shopping/api.ts | 47 +++++-- web/src/shopping/offlineQueue.test.tsx | 181 +++++++++++++++++++++---- 3 files changed, 194 insertions(+), 40 deletions(-) diff --git a/web/src/shopping/SupplyRunPage.tsx b/web/src/shopping/SupplyRunPage.tsx index 6bc4bf5..30072ef 100644 --- a/web/src/shopping/SupplyRunPage.tsx +++ b/web/src/shopping/SupplyRunPage.tsx @@ -1,4 +1,5 @@ import { Archive, Check, CloudUpload, MessageSquare } from 'lucide-react' +import { useState } from 'react' import { Link, useSearchParams } from 'react-router' import { Unavailable } from '../plan/PlanPage.tsx' import { defaultWeek, localNow, weekdayKicker, weekLabel, weekStart as weekOf } from '../plan/week.ts' @@ -249,12 +250,15 @@ function ChosenSupplyRun() { const settings = useHouseholdSettings() const [params] = useSearchParams() const chosen = params.get(WEEK_PARAM) + // Read once, not on every render: a failed read reports a problem, which + // renders this again. + const [saved] = useState(() => keep(savedSchedule)) if (chosen !== null && !isMonday(chosen)) { return {`“${chosen}” is not the Monday of a week, as YYYY-MM-DD.`} } // fetch rejects with a TypeError only when the network fails. - const schedule = settings.data ?? (settings.error instanceof TypeError ? keep(savedSchedule) : null) + const schedule = settings.data ?? (settings.error instanceof TypeError ? saved : null) if (settings.isError && !schedule) { return void settings.refetch()}>Couldn't load the household settings. {settings.error.message} } diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index 343b4f8..e9b5430 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -7,7 +7,7 @@ import { type MutationObserverOptions, type QueryClient, } from '@tanstack/react-query' -import { useEffect, useReducer, useSyncExternalStore } from 'react' +import { useEffect, useReducer, useRef, useSyncExternalStore } from 'react' import { ApiError, useApi, type ApiClient } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { meQueryKey } from '../AuthBootstrap.tsx' @@ -200,6 +200,10 @@ function toggleKey(weekStart: string) { return ['shopping', weekStart, 'toggle'] as const } +// TOGGLE_SCOPE is the one mutation scope of every toggle of every week, made +// here or replayed from the queue saved on this device. +const TOGGLE_SCOPE = 'shopping toggle' + // Toggles is the toggles of one week's Supply run: shown, the Supply run // with those still pending laid over it, the items whose toggles will sync // once the server can be reached, the latest failed toggle no later toggle @@ -228,12 +232,13 @@ function reachable(network: NetworkStatus): Promise { // made on the Supply run screen (useToggles) or replayed from the queue saved // on this device (useSupplyRunSync). // -// The week's toggles are sent one at a time, in the order made (one -// mutation scope), so the counts each is answered with include every toggle -// before it and none after. While the server cannot be reached — the device -// is offline, or the toggle's PATCH failed with a network error (fetch -// rejected with a TypeError) — a toggle waits, pending, and is sent once it -// can be: those after it wait behind it. Any answer from the server settles +// Toggles are sent one at a time, in the order made, whatever their week +// (one mutation scope, TOGGLE_SCOPE), so the counts each is answered with +// include every toggle before it and none after, and a replayed toggle never +// overlaps another replayed or made here. While the server cannot be +// reached — the device is offline, or the toggle's PATCH failed with a +// network error (fetch rejected with a TypeError) — a toggle waits, pending, +// and is sent once it can be: those after it wait behind it. Any answer from the server settles // it: a refusal (a 400, 404, 409 or any other error response) or a response // that is not the item as toggled fails it, and it is not sent again. Before // a toggle is sent, GET /api/v1/me has answered, so it carries the session's @@ -250,7 +255,7 @@ function toggleOptions( const mutationKey = toggleKey(weekStart) return { mutationKey, - scope: { id: mutationKey.join(' ') }, + scope: { id: TOGGLE_SCOPE }, networkMode: 'always', retry: false, gcTime: Infinity, @@ -312,8 +317,9 @@ function toggleOptions( // the counts it answers with there. Until then the toggle is pending and is // laid over the Supply run shown, so the item and the needed count change at // once; a refused toggle just stops being pending, which puts both back. -// While the server cannot be reached, the items with pending toggles will -// sync. +// An item whose pending toggle was made, or waited, while the server could +// not be reached will sync: it says so until that toggle settles, even once +// the server can be reached again. // // The Supply run and the toggles are read straight from the query and // mutation caches as each render begins, never from the caches' batched @@ -339,6 +345,9 @@ export function useToggles(weekStart: string): Toggles { useMutationState({ filters: { mutationKey } }) const offline = useSyncExternalStore(api.network.subscribe, api.network.offline) const [, renderAgain] = useReducer((n: number) => n + 1, 0) + // The keys of the pending toggles seen while the server could not be + // reached: every change of the network status renders again. + const waited = useRef>(new Set()) const run = queryClient.getQueryData(key) if (!run) throw new Error(`The week of ${weekStart} has no Supply run read to toggle items of.`) const toggles = queryClient @@ -346,13 +355,14 @@ export function useToggles(weekStart: string): Toggles { .findAll({ mutationKey }) .map((mutation) => ({ status: mutation.state.status, toggle: mutation.state.variables as KeyedToggle, error: mutation.state.error })) const pending = toggles.filter((t) => t.status === 'pending').map((t) => t.toggle) + waited.current = new Set(pending.filter((t) => offline || waited.current.has(t.key)).map((t) => t.key)) const failed = toggles .filter((t, i) => t.status === 'error' && !toggles.slice(i + 1).some((later) => later.toggle.id === t.toggle.id)) .at(-1) const { mutate } = useMutation(toggleOptions(weekStart, api, queryClient)) return { shown: shown(run, pending), - willSync: new Set(offline ? pending.map((toggle) => toggle.id) : []), + willSync: new Set(pending.filter((t) => waited.current.has(t.key)).map((t) => t.id)), failed: failed === undefined ? null : { toggle: { id: failed.toggle.id, needed: failed.toggle.needed }, error: failed.error! }, toggle: ({ id, needed }) => { const queuedToggle = { key: crypto.randomUUID(), weekStart, id, needed } @@ -367,9 +377,18 @@ export function useToggles(weekStart: string): Toggles { // to open offline — the household's schedule and each Supply run the server // sends (saveRun) — and replays, in the order made, the toggles saved in the // queue that are not already pending here: those of a page reloaded before -// the server answered them. A replayed toggle's answer is written over the -// Supply run saved, so the week's Supply run is cached from it first; it -// is still read again (updatedAt 0) when the Supply run screen opens. +// the server answered them. +// +// Replay is at-least-once, not exactly-once: a toggle leaves the saved queue +// only once its answer settles here, so one the server applied just before a +// reload is sent again. That duplicate is safe because a toggle sets needed +// to a fixed value — the same PATCH applied twice leaves the item and the +// counts as applying it once does — so no key is sent for the server to +// deduplicate by. +// +// A replayed toggle's answer is written over the Supply run saved, so the +// week's Supply run is cached from it first; it is still read again +// (updatedAt 0) when the Supply run screen opens. export function useSupplyRunSync() { const api = useApi() const queryClient = useQueryClient() diff --git a/web/src/shopping/offlineQueue.test.tsx b/web/src/shopping/offlineQueue.test.tsx index 8ec514c..e711612 100644 --- a/web/src/shopping/offlineQueue.test.tsx +++ b/web/src/shopping/offlineQueue.test.tsx @@ -21,6 +21,18 @@ const ITEMS = [ { id: 'i3', name: 'Taco seasoning', qty: '1 packet', needed: false }, ] +// Next week's curry: one item to buy. +const NEXT_ITEMS = [{ id: 'n1', name: 'Chicken thighs', qty: '2 lb', needed: true }] + +// The items of each week, by its Monday. +const WEEKS: Record = { '2026-10-05': ITEMS, '2026-10-12': NEXT_ITEMS } + +function itemsOf(week: string) { + const items = WEEKS[week] + if (items === undefined) throw new Error(`no week ${week}`) + return items +} + // The browser's online flag; goOffline and goOnline change it and send the // page's window the event a browser sends. let onLine: boolean @@ -53,51 +65,78 @@ function goOnline() { act(() => void win.dispatchEvent(new Event('online'))) } -// stubServer is the server's Supply run of the week: a PATCH marks its item -// and answers with its counts, unless refuse answers it instead. It counts -// as archived once archive is called. -function stubServer(refuse: (id: string) => Response | undefined = () => undefined) { - const needed = new Map(ITEMS.map((item) => [item.id, item.needed])) +// stubServer is the server's Supply runs of this week and next: a PATCH +// marks its item and answers with its week's counts, unless refuse answers +// it instead; when hold is given, the item is marked at once but the answer +// waits for the promise hold returns. This week's counts as archived once +// archive is called. +function stubServer(refuse: (id: string) => Response | undefined = () => undefined, hold?: () => Promise) { + const needed = new Map(Object.values(WEEKS).flatMap((items) => items.map((item) => [item.id, item.needed] as const))) let status = 'current' - const counts = () => ({ needed_count: [...needed.values()].filter(Boolean).length, total_count: needed.size }) - const run = () => ({ - weekStart: '2026-10-05', - label: 'Week 41 · Oct 5 – 11', + // How many PATCHes are unanswered now, and the most there have been at once. + let inFlight = 0 + let mostInFlight = 0 + const counts = (week: string) => { + const marks = itemsOf(week).map((item) => needed.get(item.id)!) + return { needed_count: marks.filter(Boolean).length, total_count: marks.length } + } + const run = (week: string) => ({ + weekStart: week, + label: week === '2026-10-05' ? 'Week 41 · Oct 5 – 11' : 'Week 42 · Oct 12 – 18', title: null, - status, + status: week === '2026-10-05' ? status : 'current', sent_to_chat_at: null, - ...counts(), + ...counts(week), groups: [ { - dinner: { id: 'd0', name: 'Beef tacos' }, - date: '2026-10-05', - items: ITEMS.map((item) => ({ ...item, needed: needed.get(item.id)! })), + dinner: { id: week === '2026-10-05' ? 'd0' : 'd1', name: week === '2026-10-05' ? 'Beef tacos' : 'Chicken curry' }, + date: week, + items: itemsOf(week).map((item) => ({ ...item, needed: needed.get(item.id)! })), }, ], }) - const fetchMock = vi.fn((input, init) => { - if (!onLine || !reachable) return Promise.reject(new TypeError('Failed to fetch')) + const fetchMock = vi.fn(async (input, init) => { + if (!onLine || !reachable) throw new TypeError('Failed to fetch') const key = `${init?.method ?? 'GET'} ${String(input)}` - if (key === 'GET /api/v1/me') return Promise.resolve(jsonResponse(200, ME)) - if (key === 'GET /api/v1/settings/household') return Promise.resolve(jsonResponse(200, HOUSEHOLD_SETTINGS)) - if (key === `GET ${THIS_WEEK}`) return Promise.resolve(jsonResponse(200, run())) - const item = ITEMS.find((i) => key === `PATCH ${ITEM(i.id)}`) - if (item === undefined) throw new Error(`unexpected fetch: ${key}`) - const refusal = refuse(item.id) - if (refusal !== undefined) return Promise.resolve(refusal) + if (key === 'GET /api/v1/me') return jsonResponse(200, ME) + if (key === 'GET /api/v1/settings/household') return jsonResponse(200, HOUSEHOLD_SETTINGS) + if (key === `GET ${THIS_WEEK}`) return jsonResponse(200, run('2026-10-05')) + const week = Object.keys(WEEKS).find((w) => itemsOf(w).some((i) => key === `PATCH ${ITEM(i.id)}`)) + if (week === undefined) throw new Error(`unexpected fetch: ${key}`) + const id = itemsOf(week).find((i) => key === `PATCH ${ITEM(i.id)}`)!.id + const refusal = refuse(id) + if (refusal !== undefined) return refusal const body = JSON.parse(String(init?.body)) as { needed: boolean } - needed.set(item.id, body.needed) - return Promise.resolve(jsonResponse(200, { id: item.id, needed: body.needed, ...counts() })) + needed.set(id, body.needed) + const answer = jsonResponse(200, { id, needed: body.needed, ...counts(week) }) + inFlight++ + mostInFlight = Math.max(mostInFlight, inFlight) + await hold?.() + inFlight-- + return answer }) vi.stubGlobal('fetch', fetchMock) return { fetchMock, + run, + mostInFlight: () => mostInFlight, archive: () => { status = 'archived' }, } } +// held is a hold for stubServer whose answers wait until release is called, +// each one released in turn. +function held() { + const waiting: (() => void)[] = [] + return { + hold: () => new Promise((resolve) => waiting.push(resolve)), + waiting: () => waiting.length, + release: () => waiting.shift()!(), + } +} + // patches lists the PATCHes sent, as "path body". function patches(fetchMock: Mock) { return fetchMock.mock.calls.filter(([, init]) => init?.method === 'PATCH').map(([input, init]) => `${String(input)} ${String(init?.body)}`) @@ -145,6 +184,7 @@ beforeEach(() => { }) afterEach(() => { + vi.restoreAllMocks() vi.useRealTimers() vi.unstubAllGlobals() Reflect.deleteProperty(window, 'matchMedia') @@ -277,3 +317,94 @@ describe('offline toggle queue', () => { await waitFor(() => expect(patches(server.fetchMock)).toEqual([`${ITEM('i1')} {"needed":false}`])) }) }) + +describe('offline toggle queue, replay', () => { + it('replays a queue spanning two weeks strictly in order, one PATCH at a time, a toggle made meanwhile after it', async () => { + const answers = held() + const server = stubServer(undefined, answers.hold) + const queued = [ + { key: 'k1', weekStart: '2026-10-05', id: 'i3', needed: true }, + { key: 'k2', weekStart: '2026-10-12', id: 'n1', needed: false }, + { key: 'k3', weekStart: '2026-10-05', id: 'i1', needed: false }, + ] + const runs = { '2026-10-05': server.run('2026-10-05'), '2026-10-12': server.run('2026-10-12') } + localStorage.setItem(SAVED_KEY, JSON.stringify({ schedule: null, runs, queue: queued })) + + await openPage() + await waitFor(() => expect(answers.waiting()).toBe(1)) + fireEvent.click(await screen.findByRole('checkbox', { name: 'Corn tortillas' })) + for (let sent = 1; sent <= 4; sent++) { + await waitFor(() => expect(patches(server.fetchMock)).toHaveLength(sent)) + // The next one is not sent before this one is answered. + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(patches(server.fetchMock)).toHaveLength(sent) + act(() => answers.release()) + } + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(server.fetchMock)).toEqual([ + `${ITEM('i3')} {"needed":true}`, + `${ITEM('n1')} {"needed":false}`, + `${ITEM('i1')} {"needed":false}`, + `${ITEM('i2')} {"needed":false}`, + ]) + expect(server.mostInFlight()).toBe(1) + await waitFor(() => expect(screen.getByText('1 / 3')).toBeInTheDocument()) + }) + + it('replays at least once: a toggle the server applied before a reload is sent again and leaves the item and count the same', async () => { + const answers = held() + const server = stubServer(undefined, answers.hold) + const first = await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + fireEvent.click(checkbox('Taco seasoning')) + // The server applied it; the page reloads before its answer arrives. + await waitFor(() => expect(answers.waiting()).toBe(1)) + expect(queue()).toEqual(['i3 true']) + first.unmount() + act(() => answers.release()) + + await openPage() + await waitFor(() => expect(answers.waiting()).toBe(1)) + act(() => answers.release()) + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(server.fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`, `${ITEM('i3')} {"needed":true}`]) + await waitFor(() => expect(screen.getByText('3 / 3')).toBeInTheDocument()) + expect(checkbox('Taco seasoning')).toBeChecked() + expect(server.run('2026-10-05')).toMatchObject({ needed_count: 3, total_count: 3 }) + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('keeps "will sync" on a queued toggle until its delayed replay is answered', async () => { + const answers = held() + stubServer(undefined, answers.hold) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + goOffline() + fireEvent.click(checkbox('Ground beef')) + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + + goOnline() + await waitFor(() => expect(answers.waiting()).toBe(1)) + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(queue()).toEqual(['i1 false']) + + act(() => answers.release()) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(queue()).toEqual([]) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + }) + + it('says the saved list is unreadable when it is corrupt offline, without rendering again and again', async () => { + stubServer() + onLine = false + localStorage.setItem(SAVED_KEY, '{corrupt') + const getItem = vi.spyOn(Storage.prototype, 'getItem') + await openPage() + + expect(await screen.findByText(/Couldn't load the household settings/)).toBeInTheDocument() + expect(screen.getByText(/can't be kept on this device while offline/)).toBeInTheDocument() + const reads = getItem.mock.calls.filter(([key]) => key === SAVED_KEY).length + await new Promise((resolve) => setTimeout(resolve, 50)) + expect(getItem.mock.calls.filter(([key]) => key === SAVED_KEY)).toHaveLength(reads) + }) +}) -- 2.55.0 From 6d45f3ed0d4b914733ca56d318a548c74a8e27c6 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 11:07:21 -0400 Subject: [PATCH 3/4] Bound Supply run toggle tries, retry without an online event, keep "will sync" across navigation (Feature 17) - Each try of a toggle (GET /api/v1/me and the PATCH) has a 10 s deadline: past it the PATCH is aborted, the network marked failed, and the toggle stays pending at the head of the queue to be tried again. - A toggle waiting while the server cannot be reached is tried again when the network status says it can be, or after a wait doubling from 1 s up to 30 s, so recovery never depends on a browser online event. - The "will sync" keys live in a page-lifetime store in offline.ts, not in a component ref, so leaving and reopening the Supply run screen keeps the marker until the toggle settles. - ApiRequest takes an optional abort signal; the client sets no deadline. Co-Authored-By: Claude Opus 5.5 --- web/src/api/client.ts | 11 ++- web/src/shopping/api.ts | 99 +++++++++++++++----- web/src/shopping/offline.ts | 19 ++++ web/src/shopping/offlineQueue.test.tsx | 122 ++++++++++++++++++++++--- 4 files changed, 212 insertions(+), 39 deletions(-) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 1fc884d..754edaf 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -41,6 +41,9 @@ export type ApiRequest = { method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' // Sent as JSON. A GET must not have one. body?: unknown + // Aborts the request; fetch then rejects with the signal's reason. No + // request has a deadline unless its caller aborts it. + signal?: AbortSignal } // ApiRead is a GET's decoded response body, and whether the service worker @@ -78,7 +81,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s let forbidden = false const listeners = new Set<() => void>() - async function send(method: string, path: string, body: unknown): Promise { + async function send(method: string, path: string, body: unknown, signal: AbortSignal | undefined): Promise { const headers: Record = { Accept: 'application/json' } if (body !== undefined) headers['Content-Type'] = 'application/json' if (method !== 'GET') { @@ -97,7 +100,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s } let response: Response try { - response = await fetch(path, { method, headers, credentials: 'same-origin', body: json }) + response = await fetch(path, { method, headers, credentials: 'same-origin', body: json, signal }) } catch (error) { // fetch rejects with a TypeError when the network fails; an abort is a // DOMException and says nothing about the network. @@ -138,14 +141,14 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s if (method === 'GET' && request.body !== undefined) { throw new Error(`GET ${path}: a GET request cannot carry a body`) } - let response = await send(method, url, request.body) + let response = await send(method, url, request.body, request.signal) if (response.status === 403) { const error = await parseForbidden(method, path, response) // Only a non-GET carries the token, so only it can be refused for it. if (error.code !== 'csrf' || method === 'GET') throw error // The session's token changed: refresh it once and retry once. await refreshMe() - response = await send(method, url, request.body) + response = await send(method, url, request.body, request.signal) if (response.status === 403) throw await parseForbidden(method, path, response) } return { body: await decode(method, path, response), cached: response.headers.has(cacheFallbackHeader) } diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index e9b5430..fb3fbdb 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -7,14 +7,26 @@ import { type MutationObserverOptions, type QueryClient, } from '@tanstack/react-query' -import { useEffect, useReducer, useRef, useSyncExternalStore } from 'react' +import { useEffect, useReducer, useSyncExternalStore } from 'react' import { ApiError, useApi, type ApiClient } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { meQueryKey } from '../AuthBootstrap.tsx' import { oneOf } from '../dinners/api.ts' import type { NetworkStatus } from '../pwa/networkStatus.ts' import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts' -import { dequeue, enqueue, keep, queued, SAVED_KEY, saveRun, savedRun, saveSchedule } from './offline.ts' +import { + dequeue, + enqueue, + keep, + markWillSync, + queued, + SAVED_KEY, + saveRun, + savedRun, + saveSchedule, + synced, + willSync, +} from './offline.ts' export const SHOPPING_PATH = '/api/v1/shopping' @@ -215,19 +227,45 @@ export type Toggles = { toggle: (toggle: Toggle) => void } -// reachable resolves once network says the server can be reached: at once -// when it says so now. -function reachable(network: NetworkStatus): Promise { +// TOGGLE_DEADLINE is how long, in milliseconds, one try of a toggle — its +// GET /api/v1/me and its PATCH — may take before it counts as the server not +// answering. Only toggles have a deadline: the API client sets none. +export const TOGGLE_DEADLINE = 10_000 + +// RETRY_FIRST and RETRY_CEILING bound, in milliseconds, how long a toggle +// waits while the server cannot be reached before it is tried again anyway: +// the first wait, doubled after each failed try up to the ceiling. +export const RETRY_FIRST = 1_000 +export const RETRY_CEILING = 30_000 + +// reachable resolves once network says the server can be reached — at once +// when it says so now — or after delay milliseconds, whichever is first. The +// server coming back says nothing by itself, and the browser sends no online +// event when it never thought it was offline, so the toggle is tried again +// after delay whatever network says. +function reachable(network: NetworkStatus, delay: number): Promise { if (!network.offline()) return Promise.resolve() return new Promise((resolve) => { - const stop = network.subscribe(() => { - if (network.offline()) return + const done = () => { + clearTimeout(timer) stop() resolve() + } + const timer = setTimeout(done, delay) + const stop = network.subscribe(() => { + if (!network.offline()) done() }) }) } +// beforeDeadline is promise, rejected with signal's reason once it aborts. +function beforeDeadline(promise: Promise, signal: AbortSignal): Promise { + return new Promise((resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason as Error), { once: true }) + promise.then(resolve, reject) + }) +} + // toggleOptions is the mutation every toggle of the week's Supply run is, // made on the Supply run screen (useToggles) or replayed from the queue saved // on this device (useSupplyRunSync). @@ -237,10 +275,16 @@ function reachable(network: NetworkStatus): Promise { // include every toggle before it and none after, and a replayed toggle never // overlaps another replayed or made here. While the server cannot be // reached — the device is offline, or the toggle's PATCH failed with a -// network error (fetch rejected with a TypeError) — a toggle waits, pending, -// and is sent once it can be: those after it wait behind it. Any answer from the server settles -// it: a refusal (a 400, 404, 409 or any other error response) or a response -// that is not the item as toggled fails it, and it is not sent again. Before +// network error (fetch rejected with a TypeError), or a try ran past its +// deadline (TOGGLE_DEADLINE) — a toggle waits, pending, and is tried again +// once network says the server can be reached, or after a wait that doubles +// from RETRY_FIRST up to RETRY_CEILING, whichever is first: those after it +// wait behind it, in order. A try past its deadline is aborted and marks the +// network failed, as a network error does, so the item says it will sync; the +// server may have applied the aborted PATCH, and sending it again is safe for +// the reason a replay is (see useSupplyRunSync). Any answer from the server +// settles it: a refusal (a 400, 404, 409 or any other error response) or a +// response that is not the item as toggled fails it, and it is not sent again. Before // a toggle is sent, GET /api/v1/me has answered, so it carries the session's // CSRF token even when the app opened offline. // @@ -259,20 +303,30 @@ function toggleOptions( networkMode: 'always', retry: false, gcTime: Infinity, - mutationFn: async ({ id, needed }: KeyedToggle): Promise => { + mutationFn: async ({ key, id, needed }: KeyedToggle): Promise => { const path = `${SHOPPING_PATH}/items/${encodeURIComponent(id)}` const where = `PATCH ${path}` let body: unknown - for (;;) { - await reachable(api.network) + for (let failures = 0; ; failures++) { + if (api.network.offline()) markWillSync(key) + await reachable(api.network, Math.min(RETRY_FIRST * 2 ** failures, RETRY_CEILING)) + const deadline = new AbortController() + const timer = setTimeout(() => deadline.abort(new DOMException(`${where}: no answer in time`, 'TimeoutError')), TOGGLE_DEADLINE) try { - await queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) - body = await api.apiFetch(path, { method: 'PATCH', body: { needed } }) + const me = queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) + await beforeDeadline(me, deadline.signal) + body = await api.apiFetch(path, { method: 'PATCH', body: { needed }, signal: deadline.signal }) break } catch (error) { + if (deadline.signal.aborted) { + api.network.requestFailed() + continue + } // The API client marks the network offline as fetch rejects; a // TypeError it did not mark is not a network failure. if (!(error instanceof TypeError) || !api.network.offline()) throw error + } finally { + clearTimeout(timer) } } if (!isRecord(body) || body.id !== id || body.needed !== needed) throw new Error(`${where}: response is not item ${id} as toggled`) @@ -307,6 +361,7 @@ function toggleOptions( if ((earlier.state.variables as KeyedToggle).id === id) cache.remove(earlier) } keep(() => dequeue(settled)) + synced(settled) }, } } @@ -319,7 +374,8 @@ function toggleOptions( // once; a refused toggle just stops being pending, which puts both back. // An item whose pending toggle was made, or waited, while the server could // not be reached will sync: it says so until that toggle settles, even once -// the server can be reached again. +// the server can be reached again, and even when the screen is left and +// opened again meanwhile (markWillSync keeps it with the page, not here). // // The Supply run and the toggles are read straight from the query and // mutation caches as each render begins, never from the caches' batched @@ -345,9 +401,6 @@ export function useToggles(weekStart: string): Toggles { useMutationState({ filters: { mutationKey } }) const offline = useSyncExternalStore(api.network.subscribe, api.network.offline) const [, renderAgain] = useReducer((n: number) => n + 1, 0) - // The keys of the pending toggles seen while the server could not be - // reached: every change of the network status renders again. - const waited = useRef>(new Set()) const run = queryClient.getQueryData(key) if (!run) throw new Error(`The week of ${weekStart} has no Supply run read to toggle items of.`) const toggles = queryClient @@ -355,14 +408,16 @@ export function useToggles(weekStart: string): Toggles { .findAll({ mutationKey }) .map((mutation) => ({ status: mutation.state.status, toggle: mutation.state.variables as KeyedToggle, error: mutation.state.error })) const pending = toggles.filter((t) => t.status === 'pending').map((t) => t.toggle) - waited.current = new Set(pending.filter((t) => offline || waited.current.has(t.key)).map((t) => t.key)) + // Every change of the network status renders again, so each pending + // toggle seen while the server cannot be reached is marked. + if (offline) pending.forEach((t) => markWillSync(t.key)) const failed = toggles .filter((t, i) => t.status === 'error' && !toggles.slice(i + 1).some((later) => later.toggle.id === t.toggle.id)) .at(-1) const { mutate } = useMutation(toggleOptions(weekStart, api, queryClient)) return { shown: shown(run, pending), - willSync: new Set(pending.filter((t) => waited.current.has(t.key)).map((t) => t.id)), + willSync: new Set(pending.filter((t) => willSync(t.key)).map((t) => t.id)), failed: failed === undefined ? null : { toggle: { id: failed.toggle.id, needed: failed.toggle.needed }, error: failed.error! }, toggle: ({ id, needed }) => { const queuedToggle = { key: crypto.randomUUID(), weekStart, id, needed } diff --git a/web/src/shopping/offline.ts b/web/src/shopping/offline.ts index 6b1c323..055fb00 100644 --- a/web/src/shopping/offline.ts +++ b/web/src/shopping/offline.ts @@ -106,6 +106,25 @@ export function dequeue(key: string) { write((saved) => ({ ...saved, queue: saved.queue.filter((toggle) => toggle.key !== key) })) } +// The keys of the toggles that will sync: made, or waiting, while the server +// could not be reached, and not settled yet. They live here, with the page, +// as the toggles in the mutation cache do: the Supply run screen is left and +// opened again while a toggle is still pending, and starts with nothing of +// its own. +const syncing = new Set() + +export function markWillSync(key: string) { + syncing.add(key) +} + +export function willSync(key: string): boolean { + return syncing.has(key) +} + +export function synced(key: string) { + syncing.delete(key) +} + // The latest failure to read or write the saved Supply run, which the Supply // run screen shows: a toggle that cannot be kept may be lost on a reload. let problem: Error | null = null diff --git a/web/src/shopping/offlineQueue.test.tsx b/web/src/shopping/offlineQueue.test.tsx index e711612..e6f0f0c 100644 --- a/web/src/shopping/offlineQueue.test.tsx +++ b/web/src/shopping/offlineQueue.test.tsx @@ -7,7 +7,7 @@ import { createNetworkStatus, type NetworkWindow } from '../pwa/networkStatus.ts import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' import { mockViewport } from '../test/matchMedia.ts' -import { SupplyRunSync } from './api.ts' +import { RETRY_FIRST, SupplyRunSync, TOGGLE_DEADLINE } from './api.ts' import { SAVED_KEY } from './offline.ts' import { SupplyRunPage } from './SupplyRunPage.tsx' @@ -68,8 +68,8 @@ function goOnline() { // stubServer is the server's Supply runs of this week and next: a PATCH // marks its item and answers with its week's counts, unless refuse answers // it instead; when hold is given, the item is marked at once but the answer -// waits for the promise hold returns. This week's counts as archived once -// archive is called. +// waits for the promise hold returns, or until the request is aborted. This +// week's counts as archived once archive is called. function stubServer(refuse: (id: string) => Response | undefined = () => undefined, hold?: () => Promise) { const needed = new Map(Object.values(WEEKS).flatMap((items) => items.map((item) => [item.id, item.needed] as const))) let status = 'current' @@ -111,8 +111,14 @@ function stubServer(refuse: (id: string) => Response | undefined = () => undefin const answer = jsonResponse(200, { id, needed: body.needed, ...counts(week) }) inFlight++ mostInFlight = Math.max(mostInFlight, inFlight) - await hold?.() - inFlight-- + try { + await new Promise((resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal!.reason as Error)) + void (hold?.() ?? Promise.resolve()).then(resolve) + }) + } finally { + inFlight-- + } return answer }) vi.stubGlobal('fetch', fetchMock) @@ -151,20 +157,35 @@ function queue(): string[] { // openPage opens the Supply run screen in a new window, as the app does: // with SupplyRunSync, and with /me loaded unless the server cannot be -// reached. +// reached. leave goes to another screen of the same page, and reopen comes +// back to the Supply run screen. async function openPage() { const api = createApiClient(fakeLocation('/plan/shopping'), createNetworkStatus(openWindow()), createVerifiedSession(localStorage)) if (onLine && reachable) await api.loadMe() - return render( + const queryClient = createQueryClient() + const app = (onScreen: boolean) => ( - + - + {onScreen && } - , + ) + const view = render(app(true)) + return { ...view, leave: () => view.rerender(app(false)), reopen: () => view.rerender(app(true)) } +} + +// reload ends the page open and its timers, as a reload does: a toggle it +// was waiting to try again is never tried, by it, again. +function reload(page: { unmount: () => void }) { + page.unmount() + vi.clearAllTimers() +} + +async function advance(ms: number) { + await act(() => vi.advanceTimersByTimeAsync(ms)) } function checkbox(name: string) { @@ -179,7 +200,9 @@ beforeEach(() => { onLine = true reachable = true mockViewport(390) - vi.useFakeTimers({ toFake: ['Date'] }) + // setTimeout moves with real time, and a test can jump past a deadline or + // a wait (advance). + vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'], shouldAdvanceTime: true }) vi.setSystemTime(new Date('2026-10-07T16:00:00Z')) }) @@ -259,7 +282,7 @@ describe('offline toggle queue', () => { expect(queue()).toEqual(['i3 true', 'i1 false']) // Reload while offline: the settings, /me and the list cannot be read. - first.unmount() + reload(first) await openPage() expect(await screen.findByText('2 / 3')).toBeInTheDocument() expect(checkbox('Taco seasoning')).toBeChecked() @@ -360,7 +383,7 @@ describe('offline toggle queue, replay', () => { // The server applied it; the page reloads before its answer arrives. await waitFor(() => expect(answers.waiting()).toBe(1)) expect(queue()).toEqual(['i3 true']) - first.unmount() + reload(first) act(() => answers.release()) await openPage() @@ -394,6 +417,79 @@ describe('offline toggle queue, replay', () => { expect(screen.getByText('1 / 3')).toBeInTheDocument() }) + it('aborts a PATCH that never answers at its deadline, keeps it pending and tries it again before the toggles after it', async () => { + let tries = 0 + // The first PATCH never answers; every later one answers at once. + const server = stubServer(undefined, () => (++tries === 1 ? new Promise(() => {}) : Promise.resolve())) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + fireEvent.click(checkbox('Taco seasoning')) + await waitFor(() => expect(patches(server.fetchMock)).toHaveLength(1)) + fireEvent.click(checkbox('Ground beef')) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + // Past the deadline the hung PATCH is aborted; both still wait, in order. + await advance(TOGGLE_DEADLINE) + expect(await within(row('Taco seasoning')).findByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(patches(server.fetchMock)).toHaveLength(1) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(server.fetchMock)).toEqual([ + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i1')} {"needed":false}`, + ]) + expect(server.mostInFlight()).toBe(1) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('tries a toggle again once the server is back, with no online event from the browser', async () => { + const { fetchMock } = stubServer() + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + // The browser says online throughout; the server cannot be reached, then can. + reachable = false + fireEvent.click(checkbox('Corn tortillas')) + expect(await within(row('Corn tortillas')).findByText('will sync')).toBeInTheDocument() + reachable = true + + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + // The first try failed at GET /api/v1/me, before its PATCH. + expect(patches(fetchMock)).toEqual([`${ITEM('i2')} {"needed":false}`]) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + }) + + it('keeps "will sync" when the Supply run screen is left and opened again while the replay is pending', async () => { + const answers = held() + stubServer(undefined, answers.hold) + const page = await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + goOffline() + fireEvent.click(checkbox('Ground beef')) + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + + page.leave() + goOnline() + await waitFor(() => expect(answers.waiting()).toBe(1)) + page.reopen() + expect(await within(row('Ground beef')).findByText('will sync')).toBeInTheDocument() + expect(queue()).toEqual(['i1 false']) + + act(() => answers.release()) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(queue()).toEqual([]) + expect(screen.getByText('1 / 3')).toBeInTheDocument() + }) + it('says the saved list is unreadable when it is corrupt offline, without rendering again and again', async () => { stubServer() onLine = false -- 2.55.0 From 54b59965ed18ce1d33ca74be512b2eaed1169a4a Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 11:27:18 -0400 Subject: [PATCH 4/4] Bound a Supply run toggle's session load and CSRF refresh by its deadline (Feature 17) - loadMe takes an optional abort signal; AuthBootstrap calls it with none. - Each toggle try loads GET /api/v1/me itself under its deadline when the session is not loaded, instead of joining a /me query already in flight that may never answer. - The /me refresh a csrf 403 makes is bounded by the request's signal: a request whose signal aborts stops waiting at once and the refresh is no longer shared, so the next csrf 403 starts a fresh /me. Requests without a signal still share it, and it is aborted only once no request waits. - Tests: a hung /me and a hung csrf refresh during a toggle exchange, both recovered on the next try in order; the client's refresh sharing and abort. Co-Authored-By: Claude Opus 5.5 --- web/src/AuthBootstrap.tsx | 2 +- web/src/api/client.test.ts | 72 ++++++++++++++++ web/src/api/client.ts | 67 ++++++++++++--- web/src/shopping/api.ts | 20 ++--- web/src/shopping/offlineQueue.test.tsx | 112 +++++++++++++++++++++++-- 5 files changed, 239 insertions(+), 34 deletions(-) diff --git a/web/src/AuthBootstrap.tsx b/web/src/AuthBootstrap.tsx index 774ca5c..c3d9522 100644 --- a/web/src/AuthBootstrap.tsx +++ b/web/src/AuthBootstrap.tsx @@ -20,7 +20,7 @@ export const meQueryKey = [ME_PATH] as const export function AuthBootstrap({ children }: { children: ReactNode }) { const api = useApi() const forbidden = useSyncExternalStore(api.subscribe, api.forbidden) - const me = useQuery({ queryKey: meQueryKey, queryFn: api.loadMe, staleTime: Infinity, networkMode: 'always' }) + const me = useQuery({ queryKey: meQueryKey, queryFn: () => api.loadMe(), staleTime: Infinity, networkMode: 'always' }) // fetch rejects with a TypeError only when the network fails. const savedPages = me.isError && me.error instanceof TypeError && api.session.verified() const { refetch } = me diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 15390e7..8155382 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -125,6 +125,66 @@ describe('apiFetch', () => { expect(api.forbidden()).toBe(false) }) + it('shares a csrf refresh between requests, but one whose signal aborts stops waiting and the next refreshes afresh', async () => { + // The first refresh answers only when answerFirst is called; every other + // request answers at once. + let answerFirst!: (response: Response) => void + const mes: (AbortSignal | null | undefined)[] = [] + const fetchMock = vi.fn((input, init) => { + if (String(input) === '/api/v1/me') { + mes.push(init?.signal) + if (mes.length === 2) return new Promise((resolve) => (answerFirst = resolve)) + return Promise.resolve(jsonResponse(200, { ...ME, csrfToken: `token-${mes.length}` })) + } + const token = new Headers(init?.headers).get(CSRF_HEADER) + return Promise.resolve(token === 'token-1' ? errorResponse(403, 'csrf') : jsonResponse(201, { token })) + }) + vi.stubGlobal('fetch', fetchMock) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + + const unbounded = api.apiFetch('/api/v1/jobs', { method: 'POST', body: {} }) + const timedOut = new AbortController() + const bounded = api.apiFetch('/api/v1/jobs', { method: 'POST', body: {}, signal: timedOut.signal }) + await vi.waitFor(() => expect(mes).toHaveLength(2)) + timedOut.abort(new DOMException('no answer in time', 'TimeoutError')) + await expect(bounded).rejects.toMatchObject({ name: 'TimeoutError' }) + // A request without a signal still waits for it, so it is not aborted. + expect(mes[1]!.aborted).toBe(false) + + // A later csrf 403 does not wait on the refresh still in flight. + await expect( + api.apiFetch('/api/v1/jobs', { method: 'POST', body: {}, signal: new AbortController().signal }), + ).resolves.toEqual({ token: 'token-3' }) + expect(mes).toHaveLength(3) + + answerFirst(jsonResponse(200, { ...ME, csrfToken: 'token-2' })) + await expect(unbounded).resolves.toEqual({ token: 'token-2' }) + expect(mes).toHaveLength(3) + expect(fetchMock.mock.calls.filter(([, init]) => init?.method === 'POST')).toHaveLength(5) + }) + + it('aborts a csrf refresh once the only request waiting for it aborts, without counting it as offline', async () => { + const mes: AbortSignal[] = [] + const fetchMock = vi.fn((input, init) => { + if (String(input) !== '/api/v1/me') return Promise.resolve(errorResponse(403, 'csrf')) + if (mes.push(init!.signal!) === 1) return Promise.resolve(jsonResponse(200, ME)) + return new Promise((_, reject) => init!.signal!.addEventListener('abort', () => reject(init!.signal!.reason as Error))) + }) + vi.stubGlobal('fetch', fetchMock) + const network = createNetworkStatus(window) + const api = createApiClient(fakeLocation(), network, createVerifiedSession(localStorage)) + await api.loadMe() + + const timedOut = new AbortController() + const bounded = api.apiFetch('/api/v1/jobs', { method: 'POST', body: {}, signal: timedOut.signal }) + await vi.waitFor(() => expect(mes).toHaveLength(2)) + timedOut.abort(new DOMException('no answer in time', 'TimeoutError')) + await expect(bounded).rejects.toMatchObject({ name: 'TimeoutError' }) + expect(mes[1]!.aborted).toBe(true) + expect(network.offline()).toBe(false) + }) + it('marks any other 403 as not in this household and notifies subscribers', async () => { const fetchMock = stubFetch(errorResponse(403, 'forbidden')) const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) @@ -153,6 +213,18 @@ describe('apiFetch', () => { }) describe('loadMe', () => { + it('aborts /me with its signal', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((_, init) => new Promise((_, reject) => init!.signal!.addEventListener('abort', () => reject(init!.signal!.reason as Error)))), + ) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + const timedOut = new AbortController() + const me = api.loadMe(timedOut.signal) + timedOut.abort(new DOMException('no answer in time', 'TimeoutError')) + await expect(me).rejects.toMatchObject({ name: 'TimeoutError' }) + }) + it('rejects a /me response without a CSRF token', async () => { stubFetch(jsonResponse(200, { ...ME, csrfToken: '' })) const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 754edaf..ae6821b 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -41,8 +41,9 @@ export type ApiRequest = { method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' // Sent as JSON. A GET must not have one. body?: unknown - // Aborts the request; fetch then rejects with the signal's reason. No - // request has a deadline unless its caller aborts it. + // Aborts the request, including the /me refresh a csrf 403 makes it wait + // for; it then rejects with the signal's reason. No request has a deadline + // unless its caller aborts it. signal?: AbortSignal } @@ -58,8 +59,9 @@ export type ApiClient = { // apiRead is apiFetch's GET that also says whether the body is the service // worker's cached copy rather than the server's answer. apiRead(path: string): Promise> - // loadMe fetches GET /api/v1/me and caches its CSRF token. - loadMe(): Promise + // loadMe fetches GET /api/v1/me and caches its CSRF token; signal aborts + // it, as it does an apiFetch. + loadMe(signal?: AbortSignal): Promise // forbidden reports whether a request was refused because the signed-in // account is not in this household; subscribe is notified when it changes. forbidden(): boolean @@ -73,11 +75,16 @@ export type ApiClient = { session: VerifiedSession } +// Refresh is the /me request refreshMe shares: controller aborts it, waiting +// counts the requests with a signal still waiting for it, and unbounded says +// whether one without a signal is. +type Refresh = { me: Promise; controller: AbortController; waiting: number; unbounded: boolean } + // createApiClient builds the app's API client. location is window.location in // the app: a 401 sends it to /auth/login with the current path to return to. export function createApiClient(location: ApiLocation, network: NetworkStatus, session: VerifiedSession): ApiClient { let csrfToken: string | null = null - let refreshing: Promise | null = null + let refreshing: Refresh | null = null let forbidden = false const listeners = new Set<() => void>() @@ -115,12 +122,46 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s } // refreshMe shares one in-flight /me request between every request that - // got a csrf 403 at the same time. - function refreshMe(): Promise { - refreshing ??= loadMe().finally(() => { - refreshing = null + // got a csrf 403 at the same time. A request whose signal aborts stops + // waiting for it at once, and it is no longer shared: the next csrf 403 + // starts a fresh /me rather than wait on one that may never answer. Once + // every request with a signal waiting for it has stopped, and none without + // one waits, it is aborted. + function refreshMe(signal: AbortSignal | undefined): Promise { + if (refreshing === null) { + const controller = new AbortController() + const refresh: Refresh = { me: loadMe(controller.signal), controller, waiting: 0, unbounded: false } + const done = () => { + if (refreshing === refresh) refreshing = null + } + refresh.me.then(done, done) + refreshing = refresh + } + const refresh = refreshing + if (signal === undefined) { + refresh.unbounded = true + return refresh.me + } + refresh.waiting++ + return new Promise((resolve, reject) => { + const stop = () => { + if (refreshing === refresh) refreshing = null + if (--refresh.waiting === 0 && !refresh.unbounded) refresh.controller.abort(signal.reason) + reject(signal.reason as Error) + } + if (signal.aborted) return stop() + signal.addEventListener('abort', stop, { once: true }) + refresh.me.then( + (me) => { + signal.removeEventListener('abort', stop) + resolve(me) + }, + (error: unknown) => { + signal.removeEventListener('abort', stop) + reject(error as Error) + }, + ) }) - return refreshing } async function apiFetch(path: string, request: ApiRequest = {}): Promise { @@ -147,7 +188,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s // Only a non-GET carries the token, so only it can be refused for it. if (error.code !== 'csrf' || method === 'GET') throw error // The session's token changed: refresh it once and retry once. - await refreshMe() + await refreshMe(request.signal) response = await send(method, url, request.body, request.signal) if (response.status === 403) throw await parseForbidden(method, path, response) } @@ -179,8 +220,8 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s return (await response.json()) as T } - async function loadMe(): Promise { - const me = await apiFetch(ME_PATH) + async function loadMe(signal?: AbortSignal): Promise { + const me = await apiFetch(ME_PATH, { signal }) if ( !isRecord(me) || typeof me.name !== 'string' || diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index fb3fbdb..8ea8afc 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -228,8 +228,9 @@ export type Toggles = { } // TOGGLE_DEADLINE is how long, in milliseconds, one try of a toggle — its -// GET /api/v1/me and its PATCH — may take before it counts as the server not -// answering. Only toggles have a deadline: the API client sets none. +// GET /api/v1/me, its PATCH and the /me refresh a csrf 403 to it makes — may +// take before it counts as the server not answering. Only toggles have a +// deadline: the API client sets none. export const TOGGLE_DEADLINE = 10_000 // RETRY_FIRST and RETRY_CEILING bound, in milliseconds, how long a toggle @@ -258,14 +259,6 @@ function reachable(network: NetworkStatus, delay: number): Promise { }) } -// beforeDeadline is promise, rejected with signal's reason once it aborts. -function beforeDeadline(promise: Promise, signal: AbortSignal): Promise { - return new Promise((resolve, reject) => { - signal.addEventListener('abort', () => reject(signal.reason as Error), { once: true }) - promise.then(resolve, reject) - }) -} - // toggleOptions is the mutation every toggle of the week's Supply run is, // made on the Supply run screen (useToggles) or replayed from the queue saved // on this device (useSupplyRunSync). @@ -313,8 +306,11 @@ function toggleOptions( const deadline = new AbortController() const timer = setTimeout(() => deadline.abort(new DOMException(`${where}: no answer in time`, 'TimeoutError')), TOGGLE_DEADLINE) try { - const me = queryClient.ensureQueryData({ queryKey: meQueryKey, queryFn: api.loadMe, networkMode: 'always' }) - await beforeDeadline(me, deadline.signal) + // Each try loads /me itself, under its deadline, rather than share + // a /me already in flight that may never answer. + if (queryClient.getQueryData(meQueryKey) === undefined) { + queryClient.setQueryData(meQueryKey, await api.loadMe(deadline.signal)) + } body = await api.apiFetch(path, { method: 'PATCH', body: { needed }, signal: deadline.signal }) break } catch (error) { diff --git a/web/src/shopping/offlineQueue.test.tsx b/web/src/shopping/offlineQueue.test.tsx index e6f0f0c..2f64ec5 100644 --- a/web/src/shopping/offlineQueue.test.tsx +++ b/web/src/shopping/offlineQueue.test.tsx @@ -68,14 +68,21 @@ function goOnline() { // stubServer is the server's Supply runs of this week and next: a PATCH // marks its item and answers with its week's counts, unless refuse answers // it instead; when hold is given, the item is marked at once but the answer -// waits for the promise hold returns, or until the request is aborted. This -// week's counts as archived once archive is called. -function stubServer(refuse: (id: string) => Response | undefined = () => undefined, hold?: () => Promise) { +// waits for the promise hold returns, or until the request is aborted; holdMe +// holds GET /api/v1/me's answer the same way. This week's counts as archived +// once archive is called. +function stubServer( + refuse: (id: string) => Response | undefined = () => undefined, + hold?: () => Promise, + holdMe?: () => Promise, +) { const needed = new Map(Object.values(WEEKS).flatMap((items) => items.map((item) => [item.id, item.needed] as const))) let status = 'current' // How many PATCHes are unanswered now, and the most there have been at once. let inFlight = 0 let mostInFlight = 0 + // How many GET /api/v1/me requests have been answered. + let meAnswered = 0 const counts = (week: string) => { const marks = itemsOf(week).map((item) => needed.get(item.id)!) return { needed_count: marks.filter(Boolean).length, total_count: marks.length } @@ -98,7 +105,11 @@ function stubServer(refuse: (id: string) => Response | undefined = () => undefin const fetchMock = vi.fn(async (input, init) => { if (!onLine || !reachable) throw new TypeError('Failed to fetch') const key = `${init?.method ?? 'GET'} ${String(input)}` - if (key === 'GET /api/v1/me') return jsonResponse(200, ME) + if (key === 'GET /api/v1/me') { + await answerAfter(init?.signal, holdMe) + meAnswered++ + return jsonResponse(200, ME) + } if (key === 'GET /api/v1/settings/household') return jsonResponse(200, HOUSEHOLD_SETTINGS) if (key === `GET ${THIS_WEEK}`) return jsonResponse(200, run('2026-10-05')) const week = Object.keys(WEEKS).find((w) => itemsOf(w).some((i) => key === `PATCH ${ITEM(i.id)}`)) @@ -112,10 +123,7 @@ function stubServer(refuse: (id: string) => Response | undefined = () => undefin inFlight++ mostInFlight = Math.max(mostInFlight, inFlight) try { - await new Promise((resolve, reject) => { - init?.signal?.addEventListener('abort', () => reject(init.signal!.reason as Error)) - void (hold?.() ?? Promise.resolve()).then(resolve) - }) + await answerAfter(init?.signal, hold) } finally { inFlight-- } @@ -126,12 +134,22 @@ function stubServer(refuse: (id: string) => Response | undefined = () => undefin fetchMock, run, mostInFlight: () => mostInFlight, + meAnswered: () => meAnswered, archive: () => { status = 'archived' }, } } +// answerAfter resolves once the promise hold returns does, at once without +// hold, or rejects with signal's reason once it aborts. +function answerAfter(signal: AbortSignal | null | undefined, hold?: () => Promise) { + return new Promise((resolve, reject) => { + signal?.addEventListener('abort', () => reject(signal.reason as Error)) + void (hold?.() ?? Promise.resolve()).then(resolve) + }) +} + // held is a hold for stubServer whose answers wait until release is called, // each one released in turn. function held() { @@ -143,6 +161,11 @@ function held() { } } +// mes counts the GET /api/v1/me requests sent. +function mes(fetchMock: Mock) { + return fetchMock.mock.calls.filter(([input, init]) => String(input) === '/api/v1/me' && (init?.method ?? 'GET') === 'GET').length +} + // patches lists the PATCHes sent, as "path body". function patches(fetchMock: Mock) { return fetchMock.mock.calls.filter(([, init]) => init?.method === 'PATCH').map(([input, init]) => `${String(input)} ${String(init?.body)}`) @@ -449,6 +472,79 @@ describe('offline toggle queue, replay', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) + it('abandons a GET /api/v1/me that never answers at its deadline and loads it afresh before the toggles after it', async () => { + let tries = 0 + // The page loaded /me; the first toggle loads it again, and that /me never + // answers. Every later /me answers at once. + const server = stubServer(undefined, undefined, () => (++tries === 2 ? new Promise(() => {}) : Promise.resolve())) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + fireEvent.click(checkbox('Taco seasoning')) + await waitFor(() => expect(tries).toBe(2)) + fireEvent.click(checkbox('Ground beef')) + expect(queue()).toEqual(['i3 true', 'i1 false']) + + // Past the deadline the hung /me is abandoned; both still wait, in order. + await advance(TOGGLE_DEADLINE) + expect(await within(row('Taco seasoning')).findByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(patches(server.fetchMock)).toEqual([]) + + // The next try sends a fresh /me, which the server answers. + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + expect(mes(server.fetchMock)).toBe(3) + expect(patches(server.fetchMock)).toEqual([`${ITEM('i3')} {"needed":true}`, `${ITEM('i1')} {"needed":false}`]) + expect(server.mostInFlight()).toBe(1) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('abandons the /me refresh of a csrf 403 that never answers at its deadline and refreshes afresh on the next try', async () => { + let tries = 0 + // The session's token changed: each PATCH is refused for it until a /me + // after the first refusal has answered. The first such refresh never + // answers. + let refusedAt: number | null = null + const server = stubServer( + () => { + if (refusedAt !== null && server.meAnswered() > refusedAt) return undefined + refusedAt ??= server.meAnswered() + return errorResponse(403, 'csrf') + }, + undefined, + () => (refusedAt !== null && ++tries === 1 ? new Promise(() => {}) : Promise.resolve()), + ) + await openPage() + expect(await screen.findByText('2 / 3')).toBeInTheDocument() + + fireEvent.click(checkbox('Taco seasoning')) + await waitFor(() => expect(tries).toBe(1)) + fireEvent.click(checkbox('Ground beef')) + expect(patches(server.fetchMock)).toHaveLength(1) + + // Past the deadline the hung refresh is abandoned; both still wait, in order. + await advance(TOGGLE_DEADLINE) + expect(await within(row('Taco seasoning')).findByText('will sync')).toBeInTheDocument() + expect(within(row('Ground beef')).getByText('will sync')).toBeInTheDocument() + expect(queue()).toEqual(['i3 true', 'i1 false']) + + // The next try is refused again and sends a fresh refresh, which answers. + await advance(RETRY_FIRST) + await waitFor(() => expect(queue()).toEqual([])) + expect(patches(server.fetchMock)).toEqual([ + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i3')} {"needed":true}`, + `${ITEM('i1')} {"needed":false}`, + ]) + await waitFor(() => expect(screen.queryByText('will sync')).not.toBeInTheDocument()) + expect(screen.getByText('2 / 3')).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + it('tries a toggle again once the server is back, with no online event from the browser', async () => { const { fetchMock } = stubServer() await openPage() -- 2.55.0