diff --git a/CHANGELOG.md b/CHANGELOG.md index 55d1b234..cc8a9982 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Chart-experience release: chart authoring, editing, rule-based styling and click ### Fixed +- Every dashboard page you had ever visited kept auto-refreshing, forever, so query load scaled with browsing history rather than with what was on screen. Measured on Chart Reference sitting on the same page with the same 18 visible widgets: 16 `/api/query` POSTs in 40s having opened only page 1, against 77 after touring six pages and returning — **4.81x the database traffic for an identical view**, ~68 hidden widgets polling for pages nobody was looking at. Tour all 20 pages and leave the tab open and roughly 230 widgets keep polling. All of it is refresh-tier work against the customer's database, which is exactly the load the query scheduler exists to shed, so one user exploring a large dashboard could crowd out interactive queries for everyone on that connector. `isActive` was already computed and already used to hide the page and to gate `onLayoutChange` — it just never gated the refresh. Visited pages still stay mounted, so tab switching remains instant and does not re-query; they simply stop polling while hidden (#1419) - Two options were advertised in the widget editor, implemented in the component library, and silently discarded in between: `trendEnabled` (single-value) and `thresholdZones` (gauge). Setting either did nothing, with no error, and the seeded reference dashboard shipped tiles demonstrating both — the `thresholdZones (3 bands)` tile was pixel-identical to `default`. The cause was not the Zod schema, which uses `.passthrough()` and preserved the keys intact; it was the plugin's explicit prop mapping, which never passed them to the chart. The worse half was the KPI: `transformToValueData` took the first column **positionally**, so following the editor's own "requires 2 rows" instruction on a `label, value` query rendered the date as the headline metric — `$2026-03`. It now picks the first numeric column and carries the previous row so a trend can be computed. A ratchet asserts that every option offered for a chart type is actually read by its plugin; it found the same bug in five more plugins, allowlisted and tracked in #1472 (#1397) - A long-format result — `category, series, value`, what `GROUP BY a, b` naturally produces — rendered a bar or line chart that looked plausible and was wrong. `resolveValueKeys` treats every non-label column as a value series, so the `series` column became a series of its own whose cells all coerced to null: 12 bars with duplicated x labels, a ghost legend swatch with no bars, and **no stacking at all** despite `stackMode` being set. Line was worse — a single revenue line drawn across duplicated x values, interleaving delivered -> shipped -> delivered, which reads as a violently spiky time series but is pure artifact. Nothing marked either as an error, and the seeded reference dashboard itself demonstrated two `stackMode` options with this exact query shape, which is how easy it is to fall into. Bar and line now reject a result whose plotted value columns contain no numeric values, naming the offending column and showing the pivot that fixes it. A column that is entirely null stays legal — that is a sparse series, what a LEFT JOIN produces. `validate` now receives the same column mapping as the transform, so a result whose text columns the user already mapped away is not wrongly rejected. The 11 affected reference tiles are rewritten to wide format, so the stacking options finally demonstrate stacking (#1400) - `numberFormat: "percent"` appended a `%` sign and scaled nothing, so a KPI computing `401 / 2000` rendered **`0.2%`** where the true share was **`20.05%`** — wrong by a factor of 100, rendered cleanly, with nothing in the UI to distinguish it from a real value. `0.2%` and `20.05%` are both believable share figures. The convention was never stated anywhere and the two option surfaces documented **opposite** contracts: the table's option was labelled `Percent (12%)`, asserting input was already 0–100, while the seeded reference tile was authored with ratio semantics — the clearest evidence the contract was not discoverable. `percent` now takes a **ratio** and scales it, via `Intl.NumberFormat`'s own `style: "percent"`, which also brings locale grouping (`12.5` → `1,250%`) that the hand-rolled branch never had. With no explicit decimal places it caps at 2 rather than Intl's default of 0, since rounding `20.05%` to `20%` is the same class of silent precision loss. **Breaking** for anyone who worked around the old behaviour by pre-multiplying by 100 in their query. Both option descriptions, both option labels, the single-value docs page and the demo tile now state the expected input range (#1396) diff --git a/app/e2e/hidden-page-refresh.spec.ts b/app/e2e/hidden-page-refresh.spec.ts new file mode 100644 index 00000000..7d430e53 --- /dev/null +++ b/app/e2e/hidden-page-refresh.spec.ts @@ -0,0 +1,133 @@ +import { test, expect, ALICE, createTestDashboard } from "./fixtures"; + +/** + * #1419 — every page you had ever visited kept auto-refreshing, so query load + * scaled with browsing history rather than with what was on screen. Measured on + * the Chart Reference dashboard: 4.81x the `/api/query` volume for the same 18 + * visible widgets after touring six pages. + * + * This is that measurement as a regression test. It counts `/api/query` POSTs + * over a fixed window while sitting on page 1, then repeats the count after + * visiting the other pages and returning to page 1. The visible view is + * identical in both windows, so the counts must be too. + */ + +const REFRESH_SECONDS = 5; +/** Long enough for two refresh ticks, short enough to keep the test bearable. */ +const SAMPLE_MS = 12_000; + +/** + * Each page must query something *different*. + * + * `use-widget-query` keys on `[connectionId, database, query, params, + * staleTime]` with no widget id, so widgets sharing a query across pages share + * one TanStack cache entry — and therefore one refetch, however many are + * mounted. An earlier version of this fixture gave every page the same query + * and passed identically with and against the fix, measuring nothing. The + * distinct `LIMIT` is what makes the pages independently pollable. + */ +function page(id: string, title: string, limit: number) { + return { + id, + title, + widgets: [ + { + id: `${id}-w1`, + chartType: "table", + connectionId: "conn-neo4j-001", + query: `MATCH (m:Movie) RETURN m.title AS title LIMIT ${limit}`, + settings: { title: `${title} widget` }, + }, + ], + gridLayout: [{ i: `${id}-w1`, x: 0, y: 0, w: 12, h: 4 }], + }; +} + +test.describe("Hidden pages do not auto-refresh (#1419)", () => { + test.beforeEach(async ({ authPage }) => { + await authPage.login(ALICE.email, ALICE.password); + }); + + test("query volume depends on the visible page, not on browsing history", async ({ + page: pw, + }) => { + test.setTimeout(120_000); + + const { id, cleanup } = await createTestDashboard( + pw.request, + `Hidden refresh ${Date.now()}`, + ); + + try { + await pw.request.put(`/api/dashboards/${id}`, { + data: { + layoutJson: { + version: 2, + pages: [ + page("p1", "One", 1), + page("p2", "Two", 2), + page("p3", "Three", 3), + ], + settings: { + autoRefresh: true, + refreshIntervalSeconds: REFRESH_SECONDS, + }, + }, + }, + }); + + // Count every widget query the browser issues, regardless of page. + let queries = 0; + pw.on("request", (req) => { + if (req.url().includes("/api/query") && req.method() === "POST") { + queries += 1; + } + }); + + await pw.goto(`/${id}`); + await expect( + pw.locator("[data-testid='widget-card']").first(), + ).toBeVisible({ + timeout: 20_000, + }); + await expect(pw.locator("table").first()).toBeVisible({ + timeout: 20_000, + }); + + // ── Baseline: only page 1 has ever been opened ──────────────────── + queries = 0; + await pw.waitForTimeout(SAMPLE_MS); + const baseline = queries; + + // Auto-refresh must actually be running, or this test proves nothing. + expect( + baseline, + "expected auto-refresh to issue queries during the baseline window", + ).toBeGreaterThan(0); + + // ── Tour the other pages, then come back to page 1 ──────────────── + for (const title of ["Two", "Three", "One"]) { + await pw.getByRole("tab", { name: title }).click(); + await pw.waitForTimeout(500); + } + await expect(pw.locator("table").first()).toBeVisible({ + timeout: 20_000, + }); + + // ── Same visible view, same window, same count ──────────────────── + queries = 0; + await pw.waitForTimeout(SAMPLE_MS); + const afterTour = queries; + + // Before the fix this was ~3x baseline with three pages mounted. Allow + // one extra tick of slack for timer alignment rather than demanding + // equality, but nothing close to a second page's worth. + expect( + afterTour, + `hidden pages are still polling: ${afterTour} queries after touring vs ${baseline} on a fresh load`, + ).toBeLessThanOrEqual(baseline + 1); + } finally { + await cleanup(); + } + }); +}); diff --git a/app/src/components/__tests__/dashboard-workspace.test.tsx b/app/src/components/__tests__/dashboard-workspace.test.tsx index 1d35d565..115c9e85 100644 --- a/app/src/components/__tests__/dashboard-workspace.test.tsx +++ b/app/src/components/__tests__/dashboard-workspace.test.tsx @@ -614,6 +614,77 @@ describe("DashboardWorkspace", () => { ).toBe("false"); }); + // #1419 — visited pages stay mounted so tab switching is instant, but they + // kept polling too. Query load scaled with browsing history rather than with + // what is on screen: 4.81x the /api/query volume for the same 18 visible + // widgets after touring six pages. + describe("hidden pages do not auto-refresh (#1419)", () => { + function withAutoRefresh() { + const d = makeDashboard(); + (d.layoutJson as unknown as { settings: unknown }).settings = { + autoRefresh: true, + refreshIntervalSeconds: 30, + }; + return d; + } + + function intervals() { + return screen + .getAllByTestId("dashboard-container") + .map((el) => el.getAttribute("data-refetch-interval")); + } + + it("only the active page carries the refresh interval", async () => { + dashboard = withAutoRefresh(); + render(); + + // Page 1 is active and alone. + expect(intervals()).toEqual(["30000"]); + + // Visiting page 2 keeps page 1 mounted — that is deliberate — but it + // must stop polling. + await userEvent.click(screen.getAllByTestId("page-tab")[1]); + expect(intervals()).toEqual(["false", "30000"]); + }); + + it("stops the previously active page when returning to the first", async () => { + dashboard = withAutoRefresh(); + render(); + + await userEvent.click(screen.getAllByTestId("page-tab")[1]); + await userEvent.click(screen.getAllByTestId("page-tab")[2]); + await userEvent.click(screen.getAllByTestId("page-tab")[0]); + + // Three pages mounted, exactly one polling. + const found = intervals(); + expect(found).toHaveLength(3); + expect(found.filter((v) => v === "30000")).toHaveLength(1); + expect(found[0]).toBe("30000"); + }); + + it("never polls a hidden page, however many are mounted", async () => { + dashboard = withAutoRefresh(); + render(); + + for (const i of [1, 2]) { + await userEvent.click(screen.getAllByTestId("page-tab")[i]); + } + + // This is the invariant the 4.8x measurement violated: polling depends on + // the visible page, not on how much of the dashboard has been explored. + expect(intervals().filter((v) => v !== "false")).toHaveLength(1); + }); + + it("keeps every page off in edit mode regardless of which is active", async () => { + dashboard = withAutoRefresh(); + pathname = "/d1/edit"; + render(); + + await userEvent.click(screen.getAllByTestId("page-tab")[1]); + expect(intervals().every((v) => v === "false")).toBe(true); + }); + }); + // ── view mode must not fetch the editor's data ────────────────────── it("does not fetch connections or widget templates in view mode", () => { render(); diff --git a/app/src/components/dashboard-workspace.tsx b/app/src/components/dashboard-workspace.tsx index 5f1e5941..d35cde6d 100644 --- a/app/src/components/dashboard-workspace.tsx +++ b/app/src/components/dashboard-workspace.tsx @@ -803,7 +803,16 @@ export function DashboardWorkspace({