1db6b8841c
Quality-fix iteration on 078c9ad. Resolves the 4 Important issues
flagged in the code review while preserving everything that's working.
Issue 1 — ProviderDrawer silently swallowed error states (infinite
skeleton on any failure). Now mirrors ClaimDrawer/RemitDrawer:
- Destructure { data, isLoading, isError, error, refetch }
- Compute errorKind = ApiError(404) → not_found, else → network
- New ProviderDrawerError.tsx renders the right copy + retry/close
- Body branches on errorKind → loading → data
Issue 2 — useProviderDetail did not match the useClaimDetail contract:
- 404 retry guard (no retries on 404, <=3 on everything else)
- Explicit typed return shape { data, isLoading, isError, error,
refetch }
- npi === null short-circuit after the useQuery call
Issue 3 — Provider drilldown was non-functional in demo mode. Added
the useSyncExternalStore fallback to useAppStore.providers (same
pattern as useProviders), gated by api.isConfigured. Unknown NPI in
demo mode surfaces via the error branch instead of an infinite
skeleton.
Issue 4 — Test was shape-only (1 case). Rewrote with vi.hoisted +
vi.fn() hook mock and 7 cases covering null/loading/404/network/
close/success/hook-call-shape.
Issue 5 — Replaced h-[calc(100%-64px)] with a flex layout
(flex h-full flex-col on DialogContent, flex h-full flex-col
overflow-y-auto on inner wrappers). Mirrors RemitDrawer's pattern;
no more coupling to DrillDrawerHeader's padding/font sizes.
Issue 7 — Removed the dead npi === null ? null : (...) wrapper
inside DialogContent; Dialog open={npi !== null} already gates it.
Issue 8 — Trailing newlines added across all touched + created files.
Self-review: tsc clean on changed files; drawer test suite 211/211
passing; full test suite 369/372 (3 pre-existing Inbox/Reconciliation
failures unchanged). ProviderDrawer now 7/7.
88 lines
2.8 KiB
TypeScript
88 lines
2.8 KiB
TypeScript
import { useQuery } from "@tanstack/react-query";
|
|
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).
|
|
*
|
|
* 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): {
|
|
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<Provider>({
|
|
queryKey: ["provider-detail", npi],
|
|
queryFn: () => api.getProvider(npi as string),
|
|
enabled: npi !== null && api.isConfigured,
|
|
staleTime: 60 * 1000,
|
|
retry: (failureCount, error) => {
|
|
if (error instanceof ApiError && error.status === 404) return false;
|
|
return failureCount < 3;
|
|
},
|
|
});
|
|
|
|
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();
|
|
},
|
|
};
|
|
}
|