From 6c105416a9357ebacc23e0dce8af76ed8d7f9e6b Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 02:43:06 -0400 Subject: [PATCH] Keep decided Side quests off the deck and show refused decisions A saved decision now reads the deck again whenever a read started before the save is still under way, so a late answer can no longer put the decided Side quest back. Optimistic Try it adds its dinner to the Quest log only when it is not there yet, and a refusal removes only that addition. A refused decision's ErrorBanner with Retry now shows whatever the deck has come to, including when a read emptied it during the exit. Co-Authored-By: Claude Opus 5.5 --- web/src/discover/DiscoverPage.test.tsx | 93 +++++++++++++++++++++++++- web/src/discover/DiscoverPage.tsx | 9 ++- web/src/discover/api.ts | 38 ++++++----- 3 files changed, 115 insertions(+), 25 deletions(-) diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 3ee3d1b..1a76239 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -8,6 +8,7 @@ import { declaredStyle } from '../test/cssRules.ts' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' import { expectMinHitArea } from '../test/hitArea.ts' import { mockViewport } from '../test/matchMedia.ts' +import { DECK_KEY } from './api.ts' import { DiscoverPage } from './DiscoverPage.tsx' const DECK = '/api/v1/suggestions/deck' @@ -102,13 +103,13 @@ async function finishRunning(animations: FakeAnimation[]) { } // renderPage loads /me (for the CSRF token every write needs) and renders -// the Discover page. -async function renderPage() { +// the Discover page with queryClient. +async function renderPage(queryClient = createQueryClient()) { const api = createApiClient(fakeLocation('/discover'), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() return render( - + , @@ -610,6 +611,92 @@ describe('DiscoverPage', () => { expect(screen.getByText('Quest log · 7')).toBeInTheDocument() }) + it('keeps a saved decision off the deck when a read started during it answers late', async () => { + const decline = deferred() + let late: ReturnType | null = null + const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/decline': () => decline.promise, + [`GET ${DECK}`]: () => late?.promise ?? jsonResponse(200, { suggestions: server.deck }), + }) + const queryClient = createQueryClient() + await renderPage(queryClient) + 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'])) + expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala') + + // A read of the deck starts before the decision is saved, and answers + // only after: with the Side quest still pending. + late = deferred() + const stale = late + await act(async () => void queryClient.invalidateQueries({ queryKey: DECK_KEY })) + expect(deckReads(server.fetchMock)).toBe(2) + late = null + server.deck = [TIKKA, ORZO, PAELLA] + decline.resolve(declined(BOWLS)) + await waitFor(() => expect(screen.getByRole('button', { name: 'Pass' })).toBeEnabled()) + await act(async () => stale.resolve(jsonResponse(200, { suggestions: [BOWLS, TIKKA, ORZO, PAELLA] }))) + + // The late answer is dropped, and the deck is read again after the save. + await act(async () => await new Promise((resolve) => setTimeout(resolve, 0))) + expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument() + await waitFor(() => expect(deckReads(server.fetchMock)).toBe(3)) + expect(screen.getAllByRole('article', { hidden: true }).map((card) => card.querySelector('h2')!.textContent)).toEqual([ + 'Crock pot chicken tikka masala', + 'Lemon garlic shrimp orzo', + 'Sheet pan paella', + ]) + }) + + it('leaves the Quest log count alone for a dinner already in it, on Try it and on its refusal', async () => { + const accept = deferred() + stubServer([BOWLS, TIKKA, ORZO, PAELLA], { + 'POST /api/v1/suggestions/s1/accept': () => accept.promise, + // A read of the Quest log already has the dinner the stale deck offers. + [`GET ${QUEST_LOG}`]: () => + jsonResponse(200, { + dinners: [ + { id: 'q0', name: 'Quest 0' }, + { id: 'dinner-s1', name: 'Korean beef bowls' }, + ], + }), + }) + await renderPage() + await screen.findByRole('article', { name: 'Korean beef bowls' }) + await screen.findByText('Quest log · 2') + + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + await waitFor(() => expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')) + expect(screen.getByText('Quest log · 2')).toBeInTheDocument() + + accept.resolve(errorResponse(500, 'internal')) + await screen.findByRole('alert') + expect(topCard()).toHaveAccessibleName('Korean beef bowls') + expect(screen.getByText('Quest log · 2')).toBeInTheDocument() + }) + + it('shows a decision refused because the deck changed during the exit, with Retry', async () => { + const server = stubServer([BOWLS]) + const queryClient = createQueryClient() + const animations = stubAnimate({ autoFinish: false }) + await renderPage(queryClient) + await screen.findByRole('article', { name: 'Korean beef bowls' }) + + // While the card leaves, a read of the deck takes the last card away. + fireEvent.click(screen.getByRole('button', { name: 'Try it' })) + server.deck = [] + await act(async () => await queryClient.invalidateQueries({ queryKey: DECK_KEY })) + await screen.findByRole('heading', { name: 'All caught up' }) + + await finishRunning(animations) + await finishRunning(animations) + const alert = await screen.findByRole('alert') + expect(alert).toHaveTextContent("Couldn't save that choice. suggestion s1 is not in the deck") + expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(sent(server.fetchMock)).toEqual([]) + }) + 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/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 2fadb80..114d566 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -362,11 +362,6 @@ export function DiscoverPage() { - {decision.isError && ( - submit(decision.variables!)}> - Couldn't save that choice. {decision.error.message} - - )} ) } @@ -384,6 +379,10 @@ export function DiscoverPage() { void questLog.refetch()}>Couldn't load the Quest log. {questLog.error.message} )} {body} + {/* A refused decision is shown whatever the deck has come to since. */} + {decision.isError && ( + submit(decision.variables!)}>Couldn't save that choice. {decision.error.message} + )} ) } diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index ede8008..52f67ab 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -135,8 +135,9 @@ export function useStartBatch() { } // Decided is the context a decision's optimistic update leaves for its -// rollback: the Side quest taken off the deck. -type Decided = { quest: SideQuest } +// rollback: the Side quest taken off the deck, and whether its dinner was +// added to the Quest log. +type Decided = { quest: SideQuest; added: boolean } // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass // (POST /api/v1/suggestions/{id}/decline). The answer must be that @@ -144,10 +145,11 @@ type Decided = { quest: SideQuest } // replenish job the decision started, or null. // // The decision is applied at once: the Side quest leaves the deck and, on -// Try it, its dinner joins the Quest log. A refused decision puts the Side -// quest back on top of the deck and takes its dinner out of the Quest log -// again. A saved one reads the Quest log again, and the deck too once fewer -// than REFILL_BELOW Side quests are left in it. +// 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 and takes +// out of the Quest log only the dinner it added. 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() @@ -176,28 +178,30 @@ export function useDecide() { const quest = queryClient.getQueryData(DECK_KEY)?.find((q) => q.id === id) if (quest === undefined) throw new Error(`suggestion ${id} is not in the deck`) queryClient.setQueryData(DECK_KEY, (deck) => deck?.filter((q) => q.id !== id)) - if (choice === 'try') { - queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => - dinners?.concat({ id: quest.dinner_id, name: quest.name }), - ) - } - return { quest } + // A stale deck can offer a Side quest whose dinner a read of the Quest + // log already has. + const dinners = queryClient.getQueryData(QUEST_LOG_KEY) + const added = choice === 'try' && dinners !== undefined && !dinners.some((d) => d.id === quest.dinner_id) + if (added) queryClient.setQueryData(QUEST_LOG_KEY, dinners.concat({ id: quest.dinner_id, name: quest.name })) + return { quest, added } }, - onError: (_error, { choice }, context) => { + onError: (_error, _variables, context) => { // onMutate refusing leaves nothing to undo. if (context === undefined) return - const { quest } = context + const { quest, added } = context queryClient.setQueryData(DECK_KEY, (deck) => deck && [quest, ...deck.filter((q) => q.id !== quest.id)]) - if (choice === 'try') { + if (added) { queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.id !== quest.dinner_id)) } }, onSuccess: async (_replenishJobId, { id }) => { // A read that landed while the decision was in flight may have put - // the Side quest back. + // the Side quest back, and one still under way may yet: invalidating + // cancels it for a read made after the decision was saved. + const reading = queryClient.isFetching({ queryKey: DECK_KEY }) > 0 const deck = queryClient.setQueryData(DECK_KEY, (current) => current?.filter((q) => q.id !== id)) await Promise.all([ - deck !== undefined && deck.length < REFILL_BELOW ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined, + reading || (deck !== undefined && deck.length < REFILL_BELOW) ? queryClient.invalidateQueries({ queryKey: DECK_KEY }) : undefined, queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }), ]) },