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`