From 4996f4b69cdb2e6e575088d2a23fb19d72a05a10 Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Wed, 5 Aug 2026 15:04:02 +0200 Subject: [PATCH] fix(dashboard): only the visible page auto-refreshes (#1419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every page a user had visited kept polling, so query load scaled with browsing history instead of with what was on screen. Measured on Chart Reference, same page and same 18 visible widgets both times: only page 1 opened 18 tiles 16 POSTs / 40s after touring 6 pages 86 tiles 77 POSTs / 40s 4.81x the database traffic for an identical view. That load is refresh-tier work against the customer's database — the exact category the scheduler exists to shed — so one user browsing a large dashboard could crowd out interactive queries for everyone on that connector. `isActive` was already computed, already used two lines above for the `hidden` class and one line below to gate `onLayoutChange`. It just never gated the refresh. Pages stay mounted, which is deliberate: tab switches remain instant and do not re-query. They simply stop polling while hidden. Scope: this is part one of the issue. The unbounded mount cache — 131 tiles and 87 canvases after 11 tabs, 146.5 MB peak heap — is a separate behaviour change with a state-loss edge case around unsaved form input, and is tracked separately. The E2E was verified in both directions. Against the unfixed build it fails with "7 queries after touring vs 2 on a fresh load", on the initial run and the retry; with the fix it passes. An earlier version of it passed either way: all three pages shared one query, and use-widget-query keys on (connection, database, query, params, staleTime) with no widget id, so TanStack collapsed them into a single fetch. The pages now query distinct data, and the reason is commented in the fixture. Refs #1419 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + app/e2e/hidden-page-refresh.spec.ts | 133 ++++++++++++++++++ .../__tests__/dashboard-workspace.test.tsx | 71 ++++++++++ app/src/components/dashboard-workspace.tsx | 11 +- 4 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 app/e2e/hidden-page-refresh.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e8c7fab..33018105 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) - 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) - A client-side `groupBy` reported `count` as the number of **rows** in the group while `sum`, `avg`, `min` and `max` all skipped nulls, so the three contradicted each other: a group of `[100, 200, null]` returned `count: 3, sum: 300, avg: 150`, and `sum / count` came to 100. A reader saw "3 revenue values totalling 300" with no way to know only two rows had revenue at all — and because `count` is typically the denominator someone divides by, the error propagated into everything derived from it. Nulls in an aggregated column are not an edge case; they are what a `LEFT JOIN` produces. `count` now counts non-null values, matching SQL's `COUNT(col)` — the output key is `revenue_count`, named for the column, so the column form is the only one it could have meant. The existing test used sample data with no nulls and passed on both the wrong and the right behaviour; the new invariant test (`sum / count === avg` for every group) fails on the old code and cannot be silently re-broken (#1414) 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({