From 14c182ff37196c52838969cd742d00ff4a4ec64a Mon Sep 17 00:00:00 2001 From: MikeTuev Date: Sun, 12 Jul 2026 05:36:25 +0500 Subject: [PATCH] fix(dashboard): logs detail modal no longer reopens on first close (#6830) LogsPage recomputed initialId from window.location on every render, but the App Router syncs window.location only after the navigation commits. Closing the detail modal re-rendered the page while the URL still carried ?id=X, so initialSelectedId flipped null -> X and the child's one-shot deep-link effect (guard still unarmed after the open-click render, where location was stale in the other direction) reopened the modal. Only the second close worked. Read the id once via lazy useState so the prop stays stable for the page's lifetime; deep links still open the modal on mount. Regression test reproduces the App Router ordering with a router.replace mock that re-renders the page before committing the URL. --- src/app/(dashboard)/dashboard/logs/page.tsx | 10 +- ...page-detail-modal-reopen-on-close.test.tsx | 206 ++++++++++++++++++ 2 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 tests/unit/ui/logs-page-detail-modal-reopen-on-close.test.tsx diff --git a/src/app/(dashboard)/dashboard/logs/page.tsx b/src/app/(dashboard)/dashboard/logs/page.tsx index 4b88afde7e..d7051f8612 100644 --- a/src/app/(dashboard)/dashboard/logs/page.tsx +++ b/src/app/(dashboard)/dashboard/logs/page.tsx @@ -46,9 +46,13 @@ export default function LogsPage() { return () => document.removeEventListener("mousedown", handleClickOutside); }, []); - // initial id from URL (synchronously on client) so child can open on mount - const initialId = - typeof window !== "undefined" ? new URL(window.location.href).searchParams.get("id") : null; + // initial id from URL (synchronously on client) so child can open on mount. + // Read once via lazy state: window.location lags router.replace() by one + // render, so re-reading it on every render flips this prop mid-session and + // re-triggers the child's deep-link effect right when the modal closes. + const [initialId] = useState(() => + typeof window !== "undefined" ? new URL(window.location.href).searchParams.get("id") : null + ); async function handleExport(hours: number) { setExporting(true); diff --git a/tests/unit/ui/logs-page-detail-modal-reopen-on-close.test.tsx b/tests/unit/ui/logs-page-detail-modal-reopen-on-close.test.tsx new file mode 100644 index 0000000000..27d418ee46 --- /dev/null +++ b/tests/unit/ui/logs-page-detail-modal-reopen-on-close.test.tsx @@ -0,0 +1,206 @@ +// @vitest-environment jsdom +/** + * TDD regression: dashboard/logs — closing the request-detail modal after the + * FIRST row click immediately reopens it; only the second close works. + * + * Root cause: LogsPage computes `initialId` from `window.location` on EVERY + * render, but Next.js App Router syncs `window.location` only after the + * navigation commits — i.e. after the re-render triggered by + * `router.replace()`. So: + * + * 1. Row click → openDetail → router.replace("?id=X"): page re-renders while + * location is still the old URL → initialId stays null, the child's + * one-shot `initialOpenedRef` guard is never consumed. + * 2. First close → closeDetail → router.replace(no id): page re-renders while + * location STILL carries "?id=X" → initialId flips null → "X" → the child's + * deep-link effect fires (guard still unarmed) → openDetail reopens the + * modal. + * 3. Second close works because the guard is now armed. + * + * The router mock below reproduces that ordering: replace() re-renders the + * page synchronously (like the App Router segment re-render) and the URL is + * committed to window.location separately, after the render — via + * commitPendingUrl(). + */ +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const routerControl = vi.hoisted(() => ({ + pendingUrl: null as string | null, + bumpPageRender: () => {}, +})); + +vi.mock("next-intl", () => ({ + useTranslations: () => (key: string) => key, +})); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ + replace: (url: string) => { + // Mirror App Router ordering: the segment re-renders first, the URL is + // synced to window.location only after the commit (commitPendingUrl()). + routerControl.pendingUrl = url; + routerControl.bumpPageRender(); + }, + push: vi.fn(), + prefetch: vi.fn(), + refresh: vi.fn(), + }), + usePathname: () => "/dashboard/logs", + useSearchParams: () => new URLSearchParams(globalThis.location.search), +})); + +vi.mock("@/store/emailPrivacyStore", () => ({ + default: () => ({ emailsVisible: true }), +})); + +// LogsPage imports { ConfirmModal, RequestLoggerV2 } from the barrel; keep the +// real logger (the component under test) and stub the unrelated ConfirmModal +// so the test doesn't drag the whole barrel into jsdom. +vi.mock("@/shared/components", async () => { + const { default: RequestLoggerV2 } = await import( + "../../../src/shared/components/RequestLoggerV2.tsx" + ); + const ConfirmModal = ({ isOpen }: { isOpen: boolean }) => + isOpen ?
: null; + return { RequestLoggerV2, ConfirmModal }; +}); + +const { default: LogsPage } = await import( + "../../../src/app/(dashboard)/dashboard/logs/page.tsx" +); + +// Stands in for the App Router segment root: router.replace() re-renders the +// whole page tree, which is exactly what re-evaluates LogsPage's initialId. +function Harness() { + const [, setVersion] = React.useState(0); + React.useEffect(() => { + routerControl.bumpPageRender = () => setVersion((v) => v + 1); + return () => { + routerControl.bumpPageRender = () => {}; + }; + }, []); + return ; +} + +function commitPendingUrl() { + if (routerControl.pendingUrl != null) { + window.history.replaceState(null, "", routerControl.pendingUrl); + routerControl.pendingUrl = null; + } +} + +class FakeIntersectionObserver { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } +} + +const LOG_ROW = { + id: "log-1", + status: 200, + timestamp: new Date().toISOString(), + model: "gpt-4o", + provider: "openai", + account: "user@example.com", + tokens: { in: 10, out: 20 }, + duration: 1234, +}; + +let container: HTMLElement; +let root: Root; + +async function settle() { + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); +} + +beforeEach(() => { + localStorage.clear(); + routerControl.pendingUrl = null; + routerControl.bumpPageRender = () => {}; + vi.stubGlobal("IntersectionObserver", FakeIntersectionObserver); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.startsWith("/api/usage/call-logs")) { + return Response.json([LOG_ROW]); + } + if (url.startsWith("/api/logs/detail")) { + return Response.json({ enabled: false }); + } + if (url.startsWith(`/api/logs/${LOG_ROW.id}`)) { + return Response.json({ ...LOG_ROW, active: false }); + } + if (url.startsWith("/api/provider-nodes")) { + return Response.json({ nodes: [] }); + } + return Response.json({}); + }) + ); + vi.useFakeTimers(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(async () => { + await act(async () => { + root.unmount(); + }); + container.remove(); + vi.useRealTimers(); + vi.unstubAllGlobals(); + window.history.replaceState(null, "", "/dashboard/logs"); +}); + +describe("LogsPage detail modal — first-close reopen regression", () => { + it("closing the modal after the first row click keeps it closed", async () => { + window.history.replaceState(null, "", "/dashboard/logs"); + + await act(async () => { + root.render(); + }); + await settle(); + + // First click on the log row opens the detail modal. + const row = container.querySelector("tbody tr") as HTMLTableRowElement; + expect(row).not.toBeNull(); + await act(async () => { + row.click(); + }); + await settle(); + // Navigation commits after the render: URL now carries ?id=log-1. + commitPendingUrl(); + expect(window.location.search).toContain("id=log-1"); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + + // First close: closeDetail() re-renders the page while window.location + // still has ?id=log-1 (the close navigation has not committed yet). + const dialog = container.querySelector('[role="dialog"]') as HTMLElement; + await act(async () => { + dialog.click(); // backdrop click → onClose + }); + await settle(); + + // The modal must stay closed — the stale ?id in location must not reopen it. + expect(container.querySelector('[role="dialog"]')).toBeNull(); + }); + + it("deep link ?id= still opens the modal on mount", async () => { + window.history.replaceState(null, "", `/dashboard/logs?id=${LOG_ROW.id}`); + + await act(async () => { + root.render(); + }); + await settle(); + + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + }); +});