web/src/discover/DiscoverPage.test.tsx

@@ -948,6 +948,60 @@ describe('DiscoverPage', () => {
       )
     })
 
+    it('does not put a failed decision back on the deck when a later read, landing before it settles, has it decided', async () => {
+      let log: ReturnType<typeof deferred> | null = null
+      const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/accept': () => {
+          log = deferred()
+          return errorResponse(500, 'internal')
+        },
+        [`GET ${QUEST_LOG}`]: () => log?.promise ?? jsonResponse(200, { dinners: [] }),
+      })
+      const queryClient = createQueryClient()
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+
+      // The decision fails; the deck read after it still has the Side quest
+      // pending, and the Quest log read after it is slow.
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      await waitFor(() => expect(log).not.toBeNull())
+      const slow = log!
+      log = null
+      await waitFor(() => expect(deckReads(server.fetchMock)).toBe(2))
+
+      // Another session decides it, and a read of the deck lands meanwhile.
+      server.deck = [TIKKA, ORZO, PAELLA]
+      await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY }))
+      await act(async () => slow.resolve(jsonResponse(200, { dinners: [] })))
+
+      const alert = await screen.findByRole('alert')
+      expect(screen.queryByRole('article', { name: 'Korean beef bowls' })).not.toBeInTheDocument()
+      expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala')
+      expect(alert).toHaveTextContent('That side quest has been decided already.')
+      expect(within(alert).queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument()
+    })
+
+    it('withdraws the Retry for a failed decision once a later read of the deck has it decided', async () => {
+      const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], {
+        'POST /api/v1/suggestions/s1/accept': () => errorResponse(500, 'internal'),
+      })
+      const queryClient = createQueryClient()
+      await renderPage(queryClient)
+      await screen.findByRole('article', { name: 'Korean beef bowls' })
+
+      fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
+      const alert = await screen.findByRole('alert')
+      expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument()
+      expect(topCard()).toHaveAccessibleName('Korean beef bowls')
+
+      // Another session decides it, and the next read of the deck says so.
+      server.deck = [TIKKA, ORZO, PAELLA]
+      await act(async () => await queryClient.refetchQueries({ queryKey: DECK_KEY }))
+      await waitFor(() => expect(topCard()).toHaveAccessibleName('Crock pot chicken tikka masala'))
+      expect(screen.getByRole('alert')).toHaveTextContent('That side quest has been decided already.')
+      expect(screen.queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument()
+    })
+
     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], {

web/src/discover/DiscoverPage.tsx

@@ -12,6 +12,7 @@ import {
   BATCH_JOB_KIND,
   DECK_KEY,
   parseBatchResult,
+  stillPending,
   useDeck,
   useDecide,
   useQuestLog,
@@ -90,8 +91,8 @@ function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[
 // at a time from the top card. A decision from the buttons, the ← / → keys
 // or a swipe of the top card throws it out (motion.ts) and sends the
 // decision once it is gone, taking the card off the deck at once (see
-// useDecide); Retry sends a failed one again while the deck it read after
+// useDecide); Retry sends a failed one again while the deck's newest read
-// failing still had its Side quest pending. Input is locked from the press
+// still has its Side quest pending. Input is locked from the press
 // until the decision settles, which is once the deck and the Quest log have
 // been read again after it, so no decision starts under those reads. The lock is a
 // ref, set before anything starts, so a second press before the next render
@@ -322,10 +323,9 @@ export function DiscoverPage() {
       .then(() => (after ? queryClient.invalidateQueries({ queryKey: DECK_KEY }, { cancelRefetch: false }) : undefined))
   }, [queryClient, job.status, stored])
 
-  // A failed decision can be retried only while the deck it read after
+  // A failed decision can be retried only while the deck's newest read
-  // failing still has its Side quest pending: anything else decided it
+  // still has its Side quest pending: anything else decided it already.
-  // already.
+  const pending = decision.isError ? stillPending(deck, decision.variables.quest.id) : undefined
-  const pending = decision.isError ? decision.error.pending : undefined
   const retryable = pending !== undefined && pending !== null
 
   let body

web/src/discover/api.ts

@@ -130,18 +130,13 @@ export function useStartBatch() {
   })
 }
 
