Keep Push closed over a cached plan; bound kept week mutations
CI / ci (push) Successful in 4m21s
CI / ci (pull_request) Successful in 4m21s

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 <noreply@anthropic.com>
This commit is contained in:
2026-10-08 01:45:33 -04:00
co-authored by Claude Opus 5.5
parent f77960ad05
commit 8a02e04110
4 changed files with 171 additions and 12 deletions
+51
View File
@@ -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<Response> = () => jsonResponse(200, planBody('draft'))
let push = () => errorResponse(409, 'plan_changed')
+15 -6
View File
@@ -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 ? (
<PushFailure error={push.error} onRetry={pushToCalendar} />
<PushFailure error={push.error} cached={cached} onRetry={cached ? () => void plan.refetch() : pushToCalendar} />
) : swap.failed !== null ? (
<ErrorBanner onRetry={() => 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 (
<ErrorBanner onRetry={onRetry}>
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}
</ErrorBanner>
)
}
+68 -1
View File
@@ -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 }) => (
<ApiProvider client={api}>
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
</ApiProvider>
)
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([])
})
})
+37 -5
View File
@@ -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<Plan, Error, Swap, { previous: PlanDay; swapped: PlanDay }>(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<Plan, Error, void, unknown>(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`