Keep Quick dinners start failures and adds across tab switches
A search that could not start is now the tab's startError in the per-tab cache, set by the mutation's own onError, so it survives the panel unmounting. The first page is no longer asked for again by itself, and the ErrorBanner with Retry stays until Retry clears it. An add to the Quest log now marks the row added from the mutation's own onSuccess, which runs even when the row has unmounted, so the check is there when the tab is shown again. Review findings F1 and F2 on PR #152; task db934d36-f554-41ef-9061-5f100a0b6452. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -279,6 +279,28 @@ describe('QuickDinnersPage', () => {
|
||||
expect(within(row(QUESADILLAS.name)).getByRole('img', { name: 'In the Quest log' })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('marks an add that settles while another tab is shown', async () => {
|
||||
const added = Promise.withResolvers<Response>()
|
||||
const { fetchMock, jobs } = quickServer(['j1', 'j2'], { 'POST /api/v1/ideas/j1/items/0/quest-log': () => added.promise })
|
||||
jobs.j1 = state('succeeded', 'done', 'under30', [ORZO, QUESADILLAS])
|
||||
jobs.j2 = state('succeeded', 'done', 'crockpot', [CHILI])
|
||||
const queryClient = createQueryClient()
|
||||
await renderPage(queryClient)
|
||||
await screen.findByRole('listitem', { name: ORZO.name })
|
||||
|
||||
fireEvent.click(addButton(ORZO.name))
|
||||
await waitFor(() => expect(calls(fetchMock, 'POST /api/v1/ideas/j1/items/0/quest-log')).toHaveLength(1))
|
||||
fireEvent.click(tab('Crock pot'))
|
||||
await screen.findByRole('listitem', { name: CHILI.name })
|
||||
added.resolve(jsonResponse(201, dinnerBody('dn-1', ORZO.name, { status: 'quest_log', origin: 'quick' })))
|
||||
await waitFor(() => expect(queryClient.isMutating()).toBe(0))
|
||||
|
||||
fireEvent.click(tab('30 min or less'))
|
||||
expect(within(row(ORZO.name)).getByRole('img', { name: 'In the Quest log' })).toBeInTheDocument()
|
||||
expect(addButton(QUESADILLAS.name)).toBeEnabled()
|
||||
expect(calls(fetchMock, 'POST /api/v1/ideas/j1/items/0/quest-log')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('shows its failure in its row and lets it be pressed again', async () => {
|
||||
let fail = true
|
||||
const { jobs } = quickServer(['j1'], {
|
||||
@@ -387,6 +409,72 @@ describe('QuickDinnersPage', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps a first page that cannot start across a tab switch, asking again only on Retry', async () => {
|
||||
let under30 = 0
|
||||
const { fetchMock, jobs } = stubServer({
|
||||
[QUICK]: (init) => {
|
||||
const { kind } = JSON.parse(String(init?.body)) as { kind: string }
|
||||
if (kind === 'crockpot') return jsonResponse(202, { jobId: 'j2' })
|
||||
return ++under30 === 1 ? errorResponse(500, 'internal') : jsonResponse(202, { jobId: 'j1' })
|
||||
},
|
||||
})
|
||||
jobs.j1 = state('succeeded', 'done', 'under30', [ORZO])
|
||||
jobs.j2 = state('succeeded', 'done', 'crockpot', [CHILI])
|
||||
await renderPage()
|
||||
await screen.findByRole('alert')
|
||||
|
||||
fireEvent.click(tab('Crock pot'))
|
||||
await screen.findByRole('listitem', { name: CHILI.name })
|
||||
fireEvent.click(tab('30 min or less'))
|
||||
|
||||
const alert = screen.getByRole('alert')
|
||||
expect(alert).toHaveTextContent("Couldn't start looking for Quick dinners. internal message")
|
||||
expect(screen.queryAllByTestId('skeleton-row')).toHaveLength(0)
|
||||
expect(searches(fetchMock)).toEqual([
|
||||
{ kind: 'under30', exclude: [] },
|
||||
{ kind: 'crockpot', exclude: [] },
|
||||
])
|
||||
|
||||
fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
|
||||
await screen.findByRole('listitem', { name: ORZO.name })
|
||||
expect(screen.queryByRole('alert')).toBeNull()
|
||||
expect(calls(fetchMock, QUICK)).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('keeps a later page that cannot start across a tab switch, asking again only on Retry', async () => {
|
||||
let under30 = 0
|
||||
const { fetchMock, jobs } = stubServer({
|
||||
[QUICK]: (init) => {
|
||||
const { kind } = JSON.parse(String(init?.body)) as { kind: string }
|
||||
if (kind === 'crockpot') return jsonResponse(202, { jobId: 'j2' })
|
||||
under30++
|
||||
return under30 === 2 ? errorResponse(500, 'internal') : jsonResponse(202, { jobId: under30 === 1 ? 'j1' : 'j3' })
|
||||
},
|
||||
})
|
||||
jobs.j1 = state('succeeded', 'done', 'under30', [ORZO])
|
||||
jobs.j2 = state('succeeded', 'done', 'crockpot', [CHILI])
|
||||
jobs.j3 = state('succeeded', 'done', 'under30', [QUESADILLAS])
|
||||
await renderPage()
|
||||
await screen.findByRole('listitem', { name: ORZO.name })
|
||||
fireEvent.click(await showMore())
|
||||
await screen.findByRole('alert')
|
||||
|
||||
fireEvent.click(tab('Crock pot'))
|
||||
await screen.findByRole('listitem', { name: CHILI.name })
|
||||
fireEvent.click(tab('30 min or less'))
|
||||
|
||||
const alert = screen.getByRole('alert')
|
||||
expect(alert).toHaveTextContent("Couldn't start looking for Quick dinners. internal message")
|
||||
expect(rowNames()).toEqual([ORZO.name])
|
||||
expect(screen.queryByRole('button', { name: 'Show me more' })).toBeNull()
|
||||
expect(calls(fetchMock, QUICK)).toHaveLength(3)
|
||||
|
||||
fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
|
||||
await screen.findByRole('listitem', { name: QUESADILLAS.name })
|
||||
expect(searches(fetchMock)[3]).toEqual({ kind: 'under30', exclude: [ORZO.name] })
|
||||
expect(screen.queryByRole('alert')).toBeNull()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['ai_failed', "The AI assistant couldn't come up with ideas."],
|
||||
['no_ideas_found', 'No ideas came back with a recipe to match.'],
|
||||
|
||||
@@ -59,9 +59,20 @@ function meta(idea: QuickIdea): string {
|
||||
// QuickRow is one idea (mock 1j): its washed photo, name and meta line, and
|
||||
// the teal "+" that adds it to the Quest log, a check once it is there.
|
||||
// ideaRef is null while its job runs: the server takes an idea only once
|
||||
// the job has succeeded.
|
||||
function QuickRow({ idea, ideaRef, added, onAdded }: { idea: QuickIdea; ideaRef: IdeaRef | null; added: boolean; onAdded: () => void }) {
|
||||
const questLog = useAddIdeaToQuestLog()
|
||||
// the job has succeeded. onAdded marks the idea added, also if the row has
|
||||
// unmounted before the add settles.
|
||||
function QuickRow({
|
||||
idea,
|
||||
ideaRef,
|
||||
added,
|
||||
onAdded,
|
||||
}: {
|
||||
idea: QuickIdea
|
||||
ideaRef: IdeaRef | null
|
||||
added: boolean
|
||||
onAdded: (idea: IdeaRef) => void
|
||||
}) {
|
||||
const questLog = useAddIdeaToQuestLog(onAdded)
|
||||
return (
|
||||
<li className={styles.row} aria-label={idea.name}>
|
||||
<Photo id={idea.photo_id} name={idea.name} radius="50%" sizes="64px" className={styles.photo} />
|
||||
@@ -84,7 +95,7 @@ function QuickRow({ idea, ideaRef, added, onAdded }: { idea: QuickIdea; ideaRef:
|
||||
tone="teal"
|
||||
className={styles.add}
|
||||
disabled={ideaRef === null || questLog.isPending}
|
||||
onClick={() => questLog.mutate(ideaRef!, { onSuccess: onAdded })}
|
||||
onClick={() => questLog.mutate(ideaRef!)}
|
||||
>
|
||||
<Icon icon={Plus} size="nav" />
|
||||
</IconButton>
|
||||
@@ -108,20 +119,20 @@ function Skeletons({ count }: { count: number }) {
|
||||
// the tab is first shown; what it finds is kept for the session.
|
||||
function QuickTabPanel({ kind }: { kind: QuickKind }) {
|
||||
const [tab, update] = useQuickTab(kind)
|
||||
const find = useFindQuickIdeas(kind)
|
||||
const { mutate } = find
|
||||
const { mutate } = useFindQuickIdeas(kind)
|
||||
// A page being asked for, also by this tab before it was last shown.
|
||||
const starting = useIsMutating({ mutationKey: quickTabKey(kind) }) > 0
|
||||
const job = useJob<unknown>(QUICK_JOB_KIND, tab.jobId)
|
||||
const names = shownNames(tab)
|
||||
|
||||
// ask asks for the next page, excluding every name shown; Retry asks
|
||||
// again for the page that failed.
|
||||
// again for the page that failed. A page that could not start stays the
|
||||
// tab's startError until Retry: nothing asks again by itself.
|
||||
const ask = () => mutate(names)
|
||||
|
||||
// The first page, asked for once.
|
||||
const asked = useRef(false)
|
||||
const fresh = tab.pages.length === 0 && tab.jobId === null && !tab.ended && !starting
|
||||
const fresh = tab.pages.length === 0 && tab.jobId === null && !tab.ended && tab.startError === null && !starting
|
||||
useEffect(() => {
|
||||
if (!fresh || asked.current) return
|
||||
asked.current = true
|
||||
@@ -152,7 +163,7 @@ function QuickTabPanel({ kind }: { kind: QuickKind }) {
|
||||
else if (exhausted) update((t) => (t.jobId === jobId ? { ...t, jobId: null, ended: true } : t))
|
||||
}, [jobId, ideas, exhausted, update])
|
||||
|
||||
const markAdded = (key: string) => update((t) => ({ ...t, added: [...t.added, key] }))
|
||||
const markAdded = ({ jobId, index }: IdeaRef) => update((t) => ({ ...t, added: [...t.added, `${jobId}:${index}`] }))
|
||||
|
||||
// The rows of every page found, then of the page being found, in one
|
||||
// keyed list: a page kept with the tab's pages keeps its rows, and with
|
||||
@@ -164,10 +175,10 @@ function QuickTabPanel({ kind }: { kind: QuickKind }) {
|
||||
let next = null
|
||||
if (starting) {
|
||||
next = <Skeletons count={QUICK_IDEAS_KEPT} />
|
||||
} else if (find.isError) {
|
||||
} else if (tab.startError !== null) {
|
||||
next = (
|
||||
<li>
|
||||
<ErrorBanner onRetry={ask}>Couldn't start looking for Quick dinners. {find.error.message}</ErrorBanner>
|
||||
<ErrorBanner onRetry={ask}>Couldn't start looking for Quick dinners. {tab.startError.message}</ErrorBanner>
|
||||
</li>
|
||||
)
|
||||
} else if (jobId !== null && failure !== null && !exhausted) {
|
||||
@@ -203,7 +214,7 @@ function QuickTabPanel({ kind }: { kind: QuickKind }) {
|
||||
|
||||
// Show me more once the tab is between pages; the server takes at most
|
||||
// MAX_EXCLUDE names to leave out, so past that there is no more to ask.
|
||||
const between = !starting && !find.isError && jobId === null && tab.pages.length > 0
|
||||
const between = !starting && tab.startError === null && jobId === null && tab.pages.length > 0
|
||||
const allShown = tab.ended || names.length > MAX_EXCLUDE
|
||||
const busy = starting || (jobId !== null && failure === null && found.error === null && job.status !== 'succeeded')
|
||||
|
||||
@@ -212,7 +223,7 @@ function QuickTabPanel({ kind }: { kind: QuickKind }) {
|
||||
<p className={styles.helper}>{helpers[kind]}</p>
|
||||
<ul aria-busy={busy} aria-label="Quick dinners" className={styles.rows}>
|
||||
{rows.map(({ idea, ref, key }) => (
|
||||
<QuickRow key={key} idea={idea} ideaRef={ref} added={tab.added.includes(key)} onAdded={() => markAdded(key)} />
|
||||
<QuickRow key={key} idea={idea} ideaRef={ref} added={tab.added.includes(key)} onAdded={markAdded} />
|
||||
))}
|
||||
{next}
|
||||
</ul>
|
||||
|
||||
+16
-8
@@ -220,11 +220,12 @@ export type QuickPage = { jobId: string; ideas: QuickIdea[] }
|
||||
|
||||
// QuickTab is what one tab of Quick dinners has found this session: the
|
||||
// pages found, the job finding the next one, whether the server has said
|
||||
// there are no more, and the ideas added to the Quest log, as
|
||||
// there are no more, why the last page asked for could not start, kept
|
||||
// until Retry asks again, and the ideas added to the Quest log, as
|
||||
// "jobId:index".
|
||||
export type QuickTab = { pages: QuickPage[]; jobId: string | null; ended: boolean; added: string[] }
|
||||
export type QuickTab = { pages: QuickPage[]; jobId: string | null; ended: boolean; startError: Error | null; added: string[] }
|
||||
|
||||
const NEW_TAB: QuickTab = { pages: [], jobId: null, ended: false, added: [] }
|
||||
const NEW_TAB: QuickTab = { pages: [], jobId: null, ended: false, startError: null, added: [] }
|
||||
|
||||
export function quickTabKey(kind: QuickKind) {
|
||||
return ['ideas', 'quick', kind] as const
|
||||
@@ -258,20 +259,23 @@ export function useQuickTab(kind: QuickKind) {
|
||||
|
||||
// useFindQuickIdeas starts finding a page of Quick dinner ideas of kind,
|
||||
// none named in exclude (POST /api/v1/ideas/quick), and makes its job the
|
||||
// tab's job, also if the screen has been left meanwhile. Its mutationKey is
|
||||
// the tab's, so a tab shown again knows a page is still being asked for.
|
||||
// tab's job, or its failure the tab's startError, also if the screen has
|
||||
// been left meanwhile. Its mutationKey is the tab's, so a tab shown again
|
||||
// knows a page is still being asked for.
|
||||
export function useFindQuickIdeas(kind: QuickKind) {
|
||||
const { apiFetch } = useApi()
|
||||
const queryClient = useQueryClient()
|
||||
return useMutation({
|
||||
mutationKey: quickTabKey(kind),
|
||||
mutationFn: async (exclude: string[]): Promise<string> => {
|
||||
queryClient.setQueryData<QuickTab>(quickTabKey(kind), (tab) => ({ ...(tab ?? NEW_TAB), startError: null }))
|
||||
const body = await apiFetch<unknown>(QUICK_PATH, { method: 'POST', body: { kind, exclude } })
|
||||
if (!isRecord(body) || typeof body.jobId !== 'string') throw new Error(`POST ${QUICK_PATH}: response has no jobId`)
|
||||
const jobId = body.jobId
|
||||
queryClient.setQueryData<QuickTab>(quickTabKey(kind), (tab) => ({ ...(tab ?? NEW_TAB), jobId }))
|
||||
return jobId
|
||||
},
|
||||
onError: (startError: Error) => queryClient.setQueryData<QuickTab>(quickTabKey(kind), (tab) => ({ ...(tab ?? NEW_TAB), startError })),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -285,8 +289,9 @@ function ideaPath({ jobId, index }: IdeaRef, action: 'quest-log' | 'cook-tonight
|
||||
// useAddIdeaToQuestLog adds an idea to the Quest log (POST
|
||||
// /api/v1/ideas/{jobId}/items/{i}/quest-log) and resolves with the
|
||||
// dinner's name. Asking again answers the same dinner. The Quest log is
|
||||
// read again after it.
|
||||
export function useAddIdeaToQuestLog() {
|
||||
// read again after it, and onAdded told, also if the caller has unmounted
|
||||
// meanwhile.
|
||||
export function useAddIdeaToQuestLog(onAdded?: (idea: IdeaRef) => void) {
|
||||
const { apiFetch } = useApi()
|
||||
const queryClient = useQueryClient()
|
||||
return useMutation({
|
||||
@@ -294,7 +299,10 @@ export function useAddIdeaToQuestLog() {
|
||||
const path = ideaPath(idea, 'quest-log')
|
||||
return parseDinner(`POST ${path}`, await apiFetch<unknown>(path, { method: 'POST' })).name
|
||||
},
|
||||
onSuccess: () => queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }),
|
||||
onSuccess: (_name, idea) => {
|
||||
onAdded?.(idea)
|
||||
return queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user