-// DecisionError is a failed decision, with what the deck read after it
+// stillPending says whether the deck's newest read has the Side quest id
-// found: pending is its Side quest while the server still has it pending,
+// pending, after a failed decision on it: the Side quest when it does, null
-// null once it is decided, and undefined when that read failed too (the
+// when it does not, and undefined when that read failed, which says neither.
-// deck query holds the read's error).
+// deck is the deck query's state, from the cache or useDeck.
-export class DecisionError extends Error {
+export function stillPending(deck: { status: string; data: SideQuest[] | undefined } | undefined, id: string): SideQuest | null | undefined {
-  readonly pending: SideQuest | null | undefined
+  if (deck?.status !== 'success') return undefined
-
+  return deck.data!.find((q) => q.id === id) ?? null
-  constructor(cause: unknown, pending: SideQuest | null | undefined) {
-    super(cause instanceof Error ? cause.message : String(cause), { cause })
-    this.name = 'DecisionError'
-    this.pending = pending
-  }
 }
 
 // useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass
@@ -158,9 +153,10 @@ export class DecisionError extends Error {
 // (which DiscoverPage locks out until then) cannot start under them. A read
 // under way when the decision ends is replaced by the read made after it,
 // and DiscoverPage's batch reads wait for that one rather than replace it.
-// A failed decision is a DecisionError, settled on the read it made and
+// A failed decision waits for the deck read it makes itself. Whether its
-// waited for itself: a Side quest that read still has is still pending, and
+// Side quest is still pending is then the deck query's to say, from its
-// goes back on top of the deck.
+// newest successful read: once the reads have landed, a Side quest the deck
+// still has goes back on top of it.
 export function useDecide() {
   const { apiFetch } = useApi()
   const queryClient = useQueryClient()
@@ -184,21 +180,20 @@ export function useDecide() {
     return body.replenish_job_id
   }
 
-  return useMutation<string | null, DecisionError, { quest: SideQuest; choice: Choice }>({
+  return useMutation<string | null, Error, { quest: SideQuest; choice: Choice }>({
     mutationFn: async ({ quest, choice }) => {
       try {
         return await send(quest.id, choice)
       } catch (error) {
         // The deck is read again as the decision's own fetch, replacing any
-        // read already out; its answer, not the cache's, settles it.
+        // read already out. A failed read is the deck query's error, shown
-        let read: SideQuest[]
+        // with the deck; the decision's own error is the one thrown.
-        try {
+        await queryClient
-          read = await queryClient.getQueryCache().build<SideQuest[]>(queryClient, { queryKey: DECK_KEY }).fetch(undefined, { cancelRefetch: true })
+          .getQueryCache()
-        } catch {
+          .build<SideQuest[]>(queryClient, { queryKey: DECK_KEY })
-          // The deck query holds the read's error, shown with the deck.
+          .fetch(undefined, { cancelRefetch: true })
-          throw new DecisionError(error, undefined)
+          .catch(() => undefined)
-        }
+        throw error
-        throw new DecisionError(error, read.find((q) => q.id === quest.id) ?? null)
       }
     },
     onMutate: ({ quest, choice }) => {
@@ -209,16 +204,16 @@ export function useDecide() {
         dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) ? dinners : dinners.concat({ id: quest.dinner_id, name: quest.name }),
       )
     },
-    onSettled: async (_replenishJobId, error) => {
+    onSettled: async (_replenishJobId, error, { quest }) => {
       if (error === null) {
         await Promise.all([queryClient.invalidateQueries({ queryKey: DECK_KEY }), queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })])
         return
       }
-      // A failed decision has read the deck already.
+      // A failed decision has read the deck already; any read since is newer.
       await queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY })
-      const { pending } = error
+      const pending = stillPending(queryClient.getQueryState<SideQuest[]>(DECK_KEY), quest.id)
       if (pending === null || pending === undefined) return
-      queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => [pending, ...(deck ?? []).filter((q) => q.id !== pending.id)])
+      queryClient.setQueryData<SideQuest[]>(DECK_KEY, (deck) => [pending, ...deck!.filter((q) => q.id !== pending.id)])
     },
   })
 }