From 8a02e04110b149ffff7eb2e912c63a1b3e459ab4 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 01:45:33 -0400 Subject: [PATCH] Keep Push closed over a cached plan; bound kept week mutations A plan_changed reread answered from the service worker's cache left the old plan shown with Push and Retry open. Push now needs a plan read from the server (Plan.cached, set from cacheFallbackHeader), and the push failure's Retry reads the plan again while the plan shown is cached. Settled swaps and pushes were kept for every week ever touched. When a week is shown, the settled ones of every other week with none pending are now removed; pending ones still survive leaving the week and coming back. Co-Authored-By: Claude Opus 5.5 --- web/src/plan/PlanPage.test.tsx | 51 +++++++++++++++++++++++++ web/src/plan/PlanPage.tsx | 21 ++++++++--- web/src/plan/api.test.tsx | 69 +++++++++++++++++++++++++++++++++- web/src/plan/api.ts | 42 ++++++++++++++++++--- 4 files changed, 171 insertions(+), 12 deletions(-) diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index a63e447..f3d73bf 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -467,6 +467,57 @@ describe('PlanPage', () => { expect(posts(fetchMock)).toEqual([`${PUSH} undefined`, `${PUSH} undefined`]) }) + it("pushes nothing again while the changed plan's read is answered from the cache", async () => { + let read = () => jsonResponse(200, planBody('draft')) + let push = () => errorResponse(409, 'plan_changed') + const fetchMock = stubServer({ + [`GET ${THIS_WEEK}`]: () => read(), + [`POST ${PUSH}`]: () => push(), + }) + await renderPage() + + await screen.findByText('Beef tacos') + // The server can't be reached: the service worker answers the read of + // the changed plan with the plan it cached before. + read = () => { + const response = jsonResponse(200, planBody('draft')) + response.headers.set(cacheFallbackHeader, '1') + return response + } + fireEvent.click(screen.getByRole('button', { name: 'Looks good — push to Calendar' })) + const cachedCopy = "The plan changed while it was being pushed. The plan shown came from this device's cache, not the server; read it again before pushing." + const failed = await screen.findByRole('alert') + expect(planReads(fetchMock)).toBe(2) + expect(screen.getByText('Beef tacos')).toBeInTheDocument() + const pushButton = screen.getByRole('button', { name: 'Looks good — push to Calendar' }) + expect(pushButton).toBeDisabled() + fireEvent.click(pushButton) + expect(failed).toHaveTextContent(cachedCopy) + + // Retry reads the plan again rather than pushing: still the cache, so + // nothing is pushed. + fireEvent.click(within(screen.getByRole('alert')).getByRole('button', { name: 'Retry' })) + await waitFor(() => expect(planReads(fetchMock)).toBe(3)) + expect(await screen.findByRole('alert')).toHaveTextContent(cachedCopy) + expect(screen.getByRole('button', { name: 'Looks good — push to Calendar' })).toBeDisabled() + expect(posts(fetchMock)).toEqual([`${PUSH} undefined`]) + + // The server is back: Retry shows the changed plan, and only then may it + // be pushed. + read = () => jsonResponse(200, planBody('draft', ['Lasagna', ...NAMES.slice(1)])) + fireEvent.click(within(screen.getByRole('alert')).getByRole('button', { name: 'Retry' })) + expect(await screen.findByText('Lasagna')).toBeInTheDocument() + const alert = screen.getByRole('alert') + expect(alert).toHaveTextContent("Couldn't push to Calendar. The plan changed while it was being pushed; push it again.") + expect(screen.getByRole('button', { name: 'Looks good — push to Calendar' })).toBeEnabled() + expect(posts(fetchMock)).toEqual([`${PUSH} undefined`]) + + push = () => jsonResponse(200, syncedNow()) + fireEvent.click(within(alert).getByRole('button', { name: 'Retry' })) + expect(await screen.findByText('Synced to Google Calendar just now')).toBeInTheDocument() + expect(posts(fetchMock)).toEqual([`${PUSH} undefined`, `${PUSH} undefined`]) + }) + it('shows the changed plan read while the week was left, and pushes it', async () => { let read: () => Response | Promise = () => jsonResponse(200, planBody('draft')) let push = () => errorResponse(409, 'plan_changed') diff --git a/web/src/plan/PlanPage.tsx b/web/src/plan/PlanPage.tsx index 62e42cc..4cf8667 100644 --- a/web/src/plan/PlanPage.tsx +++ b/web/src/plan/PlanPage.tsx @@ -303,8 +303,11 @@ function WeekBoard({ // A push sends the plan shown: it waits for a draft or a swap of the week // to settle, and neither starts while it runs, even across leaving the - // week and coming back. - const canPush = plan.isSuccess && plan.data !== null && !drafting && !swap.pending && !pushing + // week and coming back. Nor does it send a plan from this device's cache, + // which may not be the server's: after a push refused because the plan + // changed, the read of the changed plan may be answered from the cache. + const cached = plan.isSuccess && plan.data !== null && plan.data.cached + const canPush = plan.isSuccess && plan.data !== null && !cached && !drafting && !swap.pending && !pushing function pushToCalendar() { if (!canPush) return swap.dismiss() @@ -339,7 +342,7 @@ function WeekBoard({ } const failure = push.error !== null ? ( - + void plan.refetch() : pushToCalendar} /> ) : swap.failed !== null ? ( swapIn(swap.failed!.swap)}> Couldn't swap {weekdayName(swap.failed.swap.date)}'s dinner. {swap.failed.error.message} @@ -569,8 +572,10 @@ function SyncStatus({ plan, className }: { plan: Plan; className?: string }) { } // PushFailure says why Push to Calendar failed. A failure only the Google -// Calendar settings can fix links there; any other offers to push again. -function PushFailure({ error, onRetry }: { error: Error; onRetry: () => void }) { +// Calendar settings can fix links there; any other offers to push again, +// or, while the plan shown is this device's cached copy, to read it again +// from the server first. +function PushFailure({ error, cached, onRetry }: { error: Error; cached: boolean; onRetry: () => void }) { const code = error instanceof ApiError ? error.code : null const settings = code !== null && Object.hasOwn(settingsFailures, code) ? settingsFailures[code] : undefined if (settings !== undefined) { @@ -586,7 +591,11 @@ function PushFailure({ error, onRetry }: { error: Error; onRetry: () => void }) return ( Couldn't push to Calendar.{' '} - {code === 'plan_changed' ? 'The plan changed while it was being pushed; push it again.' : error.message} + {cached + ? `${code === 'plan_changed' ? 'The plan changed while it was being pushed.' : error.message} The plan shown came from this device's cache, not the server; read it again before pushing.` + : code === 'plan_changed' + ? 'The plan changed while it was being pushed; push it again.' + : error.message} ) } diff --git a/web/src/plan/api.test.tsx b/web/src/plan/api.test.tsx index 438c2de..138fe65 100644 --- a/web/src/plan/api.test.tsx +++ b/web/src/plan/api.test.tsx @@ -5,7 +5,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' -import { planKey, usePlan, useSwapDay, type Plan } from './api.ts' +import { planKey, usePlan, usePushToCalendar, useSwapDay, type Plan } from './api.ts' const WEEK = '2026-10-05' const PLAN = `/api/v1/plans/${WEEK}` @@ -203,3 +203,70 @@ describe('useSwapDay', () => { await act(async () => friday.send(jsonResponse(200, planBody()))) }) }) + +describe('week mutations', () => { + const NEXT = '2026-10-12' + const LATER = '2026-10-19' + + // weekMutations lists the swaps and pushes the mutation cache holds for + // the week starting on weekStart, with their status. + function weekMutations(queryClient: QueryClient, weekStart: string) { + return queryClient + .getMutationCache() + .findAll({ mutationKey: ['plans', weekStart] }) + .map((mutation) => `${String(mutation.options.mutationKey![2])} ${mutation.state.status}`) + } + + it('keeps settled swaps and pushes while their week is shown, and leaves them behind once it is not, unless one is pending', async () => { + const nextPush = deferred() + stubServer({ + [`GET ${PLAN}`]: () => jsonResponse(200, planBody()), + [`PUT ${FRIDAY}`]: () => jsonResponse(200, planBody({ 4: { id: 'q1', name: 'Lemon orzo', is_new: true } })), + [`POST ${PLAN}/push-calendar`]: () => errorResponse(500, 'internal'), + [`GET /api/v1/plans/${NEXT}`]: () => errorResponse(404, 'not_found'), + [`POST /api/v1/plans/${NEXT}/push-calendar`]: () => nextPush.reply, + [`GET /api/v1/plans/${LATER}`]: () => errorResponse(404, 'not_found'), + }) + const api = createApiClient(fakeLocation('/plan'), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + const queryClient = createQueryClient() + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ) + const hook = renderHook( + ({ week }) => ({ plan: usePlan(week), swap: useSwapDay(week), push: usePushToCalendar(week) }), + { wrapper, initialProps: { week: WEEK } }, + ) + await waitFor(() => expect(hook.result.current.plan.isSuccess).toBe(true)) + + // A swap and a push of the week settle: both stay while it is shown, + // the push's failure speaking for the week. + act(() => hook.result.current.swap.mutate(ORZO)) + await waitFor(() => expect(hook.result.current.swap.pending).toBe(false)) + act(() => hook.result.current.push.mutate()) + await waitFor(() => expect(hook.result.current.push.error).not.toBeNull()) + expect(weekMutations(queryClient, WEEK)).toEqual(['swap success', 'push error']) + + // Another week shown leaves them behind; a push sent there stays + // pending across showing a third week. + hook.rerender({ week: NEXT }) + await waitFor(() => expect(hook.result.current.plan.isSuccess).toBe(true)) + expect(weekMutations(queryClient, WEEK)).toEqual([]) + act(() => hook.result.current.push.mutate()) + hook.rerender({ week: LATER }) + await waitFor(() => expect(hook.result.current.plan.isSuccess).toBe(true)) + expect(weekMutations(queryClient, NEXT)).toEqual(['push pending']) + hook.rerender({ week: NEXT }) + expect(hook.result.current.push.pending).toBe(true) + + // Once it settles, it stays while its week is shown and is left behind + // once another is. + await act(async () => nextPush.send(errorResponse(500, 'internal'))) + await waitFor(() => expect(hook.result.current.push.error).not.toBeNull()) + expect(weekMutations(queryClient, NEXT)).toEqual(['push error']) + hook.rerender({ week: LATER }) + expect(weekMutations(queryClient, NEXT)).toEqual([]) + }) +}) diff --git a/web/src/plan/api.ts b/web/src/plan/api.ts index fd580f4..4a058e0 100644 --- a/web/src/plan/api.ts +++ b/web/src/plan/api.ts @@ -1,3 +1,4 @@ +import { useEffect } from 'react' import { useMutation, useMutationState, useQuery, useQueryClient } from '@tanstack/react-query' import { ApiError, useApi, type ApiRead } from '../api/index.ts' import { isRecord } from '../api/errors.ts' @@ -177,6 +178,30 @@ function sameDay(a: PlanDay, b: PlanDay): boolean { ) } +// WeekMutation is a kind of mutation of a week the mutation cache keeps for +// it: its swaps and its pushes. +type WeekMutation = 'swap' | 'push' + +// useWeekMutationsKept bounds the mutations of kind the cache keeps. When a +// week is shown, the settled ones of every other week are removed, unless +// that week still has one pending: a pending one, and those an earlier +// pending one must see, stay across leaving its week and coming back. The +// cache holds at most the week shown's and those of weeks left pending. +function useWeekMutationsKept(weekStart: string, kind: WeekMutation) { + const queryClient = useQueryClient() + useEffect(() => { + const mutations = queryClient.getMutationCache() + const left = mutations.findAll({ + predicate: ({ options: { mutationKey: key } }) => + key !== undefined && key.length === 3 && key[0] === 'plans' && key[1] !== weekStart && key[2] === kind, + }) + const pending = new Set(left.filter((m) => m.state.status === 'pending').map((m) => m.options.mutationKey![1])) + for (const mutation of left) { + if (!pending.has(mutation.options.mutationKey![1])) mutations.remove(mutation) + } + }, [queryClient, weekStart, kind]) +} + // swapKey keys the swaps of the week starting on weekStart in the mutation // cache, where they outlive the Draft board that sent them. function swapKey(weekStart: string) { @@ -206,10 +231,12 @@ export type SwapDay = { // earlier one is answered with is not written over a later one, and its // failure is not the week's. dismiss leaves settled swaps behind; it is // refused while one is pending, so a later swap is always in the cache for -// an earlier, pending one to see. +// an earlier, pending one to see. Settled swaps of a week no longer shown +// are left behind too (useWeekMutationsKept). export function useSwapDay(weekStart: string): SwapDay { const { apiFetch } = useApi() const queryClient = useQueryClient() + useWeekMutationsKept(weekStart, 'swap') const key = planKey(weekStart) const mutationKey = swapKey(weekStart) const mutations = queryClient.getMutationCache() @@ -225,7 +252,8 @@ export function useSwapDay(weekStart: string): SwapDay { const isLatest = () => mutations.findAll({ mutationKey }).every((other) => other.mutationId <= mutation.mutationId) const mutation = mutations.build(queryClient, { mutationKey, - // Kept until dismissed: a later swap must stay in the cache while an + // Kept until dismissed or its week is left settled + // (useWeekMutationsKept): a later swap must stay in the cache while an // earlier one is pending. gcTime: Infinity, mutationFn: async ({ date, dinner }) => { @@ -302,10 +330,13 @@ export type PushToCalendar = { // // The week's pushes live in the mutation cache, not in the component, as its // swaps do, so leaving the week and coming back keeps one pending. reset -// leaves settled pushes behind; it is refused while one is pending. +// leaves settled pushes behind; it is refused while one is pending. Settled +// pushes of a week no longer shown are left behind too +// (useWeekMutationsKept). export function usePushToCalendar(weekStart: string): PushToCalendar { const { apiFetch } = useApi() const queryClient = useQueryClient() + useWeekMutationsKept(weekStart, 'push') const key = planKey(weekStart) const mutationKey = pushKey(weekStart) const mutations = queryClient.getMutationCache() @@ -319,8 +350,9 @@ export function usePushToCalendar(weekStart: string): PushToCalendar { function mutate() { const mutation = mutations.build(queryClient, { mutationKey, - // Kept until reset: a push pending in a week left must still be found - // when it is shown again. + // Kept until reset or its week is left settled (useWeekMutationsKept): + // a push pending in a week left must still be found when it is shown + // again. gcTime: Infinity, mutationFn: async () => { const path = `${planPath(weekStart)}/push-calendar`