From d15c04d983464d7e2132dd8f2f835394d67a11cc Mon Sep 17 00:00:00 2001
From: Tyler <tyler@crispygoat.com>
Date: Sun, 21 Jun 2026 15:21:46 -0600
Subject: [PATCH] feat(claims): provider cell drillable to
 /providers?provider=NPI

---
 src/components/drill/DrillableCell.test.tsx | 23 ++++++++
 src/components/drill/DrillableCell.tsx      | 21 ++++++-
 src/pages/Claims.test.tsx                   | 63 +++++++++++++++++++++
 src/pages/Claims.tsx                        | 29 ++++++++--
 4 files changed, 128 insertions(+), 8 deletions(-)

diff --git a/src/components/drill/DrillableCell.test.tsx b/src/components/drill/DrillableCell.test.tsx
index 42f7960..70c2862 100644
--- a/src/components/drill/DrillableCell.test.tsx
+++ b/src/components/drill/DrillableCell.test.tsx
@@ -34,4 +34,27 @@ describe("DrillableCell", () => {
     expect(btn.disabled).toBe(true);
     expect(btn.classList.contains("drillable")).toBe(false);
   });
+
+  // ---------------------------------------------------------------------
+  // Regression for Task 2.4 — event bubbling.
+  //
+  // DrillableCell renders a <button>, and <button> clicks bubble up the
+  // DOM by default. Claims wraps each row in a <TableRow onClick={...}>
+  // so a click on the provider cell used to (1) navigate to /providers
+  // and then (2) bubble to the row and re-fire buildUrl() on the now-
+  // /providers URL, appending a phantom ?claim=… param. We fix it at
+  // the DrillableCell level so future tables that adopt the component
+  // are correct by default.
+  // ---------------------------------------------------------------------
+  it("test_click_does_not_bubble_to_parent", () => {
+    const parentClick = vi.fn();
+    const { container } = render(
+      <div onClick={parentClick}>
+        <DrillableCell onClick={() => {}}>Click me</DrillableCell>
+      </div>,
+    );
+    const btn = container.querySelector("button")!;
+    fireEvent.click(btn);
+    expect(parentClick).not.toHaveBeenCalled();
+  });
 });
