diff --git a/src/components/ProviderDrawer/ProviderDrawer.test.tsx b/src/components/ProviderDrawer/ProviderDrawer.test.tsx index 800f5c6..6bccef4 100644 --- a/src/components/ProviderDrawer/ProviderDrawer.test.tsx +++ b/src/components/ProviderDrawer/ProviderDrawer.test.tsx @@ -1,46 +1,196 @@ // @vitest-environment happy-dom -// ProviderDrawer wires `useProviderDetail` (TanStack Query) and renders a -// Radix Dialog portal — both need an act-aware, DOM-backed environment or -// React logs warnings and the portal can't mount. +// ProviderDrawer wires `useProviderDetail` (TanStack Query) and renders +// a Radix Dialog portal — both need an act-aware, DOM-backed environment +// or React logs warnings and the portal can't mount. (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; import { afterEach, describe, it, expect, vi } from "vitest"; -import { cleanup, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { ApiError } from "@/lib/api"; import { ProviderDrawer } from "@/components/ProviderDrawer"; +import type { Provider } from "@/types"; // Mock the hook BEFORE the import above is resolved (vitest hoists // `vi.mock` to the top of the file regardless of where it appears -// syntactically). The drawer renders the provider fields into the -// Overview tab from the `data` returned by this hook, so the mock just -// needs to supply the fields the component reads. +// syntactically). Mocking the hook directly — rather than mocking +// `api.getProvider` — lets each test pin the hook's exact return shape +// without standing up a real `QueryClient`. +// +// `vi.hoisted` is required because `vi.mock` is hoisted ABOVE top-level +// `const` declarations — referencing a top-level `vi.fn()` directly from +// the factory would hit "Cannot access before initialization" at runtime. +const { useProviderDetail } = vi.hoisted(() => ({ + useProviderDetail: vi.fn(), +})); vi.mock("@/hooks/useProviderDetail", () => ({ - useProviderDetail: () => ({ - data: { - npi: "1881068062", - name: "Montrose Memorial", - taxId: "721587149", - address: "123 Main St", - city: "Montrose", - state: "CO", - zip: "81401", - phone: "(970) 555-1234", - claimCount: 184, - outstandingAr: 12450, - }, + useProviderDetail, +})); + +/** + * Minimal valid `Provider` fixture — every required key present so the + * component typechecks. The fields are what `ProviderOverview` reads, + * so the success-path render exercises every prop. + */ +const SAMPLE_PROVIDER: Provider = { + npi: "1881068062", + name: "Montrose Memorial", + taxId: "721587149", + address: "123 Main St", + city: "Montrose", + state: "CO", + zip: "81401", + phone: "(970) 555-1234", + claimCount: 184, + outstandingAr: 12450, +}; + +/** + * Configure the mocked hook's return value for a single test. The + * `refetch` default is a fresh `vi.fn()` — tests that need to assert + * on it can override via `overrides.refetch`. + */ +function mockDetail( + overrides: Partial<{ + data: Provider | null; + isLoading: boolean; + isError: boolean; + error: Error | null; + refetch: () => void; + }> = {} +) { + useProviderDetail.mockReturnValue({ + data: null, isLoading: false, isError: false, - }), -})); + error: null, + refetch: vi.fn(), + ...overrides, + }); +} // happy-dom keeps `document.body` between tests; without cleanup, // `screen.getByText(...)` would find nodes from earlier renders. -afterEach(() => cleanup()); +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); describe("ProviderDrawer", () => { - it("renders Overview tab content for a known provider", () => { - render( {}} />); - expect(screen.getByText("Montrose Memorial")).toBeTruthy(); - expect(screen.getByText(/1881068062/)).toBeTruthy(); - expect(screen.getByText(/721587149/)).toBeTruthy(); + it("test_renders_nothing_when_npi_is_null", () => { + mockDetail({ data: null }); + render( {}} />); + + // No provider content should be in the document when the drawer + // is closed — Radix's Dialog gates the portal on `open`. + expect(document.body.textContent).not.toContain("Montrose Memorial"); }); -}); \ No newline at end of file + + it("test_calls_useProviderDetail_with_npi", () => { + mockDetail({ data: SAMPLE_PROVIDER }); + render( {}} />); + + expect(useProviderDetail).toHaveBeenCalledWith("1881068062"); + }); + + it("test_renders_provider_overview_on_success", () => { + mockDetail({ data: SAMPLE_PROVIDER }); + render( {}} />); + + // ProviderOverview renders NPI + Tax ID as Field values; the + // drawer header shows the provider name as the title. + expect(document.body.textContent).toContain("Montrose Memorial"); + expect(document.body.textContent).toContain("1881068062"); + expect(document.body.textContent).toContain("721587149"); + }); + + it("test_renders_skeleton_while_loading", () => { + mockDetail({ isLoading: true }); + render( {}} />); + + // The `Skeleton` primitive sets `aria-busy="true"` — a stable hook + // for the loading state that doesn't require a custom testid. + expect(document.querySelectorAll('[aria-busy="true"]').length).toBeGreaterThan(0); + // And the provider name should NOT have leaked in yet. + expect(document.body.textContent).not.toContain("Montrose Memorial"); + }); + + it("test_renders_not_found_error_on_404", () => { + mockDetail({ isError: true, error: new ApiError(404, "Provider ghost not found") }); + render( {}} />); + + const errEl = document.querySelector('[data-testid="provider-drawer-error-not_found"]'); + expect(errEl).not.toBeNull(); + // Body should mention "doesn't exist" — the not_found COPY key. + expect(errEl?.textContent).toContain("doesn't exist"); + // And the retry button should NOT be present (not_found has no + // retry affordance — retrying a 404 won't help). + expect(document.querySelector('[data-testid="error-retry"]')).toBeNull(); + }); + + it("test_renders_network_error_with_retry", () => { + mockDetail({ isError: true, error: new Error("network down") }); + render( {}} />); + + const errEl = document.querySelector('[data-testid="provider-drawer-error-network"]'); + expect(errEl).not.toBeNull(); + expect(errEl?.textContent).toContain("Couldn't reach the server"); + + // Network variant shows a Retry button. + const retryBtn = document.querySelector( + '[data-testid="error-retry"]' + ) as HTMLButtonElement | null; + expect(retryBtn).not.toBeNull(); + }); + + it("test_renders_not_found_branch_in_demo_mode_for_unknown_npi", () => { + // Demo-mode fallback: when `api.isConfigured` is false, the hook + // reads from the in-memory zustand store instead of fetching. If the + // queried NPI isn't in the providers list, the hook returns + // `isError: true` with an `ApiError(404, "Provider not found")`. + // The drawer's `errorKind` computation must route this to the + // `not_found` branch — NOT the generic `network` branch, which + // would mislead the user into thinking the server is down when + // there is simply no backend configured (or the NPI is just not + // in the local store). Regression guard for the `new Error` → + // `new ApiError(404, ...)` fix on the demo-mode fallback branch. + mockDetail({ + isError: true, + error: new ApiError(404, "Provider not found"), + }); + render( {}} />); + + // The not_found branch renders — with the "doesn't exist" copy. + const notFoundEl = document.querySelector( + '[data-testid="provider-drawer-error-not_found"]' + ); + expect(notFoundEl).not.toBeNull(); + expect(notFoundEl?.textContent).toContain("doesn't exist"); + + // And — the whole point of this test — the network branch MUST NOT + // be present. A plain `Error` from the demo-mode fallback would + // have landed here and shown "Couldn't reach the server", which + // is wrong for a known-local-miss. + expect( + document.querySelector('[data-testid="provider-drawer-error-network"]') + ).toBeNull(); + expect(document.body.textContent).not.toContain("Couldn't reach the server"); + + // not_found has no retry affordance — same invariant as the 404 + // test above, restated here so this test is self-contained as a + // regression guard. + expect(document.querySelector('[data-testid="error-retry"]')).toBeNull(); + }); + + it("test_close_button_calls_onClose", () => { + const onClose = vi.fn<() => void>(); + mockDetail({ isError: true, error: new Error("network down") }); + render(); + + const closeBtn = document.querySelector( + '[data-testid="error-close"]' + ) as HTMLButtonElement | null; + expect(closeBtn).not.toBeNull(); + fireEvent.click(closeBtn!); + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/components/ProviderDrawer/ProviderDrawer.tsx b/src/components/ProviderDrawer/ProviderDrawer.tsx index 36a99ab..6c5a920 100644 --- a/src/components/ProviderDrawer/ProviderDrawer.tsx +++ b/src/components/ProviderDrawer/ProviderDrawer.tsx @@ -1,8 +1,10 @@ import { Dialog, DialogContent } from "@/components/ui/dialog"; +import { ApiError } from "@/lib/api"; import { DrillDrawerHeader } from "@/components/drill/DrillDrawerHeader"; -import { ProviderOverview } from "./ProviderOverview"; -import { useProviderDetail } from "@/hooks/useProviderDetail"; import { Skeleton } from "@/components/ui/skeleton"; +import { useProviderDetail } from "@/hooks/useProviderDetail"; +import { ProviderOverview } from "./ProviderOverview"; +import { ProviderDrawerError } from "./ProviderDrawerError"; interface Props { npi: string | null; @@ -20,36 +22,66 @@ interface Props { * Layout mirrors the ClaimDrawer / RemitDrawer pattern: Radix Dialog * repositioned to the right edge as a fixed-height side panel, with * the shared ``DrillDrawerHeader`` on top and a scrollable body below. + * The DialogContent is the flex parent; the header and body stack + * naturally inside it without an explicit `calc(100% - 64px)` height + * that would couple to DrillDrawerHeader's padding/font sizes. + * + * Error branching mirrors the peer drawers: + * - `ApiError(404)` → "not_found" (no retry, the provider is gone) + * - anything else → "network" (retry available) + * - demo mode + unknown NPI → ApiError(404), routes to the same + * "not_found" branch (the hook raises `ApiError(404, "Provider not + * found")` in this case, matching the real backend 404 path). */ export function ProviderDrawer({ npi, onClose }: Props) { - const { data, isLoading } = useProviderDetail(npi); + const { data, isLoading, isError, error, refetch } = useProviderDetail(npi); + + const errorKind: "not_found" | "network" | null = isError + ? error instanceof ApiError && error.status === 404 + ? "not_found" + : "network" + : null; + return ( { if (!o) onClose(); }}> - {npi === null ? null : ( - <> + {errorKind ? ( + { + void refetch(); + }} + onClose={onClose} + /> + ) : isLoading || !data ? ( +
-
- {isLoading || !data ? ( -
- - - -
- ) : ( - - )} +
+ + +
- +
+ ) : ( +
+ +
+ +
+
)}
); -} \ No newline at end of file +} diff --git a/src/components/ProviderDrawer/ProviderDrawerError.tsx b/src/components/ProviderDrawer/ProviderDrawerError.tsx new file mode 100644 index 0000000..4a664f4 --- /dev/null +++ b/src/components/ProviderDrawer/ProviderDrawerError.tsx @@ -0,0 +1,70 @@ +import { AlertCircle, WifiOff } from "lucide-react"; +import { Button } from "@/components/ui/button"; + +type ProviderDrawerErrorProps = { + kind: "not_found" | "network"; + onRetry?: () => void; + onClose: () => void; +}; + +const COPY = { + not_found: { + eyebrow: "NOT FOUND", + message: "This provider doesn't exist or has been removed.", + }, + network: { + eyebrow: "CONNECTION", + message: "Couldn't reach the server. Check your connection and try again.", + }, +} as const; + +/** + * Error state for the provider drill-down drawer (SP21 Task 2.2). + * + * Two shapes — `not_found` (the NPI in the URL doesn't resolve on the + * server, e.g. a stale deep link) and `network` (the request failed). + * The not_found variant has no retry affordance (retrying won't help), + * the network variant does when `onRetry` is supplied. + * + * Visual twin of `RemitDrawerError` / `ClaimDrawerError` so the drawer + * family feels like one component family — same icon size, same eyebrow + * color, same button spacing. + */ +export function ProviderDrawerError({ + kind, + onRetry, + onClose, +}: ProviderDrawerErrorProps) { + const { eyebrow, message } = COPY[kind]; + const Icon = kind === "network" ? WifiOff : AlertCircle; + + return ( +
+
+ + + {eyebrow} + +
+

{message}

+
+ {kind === "network" && onRetry ? ( + + ) : null} + +
+
+ ); +} diff --git a/src/components/ProviderDrawer/ProviderOverview.tsx b/src/components/ProviderDrawer/ProviderOverview.tsx index 050000f..5a7ba47 100644 --- a/src/components/ProviderDrawer/ProviderOverview.tsx +++ b/src/components/ProviderDrawer/ProviderOverview.tsx @@ -40,4 +40,4 @@ function Field({ label, value, mono }: { label: string; value: string; mono?: bo
{value}
); -} \ No newline at end of file +} diff --git a/src/components/ProviderDrawer/index.ts b/src/components/ProviderDrawer/index.ts index da2ebaf..4ed6485 100644 --- a/src/components/ProviderDrawer/index.ts +++ b/src/components/ProviderDrawer/index.ts @@ -1,3 +1,3 @@ // Barrel export for the ProviderDrawer module (SP21 Task 2.2). -export { ProviderDrawer } from "./ProviderDrawer"; \ No newline at end of file +export { ProviderDrawer } from "./ProviderDrawer"; diff --git a/src/components/drill/DrillDrawerHeader.tsx b/src/components/drill/DrillDrawerHeader.tsx index 398f616..062e8ff 100644 --- a/src/components/drill/DrillDrawerHeader.tsx +++ b/src/components/drill/DrillDrawerHeader.tsx @@ -34,4 +34,4 @@ export function DrillDrawerHeader({ eyebrow, title, onClose }: Props) { ); -} \ No newline at end of file +} diff --git a/src/hooks/useProviderDetail.ts b/src/hooks/useProviderDetail.ts index d2c91fb..72994a7 100644 --- a/src/hooks/useProviderDetail.ts +++ b/src/hooks/useProviderDetail.ts @@ -1,26 +1,87 @@ import { useQuery } from "@tanstack/react-query"; -import { api } from "@/lib/api"; +import { useSyncExternalStore } from "react"; +import { api, ApiError } from "@/lib/api"; +import { useAppStore } from "@/store"; import type { Provider } from "@/types"; /** * Per-provider detail drawer query (SP21 Task 2.2). * - * Drives ``GET /api/config/providers/{npi}`` via ``api.getProvider``. - * The endpoint (added in Task 1.6) returns the full ``Provider`` shape - * with ``recent_claims`` and ``recent_activity`` arrays populated when - * the backend can serve them — Phase 3 will render those; Phase 2 only - * consumes the base fields. - * - * `npi === null` (drawer closed) disables the query so no fetch is - * issued. ``staleTime`` is 60 s — provider directory rows change - * infrequently, but we still want the drawer to refresh when a user - * reopens it across a long session. + * Twin of `useClaimDetail` — same return shape, same retry semantics, + * same demo-mode fallback story. Returns `{ data, isLoading, isError, + * error, refetch }`: + * - `npi === null` (drawer closed): the query is disabled and the + * hook short-circuits to the empty drawer state so a closed drawer + * doesn't burn a network request or a TanStack cache slot. + * - `npi` is set AND a backend is configured: fetches + * `GET /api/config/providers/{npi}` via `api.getProvider`. Cached + * 60 s — provider directory rows change infrequently, but we still + * want the drawer to refresh when a user reopens it across a long + * session. + * - On 404: the hook's retry predicate short-circuits (no retries) so + * the drawer's not-found state appears immediately rather than + * being masked by three back-to-back retry attempts. + * - `!api.isConfigured` (demo mode): no fetch is issued. The hook + * reads from the in-memory zustand store (`useAppStore.providers`). + * An unknown NPI surfaces as `isError: true` so the drawer's + * error branch handles it instead of spinning on a missing fetch. */ -export function useProviderDetail(npi: string | null) { - return useQuery({ +export function useProviderDetail(npi: string | null): { + data: Provider | null; + isLoading: boolean; + isError: boolean; + error: Error | null; + refetch: () => void; +} { + const fallback = useSyncExternalStore( + (cb) => useAppStore.subscribe(cb), + () => useAppStore.getState().providers, + () => useAppStore.getState().providers + ); + + const q = useQuery({ queryKey: ["provider-detail", npi], queryFn: () => api.getProvider(npi as string), - enabled: npi !== null, + enabled: npi !== null && api.isConfigured, staleTime: 60 * 1000, + retry: (failureCount, error) => { + if (error instanceof ApiError && error.status === 404) return false; + return failureCount < 3; + }, }); -} \ No newline at end of file + + if (!api.isConfigured) { + const provider = + npi === null ? null : fallback.find((p) => p.npi === npi) ?? null; + return { + data: provider, + isLoading: false, + isError: provider === null && npi !== null, + error: + provider === null && npi !== null + ? new ApiError(404, "Provider not found") + : null, + refetch: () => {}, + }; + } + + if (npi === null) { + return { + data: null, + isLoading: false, + isError: false, + error: null, + refetch: () => {}, + }; + } + + return { + data: q.data ?? null, + isLoading: q.isLoading, + isError: q.isError, + error: q.error, + refetch: () => { + void q.refetch(); + }, + }; +}