Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
133 changes: 133 additions & 0 deletions app/e2e/hidden-page-refresh.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
}
});
});
71 changes: 71 additions & 0 deletions app/src/components/__tests__/dashboard-workspace.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<DashboardWorkspace id="d1" editMode={false} />);

// 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(<DashboardWorkspace id="d1" editMode={false} />);

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(<DashboardWorkspace id="d1" editMode={false} />);

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(<DashboardWorkspace id="d1" editMode={true} />);

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(<DashboardWorkspace id="d1" editMode={false} />);
Expand Down
11 changes: 10 additions & 1 deletion app/src/components/dashboard-workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,16 @@ export function DashboardWorkspace({
<DashboardContainer
page={page}
editable={editMode}
refetchInterval={editMode ? false : viewRefetchInterval}
// Only the visible page polls. Visited pages stay mounted so
// tab switches are instant and don't re-query — but keeping
// them *polling* made query load scale with browsing history
// instead of with what is on screen: 4.81x the /api/query
// volume for the same 18 visible widgets after touring six
// pages, all of it refresh-tier work against the customer's
// database for nobody's benefit (#1419).
refetchInterval={
editMode || !isActive ? false : viewRefetchInterval
}
actions={
editMode
? {
Expand Down
Loading