From 4ba647107077f5730f490aa00bf2e1bc261ad6ee Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 10:20:23 -0400 Subject: [PATCH] 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) + }) +})