diff --git a/web/src/onboarding/OnboardingPage.test.tsx b/web/src/onboarding/OnboardingPage.test.tsx index e32f1cf..f2bb78d 100644 --- a/web/src/onboarding/OnboardingPage.test.tsx +++ b/web/src/onboarding/OnboardingPage.test.tsx @@ -206,6 +206,43 @@ describe('onboarding step 2 (mock 1h)', () => { expect(regularNames()).toEqual(['Chili']) }) + it('says why an add was refused while another add is still unanswered', async () => { + const server = await regularsStep() + fireEvent.click(screen.getByRole('button', { name: 'Add Burgers' })) + await vi.waitFor(() => expect(server.held).toHaveLength(1)) + fireEvent.click(screen.getByRole('button', { name: 'Add Chili' })) + await vi.waitFor(() => expect(server.held).toHaveLength(2)) + expect(regularNames()).toEqual(['Burgers', 'Chili']) + + server.held[0]!.answer(errorResponse(409, 'duplicate_dinner')) + expect(await screen.findByRole('alert')).toHaveTextContent('Could not add Burgers: duplicate_dinner message') + expect(regularNames()).toEqual(['Chili']) + + // The other add's answer does not hide the refusal. + created(server, server.held[1]!, 'd1') + await screen.findByRole('button', { name: 'Remove Chili' }) + expect(screen.getByRole('alert')).toHaveTextContent('Could not add Burgers: duplicate_dinner message') + expect(regularNames()).toEqual(['Chili']) + }) + + it('says why each of two overlapping adds was refused', async () => { + const server = await regularsStep() + fireEvent.click(screen.getByRole('button', { name: 'Add Burgers' })) + await vi.waitFor(() => expect(server.held).toHaveLength(1)) + fireEvent.click(screen.getByRole('button', { name: 'Add Chili' })) + await vi.waitFor(() => expect(server.held).toHaveLength(2)) + + server.held[0]!.answer(errorResponse(409, 'duplicate_dinner')) + server.held[1]!.answer(errorResponse(500, 'internal')) + await vi.waitFor(() => + expect(screen.getAllByRole('alert').map((alert) => alert.textContent)).toEqual([ + 'Could not add Burgers: duplicate_dinner message', + 'Could not add Chili: internal message', + ]), + ) + expect(regularNames()).toEqual([]) + }) + it('removes a Regular at once and archives it on the server', async () => { const server = await regularsStep([ { id: 'd1', name: 'Beef tacos' }, @@ -234,6 +271,30 @@ describe('onboarding step 2 (mock 1h)', () => { expect(regularNames()).toEqual(['Beef tacos', 'Pizza night']) }) + it('puts a refused removal back and says why while another removal is still unanswered', async () => { + const server = await regularsStep([ + { id: 'd1', name: 'Beef tacos' }, + { id: 'd2', name: 'Pizza night' }, + { id: 'd3', name: 'Chili' }, + ]) + fireEvent.click(screen.getByRole('button', { name: 'Remove Beef tacos' })) + await vi.waitFor(() => expect(server.held).toHaveLength(1)) + fireEvent.click(screen.getByRole('button', { name: 'Remove Pizza night' })) + await vi.waitFor(() => expect(server.held).toHaveLength(2)) + expect(regularNames()).toEqual(['Chili']) + + server.held[0]!.answer(errorResponse(500, 'internal')) + expect(await screen.findByRole('alert')).toHaveTextContent('Could not remove Beef tacos: internal message') + expect(regularNames()).toEqual(['Beef tacos', 'Chili']) + + // The other removal's answer does not hide the refusal. + server.regulars.splice(1, 1) + server.held[1]!.answer(new Response(null, { status: 204 })) + await vi.waitFor(() => expect(server.fetchMock.mock.calls.filter(([path]) => String(path).includes('status=regular'))).toHaveLength(2)) + await vi.waitFor(() => expect(regularNames()).toEqual(['Beef tacos', 'Chili'])) + expect(screen.getByRole('alert')).toHaveTextContent('Could not remove Beef tacos: internal message') + }) + it('enables "Next: notifications" once the server has saved 3 Regulars', async () => { const server = await regularsStep([ { id: 'd1', name: 'Beef tacos' }, diff --git a/web/src/onboarding/RegularsStep.tsx b/web/src/onboarding/RegularsStep.tsx index 07cc246..09ac65e 100644 --- a/web/src/onboarding/RegularsStep.tsx +++ b/web/src/onboarding/RegularsStep.tsx @@ -31,6 +31,10 @@ function sameName(a: string, b: string): boolean { return a.trim().toLowerCase() === b.trim().toLowerCase() } +// Refusal is one add or removal the server turned down. key names the +// change, so trying it again replaces its refusal and no other. +type Refusal = { key: string; text: string } + // RegularsStep is onboarding step 2, handoff mock 1h: the household adds // the dinners it makes on repeat. Each add and remove shows at once and is // undone if the server refuses it. @@ -40,6 +44,10 @@ export function RegularsStep() { const add = useAddRegular() const remove = useRemoveRegular() const [draft, setDraft] = useState('') + // Each change keeps its own refusal: the mutation hooks only report their + // latest change, which would hide a refusal while another change is in + // flight. + const [refusals, setRefusals] = useState([]) const list = regulars.data ?? [] const picks = commonPicks.filter((pick) => !list.some((regular) => sameName(regular.name, pick))) // Next waits for every add to be answered, so a refused add's error shows @@ -47,11 +55,29 @@ export function RegularsStep() { const adding = list.some((regular) => regular.pending) const saved = list.filter((regular) => !regular.pending).length + function track(key: string, what: string, change: Promise) { + setRefusals((all) => all.filter((refusal) => refusal.key !== key)) + change.catch((error: Error) => { + setRefusals((all) => [ + ...all.filter((refusal) => refusal.key !== key), + { key, text: `Could not ${what}: ${error.message}` }, + ]) + }) + } + + function addRegular(name: string) { + track(`add:${name.toLowerCase()}`, `add ${name}`, add.mutateAsync(name)) + } + + function removeRegular(regular: Regular) { + track(`remove:${regular.id}`, `remove ${regular.name}`, remove.mutateAsync(regular)) + } + function onSubmit(event: FormEvent) { event.preventDefault() const name = draft.trim() if (name === '') return - add.mutate(name) + addRegular(name) setDraft('') } @@ -89,16 +115,11 @@ export function RegularsStep() { - {add.isError && ( -

- Could not add {add.variables}: {add.error.message} + {refusals.map((refusal) => ( +

+ {refusal.text}

- )} - {remove.isError && ( -

- Could not remove {remove.variables.name}: {remove.error.message} -

- )} + ))}

Your regulars ยท {list.length}

{regulars.isPending && } @@ -117,7 +138,7 @@ export function RegularsStep() { label={regular.name} className={regular.pending && styles.pending} aria-busy={regular.pending} - onRemove={regular.pending ? undefined : () => remove.mutate(regular)} + onRemove={regular.pending ? undefined : () => removeRegular(regular)} /> ))} @@ -130,7 +151,7 @@ export function RegularsStep() {
    {picks.map((pick) => (
  • -