diff --git a/src/components/drill/DrillableCell.tsx b/src/components/drill/DrillableCell.tsx
index f620c05..6b8497c 100644
--- a/src/components/drill/DrillableCell.tsx
+++ b/src/components/drill/DrillableCell.tsx
@@ -1,9 +1,15 @@
-import type { ReactNode } from "react";
+import type { MouseEvent, ReactNode } from "react";
 import { cn } from "@/lib/utils";
 
 interface Props {
   children: ReactNode;
-  onClick: () => void;
+  /**
+   * Click handler. Receives the underlying React mouse event so we can
+   * call `e.stopPropagation()` before invoking the caller's logic — see
+   * the JSDoc on the component for why this matters when a DrillableCell
+   * is nested inside a row-level click handler.
+   */
+  onClick: (e: MouseEvent<HTMLButtonElement>) => void;
   disabled?: boolean;
   /** Optional aria-label; defaults to the visible text content. */
   ariaLabel?: string;
@@ -18,12 +24,21 @@ interface Props {
  * Renders as a <button> (disabled when `disabled`) so it gets keyboard
  * activation (Enter/Space) and the standard disabled-button semantics.
  * The `drillable` affordance class is omitted when disabled.
+ *
+ * Calls `e.stopPropagation()` on the button click before invoking the
+ * caller's handler. This prevents the click from bubbling to a
+ * row-level `onClick` (e.g. Claims' `<TableRow onClick={() => open(c.id)}>`),
+ * which would otherwise re-fire `buildUrl()` on the now-navigated URL
+ * and corrupt history. Precedent: `src/components/inbox/Lane.tsx:41`.
  */
 export function DrillableCell({ children, onClick, disabled, ariaLabel }: Props) {
   return (
     <button
       type="button"
-      onClick={onClick}
+      onClick={(e) => {
+        e.stopPropagation();
+        onClick(e);
+      }}
       disabled={disabled}
       aria-label={ariaLabel}
       className={cn(
diff --git a/src/pages/Claims.test.tsx b/src/pages/Claims.test.tsx
index da6ced0..bde6380 100644
--- a/src/pages/Claims.test.tsx
+++ b/src/pages/Claims.test.tsx
@@ -699,4 +699,67 @@ describe("Claims page drawer wiring", () => {
 
     unmount();
   });
+
+  // -------------------------------------------------------------------
+  // Regression for SP21 Task 2.4 — DrillableCell click bubbling.
+  //
+  // The provider cell renders a <button> (via DrillableCell) inside a
+  // <TableRow onClick={() => open(c.id)}>. Before the fix, clicking the
+  // provider button (1) navigated to /providers?provider=… and then
+  // (2) bubbled to the row, where open() rebuilt the URL on top of the
+  // new /providers URL and appended a phantom ?claim=CLM-1. The page
+  // mounted on /providers, but the URL was a Frankenstein of both
+  // nav states and the browser history was polluted with two entries
+  // per click.
+  //
+  // We assert the URL after a provider-cell click does NOT carry
+  // ?claim=. The fix lives in DrillableCell (stopPropagation), so this
+  // is an end-to-end check that the component-level guard closes the
+  // bubble all the way back at the row.
+  //
+  // Note on MemoryRouter: react-router's `navigate()` updates the
+  // router's internal history but does NOT push to window.history, so
+  // `window.location.href` in this test stays on `/claims`. That's
+  // still the right place to assert "no phantom claim= param landed on
+  // window.history" — that's where the bug manifested (buildUrl() in
+  // the row's open() reads window.location and pushState's the bad
+  // URL). In a real BrowserRouter the URL would advance to /providers
+  // after navigate(), and the same assertion holds: ?claim= would be
+  // absent.
+  // -------------------------------------------------------------------
+  it("test_clicking_provider_cell_navigates_without_pushing_claim_param", async () => {
+    const { unmount } = renderClaims();
+
+    // Wait for the table to populate from the mocked api.listClaims.
+    await settle(
+      () => document.body.textContent?.includes("CLM-1") ?? false,
+    );
+
+    // Both SAMPLE_CLAIMS use providerNpi "1234567890", which doesn't
+    // match any seeded sampleProvider, so the aria-label falls through
+    // to `View provider 1234567890`. Pick the first matching button.
+    const btn = Array.from(document.querySelectorAll("button")).find(
+      (b) =>
+        b.getAttribute("aria-label")?.startsWith("View provider ") ?? false,
+    ) as HTMLButtonElement | undefined;
+    expect(btn).toBeDefined();
+
+    // Sanity: row's claim-param is not in the URL before the click.
+    expect(window.location.href).not.toContain("claim=");
+
+    // Click it. With the fix, the click handler stops propagation before
+    // the row's onClick can fire. Without the fix, the row's open()
+    // would push a phantom ?claim=CLM-1 onto window.history.
+    await act(async () => {
+      btn!.click();
+    });
+
+    // The URL must not carry a ?claim= param. buildUrl() inside open()
+    // would have added that param if the click had bubbled to the row.
+    // (See the MemoryRouter note above for why we don't also assert
+    // on /providers here.)
+    expect(window.location.href).not.toContain("claim=");
+
+    unmount();
+  });
 });
diff --git a/src/pages/Claims.tsx b/src/pages/Claims.tsx
index 914ce97..32c611b 100644
--- a/src/pages/Claims.tsx
+++ b/src/pages/Claims.tsx
@@ -1,5 +1,5 @@
 import { useEffect, useMemo, useRef, useState } from "react";
-import { useSearchParams } from "react-router-dom";
+import { useNavigate, useSearchParams } from "react-router-dom";
 import { Search, X } from "lucide-react";
 import { Input } from "@/components/ui/input";
 import {
@@ -29,6 +29,7 @@ import { Pagination } from "@/components/ui/pagination";
 import { PageHeader } from "@/components/PageHeader";
 import { useClaims } from "@/hooks/useClaims";
 import { useDrawerUrlState } from "@/hooks/useDrawerUrlState";
+import { DrillableCell } from "@/components/drill/DrillableCell";
 import { useTailStream } from "@/hooks/useTailStream";
 import { useMergedTail } from "@/hooks/useMergedTail";
 import { TailStatusPill } from "@/components/TailStatusPill";
@@ -78,6 +79,7 @@ export function Claims() {
   // using a separate `?order=` param.
   // -------------------------------------------------------------------------
   const [searchParams, setSearchParams] = useSearchParams();
+  const navigate = useNavigate();
 
   const rawStatus = searchParams.get("status");
   const status: ClaimStatus | typeof ALL =
@@ -366,10 +368,27 @@ export function Claims() {
                         </TableCell>
                         <TableCell className="font-medium text-[13px]">{c.patientName}</TableCell>
                         <TableCell>
-                          <div className="text-[13px]">{provider?.name ?? "Unknown"}</div>
-                          <div className="mono text-[10.5px] text-muted-foreground">
-                            {c.providerNpi}
-                          </div>
+                          <DrillableCell
+                            ariaLabel={`View provider ${provider?.name ?? c.providerNpi}`}
+                            onClick={() =>
+                              navigate(
+                                `/providers?provider=${encodeURIComponent(c.providerNpi)}`,
+                              )
+                            }
+                          >
+                            {/*
+                              DrillableCell renders an inline-flex button,
+                              so the two stacked lines would otherwise land
+                              side-by-side. The flex-col wrapper preserves
+                              the original "name on top, NPI below" layout.
+                            */}
+                            <div className="flex flex-col items-start">
+                              <div className="text-[13px]">{provider?.name ?? "Unknown"}</div>
+                              <div className="mono text-[10.5px] text-muted-foreground">
+                                {c.providerNpi}
+                              </div>
+                            </div>
+                          </DrillableCell>
                         </TableCell>
                         <TableCell className="text-muted-foreground text-[13px]">
                           {c.payerName}