diff --git a/web-common/src/features/dashboards/filters/ExpressionFilterManager.spec.ts b/web-common/src/features/dashboards/filters/ExpressionFilterManager.spec.ts index 86107a23281..b8ad1ec6d99 100644 --- a/web-common/src/features/dashboards/filters/ExpressionFilterManager.spec.ts +++ b/web-common/src/features/dashboards/filters/ExpressionFilterManager.spec.ts @@ -666,6 +666,81 @@ describe("metrics view names change", () => { }); }); +describe("yaml config change", () => { + // A canvas applies its metrics views and then its config, both from the same ResolveCanvas + // response. When the metrics view specs are already loaded, applying the names parses the + // queued params right away, before the config is known. + it("adds required and pinned chips when the config lands after the params", async () => { + const provider = await createTestMetricsViewsProvider([ + AD_BIDS_METRICS_NAME, + ]); + provider.value.setMetricsViewNames([]); + const yamlConfigProvider = new YAMLConfigProvider(); + const { value: filterManager, destroy } = createInEffectRoot( + () => new ExpressionFilterManager(provider.value, yamlConfigProvider), + ); + cleanups.push(() => { + destroy(); + provider.value.cleanup(); + provider.destroy(); + }); + expect(filterManager.ready).toBe(false); + + // Queued until the specs are ready, which they are as soon as the names are applied. + filterManager.storeSync.setUrlParams(new URLSearchParams()); + provider.value.setMetricsViewNames([AD_BIDS_METRICS_NAME]); + expect(filterManager.ready).toBe(true); + expect(filterManager.sortedFilterManagers.dimensions).toEqual([]); + + yamlConfigProvider.update( + {}, + [AD_BIDS_PUBLISHER_DIMENSION, AD_BIDS_IMPRESSIONS_MEASURE], + [AD_BIDS_DOMAIN_DIMENSION], + ); + + expect(names(filterManager.sortedFilterManagers.dimensions)).toEqual([ + AD_BIDS_DOMAIN_DIMENSION, + AD_BIDS_PUBLISHER_DIMENSION, + ]); + expect(names(filterManager.sortedFilterManagers.measures)).toEqual([ + AD_BIDS_IMPRESSIONS_MEASURE, + ]); + expect(exprOf(filterManager, AD_BIDS_METRICS_NAME)).toBeUndefined(); + }); + + it("keeps the chips it has when the config is applied again", () => { + const yamlConfigProvider = new YAMLConfigProvider(); + yamlConfigProvider.update({}, [AD_BIDS_PUBLISHER_DIMENSION], []); + const filterManager = createFilterManager(yamlConfigProvider); + filterManager.storeSync.setUrlParams( + perMetricsViewParams({ + [AD_BIDS_METRICS_NAME]: `${AD_BIDS_DOMAIN_DIMENSION} IN ('google.com')`, + }), + ); + const chipsBefore = filterManager.sortedFilterManagers.dimensions; + expect(names(chipsBefore)).toEqual([ + AD_BIDS_PUBLISHER_DIMENSION, + AD_BIDS_DOMAIN_DIMENSION, + ]); + + yamlConfigProvider.update({}, [AD_BIDS_PUBLISHER_DIMENSION], []); + + const chipsAfter = filterManager.sortedFilterManagers.dimensions; + expect(chipsAfter.length).toBe(chipsBefore.length); + chipsAfter.forEach((chip, i) => expect(chip).toBe(chipsBefore[i])); + }); + + it("skips pinned filters the metrics views do not define", () => { + const yamlConfigProvider = new YAMLConfigProvider(); + const filterManager = createFilterManager(yamlConfigProvider); + + yamlConfigProvider.update({}, ["not_a_field"], []); + + expect(filterManager.sortedFilterManagers.dimensions).toEqual([]); + expect(filterManager.sortedFilterManagers.measures).toEqual([]); + }); +}); + describe("applyFilterToParams", () => { it("writes a param per metrics view and drops the legacy singular one", () => { const filterManager = createFilterManager(); diff --git a/web-common/src/features/dashboards/filters/ExpressionFilterManager.svelte.ts b/web-common/src/features/dashboards/filters/ExpressionFilterManager.svelte.ts index fd0a4682cf8..a10a32c2a4e 100644 --- a/web-common/src/features/dashboards/filters/ExpressionFilterManager.svelte.ts +++ b/web-common/src/features/dashboards/filters/ExpressionFilterManager.svelte.ts @@ -66,7 +66,7 @@ export class ExpressionFilterManager implements UrlParamsStore { public paramKeys = new Set([ExploreStateURLParams.Filters]); public readonly storeSync: UrlParamsChangeTracker; - // Unsubscribes from `metricsViewsProvider`, which can outlive this manager. See `cleanup`. + // Unsubscribes from the providers, which can outlive this manager. See `cleanup`. private readonly unsubscribers: (() => void)[]; public constructor( @@ -100,7 +100,11 @@ export class ExpressionFilterManager implements UrlParamsStore { this.ready = true; this.events.emit("ready"); }); - this.unsubscribers = [unsubUpdate, unsubSpecsLoaded]; + // The config can land after the params were parsed. See `addRequiredAndPinnedFilters`. + const unsubYamlConfig = yamlConfigProvider.on("update", () => + this.topLevelJoiner.addRequiredAndPinnedFilters(), + ); + this.unsubscribers = [unsubUpdate, unsubSpecsLoaded, unsubYamlConfig]; this.topLevelJoiner = $state( JoinerFilterManager.parse( @@ -157,8 +161,8 @@ export class ExpressionFilterManager implements UrlParamsStore { } /** - * Removes the listeners on `metricsViewsProvider`. - * Call this when the manager is discarded before the provider, e.g. a temporary `clone`. + * Removes the listeners on `metricsViewsProvider` and `yamlConfigProvider`. + * Call this when the manager is discarded before the providers, e.g. a temporary `clone`. */ public cleanup() { this.unsubscribers.forEach((unsub) => unsub()); diff --git a/web-common/src/features/dashboards/filters/JoinerFilterManager.svelte.ts b/web-common/src/features/dashboards/filters/JoinerFilterManager.svelte.ts index d378cce1002..8880ef69d70 100644 --- a/web-common/src/features/dashboards/filters/JoinerFilterManager.svelte.ts +++ b/web-common/src/features/dashboards/filters/JoinerFilterManager.svelte.ts @@ -268,6 +268,39 @@ export class JoinerFilterManager { }; } + /** + * Adds a chip for each required and pinned filter that does not have one yet. + * `parse` only adds the filters the YAML config held at the time, and the config can land after + * the params: a canvas applies its metrics views before its config, both from the same spec, + * and when the metrics view specs are already loaded the queued params are parsed in between. + */ + public addRequiredAndPinnedFilters() { + if (!this.yamlConfigProvider) return; + + Object.keys(this.yamlConfigProvider.requiredFilters) + .concat(Object.keys(this.yamlConfigProvider.pinnedFilters)) + .forEach((name) => { + if (this.managerLookup[name]) return; + + const dimensionManager = DimensionFilterManager.createForMetricsViews( + this.metricsViewsProvider, + name, + { events: this.events }, + ); + if (dimensionManager) { + this.maybeAddDimensionFilter(dimensionManager); + return; + } + + const measureManager = MeasureFilterManager.createForMetricsViews( + this.metricsViewsProvider, + name, + { events: this.events }, + ); + if (measureManager) this.maybeAddMeasureFilter(measureManager); + }); + } + public removeEmptyManagers() { this.managers = { ...this.managers, diff --git a/web-common/src/features/dashboards/providers/YAMLConfigProvider.svelte.ts b/web-common/src/features/dashboards/providers/YAMLConfigProvider.svelte.ts index 019ab396622..4530fa61e1d 100644 --- a/web-common/src/features/dashboards/providers/YAMLConfigProvider.svelte.ts +++ b/web-common/src/features/dashboards/providers/YAMLConfigProvider.svelte.ts @@ -1,4 +1,10 @@ import { type V1Expression } from "@rilldata/web-common/runtime-client"; +import { EventEmitter } from "@rilldata/web-common/lib/event-emitter.ts"; + +type YAMLConfigProviderEvents = { + /** Fired after `update` has applied the config from a spec. */ + update: void; +}; /** * A provider for YAML only configuration. These are only mutable during yaml editing. @@ -13,6 +19,11 @@ export class YAMLConfigProvider { public cleanup: (() => void) | undefined = undefined; + private events = new EventEmitter(); + public readonly on = this.events.on.bind( + this.events, + ) as typeof this.events.on; + public update( defaultFilters: Record, pinnedFilters: string[], @@ -31,6 +42,8 @@ export class YAMLConfigProvider { ); this.requiredFilters = { ...requiredFiltersRec }; this.specRequiredFilters = { ...requiredFiltersRec }; + + this.events.emit("update"); } public setEditable(newEditable: boolean) {