From d917a40fa2f561fd83f6b167d51c832aff740a07 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Thu, 8 Oct 2026 14:36:25 -0400 Subject: [PATCH] Settle a push's unmade Supply run however the shopping list is made Only the Plan's own "Make the shopping list" settled a push whose Supply run was not made. Making the list on the Supply run screen, through the same generate endpoint, left the push in the mutation cache, so coming back to the Plan still said the Supply run couldn't be made and offered to make it again, although the list existed (F17-R3). The settlement now lives in useGenerateSupplyRun, which both screens use: onMutate notes the week's pushes sent so far (settleUnmadePushes) and onSuccess removes those whose Supply run was not made. A failed attempt leaves them shown. usePushToCalendar's settle, the Plan-only path, is gone. Tests: the list made on the Supply run screen is no longer said to be unmade on the Plan, a failure there still is, and the success-after-navigation test now checks the push is kept while the week is left and removed once made, so settlement is observed apart from the week sweep. Maestro task e75db0f0-817f-4eef-84b2-266e793c018a. Co-Authored-By: Claude Opus 5.5 --- web/src/plan/PlanPage.test.tsx | 81 +++++++++++++++++++++++++++++++--- web/src/plan/PlanPage.tsx | 9 ++-- web/src/plan/api.ts | 35 ++++++++++----- web/src/shopping/api.ts | 10 ++++- 4 files changed, 110 insertions(+), 25 deletions(-) diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index cde7881..1f027a9 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -1,9 +1,10 @@ -import { QueryClientProvider } from '@tanstack/react-query' +import { QueryClientProvider, type QueryClient } from '@tanstack/react-query' import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { Link, MemoryRouter, Route, Routes, useLocation } 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 } from '../pwa/networkStatus.ts' +import { SupplyRunPage } from '../shopping/SupplyRunPage.tsx' import { cacheFallbackHeader } from '../sw/routes.ts' import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx' import { declaredStyle } from '../test/cssRules.ts' @@ -159,17 +160,17 @@ function planReads(fetchMock: Mock) { } // renderPage loads /me (for the CSRF token every write needs) and renders -// the Plan page. -async function renderPage() { +// the Plan page, over queryClient, with shopping as the Supply run screen. +async function renderPage(queryClient: QueryClient = createQueryClient(), shopping = ) { const api = createApiClient(fakeLocation('/plan'), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() return render( - + } /> - } /> + @@ -189,6 +190,22 @@ function Opened() { ) } +// pushesKept counts the pushes of the week of Oct 5 the mutation cache of +// queryClient keeps. +function pushesKept(queryClient: QueryClient) { + return queryClient.getMutationCache().findAll({ mutationKey: ['plans', '2026-10-05', 'push'] }).length +} + +// SupplyRunScreen is the Supply run screen, with a link back to the Plan. +function SupplyRunScreen() { + return ( + <> + + Back to the Plan + + ) +} + // supplyRun is the Supply run of the week of Oct 5 with needed of its 32 // items still to buy. function supplyRun(needed: number) { @@ -612,15 +629,20 @@ describe('PlanPage', () => { [`POST ${PUSH}`]: () => jsonResponse(200, syncedNow(SUPPLY_RUN_FAILED)), 'POST /api/v1/shopping/2026-10-05/generate': () => made.response, }) - await renderPage() + const queryClient = createQueryClient() + await renderPage(queryClient) fireEvent.click(await screen.findByRole('button', { name: 'Looks good — push to Calendar' })) fireEvent.click(within(await screen.findByRole('alert')).getByRole('button', { name: 'Make the shopping list' })) expect(await screen.findByRole('alert')).toHaveTextContent('Making the shopping list…') + // The push is kept while the week is left, until the list is made: it + // is then settled, not merely swept with the week. fireEvent.click(screen.getByRole('button', { name: 'Next week' })) expect(await screen.findByRole('button', { name: 'Draft this week' })).toBeInTheDocument() + expect(pushesKept(queryClient)).toBe(1) await act(async () => made.send(jsonResponse(200, { ...supplyRun(28), skipped: [] }))) + expect(pushesKept(queryClient)).toBe(0) fireEvent.click(screen.getByRole('button', { name: 'Previous week' })) expect(await screen.findByText('Beef tacos')).toBeInTheDocument() @@ -628,6 +650,53 @@ describe('PlanPage', () => { expect(screen.queryByRole('button', { name: 'Make the shopping list' })).not.toBeInTheDocument() }) + it('no longer says the Supply run could not be made once the shopping list is made on the Supply run screen', async () => { + stubServer({ + [`GET ${THIS_WEEK}`]: () => jsonResponse(200, planBody('draft')), + [`POST ${PUSH}`]: () => jsonResponse(200, syncedNow(SUPPLY_RUN_FAILED)), + 'POST /api/v1/shopping/2026-10-05/generate': () => jsonResponse(200, { ...supplyRun(28), skipped: [] }), + }) + const queryClient = createQueryClient() + await renderPage(queryClient, ) + + fireEvent.click(await screen.findByRole('button', { name: 'Looks good — push to Calendar' })) + expect(await screen.findByRole('alert')).toHaveTextContent("Pushed to Calendar, but this week's Supply run couldn't be made.") + + // The list is made on the Supply run screen, not from the Plan's alert. + fireEvent.click(screen.getByRole('link', { name: 'Shopping list' })) + fireEvent.click(await screen.findByRole('button', { name: 'Make the shopping list' })) + await waitFor(() => expect(screen.queryByRole('button', { name: 'Make the shopping list' })).not.toBeInTheDocument()) + expect(pushesKept(queryClient)).toBe(0) + + fireEvent.click(screen.getByRole('link', { name: 'Back to the Plan' })) + expect(await screen.findByRole('link', { name: 'Shopping list · 28' })).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Make the shopping list' })).not.toBeInTheDocument() + }) + + it('still says the Supply run could not be made when making it on the Supply run screen fails', async () => { + stubServer({ + [`GET ${THIS_WEEK}`]: () => jsonResponse(200, planBody('draft')), + [`POST ${PUSH}`]: () => jsonResponse(200, syncedNow(SUPPLY_RUN_FAILED)), + 'POST /api/v1/shopping/2026-10-05/generate': () => errorResponse(500, 'internal'), + }) + const queryClient = createQueryClient() + await renderPage(queryClient, ) + + fireEvent.click(await screen.findByRole('button', { name: 'Looks good — push to Calendar' })) + expect(await screen.findByRole('alert')).toHaveTextContent("Pushed to Calendar, but this week's Supply run couldn't be made.") + + fireEvent.click(screen.getByRole('link', { name: 'Shopping list' })) + fireEvent.click(await screen.findByRole('button', { name: 'Make the shopping list' })) + expect(await screen.findByRole('alert')).toHaveTextContent("Couldn't make the shopping list.") + expect(pushesKept(queryClient)).toBe(1) + + fireEvent.click(screen.getByRole('link', { name: 'Back to the Plan' })) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Pushed to Calendar, but this week's Supply run couldn't be made.") + expect(within(alert).getByRole('button', { name: 'Make the shopping list' })).toBeEnabled() + }) + it('pushes nothing again until the changed plan is read and shown', 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 baf6257..87124ba 100644 --- a/web/src/plan/PlanPage.tsx +++ b/web/src/plan/PlanPage.tsx @@ -373,12 +373,11 @@ function WeekBoard({ const retryPush = cached ? () => void plan.refetch() : pushToCalendar // makeShoppingList makes the week's Supply run again after a push could - // not make it. Once made, the push is settled, even if the Plan was left - // meanwhile: coming back does not say again that it couldn't be made. A - // failure is the mutation's state, which generate.error reports. + // not make it. Once made, here or on the Supply run screen, the push is + // settled (useGenerateSupplyRun). A failure is the mutation's state, which + // generate.error reports. function makeShoppingList() { - const { settle } = push - generate.mutateAsync().then(settle, () => undefined) + generate.mutate() } // A push's Supply run made again on the Plan stands for the push's, which // is settled. diff --git a/web/src/plan/api.ts b/web/src/plan/api.ts index 7e9341d..e6094fc 100644 --- a/web/src/plan/api.ts +++ b/web/src/plan/api.ts @@ -439,7 +439,6 @@ export type PushToCalendar = { supplyRun: PushedSupplyRun | null mutate: () => void reset: () => void - settle: () => void } // usePushToCalendar pushes the plan of the week starting on weekStart to the @@ -460,11 +459,11 @@ 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. settle -// leaves behind the push shown, once its failed Supply run has been made -// another way, so it is not said again to be unmade; a push sent since is -// kept. Settled pushes of a week no longer shown are left behind too, but -// for the latest when its Supply run was not made (keepWeekMutations). +// leaves settled pushes behind; it is refused while one is pending. A push +// whose Supply run was not made is left behind once the list is made +// (settleUnmadePushes). Settled pushes of a week no longer shown are left +// behind too, but for the latest when its Supply run was not made +// (keepWeekMutations). export function usePushToCalendar(weekStart: string): PushToCalendar { const { apiFetch } = useApi() const queryClient = useQueryClient() @@ -521,17 +520,29 @@ export function usePushToCalendar(weekStart: string): PushToCalendar { for (const mutation of week) mutations.remove(mutation) } - function settle() { - const shown = mutations.findAll({ mutationKey }).find((mutation) => mutation.mutationId === latest?.mutationId) - if (shown !== undefined) mutations.remove(shown) - } - return { pending: pushes.some((push) => push.status === 'pending'), error: latest?.status === 'error' ? latest.error! : null, supplyRun: latest?.status === 'success' ? latest.data!.supply_run : null, mutate, reset, - settle, + } +} + +// settleUnmadePushes is called as the Supply run of the week starting on +// weekStart starts being made, from the Plan or the Supply run screen +// (useGenerateSupplyRun). The function it answers with is called once the +// list is made: it leaves behind the week's pushes sent before whose Supply +// run was not made, so the Plan does not say again that it is unmade, even +// if the Plan was left meanwhile. A push sent since is kept, and a list not +// made leaves them as they are. +export function settleUnmadePushes(queryClient: QueryClient, weekStart: string): () => void { + const mutations = queryClient.getMutationCache() + const mutationKey = pushKey(weekStart) + const sent = new Set(mutations.findAll({ mutationKey })) + return () => { + for (const mutation of mutations.findAll({ mutationKey })) { + if (sent.has(mutation) && unmade(mutation)) mutations.remove(mutation) + } } } diff --git a/web/src/shopping/api.ts b/web/src/shopping/api.ts index 6119f03..09fc0a3 100644 --- a/web/src/shopping/api.ts +++ b/web/src/shopping/api.ts @@ -12,6 +12,7 @@ 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 { settleUnmadePushes } from '../plan/api.ts' import type { NetworkStatus } from '../pwa/networkStatus.ts' import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts' import { @@ -187,16 +188,21 @@ export function useSupplyRun(weekStart: string) { // useGenerateSupplyRun makes the week's Supply run from its plan (POST // /api/v1/shopping/{weekStart}/generate); the Supply run it answers with is -// the one shown. +// the one shown. Once made, from whichever screen, a push of the week whose +// Supply run was not made is settled (settleUnmadePushes). export function useGenerateSupplyRun(weekStart: string) { const { apiFetch } = useApi() const queryClient = useQueryClient() return useMutation({ + onMutate: () => settleUnmadePushes(queryClient, weekStart), mutationFn: async (): Promise => { const path = `${supplyRunPath(weekStart)}/generate` return parseSupplyRun(`POST ${path}`, weekStart, await apiFetch(path, { method: 'POST' })) }, - onSuccess: (run) => queryClient.setQueryData(supplyRunKey(weekStart), run), + onSuccess: (run, _, settle) => { + queryClient.setQueryData(supplyRunKey(weekStart), run) + settle() + }, }) }