From f29b067a0221d167cfe14e3e6328e5970d1ee035 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 16 Sep 2026 16:19:50 -0400 Subject: [PATCH] Fix Answer Question navigation on the runs board --- apps/fabro-web/app/routes/run-detail.test.ts | 78 ++++++++++++++++++-- apps/fabro-web/app/routes/runs.tsx | 4 +- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/apps/fabro-web/app/routes/run-detail.test.ts b/apps/fabro-web/app/routes/run-detail.test.ts index e0462d724..7df596d92 100644 --- a/apps/fabro-web/app/routes/run-detail.test.ts +++ b/apps/fabro-web/app/routes/run-detail.test.ts @@ -19,6 +19,7 @@ import { TEST_PRINCIPAL, makeUsage } from "../lib/test-fixtures"; let currentRunSummary: any = null; let currentRunState: any = null; let currentQuestions: any[] = []; +let currentBoardRuns: any[] = []; let deleteRunApiResult: Promise | null = null; const mountedRenderers: TestRenderer.ReactTestRenderer[] = []; @@ -27,6 +28,12 @@ const deleteRunApiMock = mock((_id: string) => ); const mutateRunListCachesMock = mock((_mutate: unknown) => undefined); const swrMutateMock = mock((_key: unknown) => Promise.resolve(undefined)); +const questionQueryMock = mock((_runId: string, _enabled: boolean) => ({ data: currentQuestions })); +const submitAnswerMock = mock((_answer: unknown) => Promise.resolve(undefined)); +const submitAnswerHookMock = mock((_runId: string) => ({ + isMutating: false, + trigger: submitAnswerMock, +})); mock.module("@headlessui/react", () => ({ Dialog: ({ open, children }: any) => @@ -48,6 +55,10 @@ mock.module("@headlessui/react", () => ({ })); mock.module("../lib/queries", () => ({ + useAllRuns: () => ({ data: { data: currentBoardRuns }, isLoading: false }), + useRunsPage: () => ({ data: null, isLoading: false }), + useAuthConfig: () => ({ data: { methods: [] } }), + useSystemInfo: () => ({ data: null }), useRun: () => ({ data: currentRunSummary, isLoading: false, @@ -56,9 +67,7 @@ mock.module("../lib/queries", () => ({ data: null, isLoading: false, }), - useRunQuestions: () => ({ - data: currentQuestions, - }), + useRunQuestions: questionQueryMock, useRunPullRequest: () => ({ data: null, isLoading: false, @@ -79,6 +88,11 @@ mock.module("../lib/run-events", () => ({ useRunEvents: () => undefined, })); +mock.module("../lib/board-events", () => ({ + shouldRefreshBoardForEvent: () => false, + useBoardEvents: () => undefined, +})); + mock.module("../hooks/use-run-toasts", () => ({ useRunToasts: () => undefined, })); @@ -169,7 +183,7 @@ mock.module("../lib/mutations", () => ({ usePreviewRun: mutationState, useRetryRun: mutationState, useSteerRun: mutationState, - useSubmitInterviewAnswer: mutationState, + useSubmitInterviewAnswer: submitAnswerHookMock, useUpdateRunTitle: mutationState, useUnarchiveRun: mutationState, })); @@ -187,6 +201,7 @@ const { default: RunDetail, resolveDockClearance, } = await import("./run-detail"); +const { default: Runs } = await import("./runs"); mock.restore(); type LifecycleToastState = import("./run-detail/lifecycle-toasts").LifecycleToastState; type RunDetailActionResult = import("./run-detail/lifecycle-toasts").RunDetailActionResult; @@ -334,7 +349,7 @@ async function renderRunDetailHarness({ [ { path: "/runs", - element: h("div", { "data-route": "runs-index" }, "Runs"), + element: h(Runs), }, { path: "/runs/:id", @@ -637,6 +652,10 @@ describe("RunDetail full-height child routes", () => { currentRunSummary = null; currentRunState = null; currentQuestions = []; + currentBoardRuns = []; + questionQueryMock.mockClear(); + submitAnswerMock.mockClear(); + submitAnswerHookMock.mockClear(); deleteRunApiResult = null; deleteRunApiMock.mockClear(); mutateRunListCachesMock.mockClear(); @@ -644,6 +663,55 @@ describe("RunDetail full-height child routes", () => { delete (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT; }); + test("Answer Question on a blocked board card opens that run's pending interview", async () => { + currentBoardRuns = [ + { ...makeRunSummary(), id: "other-run" }, + makeRunSummary({ status: "blocked" }), + ]; + const { renderer, router } = await renderRunDetailHarness({ + initialEntry: "/runs?view=columns", + status: "blocked", + questions: [makeQuestion()], + }); + + await act(async () => { + findButtonByText(renderer, "Answer Question")!.props.onClick(); + }); + + expect(router.state.location.pathname).toBe("/runs/run_1"); + expect(questionQueryMock).toHaveBeenCalledWith("run_1", true); + const interview = renderer.root.findByProps({ "aria-label": "Interview question" }); + expect(textFromTestNode(interview)).toContain("Approve?"); + const answer = interview.findByProps({ "aria-label": "Answer yes" }); + expect(answer.props.disabled).toBe(false); + await act(async () => { + answer.props.onClick(); + }); + expect(submitAnswerHookMock).toHaveBeenCalledWith("run_1"); + expect(submitAnswerMock).toHaveBeenCalledWith({ questionId: "q_1", answer: { kind: "yes" } }); + }); + + for (const status of ["blocked", "running"]) { + test(`a stale blocked card opens the ${status} run without an already answered question`, async () => { + currentBoardRuns = [makeRunSummary({ status: "blocked" })]; + const { renderer, router } = await renderRunDetailHarness({ + initialEntry: "/runs?view=columns", + status, + questions: [], + }); + + await act(async () => { + findButtonByText(renderer, "Answer Question")!.props.onClick(); + }); + + expect(router.state.location.pathname).toBe("/runs/run_1"); + expect(questionQueryMock).toHaveBeenCalledWith("run_1", status === "blocked"); + expect(textFromNode(renderer.toJSON())).toContain("Overview"); + expect(renderer.root.findAllByProps({ "aria-label": "Interview question" })).toHaveLength(0); + expect(submitAnswerMock).not.toHaveBeenCalled(); + }); + } + test("uses a full-height flex wrapper for fullHeight child routes", async () => { const renderer = await renderRunDetail({ initialEntry: "/runs/run_1/files", diff --git a/apps/fabro-web/app/routes/runs.tsx b/apps/fabro-web/app/routes/runs.tsx index 9aeceb78c..69ae6ff04 100644 --- a/apps/fabro-web/app/routes/runs.tsx +++ b/apps/fabro-web/app/routes/runs.tsx @@ -1,5 +1,5 @@ import { useState, useCallback, useMemo, useRef } from "react"; -import { Link, Navigate } from "react-router"; +import { Link, Navigate, useNavigate } from "react-router"; import { CheckIcon, ChevronDownIcon, CommandLineIcon } from "@heroicons/react/24/outline"; import { EllipsisVerticalIcon } from "@heroicons/react/20/solid"; import { Menu, MenuButton, MenuItem, MenuItems } from "@headlessui/react"; @@ -360,6 +360,7 @@ function PrCard({ // piece as a sibling `
` below the card body recreates a recurring bug // where stats stack onto separate lines instead of sitting next to size/actions. function PrCardFooter({ pr, actions }: { pr: RunItem; actions?: string[] }) { + const navigate = useNavigate(); const hasActions = actions != null && actions.length > 0; const hasStats = pr.resources != null || @@ -399,6 +400,7 @@ function PrCardFooter({ pr, actions }: { pr: RunItem; actions?: string[] }) { key={label} type="button" disabled={pr.actionDisabled} + onClick={label === "Answer Question" ? () => navigate(`/runs/${pr.id}`) : undefined} className={`inline-flex items-center gap-1.5 rounded-md border px-2.5 py-1 text-[11px] font-medium transition-colors disabled:cursor-not-allowed disabled:text-fg-muted disabled:border-line ${ label === "Merge" ? "border-mint/20 text-mint hover:border-mint/50 hover:text-fg"