Remediate feature-review HIGH: report the Supply run outcome after Push to Calendar #141
@@ -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 { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||||
import { Link, MemoryRouter, Route, Routes, useLocation } from 'react-router'
|
import { Link, MemoryRouter, Route, Routes, useLocation } from 'react-router'
|
||||||
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'
|
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'
|
||||||
import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts'
|
import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts'
|
||||||
import { createNetworkStatus } from '../pwa/networkStatus.ts'
|
import { createNetworkStatus } from '../pwa/networkStatus.ts'
|
||||||
|
import { SupplyRunPage } from '../shopping/SupplyRunPage.tsx'
|
||||||
import { cacheFallbackHeader } from '../sw/routes.ts'
|
import { cacheFallbackHeader } from '../sw/routes.ts'
|
||||||
import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx'
|
import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx'
|
||||||
import { declaredStyle } from '../test/cssRules.ts'
|
import { declaredStyle } from '../test/cssRules.ts'
|
||||||
@@ -159,17 +160,17 @@ function planReads(fetchMock: Mock<typeof fetch>) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// renderPage loads /me (for the CSRF token every write needs) and renders
|
// renderPage loads /me (for the CSRF token every write needs) and renders
|
||||||
// the Plan page.
|
// the Plan page, over queryClient, with shopping as the Supply run screen.
|
||||||
async function renderPage() {
|
async function renderPage(queryClient: QueryClient = createQueryClient(), shopping = <Opened />) {
|
||||||
const api = createApiClient(fakeLocation('/plan'), createNetworkStatus(window), createVerifiedSession(localStorage))
|
const api = createApiClient(fakeLocation('/plan'), createNetworkStatus(window), createVerifiedSession(localStorage))
|
||||||
await api.loadMe()
|
await api.loadMe()
|
||||||
return render(
|
return render(
|
||||||
<ApiProvider client={api}>
|
<ApiProvider client={api}>
|
||||||
<QueryClientProvider client={createQueryClient()}>
|
<QueryClientProvider client={queryClient}>
|
||||||
<MemoryRouter initialEntries={['/plan']}>
|
<MemoryRouter initialEntries={['/plan']}>
|
||||||
<Routes>
|
<Routes>
|
||||||
<Route path="/plan" element={<PlanPage />} />
|
<Route path="/plan" element={<PlanPage />} />
|
||||||
<Route path="/plan/shopping" element={<Opened />} />
|
<Route path="/plan/shopping" element={shopping} />
|
||||||
</Routes>
|
</Routes>
|
||||||
</MemoryRouter>
|
</MemoryRouter>
|
||||||
</QueryClientProvider>
|
</QueryClientProvider>
|
||||||
@@ -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 (
|
||||||
|
<>
|
||||||
|
<SupplyRunPage />
|
||||||
|
<Link to="/plan">Back to the Plan</Link>
|
||||||
|
</>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// supplyRun is the Supply run of the week of Oct 5 with needed of its 32
|
// supplyRun is the Supply run of the week of Oct 5 with needed of its 32
|
||||||
// items still to buy.
|
// items still to buy.
|
||||||
function supplyRun(needed: number) {
|
function supplyRun(needed: number) {
|
||||||
@@ -612,15 +629,20 @@ describe('PlanPage', () => {
|
|||||||
[`POST ${PUSH}`]: () => jsonResponse(200, syncedNow(SUPPLY_RUN_FAILED)),
|
[`POST ${PUSH}`]: () => jsonResponse(200, syncedNow(SUPPLY_RUN_FAILED)),
|
||||||
'POST /api/v1/shopping/2026-10-05/generate': () => made.response,
|
'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(await screen.findByRole('button', { name: 'Looks good — push to Calendar' }))
|
||||||
fireEvent.click(within(await screen.findByRole('alert')).getByRole('button', { name: 'Make the shopping list' }))
|
fireEvent.click(within(await screen.findByRole('alert')).getByRole('button', { name: 'Make the shopping list' }))
|
||||||
expect(await screen.findByRole('alert')).toHaveTextContent('Making 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' }))
|
fireEvent.click(screen.getByRole('button', { name: 'Next week' }))
|
||||||
expect(await screen.findByRole('button', { name: 'Draft this week' })).toBeInTheDocument()
|
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: [] })))
|
await act(async () => made.send(jsonResponse(200, { ...supplyRun(28), skipped: [] })))
|
||||||
|
expect(pushesKept(queryClient)).toBe(0)
|
||||||
|
|
||||||
fireEvent.click(screen.getByRole('button', { name: 'Previous week' }))
|
fireEvent.click(screen.getByRole('button', { name: 'Previous week' }))
|
||||||
expect(await screen.findByText('Beef tacos')).toBeInTheDocument()
|
expect(await screen.findByText('Beef tacos')).toBeInTheDocument()
|
||||||
@@ -628,6 +650,53 @@ describe('PlanPage', () => {
|
|||||||
expect(screen.queryByRole('button', { name: 'Make the shopping list' })).not.toBeInTheDocument()
|
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, <SupplyRunScreen />)
|
||||||
|
|
||||||
|
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, <SupplyRunScreen />)
|
||||||
|
|
||||||
|
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 () => {
|
it('pushes nothing again until the changed plan is read and shown', async () => {
|
||||||
let read: () => Response | Promise<Response> = () => jsonResponse(200, planBody('draft'))
|
let read: () => Response | Promise<Response> = () => jsonResponse(200, planBody('draft'))
|
||||||
let push = () => errorResponse(409, 'plan_changed')
|
let push = () => errorResponse(409, 'plan_changed')
|
||||||
|
|||||||
@@ -373,12 +373,11 @@ function WeekBoard({
|
|||||||
|
|
||||||
const retryPush = cached ? () => void plan.refetch() : pushToCalendar
|
const retryPush = cached ? () => void plan.refetch() : pushToCalendar
|
||||||
// makeShoppingList makes the week's Supply run again after a push could
|
// 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
|
// not make it. Once made, here or on the Supply run screen, the push is
|
||||||
// meanwhile: coming back does not say again that it couldn't be made. A
|
// settled (useGenerateSupplyRun). A failure is the mutation's state, which
|
||||||
// failure is the mutation's state, which generate.error reports.
|
// generate.error reports.
|
||||||
function makeShoppingList() {
|
function makeShoppingList() {
|
||||||
const { settle } = push
|
generate.mutate()
|
||||||
generate.mutateAsync().then(settle, () => undefined)
|
|
||||||
}
|
}
|
||||||
// A push's Supply run made again on the Plan stands for the push's, which
|
// A push's Supply run made again on the Plan stands for the push's, which
|
||||||
// is settled.
|
// is settled.
|
||||||
|
|||||||
+23
-12
@@ -439,7 +439,6 @@ export type PushToCalendar = {
|
|||||||
supplyRun: PushedSupplyRun | null
|
supplyRun: PushedSupplyRun | null
|
||||||
mutate: () => void
|
mutate: () => void
|
||||||
reset: () => void
|
reset: () => void
|
||||||
settle: () => void
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// usePushToCalendar pushes the plan of the week starting on weekStart to the
|
// 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
|
// 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
|
// 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 settled pushes behind; it is refused while one is pending. A push
|
||||||
// leaves behind the push shown, once its failed Supply run has been made
|
// whose Supply run was not made is left behind once the list is made
|
||||||
// another way, so it is not said again to be unmade; a push sent since is
|
// (settleUnmadePushes). Settled pushes of a week no longer shown are left
|
||||||
// kept. Settled pushes of a week no longer shown are left behind too, but
|
// behind too, but for the latest when its Supply run was not made
|
||||||
// for the latest when its Supply run was not made (keepWeekMutations).
|
// (keepWeekMutations).
|
||||||
export function usePushToCalendar(weekStart: string): PushToCalendar {
|
export function usePushToCalendar(weekStart: string): PushToCalendar {
|
||||||
const { apiFetch } = useApi()
|
const { apiFetch } = useApi()
|
||||||
const queryClient = useQueryClient()
|
const queryClient = useQueryClient()
|
||||||
@@ -521,17 +520,29 @@ export function usePushToCalendar(weekStart: string): PushToCalendar {
|
|||||||
for (const mutation of week) mutations.remove(mutation)
|
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 {
|
return {
|
||||||
pending: pushes.some((push) => push.status === 'pending'),
|
pending: pushes.some((push) => push.status === 'pending'),
|
||||||
error: latest?.status === 'error' ? latest.error! : null,
|
error: latest?.status === 'error' ? latest.error! : null,
|
||||||
supplyRun: latest?.status === 'success' ? latest.data!.supply_run : null,
|
supplyRun: latest?.status === 'success' ? latest.data!.supply_run : null,
|
||||||
mutate,
|
mutate,
|
||||||
reset,
|
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)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import { ApiError, useApi, type ApiClient } from '../api/index.ts'
|
|||||||
import { isRecord } from '../api/errors.ts'
|
import { isRecord } from '../api/errors.ts'
|
||||||
import { meQueryKey } from '../AuthBootstrap.tsx'
|
import { meQueryKey } from '../AuthBootstrap.tsx'
|
||||||
import { oneOf } from '../dinners/api.ts'
|
import { oneOf } from '../dinners/api.ts'
|
||||||
|
import { settleUnmadePushes } from '../plan/api.ts'
|
||||||
import type { NetworkStatus } from '../pwa/networkStatus.ts'
|
import type { NetworkStatus } from '../pwa/networkStatus.ts'
|
||||||
import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts'
|
import { HOUSEHOLD_SETTINGS_KEY, type HouseholdSettings } from '../settings/api.ts'
|
||||||
import {
|
import {
|
||||||
@@ -187,16 +188,21 @@ export function useSupplyRun(weekStart: string) {
|
|||||||
|
|
||||||
// useGenerateSupplyRun makes the week's Supply run from its plan (POST
|
// useGenerateSupplyRun makes the week's Supply run from its plan (POST
|
||||||
// /api/v1/shopping/{weekStart}/generate); the Supply run it answers with is
|
// /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) {
|
export function useGenerateSupplyRun(weekStart: string) {
|
||||||
const { apiFetch } = useApi()
|
const { apiFetch } = useApi()
|
||||||
const queryClient = useQueryClient()
|
const queryClient = useQueryClient()
|
||||||
return useMutation({
|
return useMutation({
|
||||||
|
onMutate: () => settleUnmadePushes(queryClient, weekStart),
|
||||||
mutationFn: async (): Promise<SupplyRun> => {
|
mutationFn: async (): Promise<SupplyRun> => {
|
||||||
const path = `${supplyRunPath(weekStart)}/generate`
|
const path = `${supplyRunPath(weekStart)}/generate`
|
||||||
return parseSupplyRun(`POST ${path}`, weekStart, await apiFetch<unknown>(path, { method: 'POST' }))
|
return parseSupplyRun(`POST ${path}`, weekStart, await apiFetch<unknown>(path, { method: 'POST' }))
|
||||||
},
|
},
|
||||||
onSuccess: (run) => queryClient.setQueryData<SupplyRun | null>(supplyRunKey(weekStart), run),
|
onSuccess: (run, _, settle) => {
|
||||||
|
queryClient.setQueryData<SupplyRun | null>(supplyRunKey(weekStart), run)
|
||||||
|
settle()
|
||||||
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user