diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 8e2e31e..67cf6dc 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -588,10 +588,12 @@ describe('DiscoverPage', () => { await screen.findByText('Quest log ยท 7') expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') - accept.resolve(errorResponse(500, 'internal')) + // The server refuses the request itself: the Side quest is still pending. + accept.resolve(errorResponse(400, 'invalid_request')) const alert = await screen.findByRole('alert') expect(alert).toHaveTextContent("Couldn't save that choice.") expect(topCard()).toHaveAccessibleName('Korean beef bowls') + expect(deckReads(server.fetchMock)).toBe(1) expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ 'Korean beef bowls', 'Crock pot chicken tikka masala', @@ -771,6 +773,56 @@ describe('DiscoverPage', () => { expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') }) + it('keeps a card off the deck when another session already decided it (409), and reads the deck again', async () => { + const accept = deferred() + let read: ReturnType | null = null + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => accept.promise, + [`GET ${DECK}`]: () => read?.promise ?? jsonResponse(200, { suggestions: server.deck }), + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept'])) + + // Another session passed on it first; no read of the deck lands + // during the decision. + read = deferred() + const pending = read + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => accept.resolve(errorResponse(409, 'suggestion_decided'))) + await screen.findByRole('alert') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + + await act(async () => pending.resolve(jsonResponse(200, { suggestions: server.deck }))) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + }) + + it('reads the deck again after a 500 to a decision, rather than putting the card back', async () => { + const decline = deferred() + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/decline': () => decline.promise, + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + fireEvent.click(screen.getByRole('button', { name: 'Pass' })) + await waitFor(() => expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/decline'])) + + // The decision was saved, and the answer to it is a 500 all the same. + server.deck = [TIKKA, ORZO, PAELLA] + await act(async () => decline.resolve(errorResponse(500, 'internal'))) + await screen.findByRole('alert') + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2)) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + }) + it('reads the deck again once a saved decision leaves fewer than 3 cards', async () => { const server = stubServer([BOWLS, TIKKA, ORZO], { 'POST /api/v1/suggestions/s1/decline': () => { diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index c169e7d..d7ee63f 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -146,6 +146,14 @@ type Decided = { taken: boolean; adding: symbol | null; reads: DeckReads } // flight: gone is whether the latest left the Side quest out. type DeckReads = { gone: boolean; stop: () => void } +// stillPending is whether a failed decision's error shows its Side quest +// still pending: an API error answer refusing the request itself. A 404 or +// 409 says it is gone or decided already, a 5xx answer may have come after +// the decision was saved, and a failure without an answer may have too. +function stillPending(error: Error): boolean { + return error instanceof ApiError && error.status >= 400 && error.status < 500 && error.status !== 404 && error.status !== 409 +} + // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (POST /api/v1/suggestions/{id}/decline) on the Side quest the household // decided on, whether or not a read of the deck has since taken it away. @@ -154,14 +162,15 @@ type DeckReads = { gone: boolean; stop: () => void } // // The decision is applied at once: the Side quest leaves the deck and, on // Try it, its dinner joins the Quest log unless it is there already. A -// refused decision puts the Side quest back on top of the deck if it took -// it off and no read of the deck since has left it out, takes out of the -// Quest log only the entry it added, and reads the Quest log again, since -// the decision may have been saved all the same. A failure without an -// answer from the server, which may have saved it, reads the deck again -// too. A saved one reads the Quest log again, and the deck too once fewer -// than REFILL_BELOW Side quests are left in it or a read of the deck -// started before the decision was saved. +// failed decision takes out of the Quest log only the entry it added and +// reads the Quest log again, since the decision may have been saved all the +// same. One the server refused with the Side quest still pending (see +// stillPending) puts it back on top of the deck if it took it off and no +// read of the deck since has left it out; any other failure reads the deck +// again instead, leaving the Side quest off it until that read answers. A +// saved one reads the Quest log again, and the deck too once fewer than +// REFILL_BELOW Side quests are left in it or a read of the deck started +// before the decision was saved. export function useDecide() { const { apiFetch } = useApi() const queryClient = useQueryClient() @@ -206,10 +215,8 @@ export function useDecide() { }, onError: (error, { quest }, context) => { const { taken, adding, reads } = context! - if (taken && !reads.gone) queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) - // An API error answer is the server refusing the decision; anything - // else may have come after it saved it. - if (!(error instanceof ApiError)) void queryClient.invalidateQueries({ queryKey: DECK_KEY }) + if (!stillPending(error)) void queryClient.invalidateQueries({ queryKey: DECK_KEY }) + else if (taken && !reads.gone) queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) if (adding === null) return // A read of the Quest log that landed meanwhile has replaced the entry. queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.adding !== adding))