From 54867996b2f6b73f3a38b030378cd784ed858359 Mon Sep 17 00:00:00 2001 From: Joshua Date: Wed, 23 Sep 2026 23:48:48 -0400 Subject: [PATCH] Task e57facc2: Keep a filter change off the silent SSE refresh path History rows now carry the (filter, start, end) they were loaded for. Only a reload for that same range keeps the rows on screen (an SSE refresh); a filter or range change always goes through loading, and a failed load shows the error state instead of the previous filter's rows. Replaces the causeless silentNext flag (review finding NQ-210-1). Co-Authored-By: Claude Opus 5.5 --- admin/src/history/HistoryTab.test.tsx | 105 +++++++++++++++++++++++++- admin/src/history/HistoryTab.tsx | 34 +++++---- 2 files changed, 123 insertions(+), 16 deletions(-) diff --git a/admin/src/history/HistoryTab.test.tsx b/admin/src/history/HistoryTab.test.tsx index 99bf331..506bd08 100644 --- a/admin/src/history/HistoryTab.test.tsx +++ b/admin/src/history/HistoryTab.test.tsx @@ -1,5 +1,5 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import HistoryTab, { FOOTER_NOTE, computeStats } from "./HistoryTab"; import type { HistoryFilter, HistoryRow } from "../api/history"; import { storeTokens } from "../auth/oidc"; @@ -525,4 +525,107 @@ describe("HistoryTab live updates", () => { expect(within(row).queryByRole("button")).toBeNull(); } }); + + describe("a filter change interleaved with a quest transition", () => { + // Both land before the load effect runs, so one load serves both. + async function interleave(order: "sse-first" | "chip-first") { + await act(async () => { + const click = () => fireEvent.click(screen.getByRole("button", { name: "Reversals" })); + if (order === "chip-first") click(); + streams[0].push(frame("nestquest_quest_uncompleted", PAYLOAD)); + await new Promise((resolve) => setTimeout(resolve, 30)); + if (order === "sse-first") click(); + }); + } + + async function renderLive() { + render( + + + , + ); + await screen.findByTestId("stat-on-time"); + await waitFor(() => expect(streams).toHaveLength(1)); + expect(screen.getAllByTestId("history-row")).toHaveLength(ROWS.all.length); + } + + function titles(): string[] { + return screen.getAllByTestId("history-row").map((el) => el.textContent ?? ""); + } + + for (const order of ["sse-first", "chip-first"] as const) { + it(`${order}: shows loading, then only the active filter's rows`, async () => { + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const route = fetchMock.getMockImplementation()!; + await renderLive(); + fetchMock.mockImplementation((url: string) => { + const parsed = new URL(url, "http://x"); + if (parsed.searchParams.get("filter") === "reversals") { + return gate.then(() => route(url)); + } + return route(url); + }); + + await interleave(order); + expect(screen.getByRole("button", { name: "Reversals" }).getAttribute("aria-pressed")).toBe("true"); + // The previous filter's rows never show under the new chip. + expect(screen.getByTestId("history-loading")).toBeTruthy(); + expect(screen.queryAllByTestId("history-row")).toHaveLength(0); + + release(); + await screen.findByTestId("stat-on-time"); + expect(titles()).toHaveLength(1); + expect(titles()[0]).toContain("Make bed · Maeve"); + expect(screen.getAllByTestId("history-dot")[0].className).toContain("history-dot--uncompleted"); + }); + + it(`${order}: a failed filter-change load shows the error state, not stale rows`, async () => { + const route = fetchMock.getMockImplementation()!; + await renderLive(); + fetchMock.mockImplementation((url: string) => { + const parsed = new URL(url, "http://x"); + if (parsed.pathname === "/api/v1/admin/history") { + return Promise.resolve(jsonResponse({ detail: "boom" }, 500)); + } + return route(url); + }); + + await interleave(order); + const error = await screen.findByTestId("history-error"); + expect(error.textContent).toContain("The history could not be loaded"); + expect(screen.getByRole("button", { name: "Reversals" }).getAttribute("aria-pressed")).toBe("true"); + expect(screen.queryAllByTestId("history-row")).toHaveLength(0); + expect(screen.queryByTestId("stat-on-time")).toBeNull(); + }); + } + }); + + it("a failed transition refresh keeps the active filter's rows on screen", async () => { + render( + + + , + ); + await screen.findByTestId("stat-on-time"); + await waitFor(() => expect(streams).toHaveLength(1)); + const route = fetchMock.getMockImplementation()!; + fetchMock.mockImplementation((url: string) => { + if (new URL(url, "http://x").pathname === "/api/v1/admin/history") { + return Promise.resolve(jsonResponse({ detail: "boom" }, 500)); + } + return route(url); + }); + const before = historyRequests().length; + + streams[0].push(frame("nestquest_quest_uncompleted", PAYLOAD)); + // The "all" filter plus the missed stats query. + await waitFor(() => expect(historyRequests().length).toBe(before + 2)); + await new Promise((resolve) => setTimeout(resolve, 30)); + expect(screen.queryByTestId("history-error")).toBeNull(); + expect(screen.queryByTestId("history-loading")).toBeNull(); + expect(screen.getAllByTestId("history-row")).toHaveLength(ROWS.all.length); + }); }); diff --git a/admin/src/history/HistoryTab.tsx b/admin/src/history/HistoryTab.tsx index a69a093..33ab53d 100644 --- a/admin/src/history/HistoryTab.tsx +++ b/admin/src/history/HistoryTab.tsx @@ -53,7 +53,8 @@ interface Loaded { type LoadState = | { kind: "loading" } | { kind: "error"; message: string } - | { kind: "ready"; data: Loaded }; + /** `key` is the (filter, start, end) the rows were loaded for. */ + | { kind: "ready"; data: Loaded; key: string }; /** * The selected filter's rows plus the period stats. Stats always come from @@ -167,15 +168,13 @@ export default function HistoryTab({ today = localTodayIso() }: { today?: string const [exporting, setExporting] = useState(false); const [exportError, setExportError] = useState(null); const exportingRef = useRef(false); - /** The next load is a refresh in place: keep the current rows, no loading state. */ - const silentNext = useRef(false); const start = shiftIso(today, -(RANGE_DAYS - 1)); + const key = `${filter}|${start}|${today}`; // A quest transition is a new history row (or changes a derived missed row); // a day-complete adds none. The refetch keeps the active filter and range. useTransitions((event: TransitionEvent) => { if (event.type === "nestquest_child_day_complete") return; - silentNext.current = true; setAttempt((n) => n + 1); }); @@ -202,12 +201,13 @@ export default function HistoryTab({ today = localTodayIso() }: { today?: string useEffect(() => { let cancelled = false; - const silent = silentNext.current; - silentNext.current = false; - if (!silent) setState({ kind: "loading" }); + // Rows already loaded for this (filter, range) stay on screen while they + // refresh; any other rows give way to the loading state. + const isCurrent = (prev: LoadState) => prev.kind === "ready" && prev.key === key; + setState((prev) => (isCurrent(prev) ? prev : { kind: "loading" })); loadHistory(filter, start, today) .then((data) => { - if (!cancelled) setState({ kind: "ready", data }); + if (!cancelled) setState({ kind: "ready", data, key }); }) .catch((error: unknown) => { if (cancelled) return; @@ -216,31 +216,35 @@ export default function HistoryTab({ today = localTodayIso() }: { today?: string ? error.message : "The history could not be loaded. Check your connection and try again."; // A failed refresh keeps the rows already on screen. - setState((prev) => (silent && prev.kind === "ready" ? prev : { kind: "error", message })); + setState((prev) => (isCurrent(prev) ? prev : { kind: "error", message })); }); return () => { cancelled = true; }; - }, [filter, start, today, attempt]); + }, [filter, start, today, key, attempt]); + + // Never render one filter's rows under another's chip, even for the render + // before the load effect runs. + const shown: LoadState = state.kind === "ready" && state.key !== key ? { kind: "loading" } : state; let body; - if (state.kind === "loading") { + if (shown.kind === "loading") { body = (
Loading history…
); - } else if (state.kind === "error") { + } else if (shown.kind === "error") { body = (
-

{state.message}

+

{shown.message}

); } else { - const groups = groupByDay(state.data.rows); + const groups = groupByDay(shown.data.rows); body = groups.length > 0 ? ( groups.map((group) => ( @@ -274,7 +278,7 @@ export default function HistoryTab({ today = localTodayIso() }: { today?: string ) : null} - {state.kind === "ready" ? : null} + {shown.kind === "ready" ? : null}
{FILTERS.map((item) => (