From f0806cd5591183c314a8ab0d7fb533ef482ffb98 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 18:23:52 +0200 Subject: [PATCH] [F082] surface release-proposal query failures instead of silent hide The card collapsed any non-404 backend failure (500 / network drop) onto `!proposal` and returned null, so the CEO had no idea the release-proposal endpoint was unreachable. Distinguish the cases: isError + a Retry affordance vs the 404 null empty state that stays hidden. Mirrors PrReviewQueue. --- .../__tests__/release-proposal-card.test.tsx | 122 ++++++++++++++++++ .../dashboard/release-proposal-card.tsx | 42 +++++- 2 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 panel/src/components/dashboard/__tests__/release-proposal-card.test.tsx diff --git a/panel/src/components/dashboard/__tests__/release-proposal-card.test.tsx b/panel/src/components/dashboard/__tests__/release-proposal-card.test.tsx new file mode 100644 index 00000000..b5f3aad3 --- /dev/null +++ b/panel/src/components/dashboard/__tests__/release-proposal-card.test.tsx @@ -0,0 +1,122 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import type { ReleaseProposal } from "@/lib/api/release"; + +// Control useQuery per test; the mutation + queryClient hooks just need to exist. +const { mockUseQuery } = vi.hoisted(() => ({ + mockUseQuery: vi.fn(), +})); + +vi.mock("@tanstack/react-query", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useQuery: mockUseQuery, + useMutation: vi.fn(() => ({ mutate: vi.fn(), mutateAsync: vi.fn() })), + useQueryClient: vi.fn(() => ({ + invalidateQueries: vi.fn(), + })), + }; +}); + +// releaseApi methods never run (useQuery/useMutation are mocked) — the +// component imports releaseApi from the barrel, so provide a stub object. +vi.mock("@/lib/api", () => ({ + releaseApi: { + getProposal: vi.fn(), + approve: vi.fn(), + reject: vi.fn(), + }, +})); + +import { ReleaseProposalCard } from "../release-proposal-card"; + +function buildProposal(): ReleaseProposal { + return { + task_id: "t1", + title: "Cut v0.14.0", + status: "awaiting_ceo_approval", + required_changes: null, + report: { + proposed_version: "0.14.0", + bump_kind: "minor", + change_summary: ["feat: metrics"], + drafted_changelog: "## 0.14.0\n- metrics", + version_bump_plan: ["pyproject.toml"], + gaps: [], + migration_notes: [], + gate_state: "green", + }, + }; +} + +describe("ReleaseProposalCard — query-failure surfacing (F082)", () => { + beforeEach(() => { + mockUseQuery.mockReturnValue({ + data: undefined, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }); + }); + + it("surfaces a backend error (not a 404) instead of silently hiding", () => { + // A non-404 failure (500, network drop) rethrows in releaseApi.getProposal, + // so useQuery sees isError=true + data=undefined. Before the fix the card + // collapsed this onto `!proposal` and returned null — the CEO had no idea + // the release-proposal endpoint was unreachable. + mockUseQuery.mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + error: new Error("release service unavailable"), + refetch: vi.fn(), + }); + + render(); + + // The failure must be visible — not a silent hide. The error card surfaces + // the underlying message and a retry affordance. + expect( + screen.getByText(/couldn't load the release proposal/i), + ).toBeInTheDocument(); + expect( + screen.getByText(/release service unavailable/i), + ).toBeInTheDocument(); + // A retry affordance so the CEO can re-fetch without a full page reload. + expect(screen.getByRole("button", { name: /retry/i })).toBeInTheDocument(); + }); + + it("still hides on the 404 no-open-proposal empty state (regression guard)", () => { + // 404 → releaseApi.getProposal returns null → data=null, isError=false. + // That's the normal empty state and must stay hidden (mirrors PrReviewQueue). + mockUseQuery.mockReturnValue({ + data: null, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }); + + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it("renders the proposal card on the happy path (regression guard)", () => { + mockUseQuery.mockReturnValue({ + data: buildProposal(), + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }); + + render(); + expect(screen.getByText(/Release Proposal/i)).toBeInTheDocument(); + expect(screen.getByText("v0.14.0")).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /Approve & publish/i }), + ).toBeInTheDocument(); + }); +}); diff --git a/panel/src/components/dashboard/release-proposal-card.tsx b/panel/src/components/dashboard/release-proposal-card.tsx index e0dba36f..bd86d158 100644 --- a/panel/src/components/dashboard/release-proposal-card.tsx +++ b/panel/src/components/dashboard/release-proposal-card.tsx @@ -44,7 +44,13 @@ export function ReleaseProposalCard({ className }: { className?: string }) { const [action, setAction] = useState<"approve" | "reject" | null>(null); const [requiredChanges, setRequiredChanges] = useState(""); - const { data: proposal, isLoading } = useQuery({ + const { + data: proposal, + isLoading, + isError, + error, + refetch, + } = useQuery({ queryKey: ["release", "proposal"], queryFn: () => releaseApi.getProposal(), refetchInterval: 30000, @@ -102,8 +108,38 @@ export function ReleaseProposalCard({ className }: { className?: string }) { } }; - // Hidden entirely when there is no open proposal (mirrors PrReviewQueue). - if (isLoading || !proposal) return null; + // Loading: nothing to render yet (mirrors the prior hide). + if (isLoading) return null; + // A genuine query failure (non-404) must NOT hide silently — a 500 / network + // drop rethrows out of releaseApi.getProposal, leaving data undefined + isError + // set. Collapsing that onto `!proposal` returned null, so the CEO had no idea + // the release-proposal endpoint was unreachable. Surface it with a retry. + // (A 404 — "no open proposal" — is mapped to null in getProposal and falls + // through to the `!proposal` hide below, the normal empty state.) + if (isError) { + return ( + + + + + Release Proposal + + + Couldn't load the release proposal + {error instanceof Error ? `: ${error.message}` : ""}. + + + + + + + ); + } + // No open proposal (404 → null) — the normal empty state, hidden (mirrors + // PrReviewQueue). + if (!proposal) return null; const { report } = proposal; const pending = approveMutation.isPending || rejectMutation.isPending;