From 8032621f9a64b613e1fd076c5c3a81e6445be2f9 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 1 Oct 2026 00:44:32 +0530 Subject: [PATCH 1/3] test(canvas): lock filter scope parity between filter bar and table click --- .../pivot/pivot-click-to-filter.spec.ts | 121 +++++++++++++++++- 1 file changed, 118 insertions(+), 3 deletions(-) diff --git a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts index 75ccf300cf17..a694730f2a94 100644 --- a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts +++ b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts @@ -8,7 +8,10 @@ import { getValuesInExpression, createInExpression, } from "@rilldata/web-common/features/dashboards/stores/filter-utils"; -import type { V1Expression } from "@rilldata/web-common/runtime-client"; +import type { + V1Expression, + V1MetricsViewSpec, +} from "@rilldata/web-common/runtime-client"; import { get, writable, type Readable } from "svelte/store"; import { afterAll, @@ -24,6 +27,7 @@ import { dimKeyFromRow, } from "../../../dashboards/pivot/pivot-click-selection"; import { createPivotClickToFilter } from "./pivot-click-to-filter"; +import { DimensionFilterManager } from "@rilldata/web-common/features/dashboards/filters/dimension-filters/DimensionFilterManager.svelte.ts"; import { ExpressionFilterManager } from "@rilldata/web-common/features/dashboards/filters/ExpressionFilterManager.svelte.ts"; import type { MetricsViewsProvider } from "@rilldata/web-common/features/metrics-views/providers/MetricsViewsProvider.svelte.ts"; import { YAMLConfigProvider } from "@rilldata/web-common/features/dashboards/providers/YAMLConfigProvider.svelte.ts"; @@ -32,13 +36,31 @@ import { createTestMetricsViewsProvider, useMetricsViewMocks, } from "@rilldata/web-common/features/metrics-views/providers/test/metrics-views-test-utils.svelte.ts"; -import { PIVOT_METRICS_INIT, PIVOT_TEST_METRICS_NAME } from "./pivot-test-data"; +import { + PIVOT_COUNTRY_DIMENSION, + PIVOT_METRICS_INIT, + PIVOT_TEST_METRICS_NAME, + PIVOT_TOTAL_MEASURE, +} from "./pivot-test-data"; // --------------------------------------------------------------------------- // Shared test helpers // --------------------------------------------------------------------------- -useMetricsViewMocks({ [PIVOT_TEST_METRICS_NAME]: PIVOT_METRICS_INIT }); +// Second metrics view declaring the same dimensions, mirroring a canvas with +// two metrics views such as `duckdb_commits_metrics` and `rill_commits_metrics` +// in https://github.com/rilldata/rill/issues/9845. +const PIVOT_MIRROR_METRICS_NAME = "mv2"; +const PIVOT_METRICS_MIRROR_INIT: V1MetricsViewSpec = { + ...PIVOT_METRICS_INIT, + displayName: "PivotTest Mirror", + table: "PivotTest_Mirror_Source", +}; + +useMetricsViewMocks({ + [PIVOT_TEST_METRICS_NAME]: PIVOT_METRICS_INIT, + [PIVOT_MIRROR_METRICS_NAME]: PIVOT_METRICS_MIRROR_INIT, +}); // The specs are read-only, so a single provider serves every test in this file. // Each test still gets its own ExpressionFilterManager. @@ -1341,3 +1363,96 @@ describe("header/cell mutual exclusivity", () => { result.destroy(); }); }); + +// --------------------------------------------------------------------------- +// Canvas filter scope parity (https://github.com/rilldata/rill/issues/9845) +// +// The filter bar and a table click must write the same URL params: one chip +// for the dimension, fanned out to every metrics view that declares it. +// --------------------------------------------------------------------------- + +describe("canvas filter scope parity", () => { + let twoMvProvider: MetricsViewsProvider; + let destroyTwoMvProvider: () => void; + + beforeAll(async () => { + const provider = await createTestMetricsViewsProvider([ + PIVOT_TEST_METRICS_NAME, + PIVOT_MIRROR_METRICS_NAME, + ]); + twoMvProvider = provider.value; + destroyTwoMvProvider = provider.destroy; + }); + + afterAll(() => destroyTwoMvProvider()); + + /** A real filter manager over both metrics views, torn down after the test. */ + function createTwoMvFilterManager() { + const { value, destroy } = createInEffectRoot(() => { + return new ExpressionFilterManager( + twoMvProvider, + new YAMLConfigProvider(), + ); + }); + filterManagerCleanups.push(destroy); + return value; + } + + /** Clicks the `country` cell of a one-row pivot and returns the URL params. */ + function clickCountryCell() { + const filterManager = createTwoMvFilterManager(); + const config = makeConfig({ + rowDimensionNames: [PIVOT_COUNTRY_DIMENSION], + measureNames: [PIVOT_TOTAL_MEASURE], + isFlat: true, + }); + const data: PivotDataRow[] = [{ country: "US", total: 100 }]; + const result = createPivotClickToFilter( + createFactoryArgs({ + pivotConfig: writable(config) as Readable, + pivotDataStore: stubPivotDataStore(data), + filterManager, + activeComponent: writable("pivot-1"), + }), + ); + + result.handleCellClickToFilter( + "0", + PIVOT_COUNTRY_DIMENSION, + false, + data[0], + ); + + const searchParams = new URLSearchParams(); + filterManager.applyFilterToParams(searchParams); + result.destroy(); + return searchParams; + } + + it("table click writes the filter for every metrics view declaring the dimension", () => { + const searchParams = clickCountryCell(); + + expect(searchParams.get(`f.${PIVOT_TEST_METRICS_NAME}`)).toBe( + `${PIVOT_COUNTRY_DIMENSION} IN ('US')`, + ); + expect(searchParams.get(`f.${PIVOT_MIRROR_METRICS_NAME}`)).toBe( + `${PIVOT_COUNTRY_DIMENSION} IN ('US')`, + ); + }); + + it("filter bar writes byte-identical params for the same selection", () => { + // The filter bar edits the shared manager directly: AddExpressionFilterButton + // calls addNewFilter, then the DimensionFilter chip applies the selection. + const filterManager = createTwoMvFilterManager(); + filterManager.addNewFilter(PIVOT_COUNTRY_DIMENSION); + const dfm = filterManager.filterManagersMap[PIVOT_COUNTRY_DIMENSION]; + expect(dfm).toBeInstanceOf(DimensionFilterManager); + (dfm as DimensionFilterManager).setSelectedValues(["US"], false); + + const searchParams = new URLSearchParams(); + filterManager.applyFilterToParams(searchParams); + + expect(searchParams.toString()).toBe(clickCountryCell().toString()); + expect(filterManager.sortedFilterManagers.dimensions).toHaveLength(1); + }); +}); From e34e8c36d5adbaecb48435837bd46b4a99cd8e93 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Fri, 2 Oct 2026 08:15:16 +0530 Subject: [PATCH 2/3] test(canvas): consolidate mirror metrics view assertions into existing table tests --- .../pivot/pivot-click-to-filter.spec.ts | 144 ++++-------------- .../components/pivot/pivot-test-data.ts | 7 + 2 files changed, 40 insertions(+), 111 deletions(-) diff --git a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts index a694730f2a94..81964d69c1fa 100644 --- a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts +++ b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts @@ -8,10 +8,7 @@ import { getValuesInExpression, createInExpression, } from "@rilldata/web-common/features/dashboards/stores/filter-utils"; -import type { - V1Expression, - V1MetricsViewSpec, -} from "@rilldata/web-common/runtime-client"; +import type { V1Expression } from "@rilldata/web-common/runtime-client"; import { get, writable, type Readable } from "svelte/store"; import { afterAll, @@ -39,6 +36,8 @@ import { import { PIVOT_COUNTRY_DIMENSION, PIVOT_METRICS_INIT, + PIVOT_METRICS_MIRROR_INIT, + PIVOT_MIRROR_METRICS_NAME, PIVOT_TEST_METRICS_NAME, PIVOT_TOTAL_MEASURE, } from "./pivot-test-data"; @@ -47,16 +46,6 @@ import { // Shared test helpers // --------------------------------------------------------------------------- -// Second metrics view declaring the same dimensions, mirroring a canvas with -// two metrics views such as `duckdb_commits_metrics` and `rill_commits_metrics` -// in https://github.com/rilldata/rill/issues/9845. -const PIVOT_MIRROR_METRICS_NAME = "mv2"; -const PIVOT_METRICS_MIRROR_INIT: V1MetricsViewSpec = { - ...PIVOT_METRICS_INIT, - displayName: "PivotTest Mirror", - table: "PivotTest_Mirror_Source", -}; - useMetricsViewMocks({ [PIVOT_TEST_METRICS_NAME]: PIVOT_METRICS_INIT, [PIVOT_MIRROR_METRICS_NAME]: PIVOT_METRICS_MIRROR_INIT, @@ -71,6 +60,7 @@ const filterManagerCleanups: (() => void)[] = []; beforeAll(async () => { const provider = await createTestMetricsViewsProvider([ PIVOT_TEST_METRICS_NAME, + PIVOT_MIRROR_METRICS_NAME, ]); metricsViewsProvider = provider.value; destroyProvider = provider.destroy; @@ -102,8 +92,9 @@ function createFilterManager() { function selectedValues( fm: ExpressionFilterManager, dimensionName: string, + metricsViewName = PIVOT_TEST_METRICS_NAME, ): (string | null)[] { - const expr = fm.topLevelJoiner.expr[PIVOT_TEST_METRICS_NAME]; + const expr = fm.topLevelJoiner.expr[metricsViewName]; const dimensionExpr = expr?.cond?.exprs?.find( (e) => e.cond?.exprs?.[0]?.ident === dimensionName, ); @@ -267,15 +258,27 @@ describe("flat table: single-cell-per-row", () => { const dkRow1 = dimKeyFromRow(data[1], ["country", "city"]); it("replaces existing cell in the same row", () => { - const { result } = setup(config, data); + const { result, fm } = setup(config, data); result.handleCellClickToFilter("0", "country", false, data[0]); expect(sel(result).isCellSelected(dkRow0, "country")).toBe(true); + expect(selectedValues(fm, "country")).toEqual(["US"]); + expect(selectedValues(fm, "country", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "US", + ]); result.handleCellClickToFilter("0", "city", false, data[0]); expect(sel(result).isCellSelected(dkRow0, "country")).toBe(false); expect(sel(result).isCellSelected(dkRow0, "city")).toBe(true); expect(sel(result).cellSelections.size).toBe(1); + expect(selectedValues(fm, "city")).toEqual(["NYC"]); + expect(selectedValues(fm, "city", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "NYC", + ]); + expect(selectedValues(fm, "country")).toEqual(["US"]); + expect(selectedValues(fm, "country", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "US", + ]); result.destroy(); }); @@ -319,7 +322,7 @@ describe("nested table: multi-select", () => { const dkRow0 = dimKeyFromRow(data[0], ["country"]); it("allows multiple cells in the same row", () => { - const { result } = setup(config, data); + const { result, fm } = setup(config, data); result.handleCellClickToFilter("1", "revenue", false, data[0]); result.handleCellClickToFilter("1", "other_measure", false, data[0]); @@ -327,6 +330,10 @@ describe("nested table: multi-select", () => { expect(sel(result).isCellSelected(dkRow0, "revenue")).toBe(true); expect(sel(result).isCellSelected(dkRow0, "other_measure")).toBe(true); expect(sel(result).cellSelections.size).toBe(2); + expect(selectedValues(fm, "country")).toEqual(["US"]); + expect(selectedValues(fm, "country", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "US", + ]); result.destroy(); }); @@ -361,7 +368,7 @@ describe("nested table: cross-parent selection isolation", () => { }); it("does NOT select X under B when clicking X under A", () => { - const { result } = setup(config, data); + const { result, fm } = setup(config, data); result.handleCellClickToFilter("1.0", "revenue", false, innerRowXUnderA); @@ -378,6 +385,14 @@ describe("nested table: cross-parent selection isolation", () => { ), ).toBe(false); expect(sel(result).cellSelections.size).toBe(1); + expect(selectedValues(fm, "outer")).toEqual(["A"]); + expect(selectedValues(fm, "inner")).toEqual(["X"]); + expect(selectedValues(fm, "outer", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "A", + ]); + expect(selectedValues(fm, "inner", PIVOT_MIRROR_METRICS_NAME)).toEqual([ + "X", + ]); result.destroy(); }); @@ -1363,96 +1378,3 @@ describe("header/cell mutual exclusivity", () => { result.destroy(); }); }); - -// --------------------------------------------------------------------------- -// Canvas filter scope parity (https://github.com/rilldata/rill/issues/9845) -// -// The filter bar and a table click must write the same URL params: one chip -// for the dimension, fanned out to every metrics view that declares it. -// --------------------------------------------------------------------------- - -describe("canvas filter scope parity", () => { - let twoMvProvider: MetricsViewsProvider; - let destroyTwoMvProvider: () => void; - - beforeAll(async () => { - const provider = await createTestMetricsViewsProvider([ - PIVOT_TEST_METRICS_NAME, - PIVOT_MIRROR_METRICS_NAME, - ]); - twoMvProvider = provider.value; - destroyTwoMvProvider = provider.destroy; - }); - - afterAll(() => destroyTwoMvProvider()); - - /** A real filter manager over both metrics views, torn down after the test. */ - function createTwoMvFilterManager() { - const { value, destroy } = createInEffectRoot(() => { - return new ExpressionFilterManager( - twoMvProvider, - new YAMLConfigProvider(), - ); - }); - filterManagerCleanups.push(destroy); - return value; - } - - /** Clicks the `country` cell of a one-row pivot and returns the URL params. */ - function clickCountryCell() { - const filterManager = createTwoMvFilterManager(); - const config = makeConfig({ - rowDimensionNames: [PIVOT_COUNTRY_DIMENSION], - measureNames: [PIVOT_TOTAL_MEASURE], - isFlat: true, - }); - const data: PivotDataRow[] = [{ country: "US", total: 100 }]; - const result = createPivotClickToFilter( - createFactoryArgs({ - pivotConfig: writable(config) as Readable, - pivotDataStore: stubPivotDataStore(data), - filterManager, - activeComponent: writable("pivot-1"), - }), - ); - - result.handleCellClickToFilter( - "0", - PIVOT_COUNTRY_DIMENSION, - false, - data[0], - ); - - const searchParams = new URLSearchParams(); - filterManager.applyFilterToParams(searchParams); - result.destroy(); - return searchParams; - } - - it("table click writes the filter for every metrics view declaring the dimension", () => { - const searchParams = clickCountryCell(); - - expect(searchParams.get(`f.${PIVOT_TEST_METRICS_NAME}`)).toBe( - `${PIVOT_COUNTRY_DIMENSION} IN ('US')`, - ); - expect(searchParams.get(`f.${PIVOT_MIRROR_METRICS_NAME}`)).toBe( - `${PIVOT_COUNTRY_DIMENSION} IN ('US')`, - ); - }); - - it("filter bar writes byte-identical params for the same selection", () => { - // The filter bar edits the shared manager directly: AddExpressionFilterButton - // calls addNewFilter, then the DimensionFilter chip applies the selection. - const filterManager = createTwoMvFilterManager(); - filterManager.addNewFilter(PIVOT_COUNTRY_DIMENSION); - const dfm = filterManager.filterManagersMap[PIVOT_COUNTRY_DIMENSION]; - expect(dfm).toBeInstanceOf(DimensionFilterManager); - (dfm as DimensionFilterManager).setSelectedValues(["US"], false); - - const searchParams = new URLSearchParams(); - filterManager.applyFilterToParams(searchParams); - - expect(searchParams.toString()).toBe(clickCountryCell().toString()); - expect(filterManager.sortedFilterManagers.dimensions).toHaveLength(1); - }); -}); diff --git a/web-common/src/features/canvas/components/pivot/pivot-test-data.ts b/web-common/src/features/canvas/components/pivot/pivot-test-data.ts index 8820c6b18c8f..3d921b11b157 100644 --- a/web-common/src/features/canvas/components/pivot/pivot-test-data.ts +++ b/web-common/src/features/canvas/components/pivot/pivot-test-data.ts @@ -118,3 +118,10 @@ export const PIVOT_METRICS_INIT: V1MetricsViewSpec = { measures: PIVOT_MEASURES, dimensions: PIVOT_DIMENSIONS, }; + +export const PIVOT_MIRROR_METRICS_NAME = "mv2"; +export const PIVOT_METRICS_MIRROR_INIT: V1MetricsViewSpec = { + ...PIVOT_METRICS_INIT, + displayName: "PivotTest Mirror", + table: "PivotTest_Mirror_Source", +}; From da935135b2240986f75f3664a03fad4777832698 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Fri, 2 Oct 2026 08:16:40 +0530 Subject: [PATCH 3/3] test(canvas): drop unused imports from pivot-click-to-filter spec --- .../canvas/components/pivot/pivot-click-to-filter.spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts index 81964d69c1fa..7986eab02de8 100644 --- a/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts +++ b/web-common/src/features/canvas/components/pivot/pivot-click-to-filter.spec.ts @@ -24,7 +24,6 @@ import { dimKeyFromRow, } from "../../../dashboards/pivot/pivot-click-selection"; import { createPivotClickToFilter } from "./pivot-click-to-filter"; -import { DimensionFilterManager } from "@rilldata/web-common/features/dashboards/filters/dimension-filters/DimensionFilterManager.svelte.ts"; import { ExpressionFilterManager } from "@rilldata/web-common/features/dashboards/filters/ExpressionFilterManager.svelte.ts"; import type { MetricsViewsProvider } from "@rilldata/web-common/features/metrics-views/providers/MetricsViewsProvider.svelte.ts"; import { YAMLConfigProvider } from "@rilldata/web-common/features/dashboards/providers/YAMLConfigProvider.svelte.ts"; @@ -34,12 +33,10 @@ import { useMetricsViewMocks, } from "@rilldata/web-common/features/metrics-views/providers/test/metrics-views-test-utils.svelte.ts"; import { - PIVOT_COUNTRY_DIMENSION, PIVOT_METRICS_INIT, PIVOT_METRICS_MIRROR_INIT, PIVOT_MIRROR_METRICS_NAME, PIVOT_TEST_METRICS_NAME, - PIVOT_TOTAL_MEASURE, } from "./pivot-test-data"; // ---------------------------------------------------------------------------