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 <noreply@anthropic.com>
2026-09-23 23:48:48 -04:00
parent 26f7d3a7d1
commit 54867996b2
2 changed files with 123 additions and 16 deletions
@@ -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(
+        <TransitionsProvider>
+          <HistoryTab today={TODAY} />
+        </TransitionsProvider>,
+      );
+      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<void>((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(
+      <TransitionsProvider>
+        <HistoryTab today={TODAY} />
+      </TransitionsProvider>,
+    );
+    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);
+  });
 });
@@ -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<string | null>(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 = (
      <div className="history-status" role="status" data-testid="history-loading">
        Loading history…
      </div>
    );
-  } else if (state.kind === "error") {
+  } else if (shown.kind === "error") {
    body = (
      <div className="history-status history-status--error" role="alert" data-testid="history-error">
-        <p>{state.message}</p>
+        <p>{shown.message}</p>
        <button type="button" className="history-retry" onClick={() => setAttempt((n) => n + 1)}>
          Try again
        </button>
      </div>
    );
  } 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
            </button>
          </div>
        ) : null}
-        {state.kind === "ready" ? <StatRow stats={state.data.stats} /> : null}
+        {shown.kind === "ready" ? <StatRow stats={shown.data.stats} /> : null}
        <div className="history-chips" role="group" aria-label="Filter events">
          {FILTERS.map((item) => (
            <button