From ab898b31004821acdd0bd1183bdd42e74e9ee6d3 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Tue, 24 Feb 2026 15:40:24 +0200 Subject: [PATCH 01/25] per panel non applicability --- .../scenes/src/querying/SceneQueryRunner.ts | 15 +++ .../variables/DrilldownDependenciesManager.ts | 126 +++++++++++++++++- 2 files changed, 134 insertions(+), 7 deletions(-) diff --git a/packages/scenes/src/querying/SceneQueryRunner.ts b/packages/scenes/src/querying/SceneQueryRunner.ts index 412e98412..b6498a84d 100644 --- a/packages/scenes/src/querying/SceneQueryRunner.ts +++ b/packages/scenes/src/querying/SceneQueryRunner.ts @@ -425,6 +425,14 @@ export class SceneQueryRunner extends SceneObjectBase implemen }); } + /** + * Returns the per-panel applicability results computed before the last query run. + * Used by UI components (e.g. the panel sub-header) to display non-applicable filters. + */ + public getNonApplicableFilters() { + return this._drilldownDependenciesManager.getApplicabilityResults(); + } + private async runWithTimeRange(timeRange: SceneTimeRangeLike) { // If no maxDataPoints specified we might need to wait for container width to be set from the outside if (!this.state.maxDataPoints && this.state.maxDataPointsFromWidth && !this._containerWidth) { @@ -469,6 +477,13 @@ export class SceneQueryRunner extends SceneObjectBase implemen this._drilldownDependenciesManager.findAndSubscribeToDrilldowns(ds.uid); + await this._drilldownDependenciesManager.resolveApplicability( + ds, + queries, + timeRange.state.value, + sceneGraph.getScopes(this) + ); + const runRequest = getRunRequest(); const { primary, secondaries, processors } = this.prepareRequests(timeRange, ds); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index b0686857a..a0eb431fb 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -1,3 +1,10 @@ +import { + DataSourceApi, + // @ts-expect-error (temporary till we update grafana/data) + DrilldownsApplicability, + Scope, + TimeRange, +} from '@grafana/data'; import { findActiveAdHocFilterVariableByUid } from '../variables/adhoc/patchGetAdhocFilters'; import { findActiveGroupByVariablesByUid } from '../variables/groupby/findActiveGroupByVariablesByUid'; import { GroupByVariable } from '../variables/groupby/GroupByVariable'; @@ -8,7 +15,7 @@ import { isFilterComplete, } from '../variables/adhoc/AdHocFiltersVariable'; import { VariableDependencyConfig } from '../variables/VariableDependencyConfig'; -import { SceneObjectState } from '../core/types'; +import { SceneDataQuery, SceneObjectState } from '../core/types'; /** * Manages ad-hoc filters and group-by variables for data providers @@ -17,6 +24,8 @@ export class DrilldownDependenciesManager { private _adhocFiltersVar?: AdHocFiltersVariable; private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; + private _perPanelApplicability?: Map; + private _applicabilityResults?: DrilldownsApplicability[]; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -68,20 +77,123 @@ export class DrilldownDependenciesManager { return this._groupByVar; } + /** + * Resolves per-panel drilldown applicability before the data query runs. + * All gating logic lives here so SceneQueryRunner stays thin. + */ + public async resolveApplicability( + ds: DataSourceApi, + queries: SceneDataQuery[], + timeRange: TimeRange, + scopes: Scope[] | undefined + ): Promise { + if (!this._adhocFiltersVar && !this._groupByVar) { + return; + } + + const adhocEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; + const groupByEnabled = this._groupByVar?.state.applicabilityEnabled; + if (!adhocEnabled && !groupByEnabled) { + return; + } + + // @ts-expect-error (temporary till we update grafana/data) + if (!ds.getDrilldownsApplicability) { + return; + } + + const filters = adhocEnabled + ? [...this._adhocFiltersVar!.state.filters, ...(this._adhocFiltersVar!.state.originFilters ?? [])] + : []; + const groupByKeys = groupByEnabled + ? Array.isArray(this._groupByVar!.state.value) + ? this._groupByVar!.state.value.map((v) => String(v)) + : this._groupByVar!.state.value + ? [String(this._groupByVar!.state.value)] + : [] + : []; + + if (filters.length === 0 && groupByKeys.length === 0) { + this._perPanelApplicability = undefined; + this._applicabilityResults = undefined; + return; + } + + try { + // @ts-expect-error (temporary till we update grafana/data) + const results = await ds.getDrilldownsApplicability({ + filters, + groupByKeys, + queries, + timeRange, + scopes, + }); + this._setPerPanelApplicability(results); + } catch { + this._perPanelApplicability = undefined; + this._applicabilityResults = undefined; + } + } + + /** + * Returns the raw applicability results from the last resolveApplicability() call. + * Used by UI components to display which filters are non-applicable for this panel. + */ + public getApplicabilityResults(): DrilldownsApplicability[] | undefined { + return this._applicabilityResults; + } + + private _setPerPanelApplicability(results: DrilldownsApplicability[]): void { + this._applicabilityResults = results; + const map = new Map(); + results.forEach((r: DrilldownsApplicability) => { + map.set(r.origin ? `${r.key}-${r.origin}` : r.key, r); + }); + this._perPanelApplicability = map; + } + public getFilters(): AdHocFilterWithLabels[] | undefined { - return this._adhocFiltersVar - ? [...(this._adhocFiltersVar.state.originFilters ?? []), ...this._adhocFiltersVar.state.filters].filter( - (f) => isFilterComplete(f) && isFilterApplicable(f) - ) - : undefined; + if (!this._adhocFiltersVar) { + return undefined; + } + + const allFilters = [ + ...(this._adhocFiltersVar.state.originFilters ?? []), + ...this._adhocFiltersVar.state.filters, + ].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); + + if (!this._perPanelApplicability) { + return allFilters; + } + + return allFilters.filter((f) => { + const key = f.origin ? `${f.key}-${f.origin}` : f.key; + const entry = this._perPanelApplicability!.get(key); + return !entry || entry.applicable; + }); } public getGroupByKeys(): string[] | undefined { - return this._groupByVar ? this._groupByVar.getApplicableKeys() : undefined; + if (!this._groupByVar) { + return undefined; + } + + const keys = this._groupByVar.getApplicableKeys(); + + if (!this._perPanelApplicability) { + return keys; + } + + return keys.filter((k) => { + const entry = this._perPanelApplicability!.get(k); + return !entry || entry.applicable; + }); } public cleanup(): void { this._adhocFiltersVar = undefined; this._groupByVar = undefined; + this._perPanelApplicability = undefined; + this._applicabilityResults = undefined; } } From 74136388fe9ec1355fbdcde7c890cca10ef793de Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Thu, 26 Feb 2026 11:25:10 +0200 Subject: [PATCH 02/25] fix --- .../variables/DrilldownDependenciesManager.ts | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index a0eb431fb..6ef93a353 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -91,25 +91,19 @@ export class DrilldownDependenciesManager { return; } - const adhocEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; - const groupByEnabled = this._groupByVar?.state.applicabilityEnabled; - if (!adhocEnabled && !groupByEnabled) { - return; - } - // @ts-expect-error (temporary till we update grafana/data) if (!ds.getDrilldownsApplicability) { return; } - const filters = adhocEnabled - ? [...this._adhocFiltersVar!.state.filters, ...(this._adhocFiltersVar!.state.originFilters ?? [])] + const filters = this._adhocFiltersVar + ? [...this._adhocFiltersVar.state.filters, ...(this._adhocFiltersVar.state.originFilters ?? [])] : []; - const groupByKeys = groupByEnabled - ? Array.isArray(this._groupByVar!.state.value) - ? this._groupByVar!.state.value.map((v) => String(v)) - : this._groupByVar!.state.value - ? [String(this._groupByVar!.state.value)] + const groupByKeys = this._groupByVar + ? Array.isArray(this._groupByVar.state.value) + ? this._groupByVar.state.value.map((v) => String(v)) + : this._groupByVar.state.value + ? [String(this._groupByVar.state.value)] : [] : []; @@ -193,7 +187,5 @@ export class DrilldownDependenciesManager { public cleanup(): void { this._adhocFiltersVar = undefined; this._groupByVar = undefined; - this._perPanelApplicability = undefined; - this._applicabilityResults = undefined; } } From 33b4091a7899689dfc3f6047d2e4f9d841212104 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 27 Feb 2026 16:26:28 +0200 Subject: [PATCH 03/25] refactor --- packages/scenes/package.json | 7 ++- .../variables/DrilldownDependenciesManager.ts | 58 +++++++++++++------ .../variables/adhoc/AdHocFiltersVariable.tsx | 49 ++++++++++------ 3 files changed, 78 insertions(+), 36 deletions(-) diff --git a/packages/scenes/package.json b/packages/scenes/package.json index 37f8eed3a..16acfbb42 100644 --- a/packages/scenes/package.json +++ b/packages/scenes/package.json @@ -63,9 +63,14 @@ "@emotion/react": "11.10.5", "@eslint/compat": "1.3.0", "@eslint/js": "^9.28.0", + "@grafana/data": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-data", + "@grafana/e2e-selectors": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-e2e-selectors", "@grafana/eslint-config": "^8.1.0", - "@grafana/i18n": "12.1.0", + "@grafana/i18n": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-i18n", + "@grafana/runtime": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-runtime", + "@grafana/schema": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-schema", "@grafana/tsconfig": "^1.3.0-rc1", + "@grafana/ui": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-ui", "@rollup/plugin-dynamic-import-vars": "2.1.5", "@rollup/plugin-json": "^6.1.0", "@stylistic/eslint-plugin-ts": "3.1.0", diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 6ef93a353..070de7694 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -11,6 +11,7 @@ import { GroupByVariable } from '../variables/groupby/GroupByVariable'; import { AdHocFilterWithLabels, AdHocFiltersVariable, + drilldownApplicabilityKey, isFilterApplicable, isFilterComplete, } from '../variables/adhoc/AdHocFiltersVariable'; @@ -24,8 +25,8 @@ export class DrilldownDependenciesManager { private _adhocFiltersVar?: AdHocFiltersVariable; private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; - private _perPanelApplicability?: Map; private _applicabilityResults?: DrilldownsApplicability[]; + private _perPanelApplicability?: Map; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -108,8 +109,7 @@ export class DrilldownDependenciesManager { : []; if (filters.length === 0 && groupByKeys.length === 0) { - this._perPanelApplicability = undefined; - this._applicabilityResults = undefined; + this._clearPerPanelApplicability(); return; } @@ -124,8 +124,7 @@ export class DrilldownDependenciesManager { }); this._setPerPanelApplicability(results); } catch { - this._perPanelApplicability = undefined; - this._applicabilityResults = undefined; + this._clearPerPanelApplicability(); } } @@ -137,11 +136,16 @@ export class DrilldownDependenciesManager { return this._applicabilityResults; } + private _clearPerPanelApplicability(): void { + this._applicabilityResults = undefined; + this._perPanelApplicability = undefined; + } + private _setPerPanelApplicability(results: DrilldownsApplicability[]): void { this._applicabilityResults = results; const map = new Map(); results.forEach((r: DrilldownsApplicability) => { - map.set(r.origin ? `${r.key}-${r.origin}` : r.key, r); + map.set(drilldownApplicabilityKey(r), r); }); this._perPanelApplicability = map; } @@ -151,20 +155,27 @@ export class DrilldownDependenciesManager { return undefined; } - const allFilters = [ - ...(this._adhocFiltersVar.state.originFilters ?? []), - ...this._adhocFiltersVar.state.filters, - ].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); + const stateFilters = this._adhocFiltersVar.state.filters; + const originFilters = this._adhocFiltersVar.state.originFilters ?? []; + + // Reconstruct sent indices: resolveApplicability sends [...stateFilters, ...originFilters] + const allWithIndex: Array<{ filter: AdHocFilterWithLabels; sentIndex: number }> = [ + ...originFilters.map((f, i) => ({ filter: f, sentIndex: stateFilters.length + i })), + ...stateFilters.map((f, i) => ({ filter: f, sentIndex: i })), + ].filter(({ filter: f }) => isFilterComplete(f) && isFilterApplicable(f)); if (!this._perPanelApplicability) { - return allFilters; + return allWithIndex.map(({ filter }) => filter); } - return allFilters.filter((f) => { - const key = f.origin ? `${f.key}-${f.origin}` : f.key; - const entry = this._perPanelApplicability!.get(key); - return !entry || entry.applicable; - }); + return allWithIndex + .filter(({ filter: f, sentIndex }) => { + const entry = this._perPanelApplicability!.get( + drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: sentIndex }) + ); + return !entry || entry.applicable; + }) + .map(({ filter }) => filter); } public getGroupByKeys(): string[] | undefined { @@ -178,8 +189,21 @@ export class DrilldownDependenciesManager { return keys; } + // Rebuild the full sent groupByKeys to find each key's sent position + const val = this._groupByVar.state.value; + const allGroupByKeys = Array.isArray(val) ? val.map(String) : val ? [String(val)] : []; + const filtersCount = this._adhocFiltersVar + ? this._adhocFiltersVar.state.filters.length + (this._adhocFiltersVar.state.originFilters?.length ?? 0) + : 0; + return keys.filter((k) => { - const entry = this._perPanelApplicability!.get(k); + const sentIdx = allGroupByKeys.indexOf(k); + if (sentIdx === -1) { + return true; + } + const entry = this._perPanelApplicability!.get( + drilldownApplicabilityKey({ key: k, index: filtersCount + sentIdx }) + ); return !entry || entry.applicable; }); } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index b382d126c..f5a167da9 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -737,8 +737,8 @@ export class AdHocFiltersVariable } const responseMap = new Map(); - response.forEach((filter: DrilldownsApplicability) => { - responseMap.set(`${filter.key}${filter.origin ? `-${filter.origin}` : ''}`, filter); + response.forEach((r: DrilldownsApplicability) => { + responseMap.set(drilldownApplicabilityKey(r), r); }); const update = { @@ -747,28 +747,30 @@ export class AdHocFiltersVariable originFilters: [...(this.state.originFilters ?? [])], }; - update.filters.forEach((f) => { - const filter = responseMap.get(f.key); + const filtersCount = update.filters.length; - if (filter) { - f.nonApplicable = !filter.applicable; - f.nonApplicableReason = filter.reason; + update.filters.forEach((f, i) => { + const result = responseMap.get(drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: i })); + if (result) { + f.nonApplicable = !result.applicable; + f.nonApplicableReason = result.reason; } }); - update.originFilters?.forEach((f) => { - const filter = responseMap.get(`${f.key}-${f.origin}`); - - if (filter) { + update.originFilters?.forEach((f, i) => { + const result = responseMap.get( + drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: filtersCount + i }) + ); + if (result) { if (!f.matchAllFilter) { - f.nonApplicable = !filter.applicable; - f.nonApplicableReason = filter.reason; + f.nonApplicable = !result.applicable; + f.nonApplicableReason = result.reason; } const originalValue = this._originalValues.get(`${f.key}-${f.origin}`); if (originalValue) { - originalValue.nonApplicable = !filter.applicable; - originalValue.nonApplicableReason = filter?.reason; + originalValue.nonApplicable = !result.applicable; + originalValue.nonApplicableReason = result?.reason; } } }); @@ -843,9 +845,10 @@ export class AdHocFiltersVariable return []; } - const originFilters = this.state.originFilters?.filter((f) => f.key !== filter.key) ?? []; - // Filter out the current filter key from the list of all filters - const otherFilters = this.state.filters.filter((f) => f.key !== filter.key).concat(originFilters); + const originFilters = this.state.originFilters?.filter((f) => f.key !== filter.key && !f.nonApplicable) ?? []; + const otherFilters = this.state.filters + .filter((f) => f.key !== filter.key && !f.nonApplicable) + .concat(originFilters); const timeRange = sceneGraph.getTimeRange(this).state.value; const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; @@ -990,6 +993,16 @@ export function isFilterApplicable(filter: AdHocFilterWithLabels): boolean { return !filter.nonApplicable; } +/** + * Builds a unique composite key for matching DrilldownsApplicability results back + * to their source filters. Uses key + origin + index so duplicate keys + * (e.g. cluster=dev1, cluster=dev2) resolve to distinct entries. The index + * represents the filter's position in the input array sent to getDrilldownsApplicability. + */ +export function drilldownApplicabilityKey(entry: { key: string; origin?: string; index?: number }): string { + return `${entry.key}${entry.origin ? `-${entry.origin}` : ''}-${entry.index ?? ''}`; +} + export function isMultiValueOperator(operatorValue: string): boolean { const operator = OPERATORS.find((o) => o.value === operatorValue); if (!operator) { From bea96d71db4852efa3e16b5e0edb16476250a96f Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 27 Feb 2026 16:29:40 +0200 Subject: [PATCH 04/25] refactor --- packages/scenes/package.json | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/scenes/package.json b/packages/scenes/package.json index 16acfbb42..37f8eed3a 100644 --- a/packages/scenes/package.json +++ b/packages/scenes/package.json @@ -63,14 +63,9 @@ "@emotion/react": "11.10.5", "@eslint/compat": "1.3.0", "@eslint/js": "^9.28.0", - "@grafana/data": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-data", - "@grafana/e2e-selectors": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-e2e-selectors", "@grafana/eslint-config": "^8.1.0", - "@grafana/i18n": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-i18n", - "@grafana/runtime": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-runtime", - "@grafana/schema": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-schema", + "@grafana/i18n": "12.1.0", "@grafana/tsconfig": "^1.3.0-rc1", - "@grafana/ui": "link:/Users/mdvictor/Workspace/grafana/packages/grafana-ui", "@rollup/plugin-dynamic-import-vars": "2.1.5", "@rollup/plugin-json": "^6.1.0", "@stylistic/eslint-plugin-ts": "3.1.0", From 3f5d04a6bc72ab3c121f1c1127dfa8aee13a62c5 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 27 Feb 2026 18:45:58 +0200 Subject: [PATCH 05/25] refactor --- .../variables/DrilldownDependenciesManager.ts | 31 +++++++------------ .../variables/adhoc/AdHocFiltersVariable.tsx | 27 +++------------- 2 files changed, 17 insertions(+), 41 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 349c77c68..3fa9c39ce 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -17,7 +17,6 @@ import { GroupByVariable } from '../variables/groupby/GroupByVariable'; import { AdHocFilterWithLabels, AdHocFiltersVariable, - drilldownApplicabilityKey, isFilterApplicable, isFilterComplete, } from '../variables/adhoc/AdHocFiltersVariable'; @@ -32,7 +31,6 @@ export class DrilldownDependenciesManager { private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; private _applicabilityResults?: DrilldownsApplicability[]; - private _perPanelApplicability?: Map; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -150,16 +148,10 @@ export class DrilldownDependenciesManager { private _clearPerPanelApplicability(): void { this._applicabilityResults = undefined; - this._perPanelApplicability = undefined; } private _setPerPanelApplicability(results: DrilldownsApplicability[]): void { this._applicabilityResults = results; - const map = new Map(); - results.forEach((r: DrilldownsApplicability) => { - map.set(drilldownApplicabilityKey(r), r); - }); - this._perPanelApplicability = map; } public getFilters(): AdHocFilterWithLabels[] | undefined { @@ -176,16 +168,16 @@ export class DrilldownDependenciesManager { ...stateFilters.map((f, i) => ({ filter: f, sentIndex: i })), ].filter(({ filter: f }) => isFilterComplete(f) && isFilterApplicable(f)); - if (!this._perPanelApplicability) { + if (!this._applicabilityResults) { return allWithIndex.map(({ filter }) => filter); } return allWithIndex - .filter(({ filter: f, sentIndex }) => { - const entry = this._perPanelApplicability!.get( - drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: sentIndex }) - ); - return !entry || entry.applicable; + .filter(({ sentIndex }) => { + if (sentIndex >= this._applicabilityResults!.length) { + return true; + } + return this._applicabilityResults![sentIndex].applicable; }) .map(({ filter }) => filter); } @@ -197,7 +189,7 @@ export class DrilldownDependenciesManager { const keys = this._groupByVar.getApplicableKeys(); - if (!this._perPanelApplicability) { + if (!this._applicabilityResults) { return keys; } @@ -213,10 +205,11 @@ export class DrilldownDependenciesManager { if (sentIdx === -1) { return true; } - const entry = this._perPanelApplicability!.get( - drilldownApplicabilityKey({ key: k, index: filtersCount + sentIdx }) - ); - return !entry || entry.applicable; + const resultIdx = filtersCount + sentIdx; + if (resultIdx >= this._applicabilityResults!.length) { + return true; + } + return this._applicabilityResults![resultIdx].applicable; }); } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 07969b032..0947076f4 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -744,11 +744,6 @@ export class AdHocFiltersVariable return; } - const responseMap = new Map(); - response.forEach((r: DrilldownsApplicability) => { - responseMap.set(drilldownApplicabilityKey(r), r); - }); - const update = { applicabilityEnabled: true, filters: [...this.state.filters], @@ -758,17 +753,15 @@ export class AdHocFiltersVariable const filtersCount = update.filters.length; update.filters.forEach((f, i) => { - const result = responseMap.get(drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: i })); - if (result) { - f.nonApplicable = !result.applicable; - f.nonApplicableReason = result.reason; + if (i < response.length) { + f.nonApplicable = !response[i].applicable; + f.nonApplicableReason = response[i].reason; } }); update.originFilters?.forEach((f, i) => { - const result = responseMap.get( - drilldownApplicabilityKey({ key: f.key, origin: f.origin, index: filtersCount + i }) - ); + const responseIdx = filtersCount + i; + const result = responseIdx < response.length ? response[responseIdx] : undefined; if (result) { if (!f.matchAllFilter) { f.nonApplicable = !result.applicable; @@ -1001,16 +994,6 @@ export function isFilterApplicable(filter: AdHocFilterWithLabels): boolean { return !filter.nonApplicable; } -/** - * Builds a unique composite key for matching DrilldownsApplicability results back - * to their source filters. Uses key + origin + index so duplicate keys - * (e.g. cluster=dev1, cluster=dev2) resolve to distinct entries. The index - * represents the filter's position in the input array sent to getDrilldownsApplicability. - */ -export function drilldownApplicabilityKey(entry: { key: string; origin?: string; index?: number }): string { - return `${entry.key}${entry.origin ? `-${entry.origin}` : ''}-${entry.index ?? ''}`; -} - export function isMultiValueOperator(operatorValue: string): boolean { const operator = OPERATORS.find((o) => o.value === operatorValue); if (!operator) { From 25b514ed510cec7ec4b0f85b431b8139c327f95e Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 2 Mar 2026 19:09:09 +0200 Subject: [PATCH 06/25] fix + tests --- packages/scenes/src/index.ts | 2 + .../DrilldownDependenciesManager.test.ts | 389 ++++++++++++++++++ .../variables/DrilldownDependenciesManager.ts | 62 ++- .../variables/adhoc/AdHocFiltersVariable.tsx | 21 +- .../src/variables/applicabilityUtils.test.ts | 130 ++++++ .../src/variables/applicabilityUtils.ts | 36 ++ 6 files changed, 596 insertions(+), 44 deletions(-) create mode 100644 packages/scenes/src/variables/DrilldownDependenciesManager.test.ts create mode 100644 packages/scenes/src/variables/applicabilityUtils.test.ts create mode 100644 packages/scenes/src/variables/applicabilityUtils.ts diff --git a/packages/scenes/src/index.ts b/packages/scenes/src/index.ts index e94bc3378..04d4bbdf1 100644 --- a/packages/scenes/src/index.ts +++ b/packages/scenes/src/index.ts @@ -41,6 +41,8 @@ export { SceneTimeZoneOverride } from './core/SceneTimeZoneOverride'; export { SceneQueryRunner, type QueryRunnerState } from './querying/SceneQueryRunner'; export { DataProviderProxy } from './querying/DataProviderProxy'; +export { type ApplicabilityResults } from './variables/DrilldownDependenciesManager'; +export { buildApplicabilityMatcher } from './variables/applicabilityUtils'; export { type ExtraQueryDescriptor, type ExtraQueryProvider, diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts new file mode 100644 index 000000000..841c84558 --- /dev/null +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -0,0 +1,389 @@ +import { DataSourceApi, getDefaultTimeRange } from '@grafana/data'; + +import { AdHocFiltersVariable, AdHocFilterWithLabels } from './adhoc/AdHocFiltersVariable'; +import { DrilldownDependenciesManager } from './DrilldownDependenciesManager'; +import { GroupByVariable } from './groupby/GroupByVariable'; +import { VariableDependencyConfig } from './VariableDependencyConfig'; + +function createManager(opts: { adhocVar?: AdHocFiltersVariable; groupByVar?: GroupByVariable }) { + const mockDependencyConfig = { setVariableNames: jest.fn() } as unknown as VariableDependencyConfig; + const manager = new DrilldownDependenciesManager(mockDependencyConfig); + + if (opts.adhocVar) { + manager['_adhocFiltersVar'] = opts.adhocVar; + } + if (opts.groupByVar) { + manager['_groupByVar'] = opts.groupByVar; + } + + return manager; +} + +function createAdhocVar(filters: AdHocFilterWithLabels[], originFilters?: AdHocFilterWithLabels[]) { + return new AdHocFiltersVariable({ + datasource: { uid: 'my-ds-uid' }, + name: 'filters', + filters, + originFilters, + }); +} + +function createGroupByVar(value: string[], keysApplicability?: any[]) { + return new GroupByVariable({ + datasource: { uid: 'my-ds-uid' }, + name: 'groupby', + key: 'testGroupBy', + value, + text: value, + keysApplicability, + }); +} + +describe('DrilldownDependenciesManager', () => { + describe('resolveApplicability', () => { + it('should split response into filter and groupBy portions', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'not found' }, + { key: 'region', applicable: true, origin: 'dashboard' }, + { key: 'namespace', applicable: true }, + { key: 'pod', applicable: false, reason: 'label not found' }, + ]); + + const manager = createManager({ + adhocVar: createAdhocVar( + [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us-east', operator: '=' }, + ], + [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }] + ), + groupByVar: createGroupByVar(['namespace', 'pod']), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + const results = manager.getApplicabilityResults(); + expect(results?.filters).toEqual([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'not found' }, + { key: 'region', applicable: true, origin: 'dashboard' }, + ]); + expect(results?.groupBy).toEqual([ + { key: 'namespace', applicable: true }, + { key: 'pod', applicable: false, reason: 'label not found' }, + ]); + }); + + it('should clear results when there are no filters or groupBy keys', async () => { + const getDrilldownsApplicability = jest.fn(); + + const manager = createManager({ + adhocVar: createAdhocVar([]), + groupByVar: createGroupByVar([]), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + expect(manager.getApplicabilityResults()).toBeUndefined(); + expect(getDrilldownsApplicability).not.toHaveBeenCalled(); + }); + + it('should clear results when ds call throws', async () => { + const getDrilldownsApplicability = jest.fn().mockRejectedValue(new Error('fail')); + + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + expect(manager.getApplicabilityResults()).toBeUndefined(); + }); + + it('should skip when ds does not support getDrilldownsApplicability', async () => { + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), + }); + + const ds = {} as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + expect(manager.getApplicabilityResults()).toBeUndefined(); + }); + }); + + describe('getFilters', () => { + it('should return undefined when no adhocFiltersVar', () => { + const manager = createManager({}); + expect(manager.getFilters()).toBeUndefined(); + }); + + it('should return all complete filters when no applicability results', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(2); + expect(result[0].key).toBe('env'); + expect(result[1].key).toBe('cluster'); + }); + + it('should exclude incomplete filters', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: '', value: '', operator: '' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(1); + expect(result[0].key).toBe('env'); + }); + + it('should exclude variable-level nonApplicable filters', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'pod', value: 'abc', operator: '=', nonApplicable: true }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(1); + expect(result[0].key).toBe('env'); + }); + + it('should exclude per-panel nonApplicable filters using key+origin matching', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ]; + const originFilters: AdHocFilterWithLabels[] = [ + { key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters, originFilters) }); + + manager['_applicabilityResults'] = { + filters: [ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'not found' }, + { key: 'region', applicable: true, origin: 'dashboard' }, + ], + groupBy: [], + }; + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(2); + expect(result[0].key).toBe('env'); + expect(result[1].key).toBe('region'); + }); + + it('should keep filters when no matching response entry exists', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'extra', value: 'val', operator: '=' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + manager['_applicabilityResults'] = { + filters: [{ key: 'env', applicable: true }], + groupBy: [], + }; + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(2); + expect(result[0].key).toBe('env'); + expect(result[1].key).toBe('extra'); + }); + + it('should handle duplicate keys by matching positionally within the queue', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'env', value: 'staging', operator: '=' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + manager['_applicabilityResults'] = { + filters: [ + { key: 'env', applicable: true }, + { key: 'env', applicable: false, reason: 'value not found' }, + ], + groupBy: [], + }; + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(1); + expect(result[0].value).toBe('prod'); + }); + + it('should separate user filters from origin filters with same key via origin', () => { + const filters: AdHocFilterWithLabels[] = [{ key: 'env', value: 'prod', operator: '=' }]; + const originFilters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'staging', operator: '=', origin: 'dashboard' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters, originFilters) }); + + manager['_applicabilityResults'] = { + filters: [ + { key: 'env', applicable: false, reason: 'user filter overridden' }, + { key: 'env', applicable: true, origin: 'dashboard' }, + ], + groupBy: [], + }; + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(1); + expect(result[0].origin).toBe('dashboard'); + }); + }); + + describe('getGroupByKeys', () => { + it('should return undefined when no groupByVar', () => { + const manager = createManager({}); + expect(manager.getGroupByKeys()).toBeUndefined(); + }); + + it('should return all keys when no applicability results', () => { + const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod']) }); + expect(manager.getGroupByKeys()).toEqual(['ns', 'pod']); + }); + + it('should exclude per-panel nonApplicable groupBy keys', () => { + const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod', 'container']) }); + + manager['_applicabilityResults'] = { + filters: [], + groupBy: [ + { key: 'ns', applicable: true }, + { key: 'pod', applicable: false, reason: 'label not found' }, + { key: 'container', applicable: true }, + ], + }; + + expect(manager.getGroupByKeys()).toEqual(['ns', 'container']); + }); + + it('should keep keys with no matching response entry', () => { + const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod']) }); + + manager['_applicabilityResults'] = { + filters: [], + groupBy: [{ key: 'ns', applicable: true }], + }; + + expect(manager.getGroupByKeys()).toEqual(['ns', 'pod']); + }); + + it('should respect variable-level keysApplicability then per-panel results', () => { + const manager = createManager({ + groupByVar: createGroupByVar( + ['ns', 'pod', 'container'], + [ + { key: 'ns', applicable: true }, + { key: 'pod', applicable: false }, + { key: 'container', applicable: true }, + ] + ), + }); + + manager['_applicabilityResults'] = { + filters: [], + groupBy: [ + { key: 'ns', applicable: false, reason: 'per-panel not found' }, + { key: 'container', applicable: true }, + ], + }; + + // 'pod' excluded by variable-level keysApplicability + // 'ns' further excluded by per-panel result + expect(manager.getGroupByKeys()).toEqual(['container']); + }); + + it('should not cross-contaminate filter and groupBy results for same key', () => { + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), + groupByVar: createGroupByVar(['env']), + }); + + manager['_applicabilityResults'] = { + filters: [{ key: 'env', applicable: true }], + groupBy: [{ key: 'env', applicable: false, reason: 'groupBy env not found' }], + }; + + const filters = manager.getFilters() ?? []; + expect(filters).toHaveLength(1); + expect(filters[0].key).toBe('env'); + expect(manager.getGroupByKeys()).toEqual([]); + }); + }); + + describe('combined filters and groupBy', () => { + it('should resolve and return both filters and groupBy keys through the full flow', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'label not found' }, + { key: 'region', applicable: true, origin: 'dashboard' }, + { key: 'namespace', applicable: true }, + { key: 'pod', applicable: false, reason: 'label not found' }, + ]); + + const manager = createManager({ + adhocVar: createAdhocVar( + [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ], + [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }] + ), + groupByVar: createGroupByVar(['namespace', 'pod']), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + const filters = manager.getFilters() ?? []; + expect(filters).toHaveLength(2); + expect(filters[0].key).toBe('env'); + expect(filters[1].key).toBe('region'); + + expect(manager.getGroupByKeys()).toEqual(['namespace']); + }); + + it('should handle filters-only response when both variables exist', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'overridden' }, + ]); + + const manager = createManager({ + adhocVar: createAdhocVar([ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ]), + groupByVar: createGroupByVar([]), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + const filters = manager.getFilters() ?? []; + expect(filters).toHaveLength(1); + expect(filters[0].key).toBe('env'); + + expect(manager.getGroupByKeys()).toEqual([]); + }); + }); +}); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 3fa9c39ce..5fc11efa3 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -22,6 +22,12 @@ import { } from '../variables/adhoc/AdHocFiltersVariable'; import { VariableDependencyConfig } from '../variables/VariableDependencyConfig'; import { SceneDataQuery, SceneObject, SceneObjectState } from '../core/types'; +import { buildApplicabilityMatcher } from '../variables/applicabilityUtils'; + +export interface ApplicabilityResults { + filters: DrilldownsApplicability[]; + groupBy: DrilldownsApplicability[]; +} /** * Manages ad-hoc filters and group-by variables for data providers @@ -30,7 +36,7 @@ export class DrilldownDependenciesManager { private _adhocFiltersVar?: AdHocFiltersVariable; private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; - private _applicabilityResults?: DrilldownsApplicability[]; + private _applicabilityResults?: ApplicabilityResults; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -125,24 +131,28 @@ export class DrilldownDependenciesManager { try { // @ts-expect-error (temporary till we update grafana/data) - const results = await ds.getDrilldownsApplicability({ + const results: DrilldownsApplicability[] = await ds.getDrilldownsApplicability({ filters, groupByKeys, queries, timeRange, scopes, }); - this._setPerPanelApplicability(results); + this._setPerPanelApplicability({ + filters: results.slice(0, filters.length), + groupBy: results.slice(filters.length), + }); } catch { this._clearPerPanelApplicability(); } } /** - * Returns the raw applicability results from the last resolveApplicability() call. + * Returns the applicability results from the last resolveApplicability() call, + * split into filter and groupBy portions. * Used by UI components to display which filters are non-applicable for this panel. */ - public getApplicabilityResults(): DrilldownsApplicability[] | undefined { + public getApplicabilityResults(): ApplicabilityResults | undefined { return this._applicabilityResults; } @@ -150,7 +160,7 @@ export class DrilldownDependenciesManager { this._applicabilityResults = undefined; } - private _setPerPanelApplicability(results: DrilldownsApplicability[]): void { + private _setPerPanelApplicability(results: ApplicabilityResults): void { this._applicabilityResults = results; } @@ -162,24 +172,18 @@ export class DrilldownDependenciesManager { const stateFilters = this._adhocFiltersVar.state.filters; const originFilters = this._adhocFiltersVar.state.originFilters ?? []; - // Reconstruct sent indices: resolveApplicability sends [...stateFilters, ...originFilters] - const allWithIndex: Array<{ filter: AdHocFilterWithLabels; sentIndex: number }> = [ - ...originFilters.map((f, i) => ({ filter: f, sentIndex: stateFilters.length + i })), - ...stateFilters.map((f, i) => ({ filter: f, sentIndex: i })), - ].filter(({ filter: f }) => isFilterComplete(f) && isFilterApplicable(f)); + const applicable = [...stateFilters, ...originFilters].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); if (!this._applicabilityResults) { - return allWithIndex.map(({ filter }) => filter); + return applicable; } - return allWithIndex - .filter(({ sentIndex }) => { - if (sentIndex >= this._applicabilityResults!.length) { - return true; - } - return this._applicabilityResults![sentIndex].applicable; - }) - .map(({ filter }) => filter); + const matchResult = buildApplicabilityMatcher(this._applicabilityResults.filters); + + return applicable.filter((f) => { + const result = matchResult(f.key, f.origin); + return !result || result.applicable; + }); } public getGroupByKeys(): string[] | undefined { @@ -193,23 +197,11 @@ export class DrilldownDependenciesManager { return keys; } - // Rebuild the full sent groupByKeys to find each key's sent position - const val = this._groupByVar.state.value; - const allGroupByKeys = Array.isArray(val) ? val.map(String) : val ? [String(val)] : []; - const filtersCount = this._adhocFiltersVar - ? this._adhocFiltersVar.state.filters.length + (this._adhocFiltersVar.state.originFilters?.length ?? 0) - : 0; + const matchResult = buildApplicabilityMatcher(this._applicabilityResults.groupBy); return keys.filter((k) => { - const sentIdx = allGroupByKeys.indexOf(k); - if (sentIdx === -1) { - return true; - } - const resultIdx = filtersCount + sentIdx; - if (resultIdx >= this._applicabilityResults!.length) { - return true; - } - return this._applicabilityResults![resultIdx].applicable; + const result = matchResult(k); + return !result || result.applicable; }); } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 0947076f4..5e97d7849 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -32,6 +32,7 @@ import { getEnrichedFiltersRequest } from '../getEnrichedFiltersRequest'; import { AdHocFiltersComboboxRenderer } from './AdHocFiltersCombobox/AdHocFiltersComboboxRenderer'; import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { debounce, isEqual } from 'lodash'; +import { buildApplicabilityMatcher } from '../applicabilityUtils'; import { getAdHocFiltersFromScopes } from './getAdHocFiltersFromScopes'; import { VariableDependencyConfig } from '../VariableDependencyConfig'; import { getQueryController } from '../../core/sceneGraph/getQueryController'; @@ -586,6 +587,7 @@ export class AdHocFiltersVariable return f === filter ? { ...f, ...update } : f; }) ?? []; this.setState({ originFilters: updatedFilters }); + this._debouncedVerifyApplicability(); return; } @@ -609,6 +611,7 @@ export class AdHocFiltersVariable }); this.setState({ filters: updatedFilters }); + this._debouncedVerifyApplicability(); this._recommendations?.storeRecentFilter({ ...filter, @@ -744,24 +747,24 @@ export class AdHocFiltersVariable return; } + const matchResult = buildApplicabilityMatcher(response); + const update = { applicabilityEnabled: true, filters: [...this.state.filters], originFilters: [...(this.state.originFilters ?? [])], }; - const filtersCount = update.filters.length; - - update.filters.forEach((f, i) => { - if (i < response.length) { - f.nonApplicable = !response[i].applicable; - f.nonApplicableReason = response[i].reason; + update.filters.forEach((f) => { + const result = matchResult(f.key); + if (result) { + f.nonApplicable = !result.applicable; + f.nonApplicableReason = result.reason; } }); - update.originFilters?.forEach((f, i) => { - const responseIdx = filtersCount + i; - const result = responseIdx < response.length ? response[responseIdx] : undefined; + update.originFilters?.forEach((f) => { + const result = matchResult(f.key, f.origin); if (result) { if (!f.matchAllFilter) { f.nonApplicable = !result.applicable; diff --git a/packages/scenes/src/variables/applicabilityUtils.test.ts b/packages/scenes/src/variables/applicabilityUtils.test.ts new file mode 100644 index 000000000..271de9662 --- /dev/null +++ b/packages/scenes/src/variables/applicabilityUtils.test.ts @@ -0,0 +1,130 @@ +import { buildApplicabilityMatcher } from './applicabilityUtils'; + +describe('buildApplicabilityMatcher', () => { + it('should match entries by key', () => { + const response = [ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'not found' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('cluster')).toEqual({ key: 'cluster', applicable: false, reason: 'not found' }); + }); + + it('should match entries by key + origin', () => { + const response = [ + { key: 'env', applicable: true }, + { key: 'region', applicable: false, origin: 'dashboard' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('region', 'dashboard')).toEqual({ key: 'region', applicable: false, origin: 'dashboard' }); + }); + + it('should return undefined for unknown keys', () => { + const response = [{ key: 'env', applicable: true }]; + + const match = buildApplicabilityMatcher(response); + + expect(match('unknown')).toBeUndefined(); + }); + + it('should return undefined for empty response', () => { + const match = buildApplicabilityMatcher([]); + + expect(match('env')).toBeUndefined(); + }); + + it('should dequeue entries in order for duplicate keys', () => { + const response = [ + { key: 'env', applicable: true }, + { key: 'env', applicable: false, reason: 'value not found' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('env')).toEqual({ key: 'env', applicable: false, reason: 'value not found' }); + expect(match('env')).toBeUndefined(); + }); + + it('should dequeue entries in order for duplicate key+origin', () => { + const response = [ + { key: 'env', applicable: false, reason: 'first', origin: 'scope' }, + { key: 'env', applicable: true, origin: 'scope' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env', 'scope')).toEqual({ key: 'env', applicable: false, reason: 'first', origin: 'scope' }); + expect(match('env', 'scope')).toEqual({ key: 'env', applicable: true, origin: 'scope' }); + expect(match('env', 'scope')).toBeUndefined(); + }); + + it('should keep key-only and key+origin entries in separate queues', () => { + const response = [ + { key: 'env', applicable: true }, + { key: 'env', applicable: false, origin: 'dashboard' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('env', 'dashboard')).toEqual({ key: 'env', applicable: false, origin: 'dashboard' }); + + expect(match('env')).toBeUndefined(); + expect(match('env', 'dashboard')).toBeUndefined(); + }); + + it('should not match key+origin entry when queried without origin', () => { + const response = [{ key: 'region', applicable: false, origin: 'dashboard' }]; + + const match = buildApplicabilityMatcher(response); + + expect(match('region')).toBeUndefined(); + expect(match('region', 'dashboard')).toEqual({ key: 'region', applicable: false, origin: 'dashboard' }); + }); + + it('should not match key-only entry when queried with origin', () => { + const response = [{ key: 'region', applicable: true }]; + + const match = buildApplicabilityMatcher(response); + + expect(match('region', 'dashboard')).toBeUndefined(); + expect(match('region')).toEqual({ key: 'region', applicable: true }); + }); + + it('should handle mixed keys and origins correctly regardless of response order', () => { + const response = [ + { key: 'cluster', applicable: false, reason: 'overridden' }, + { key: 'env', applicable: true, origin: 'scope' }, + { key: 'pod', applicable: true }, + { key: 'env', applicable: false, reason: 'label not found' }, + { key: 'cluster', applicable: true, origin: 'dashboard' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('pod')).toEqual({ key: 'pod', applicable: true }); + expect(match('env')).toEqual({ key: 'env', applicable: false, reason: 'label not found' }); + expect(match('env', 'scope')).toEqual({ key: 'env', applicable: true, origin: 'scope' }); + expect(match('cluster')).toEqual({ key: 'cluster', applicable: false, reason: 'overridden' }); + expect(match('cluster', 'dashboard')).toEqual({ key: 'cluster', applicable: true, origin: 'dashboard' }); + }); + + it('should handle multiple origins for the same key', () => { + const response = [ + { key: 'env', applicable: true, origin: 'scope' }, + { key: 'env', applicable: false, origin: 'dashboard' }, + ]; + + const match = buildApplicabilityMatcher(response); + + expect(match('env', 'scope')).toEqual({ key: 'env', applicable: true, origin: 'scope' }); + expect(match('env', 'dashboard')).toEqual({ key: 'env', applicable: false, origin: 'dashboard' }); + }); +}); diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts new file mode 100644 index 000000000..891384880 --- /dev/null +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -0,0 +1,36 @@ +import { + // @ts-expect-error (temporary till we update grafana/data) + DrilldownsApplicability, +} from '@grafana/data'; + +function compositeKey(key: string, origin?: string): string { + return origin ? `${key}-${origin}` : key; +} + +/** + * Creates a matcher that pairs response entries with input filters/keys using + * key+origin queues. For each key+origin combination, entries are dequeued in + * order, so duplicates with the same key+origin are matched positionally within + * their group rather than relying on global array ordering. + */ +export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { + const queues = new Map(); + + for (const entry of response) { + const ck = compositeKey(entry.key, entry.origin); + const queue = queues.get(ck); + if (queue) { + queue.push(entry); + } else { + queues.set(ck, [entry]); + } + } + + return (key: string, origin?: string): DrilldownsApplicability | undefined => { + const queue = queues.get(compositeKey(key, origin)); + if (!queue || queue.length === 0) { + return undefined; + } + return queue.shift(); + }; +} From 5c146bcfd561490da508c5d6286b37c612753eb4 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Tue, 3 Mar 2026 13:48:57 +0200 Subject: [PATCH 07/25] add extra gate --- .../DrilldownDependenciesManager.test.ts | 55 ++++++++++++++----- .../variables/DrilldownDependenciesManager.ts | 7 +++ 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index 841c84558..cbbe38ecd 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -19,16 +19,21 @@ function createManager(opts: { adhocVar?: AdHocFiltersVariable; groupByVar?: Gro return manager; } -function createAdhocVar(filters: AdHocFilterWithLabels[], originFilters?: AdHocFilterWithLabels[]) { +function createAdhocVar( + filters: AdHocFilterWithLabels[], + originFilters?: AdHocFilterWithLabels[], + applicabilityEnabled?: boolean +) { return new AdHocFiltersVariable({ datasource: { uid: 'my-ds-uid' }, name: 'filters', filters, originFilters, + applicabilityEnabled, }); } -function createGroupByVar(value: string[], keysApplicability?: any[]) { +function createGroupByVar(value: string[], keysApplicability?: any[], applicabilityEnabled?: boolean) { return new GroupByVariable({ datasource: { uid: 'my-ds-uid' }, name: 'groupby', @@ -36,6 +41,7 @@ function createGroupByVar(value: string[], keysApplicability?: any[]) { value, text: value, keysApplicability, + applicabilityEnabled, }); } @@ -56,9 +62,10 @@ describe('DrilldownDependenciesManager', () => { { key: 'env', value: 'prod', operator: '=' }, { key: 'cluster', value: 'us-east', operator: '=' }, ], - [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }] + [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }], + true ), - groupByVar: createGroupByVar(['namespace', 'pod']), + groupByVar: createGroupByVar(['namespace', 'pod'], undefined, true), }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; @@ -80,8 +87,8 @@ describe('DrilldownDependenciesManager', () => { const getDrilldownsApplicability = jest.fn(); const manager = createManager({ - adhocVar: createAdhocVar([]), - groupByVar: createGroupByVar([]), + adhocVar: createAdhocVar([], undefined, true), + groupByVar: createGroupByVar([], undefined, true), }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; @@ -94,19 +101,34 @@ describe('DrilldownDependenciesManager', () => { it('should clear results when ds call throws', async () => { const getDrilldownsApplicability = jest.fn().mockRejectedValue(new Error('fail')); + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + expect(manager.getApplicabilityResults()).toBeUndefined(); + }); + + it('should skip when applicabilityEnabled is not set on any variable', async () => { + const getDrilldownsApplicability = jest.fn(); + const manager = createManager({ adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), + groupByVar: createGroupByVar(['namespace']), }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); expect(manager.getApplicabilityResults()).toBeUndefined(); + expect(getDrilldownsApplicability).not.toHaveBeenCalled(); }); it('should skip when ds does not support getDrilldownsApplicability', async () => { const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), }); const ds = {} as unknown as DataSourceApi; @@ -346,9 +368,10 @@ describe('DrilldownDependenciesManager', () => { { key: 'env', value: 'prod', operator: '=' }, { key: 'cluster', value: 'us', operator: '=' }, ], - [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }] + [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }], + true ), - groupByVar: createGroupByVar(['namespace', 'pod']), + groupByVar: createGroupByVar(['namespace', 'pod'], undefined, true), }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; @@ -369,11 +392,15 @@ describe('DrilldownDependenciesManager', () => { ]); const manager = createManager({ - adhocVar: createAdhocVar([ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ]), - groupByVar: createGroupByVar([]), + adhocVar: createAdhocVar( + [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ], + undefined, + true + ), + groupByVar: createGroupByVar([], undefined, true), }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 5fc11efa3..0ea40683b 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -108,6 +108,13 @@ export class DrilldownDependenciesManager { return; } + const filtersApplicabilityEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; + const groupByApplicabilityEnabled = this._groupByVar?.state.applicabilityEnabled; + + if (!filtersApplicabilityEnabled && !groupByApplicabilityEnabled) { + return; + } + // @ts-expect-error (temporary till we update grafana/data) if (!ds.getDrilldownsApplicability) { return; From b24b42fb0f00aeceba1779d07f53168a11ed4624 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Tue, 3 Mar 2026 13:50:59 +0200 Subject: [PATCH 08/25] fix --- packages/scenes/src/variables/DrilldownDependenciesManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 0ea40683b..80b0b2e85 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -121,7 +121,7 @@ export class DrilldownDependenciesManager { } const filters = this._adhocFiltersVar - ? [...this._adhocFiltersVar.state.filters, ...(this._adhocFiltersVar.state.originFilters ?? [])] + ? [...(this._adhocFiltersVar.state.originFilters ?? []), ...this._adhocFiltersVar.state.filters] : []; const groupByKeys = this._groupByVar ? Array.isArray(this._groupByVar.state.value) @@ -179,7 +179,7 @@ export class DrilldownDependenciesManager { const stateFilters = this._adhocFiltersVar.state.filters; const originFilters = this._adhocFiltersVar.state.originFilters ?? []; - const applicable = [...stateFilters, ...originFilters].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); + const applicable = [...originFilters, ...stateFilters].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); if (!this._applicabilityResults) { return applicable; From 90de9f97608ada1735e0a49a7ddd22bdd20afb97 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Tue, 3 Mar 2026 14:25:38 +0200 Subject: [PATCH 09/25] fix tests --- .../src/variables/DrilldownDependenciesManager.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index cbbe38ecd..57e45241e 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -206,8 +206,8 @@ describe('DrilldownDependenciesManager', () => { const result = manager.getFilters() ?? []; expect(result).toHaveLength(2); - expect(result[0].key).toBe('env'); - expect(result[1].key).toBe('region'); + expect(result[0].key).toBe('region'); + expect(result[1].key).toBe('env'); }); it('should keep filters when no matching response entry exists', () => { @@ -379,8 +379,8 @@ describe('DrilldownDependenciesManager', () => { const filters = manager.getFilters() ?? []; expect(filters).toHaveLength(2); - expect(filters[0].key).toBe('env'); - expect(filters[1].key).toBe('region'); + expect(filters[0].key).toBe('region'); + expect(filters[1].key).toBe('env'); expect(manager.getGroupByKeys()).toEqual(['namespace']); }); From d510a18f403af497157f4e3282ee4110768d0ab5 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Wed, 4 Mar 2026 18:53:17 +0200 Subject: [PATCH 10/25] enforce feature toggle --- .../scenes/src/variables/adhoc/AdHocFiltersVariable.tsx | 6 ++++-- packages/scenes/src/variables/groupby/GroupByVariable.tsx | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 5e97d7849..c15e48c4a 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -323,8 +323,6 @@ export class AdHocFiltersVariable this.restoreOriginalFilter(filter); } }); - - this.setState({ applicabilityEnabled: false }); }; }; @@ -738,6 +736,10 @@ export class AdHocFiltersVariable } public async _verifyApplicability() { + if (!this.state.applicabilityEnabled) { + return; + } + const filters = [...this.state.filters, ...(this.state.originFilters ?? [])]; const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 1b2d3827c..163fa1611 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -244,8 +244,6 @@ export class GroupByVariable extends MultiValueVariable { if (this.state.defaultValue) { this.restoreDefaultValues(); } - - this.setState({ applicabilityEnabled: false }); }; }; @@ -297,6 +295,10 @@ export class GroupByVariable extends MultiValueVariable { } public async _verifyApplicability() { + if (!this.state.applicabilityEnabled) { + return; + } + const queries = getQueriesForVariables(this); const value = this.state.value; From 22231aa0d6230eb59497b15e4b5d6d1e0916c44b Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Thu, 5 Mar 2026 11:50:28 +0200 Subject: [PATCH 11/25] test fixes + cache --- .../DrilldownDependenciesManager.test.ts | 117 ++++++++++++++++++ .../variables/DrilldownDependenciesManager.ts | 24 ++++ .../adhoc/AdHocFiltersVariable.test.tsx | 4 + .../groupby/GroupByRecommendations.test.tsx | 2 + .../groupby/GroupByVariable.test.tsx | 15 ++- 5 files changed, 158 insertions(+), 4 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index 57e45241e..fa4692102 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -136,6 +136,123 @@ describe('DrilldownDependenciesManager', () => { expect(manager.getApplicabilityResults()).toBeUndefined(); }); + + it('should use cached results when filters, groupBy, queries, and scopes are unchanged', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'env', applicable: true }]); + + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + const queries = [{ refId: 'A', expr: 'up{job="test"}' }] as any[]; + + await manager.resolveApplicability(ds, queries, getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); + + // Second call with same inputs should use cache + await manager.resolveApplicability(ds, queries, getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); + }); + + it('should invalidate cache when filters change', async () => { + const getDrilldownsApplicability = jest + .fn() + .mockResolvedValueOnce([{ key: 'env', applicable: true }]) + .mockResolvedValueOnce([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'not found' }, + ]); + + const adhocVar = createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true); + const manager = createManager({ adhocVar }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + + // Add a new filter + adhocVar.setState({ + filters: [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ], + }); + + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); + }); + + it('should invalidate cache when queries change', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'env', applicable: true }]); + + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + + await manager.resolveApplicability(ds, [{ refId: 'A', expr: 'up' }] as any[], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + + // Different query expression + await manager.resolveApplicability( + ds, + [{ refId: 'A', expr: 'node_cpu_seconds_total' }] as any[], + getDefaultTimeRange(), + undefined + ); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); + }); + + it('should invalidate cache when groupBy keys change', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'ns', applicable: true }]); + + const groupByVar = createGroupByVar(['ns'], undefined, true); + const manager = createManager({ groupByVar }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + + // Change groupBy value + groupByVar.changeValueTo(['ns', 'pod']); + + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); + }); + + it('should clear cache when resolveApplicability encounters an error', async () => { + const getDrilldownsApplicability = jest + .fn() + .mockResolvedValueOnce([{ key: 'env', applicable: true }]) + .mockRejectedValueOnce(new Error('fail')) + .mockResolvedValueOnce([{ key: 'env', applicable: false, reason: 'gone' }]); + + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + + // First call succeeds and caches + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); + expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); + + // Second call with same inputs fails - cache should be cleared + getDrilldownsApplicability.mockRejectedValueOnce(new Error('fail')); + // Need to bust cache to force a new call - change a filter temporarily + manager['_lastApplicabilityCacheKey'] = undefined; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(manager.getApplicabilityResults()).toBeUndefined(); + + // Third call should re-fetch since cache was cleared by error + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(3); + }); }); describe('getFilters', () => { diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 80b0b2e85..8ab481c28 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -37,6 +37,7 @@ export class DrilldownDependenciesManager { private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; private _applicabilityResults?: ApplicabilityResults; + private _lastApplicabilityCacheKey?: string; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -133,6 +134,12 @@ export class DrilldownDependenciesManager { if (filters.length === 0 && groupByKeys.length === 0) { this._clearPerPanelApplicability(); + this._lastApplicabilityCacheKey = undefined; + return; + } + + const cacheKey = this._buildApplicabilityCacheKey(filters, groupByKeys, queries, scopes); + if (cacheKey === this._lastApplicabilityCacheKey && this._applicabilityResults) { return; } @@ -149,8 +156,10 @@ export class DrilldownDependenciesManager { filters: results.slice(0, filters.length), groupBy: results.slice(filters.length), }); + this._lastApplicabilityCacheKey = cacheKey; } catch { this._clearPerPanelApplicability(); + this._lastApplicabilityCacheKey = undefined; } } @@ -215,5 +224,20 @@ export class DrilldownDependenciesManager { public cleanup(): void { this._adhocFiltersVar = undefined; this._groupByVar = undefined; + this._lastApplicabilityCacheKey = undefined; + } + + private _buildApplicabilityCacheKey( + filters: AdHocFilterWithLabels[], + groupByKeys: string[], + queries: SceneDataQuery[], + scopes: Scope[] | undefined + ): string { + const filtersPart = filters.map((f) => `${f.origin ?? ''}:${f.key}:${f.operator}:${f.value}`).join('|'); + const groupByPart = groupByKeys.join('|'); + const queriesPart = queries.map((q) => q.refId + ':' + (q.expr ?? q.expression ?? q.query ?? '')).join('|'); + const scopesPart = scopes?.map((s) => s.metadata.name).join('|') ?? ''; + + return `f[${filtersPart}]g[${groupByPart}]q[${queriesPart}]s[${scopesPart}]`; } } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx index aff9abdbe..fe630681e 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx @@ -2354,6 +2354,7 @@ describe.each(['11.1.2', '11.1.1'])('AdHocFiltersVariable', (v) => { //pod and static are non-applicable const { filtersVar, getDrilldownsApplicabilitySpy } = setup( { + applicabilityEnabled: true, filters: [ { key: 'cluster', @@ -2400,6 +2401,7 @@ describe.each(['11.1.2', '11.1.1'])('AdHocFiltersVariable', (v) => { //pod and static are non-applicable const { filtersVar, getDrilldownsApplicabilitySpy, getTagKeysSpy } = setup( { + applicabilityEnabled: true, filters: [ { key: 'cluster', @@ -2462,6 +2464,7 @@ describe.each(['11.1.2', '11.1.1'])('AdHocFiltersVariable', (v) => { //pod and static are non-applicable const { filtersVar, getDrilldownsApplicabilitySpy } = setup( { + applicabilityEnabled: true, filters: [ { key: 'cluster', @@ -2685,6 +2688,7 @@ describe.each(['11.1.2', '11.1.1'])('AdHocFiltersVariable', (v) => { it('should set non-applicable filters on activation', async () => { setup( { + applicabilityEnabled: true, filters: [ { key: 'pod', operator: '=', value: 'val1' }, { key: 'container', operator: '=', value: 'val3' }, diff --git a/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx b/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx index 6fa3e2255..7d4dbce09 100644 --- a/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx +++ b/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx @@ -266,6 +266,7 @@ describe('GroupByRecommendations', () => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1'], }, { @@ -298,6 +299,7 @@ describe('GroupByRecommendations', () => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1'], }, { diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx index 436ed7228..f1119577f 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx @@ -630,7 +630,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { { key: 'key2', applicable: false }, ]); - const { variable } = setupTest({ value: ['key1', 'key2'] }, undefined, undefined, { + const { variable } = setupTest({ value: ['key1', 'key2'], applicabilityEnabled: true }, undefined, undefined, { // @ts-expect-error (temporary till we update grafana/data) getDrilldownsApplicability: getDrilldownsApplicabilitySpy, }); @@ -658,7 +658,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should not set keysApplicability if data source does not support it', async () => { - const { variable } = setupTest({ value: ['key1'] }); + const { variable } = setupTest({ value: ['key1'], applicabilityEnabled: true }); await act(async () => { await variable._verifyApplicability(); @@ -670,7 +670,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { it('should handle empty response from getDrilldownsApplicability', async () => { const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue(null); - const { variable } = setupTest({ value: ['key1'] }, undefined, undefined, { + const { variable } = setupTest({ value: ['key1'], applicabilityEnabled: true }, undefined, undefined, { // @ts-expect-error (temporary till we update grafana/data) getDrilldownsApplicability: getDrilldownsApplicabilitySpy, }); @@ -686,7 +686,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { it('should be called during activation handler', async () => { const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'key1', applicable: true }]); - const { variable } = setupTest({ value: ['key1'] }, undefined, undefined, { + const { variable } = setupTest({ value: ['key1'], applicabilityEnabled: true }, undefined, undefined, { // @ts-expect-error (temporary till we update grafana/data) getDrilldownsApplicability: getDrilldownsApplicabilitySpy, }); @@ -714,6 +714,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { { text: 'option2', value: 'option2' }, ], allowCustomValue: true, + applicabilityEnabled: true, }, undefined, undefined, @@ -877,6 +878,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1'], }, undefined, @@ -906,6 +908,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1'], }, undefined, @@ -937,6 +940,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1', 'value2', 'value3'], }, undefined, @@ -978,6 +982,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['newValue1', 'newValue2', 'newValue3', 'newValue4'], }, undefined, @@ -1008,6 +1013,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: true, + applicabilityEnabled: true, value: ['value1', 'value2'], }, undefined, @@ -1033,6 +1039,7 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { const { variable } = setupTest( { drilldownRecommendationsEnabled: false, + applicabilityEnabled: true, value: ['value1'], }, undefined, From 76582e9997bf710a2e933172c018b64232fd8bb4 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Thu, 5 Mar 2026 17:13:25 +0200 Subject: [PATCH 12/25] use stringify for caching --- .../variables/DrilldownDependenciesManager.ts | 12 +++++----- .../variables/adhoc/AdHocFiltersVariable.tsx | 22 ++++++++++++++++++ .../src/variables/groupby/GroupByVariable.tsx | 23 +++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 8ab481c28..deb856531 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -233,11 +233,11 @@ export class DrilldownDependenciesManager { queries: SceneDataQuery[], scopes: Scope[] | undefined ): string { - const filtersPart = filters.map((f) => `${f.origin ?? ''}:${f.key}:${f.operator}:${f.value}`).join('|'); - const groupByPart = groupByKeys.join('|'); - const queriesPart = queries.map((q) => q.refId + ':' + (q.expr ?? q.expression ?? q.query ?? '')).join('|'); - const scopesPart = scopes?.map((s) => s.metadata.name).join('|') ?? ''; - - return `f[${filtersPart}]g[${groupByPart}]q[${queriesPart}]s[${scopesPart}]`; + return JSON.stringify({ + filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + groupByKeys, + queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), + scopes: scopes?.map((s) => s.metadata.name), + }); } } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index c15e48c4a..2303e08d2 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -274,6 +274,7 @@ export class AdHocFiltersVariable protected _urlSync = new AdHocFiltersVariableUrlSyncHandler(this); private _debouncedVerifyApplicability = debounce(this._verifyApplicability, 100); + private _lastApplicabilityCacheKey?: string; private _recommendations: AdHocFiltersRecommendations | undefined; @@ -318,6 +319,7 @@ export class AdHocFiltersVariable this._debouncedVerifyApplicability(); return () => { + this._lastApplicabilityCacheKey = undefined; this.state.originFilters?.forEach((filter) => { if (filter.restorable) { this.restoreOriginalFilter(filter); @@ -742,6 +744,12 @@ export class AdHocFiltersVariable const filters = [...this.state.filters, ...(this.state.originFilters ?? [])]; const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; + const scopes = sceneGraph.getScopes(this); + + const cacheKey = this._buildApplicabilityCacheKey(filters, queries ?? [], scopes); + if (cacheKey === this._lastApplicabilityCacheKey) { + return; + } const response = await this.getFiltersApplicabilityForQueries(filters, queries ?? []); @@ -749,6 +757,8 @@ export class AdHocFiltersVariable return; } + this._lastApplicabilityCacheKey = cacheKey; + const matchResult = buildApplicabilityMatcher(response); const update = { @@ -784,6 +794,18 @@ export class AdHocFiltersVariable this.setState(update); } + private _buildApplicabilityCacheKey( + filters: AdHocFilterWithLabels[], + queries: SceneDataQuery[], + scopes: Scope[] | undefined + ): string { + return JSON.stringify({ + filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), + scopes: scopes?.map((s) => s.metadata.name), + }); + } + /** * Get possible keys given current filters. Do not call from plugins directly */ diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 163fa1611..297f5b503 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -8,6 +8,7 @@ import { GetTagResponse, GrafanaTheme2, MetricFindValue, + Scope, SelectableValue, } from '@grafana/data'; import { allActiveGroupByVariables } from './findActiveGroupByVariablesByUid'; @@ -111,6 +112,7 @@ export class GroupByVariable extends MultiValueVariable { private _scopedVars = { __sceneObject: wrapInSafeSerializableSceneObject(this) }; private _recommendations: GroupByRecommendations | undefined; + private _lastApplicabilityCacheKey?: string; public validateAndUpdate(): Observable { return this.getValueOptions({}).pipe( @@ -240,6 +242,7 @@ export class GroupByVariable extends MultiValueVariable { return () => { sub.unsubscribe(); + this._lastApplicabilityCacheKey = undefined; if (this.state.defaultValue) { this.restoreDefaultValues(); @@ -301,6 +304,12 @@ export class GroupByVariable extends MultiValueVariable { const queries = getQueriesForVariables(this); const value = this.state.value; + const scopes = sceneGraph.getScopes(this); + + const cacheKey = this._buildApplicabilityCacheKey(value, queries, scopes); + if (cacheKey === this._lastApplicabilityCacheKey) { + return; + } const response = await this.getGroupByApplicabilityForQueries(value, queries); @@ -308,6 +317,8 @@ export class GroupByVariable extends MultiValueVariable { return; } + this._lastApplicabilityCacheKey = cacheKey; + if (!isEqual(response, this.state.keysApplicability)) { this.setState({ keysApplicability: response ?? undefined, applicabilityEnabled: true }); @@ -317,6 +328,18 @@ export class GroupByVariable extends MultiValueVariable { } } + private _buildApplicabilityCacheKey( + value: VariableValue, + queries: SceneDataQuery[], + scopes: Scope[] | undefined + ): string { + return JSON.stringify({ + value: Array.isArray(value) ? value.map(String) : [String(value ?? '')], + queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), + scopes: scopes?.map((s) => s.metadata.name), + }); + } + // This method is related to the defaultValue property. We check if the current value // is different from the default value. If it is, the groupBy will show a button // allowing the user to restore the default values. From 5d7ed6e916b7055cb5676cccf53c90daf1f1f39f Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 6 Mar 2026 13:23:02 +0200 Subject: [PATCH 13/25] fix --- packages/scenes/src/index.ts | 1 - .../DrilldownDependenciesManager.test.ts | 32 ++++++++----- .../variables/DrilldownDependenciesManager.ts | 47 +++++++++++-------- .../adhoc/AdHocFiltersRecommendations.tsx | 16 ++----- .../variables/adhoc/AdHocFiltersVariable.tsx | 20 +++----- .../src/variables/applicabilityUtils.ts | 26 +++++++++- .../groupby/GroupByRecommendations.tsx | 17 ++----- .../src/variables/groupby/GroupByVariable.tsx | 20 +++----- 8 files changed, 95 insertions(+), 84 deletions(-) diff --git a/packages/scenes/src/index.ts b/packages/scenes/src/index.ts index 04d4bbdf1..7058460aa 100644 --- a/packages/scenes/src/index.ts +++ b/packages/scenes/src/index.ts @@ -41,7 +41,6 @@ export { SceneTimeZoneOverride } from './core/SceneTimeZoneOverride'; export { SceneQueryRunner, type QueryRunnerState } from './querying/SceneQueryRunner'; export { DataProviderProxy } from './querying/DataProviderProxy'; -export { type ApplicabilityResults } from './variables/DrilldownDependenciesManager'; export { buildApplicabilityMatcher } from './variables/applicabilityUtils'; export { type ExtraQueryDescriptor, diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index fa4692102..4ef5c97e5 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -73,9 +73,9 @@ describe('DrilldownDependenciesManager', () => { const results = manager.getApplicabilityResults(); expect(results?.filters).toEqual([ + { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'env', applicable: true }, { key: 'cluster', applicable: false, reason: 'not found' }, - { key: 'region', applicable: true, origin: 'dashboard' }, ]); expect(results?.groupBy).toEqual([ { key: 'namespace', applicable: true }, @@ -224,16 +224,18 @@ describe('DrilldownDependenciesManager', () => { expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); }); - it('should clear cache when resolveApplicability encounters an error', async () => { + it('should clear cache when resolveApplicability encounters an error and re-fetch on next call', async () => { const getDrilldownsApplicability = jest .fn() .mockResolvedValueOnce([{ key: 'env', applicable: true }]) .mockRejectedValueOnce(new Error('fail')) - .mockResolvedValueOnce([{ key: 'env', applicable: false, reason: 'gone' }]); + .mockResolvedValueOnce([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'gone' }, + ]); - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - }); + const adhocVar = createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true); + const manager = createManager({ adhocVar }); const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; @@ -242,16 +244,24 @@ describe('DrilldownDependenciesManager', () => { expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); - // Second call with same inputs fails - cache should be cleared - getDrilldownsApplicability.mockRejectedValueOnce(new Error('fail')); - // Need to bust cache to force a new call - change a filter temporarily - manager['_lastApplicabilityCacheKey'] = undefined; + // Change inputs so the cache key differs, then make the DS call fail + adhocVar.setState({ + filters: [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'cluster', value: 'us', operator: '=' }, + ], + }); await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); expect(manager.getApplicabilityResults()).toBeUndefined(); - // Third call should re-fetch since cache was cleared by error + // Third call with same inputs should re-fetch since error cleared the cache await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); expect(getDrilldownsApplicability).toHaveBeenCalledTimes(3); + expect(manager.getApplicabilityResults()?.filters).toEqual([ + { key: 'env', applicable: true }, + { key: 'cluster', applicable: false, reason: 'gone' }, + ]); }); }); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index deb856531..909f4cbe8 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -22,7 +22,7 @@ import { } from '../variables/adhoc/AdHocFiltersVariable'; import { VariableDependencyConfig } from '../variables/VariableDependencyConfig'; import { SceneDataQuery, SceneObject, SceneObjectState } from '../core/types'; -import { buildApplicabilityMatcher } from '../variables/applicabilityUtils'; +import { buildApplicabilityMatcher, buildApplicabilityCacheKey } from '../variables/applicabilityUtils'; export interface ApplicabilityResults { filters: DrilldownsApplicability[]; @@ -138,7 +138,12 @@ export class DrilldownDependenciesManager { return; } - const cacheKey = this._buildApplicabilityCacheKey(filters, groupByKeys, queries, scopes); + const cacheKey = buildApplicabilityCacheKey({ + filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + groupByKeys, + queries, + scopes, + }); if (cacheKey === this._lastApplicabilityCacheKey && this._applicabilityResults) { return; } @@ -152,10 +157,26 @@ export class DrilldownDependenciesManager { timeRange, scopes, }); - this._setPerPanelApplicability({ - filters: results.slice(0, filters.length), - groupBy: results.slice(filters.length), - }); + + const matcher = buildApplicabilityMatcher(results); + + const filterResults: DrilldownsApplicability[] = []; + for (const f of filters) { + const result = matcher(f.key, f.origin); + if (result) { + filterResults.push(result); + } + } + + const groupByResults: DrilldownsApplicability[] = []; + for (const k of groupByKeys) { + const result = matcher(k); + if (result) { + groupByResults.push(result); + } + } + + this._setPerPanelApplicability({ filters: filterResults, groupBy: groupByResults }); this._lastApplicabilityCacheKey = cacheKey; } catch { this._clearPerPanelApplicability(); @@ -226,18 +247,4 @@ export class DrilldownDependenciesManager { this._groupByVar = undefined; this._lastApplicabilityCacheKey = undefined; } - - private _buildApplicabilityCacheKey( - filters: AdHocFilterWithLabels[], - groupByKeys: string[], - queries: SceneDataQuery[], - scopes: Scope[] | undefined - ): string { - return JSON.stringify({ - filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), - groupByKeys, - queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), - scopes: scopes?.map((s) => s.metadata.name), - }); - } } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx index c2cf60146..0565cbc7e 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx @@ -1,9 +1,5 @@ import React from 'react'; -import { - // @ts-expect-error (temporary till we update grafana/data) - DrilldownsApplicability, - store, -} from '@grafana/data'; +import { store } from '@grafana/data'; import { sceneGraph } from '../../core/sceneGraph'; import { getEnrichedDataRequest } from '../../querying/getEnrichedDataRequest'; import { getQueriesForVariables } from '../utils'; @@ -16,6 +12,7 @@ import { SceneObjectBase } from '../../core/SceneObjectBase'; import { SceneComponentProps, SceneObjectState } from '../../core/types'; import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { Unsubscribable } from 'rxjs'; +import { buildApplicabilityMatcher } from '../applicabilityUtils'; export const MAX_RECENT_DRILLDOWNS = 3; export const MAX_STORED_RECENT_DRILLDOWNS = 10; @@ -153,15 +150,12 @@ export class AdHocFiltersRecommendations extends SceneObjectBase(); - response.forEach((item: DrilldownsApplicability) => { - applicabilityMap.set(item.key, item.applicable !== false); - }); + const matcher = buildApplicabilityMatcher(response); const applicableFilters = storedFilters .filter((f) => { - const isApplicable = applicabilityMap.get(f.key); - return isApplicable === undefined || isApplicable === true; + const result = matcher(f.key, f.origin); + return !result || result.applicable; }) .slice(-MAX_RECENT_DRILLDOWNS); diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 2303e08d2..6c0ee0d9b 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -32,7 +32,7 @@ import { getEnrichedFiltersRequest } from '../getEnrichedFiltersRequest'; import { AdHocFiltersComboboxRenderer } from './AdHocFiltersCombobox/AdHocFiltersComboboxRenderer'; import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { debounce, isEqual } from 'lodash'; -import { buildApplicabilityMatcher } from '../applicabilityUtils'; +import { buildApplicabilityMatcher, buildApplicabilityCacheKey } from '../applicabilityUtils'; import { getAdHocFiltersFromScopes } from './getAdHocFiltersFromScopes'; import { VariableDependencyConfig } from '../VariableDependencyConfig'; import { getQueryController } from '../../core/sceneGraph/getQueryController'; @@ -746,7 +746,11 @@ export class AdHocFiltersVariable const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; const scopes = sceneGraph.getScopes(this); - const cacheKey = this._buildApplicabilityCacheKey(filters, queries ?? [], scopes); + const cacheKey = buildApplicabilityCacheKey({ + filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + queries: queries ?? [], + scopes, + }); if (cacheKey === this._lastApplicabilityCacheKey) { return; } @@ -794,18 +798,6 @@ export class AdHocFiltersVariable this.setState(update); } - private _buildApplicabilityCacheKey( - filters: AdHocFilterWithLabels[], - queries: SceneDataQuery[], - scopes: Scope[] | undefined - ): string { - return JSON.stringify({ - filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), - queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), - scopes: scopes?.map((s) => s.metadata.name), - }); - } - /** * Get possible keys given current filters. Do not call from plugins directly */ diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index 891384880..62a997aa7 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -1,10 +1,14 @@ import { // @ts-expect-error (temporary till we update grafana/data) DrilldownsApplicability, + Scope, } from '@grafana/data'; +import { SceneDataQuery } from '../core/types'; + +const COMPOSITE_KEY_SEPARATOR = '|'; function compositeKey(key: string, origin?: string): string { - return origin ? `${key}-${origin}` : key; + return origin ? `${key}${COMPOSITE_KEY_SEPARATOR}${origin}` : key; } /** @@ -34,3 +38,23 @@ export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { return queue.shift(); }; } + +function normalizeQuery(q: SceneDataQuery): { refId: string; expr?: unknown } { + return { refId: q.refId, expr: q.expr ?? q.expression ?? q.query }; +} + +export function buildApplicabilityCacheKey(parts: { + filters?: Array<{ origin?: string; key: string; operator: string; value: string }>; + groupByKeys?: string[]; + value?: string[]; + queries: SceneDataQuery[]; + scopes: Scope[] | undefined; +}): string { + return JSON.stringify({ + filters: parts.filters, + groupByKeys: parts.groupByKeys, + value: parts.value, + queries: parts.queries.map(normalizeQuery), + scopes: parts.scopes?.map((s) => s.metadata.name), + }); +} diff --git a/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx b/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx index 6018a26b6..5ea401046 100644 --- a/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx +++ b/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx @@ -1,10 +1,5 @@ import React from 'react'; -import { - // @ts-expect-error (temporary till we update grafana/data) - DrilldownsApplicability, - SelectableValue, - store, -} from '@grafana/data'; +import { SelectableValue, store } from '@grafana/data'; import { sceneGraph } from '../../core/sceneGraph'; import { getEnrichedDataRequest } from '../../querying/getEnrichedDataRequest'; import { getQueriesForVariables } from '../utils'; @@ -18,6 +13,7 @@ import { VariableValueSingle } from '../types'; import { isArray } from 'lodash'; import { SceneObjectBase } from '../../core/SceneObjectBase'; import { SceneComponentProps, SceneObjectState } from '../../core/types'; +import { buildApplicabilityMatcher } from '../applicabilityUtils'; import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { Unsubscribable } from 'rxjs'; @@ -159,15 +155,12 @@ export class GroupByRecommendations extends SceneObjectBase(); - response.forEach((item: DrilldownsApplicability) => { - applicabilityMap.set(item.key, item.applicable !== false); - }); + const matcher = buildApplicabilityMatcher(response); const applicableGroupings = storedGroupings .filter((g) => { - const isApplicable = applicabilityMap.get(String(g.value)); - return isApplicable === undefined || isApplicable === true; + const result = matcher(String(g.value)); + return !result || result.applicable; }) .slice(-MAX_RECENT_DRILLDOWNS); diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index d91047b29..537c31d61 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -8,7 +8,6 @@ import { GetTagResponse, GrafanaTheme2, MetricFindValue, - Scope, SelectableValue, } from '@grafana/data'; import { allActiveGroupByVariables } from './findActiveGroupByVariablesByUid'; @@ -40,6 +39,7 @@ import { getInteractionTracker } from '../../core/sceneGraph/getInteractionTrack import { GROUPBY_DIMENSIONS_INTERACTION } from '../../performance/interactionConstants'; import { css, cx } from '@emotion/css'; import { GroupByRecommendations } from './GroupByRecommendations'; +import { buildApplicabilityCacheKey } from '../applicabilityUtils'; export interface GroupByVariableState extends MultiValueVariableState { /** Defaults to "Group" */ @@ -295,7 +295,11 @@ export class GroupByVariable extends MultiValueVariable { const value = this.state.value; const scopes = sceneGraph.getScopes(this); - const cacheKey = this._buildApplicabilityCacheKey(value, queries, scopes); + const cacheKey = buildApplicabilityCacheKey({ + value: Array.isArray(value) ? value.map(String) : [String(value ?? '')], + queries, + scopes, + }); if (cacheKey === this._lastApplicabilityCacheKey) { return; } @@ -317,18 +321,6 @@ export class GroupByVariable extends MultiValueVariable { } } - private _buildApplicabilityCacheKey( - value: VariableValue, - queries: SceneDataQuery[], - scopes: Scope[] | undefined - ): string { - return JSON.stringify({ - value: Array.isArray(value) ? value.map(String) : [String(value ?? '')], - queries: queries.map((q) => ({ refId: q.refId, expr: q.expr ?? q.expression ?? q.query })), - scopes: scopes?.map((s) => s.metadata.name), - }); - } - // This method is related to the defaultValue property. We check if the current value // is different from the default value. If it is, the groupBy will show a button // allowing the user to restore the default values. From 0b5806ab2167ee558b2b85c970e215e69d35e0e3 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 6 Mar 2026 13:50:28 +0200 Subject: [PATCH 14/25] fix --- .../DrilldownDependenciesManager.test.ts | 41 ++++++++++++++++--- .../variables/DrilldownDependenciesManager.ts | 32 +++++++-------- .../variables/adhoc/AdHocFiltersVariable.tsx | 8 +++- .../src/variables/applicabilityUtils.test.ts | 26 ++++-------- .../src/variables/applicabilityUtils.ts | 32 ++++++--------- 5 files changed, 77 insertions(+), 62 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index 4ef5c97e5..4957afcef 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -47,11 +47,12 @@ function createGroupByVar(value: string[], keysApplicability?: any[], applicabil describe('DrilldownDependenciesManager', () => { describe('resolveApplicability', () => { - it('should split response into filter and groupBy portions', async () => { + it('should split response into filter and groupBy portions by count', async () => { + // DS returns results in input order: originFilters first, then filters, then groupByKeys const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'env', applicable: true }, { key: 'cluster', applicable: false, reason: 'not found' }, - { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'namespace', applicable: true }, { key: 'pod', applicable: false, reason: 'label not found' }, ]); @@ -356,7 +357,7 @@ describe('DrilldownDependenciesManager', () => { expect(result[1].key).toBe('extra'); }); - it('should handle duplicate keys by matching positionally within the queue', () => { + it('should use last entry for duplicate keys (last wins)', () => { const filters: AdHocFilterWithLabels[] = [ { key: 'env', value: 'prod', operator: '=' }, { key: 'env', value: 'staging', operator: '=' }, @@ -372,9 +373,9 @@ describe('DrilldownDependenciesManager', () => { groupBy: [], }; + // Last entry wins → applicable: false → both excluded const result = manager.getFilters() ?? []; - expect(result).toHaveLength(1); - expect(result[0].value).toBe('prod'); + expect(result).toHaveLength(0); }); it('should separate user filters from origin filters with same key via origin', () => { @@ -481,10 +482,11 @@ describe('DrilldownDependenciesManager', () => { describe('combined filters and groupBy', () => { it('should resolve and return both filters and groupBy keys through the full flow', async () => { + // DS returns results in input order: originFilters, then filters, then groupByKeys const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'env', applicable: true }, { key: 'cluster', applicable: false, reason: 'label not found' }, - { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'namespace', applicable: true }, { key: 'pod', applicable: false, reason: 'label not found' }, ]); @@ -512,6 +514,33 @@ describe('DrilldownDependenciesManager', () => { expect(manager.getGroupByKeys()).toEqual(['namespace']); }); + it('should correctly split results when filter and groupBy share the same key name', async () => { + const getDrilldownsApplicability = jest.fn().mockResolvedValue([ + // filter result for 'env' (applicable) + { key: 'env', applicable: true }, + // groupBy result for 'env' (not applicable) + { key: 'env', applicable: false, reason: 'label not found for groupBy' }, + ]); + + const manager = createManager({ + adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), + groupByVar: createGroupByVar(['env'], undefined, true), + }); + + const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; + await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); + + const results = manager.getApplicabilityResults(); + expect(results?.filters).toEqual([{ key: 'env', applicable: true }]); + expect(results?.groupBy).toEqual([{ key: 'env', applicable: false, reason: 'label not found for groupBy' }]); + + const filters = manager.getFilters() ?? []; + expect(filters).toHaveLength(1); + expect(filters[0].key).toBe('env'); + + expect(manager.getGroupByKeys()).toEqual([]); + }); + it('should handle filters-only response when both variables exist', async () => { const getDrilldownsApplicability = jest.fn().mockResolvedValue([ { key: 'env', applicable: true }, diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 909f4cbe8..2ad8b93b0 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -139,7 +139,13 @@ export class DrilldownDependenciesManager { } const cacheKey = buildApplicabilityCacheKey({ - filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + filters: filters.map((f) => ({ + origin: f.origin, + key: f.key, + operator: f.operator, + value: f.value, + values: f.values, + })), groupByKeys, queries, scopes, @@ -158,23 +164,13 @@ export class DrilldownDependenciesManager { scopes, }); - const matcher = buildApplicabilityMatcher(results); - - const filterResults: DrilldownsApplicability[] = []; - for (const f of filters) { - const result = matcher(f.key, f.origin); - if (result) { - filterResults.push(result); - } - } - - const groupByResults: DrilldownsApplicability[] = []; - for (const k of groupByKeys) { - const result = matcher(k); - if (result) { - groupByResults.push(result); - } - } + // The DS returns results in order: one entry per filter, then one per groupBy key. + // Split by count so filters and groupBy keys with the same name don't share a queue. + const filterResults: DrilldownsApplicability[] = results.slice(0, filters.length); + const groupByResults: DrilldownsApplicability[] = results.slice( + filters.length, + filters.length + groupByKeys.length + ); this._setPerPanelApplicability({ filters: filterResults, groupBy: groupByResults }); this._lastApplicabilityCacheKey = cacheKey; diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 6c0ee0d9b..70c301d00 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -747,7 +747,13 @@ export class AdHocFiltersVariable const scopes = sceneGraph.getScopes(this); const cacheKey = buildApplicabilityCacheKey({ - filters: filters.map((f) => ({ origin: f.origin, key: f.key, operator: f.operator, value: f.value })), + filters: filters.map((f) => ({ + origin: f.origin, + key: f.key, + operator: f.operator, + value: f.value, + values: f.values, + })), queries: queries ?? [], scopes, }); diff --git a/packages/scenes/src/variables/applicabilityUtils.test.ts b/packages/scenes/src/variables/applicabilityUtils.test.ts index 271de9662..9a3ba9c63 100644 --- a/packages/scenes/src/variables/applicabilityUtils.test.ts +++ b/packages/scenes/src/variables/applicabilityUtils.test.ts @@ -39,7 +39,7 @@ describe('buildApplicabilityMatcher', () => { expect(match('env')).toBeUndefined(); }); - it('should dequeue entries in order for duplicate keys', () => { + it('should return the last entry for duplicate keys', () => { const response = [ { key: 'env', applicable: true }, { key: 'env', applicable: false, reason: 'value not found' }, @@ -47,25 +47,20 @@ describe('buildApplicabilityMatcher', () => { const match = buildApplicabilityMatcher(response); - expect(match('env')).toEqual({ key: 'env', applicable: true }); expect(match('env')).toEqual({ key: 'env', applicable: false, reason: 'value not found' }); - expect(match('env')).toBeUndefined(); }); - it('should dequeue entries in order for duplicate key+origin', () => { - const response = [ - { key: 'env', applicable: false, reason: 'first', origin: 'scope' }, - { key: 'env', applicable: true, origin: 'scope' }, - ]; + it('should return the same result on repeated lookups (stateless)', () => { + const response = [{ key: 'env', applicable: true }]; const match = buildApplicabilityMatcher(response); - expect(match('env', 'scope')).toEqual({ key: 'env', applicable: false, reason: 'first', origin: 'scope' }); - expect(match('env', 'scope')).toEqual({ key: 'env', applicable: true, origin: 'scope' }); - expect(match('env', 'scope')).toBeUndefined(); + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('env')).toEqual({ key: 'env', applicable: true }); + expect(match('env')).toEqual({ key: 'env', applicable: true }); }); - it('should keep key-only and key+origin entries in separate queues', () => { + it('should keep key-only and key+origin entries separate', () => { const response = [ { key: 'env', applicable: true }, { key: 'env', applicable: false, origin: 'dashboard' }, @@ -75,9 +70,6 @@ describe('buildApplicabilityMatcher', () => { expect(match('env')).toEqual({ key: 'env', applicable: true }); expect(match('env', 'dashboard')).toEqual({ key: 'env', applicable: false, origin: 'dashboard' }); - - expect(match('env')).toBeUndefined(); - expect(match('env', 'dashboard')).toBeUndefined(); }); it('should not match key+origin entry when queried without origin', () => { @@ -98,19 +90,17 @@ describe('buildApplicabilityMatcher', () => { expect(match('region')).toEqual({ key: 'region', applicable: true }); }); - it('should handle mixed keys and origins correctly regardless of response order', () => { + it('should handle mixed keys and origins correctly', () => { const response = [ { key: 'cluster', applicable: false, reason: 'overridden' }, { key: 'env', applicable: true, origin: 'scope' }, { key: 'pod', applicable: true }, - { key: 'env', applicable: false, reason: 'label not found' }, { key: 'cluster', applicable: true, origin: 'dashboard' }, ]; const match = buildApplicabilityMatcher(response); expect(match('pod')).toEqual({ key: 'pod', applicable: true }); - expect(match('env')).toEqual({ key: 'env', applicable: false, reason: 'label not found' }); expect(match('env', 'scope')).toEqual({ key: 'env', applicable: true, origin: 'scope' }); expect(match('cluster')).toEqual({ key: 'cluster', applicable: false, reason: 'overridden' }); expect(match('cluster', 'dashboard')).toEqual({ key: 'cluster', applicable: true, origin: 'dashboard' }); diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index 62a997aa7..fbcc68a64 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -12,30 +12,19 @@ function compositeKey(key: string, origin?: string): string { } /** - * Creates a matcher that pairs response entries with input filters/keys using - * key+origin queues. For each key+origin combination, entries are dequeued in - * order, so duplicates with the same key+origin are matched positionally within - * their group rather than relying on global array ordering. + * Builds a stateless lookup from a DS applicability response. + * Maps each key+origin to its result. For duplicate keys the last entry wins, + * which is the "active" one (the DS marks earlier duplicates as overridden). */ export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { - const queues = new Map(); + const map = new Map(); for (const entry of response) { - const ck = compositeKey(entry.key, entry.origin); - const queue = queues.get(ck); - if (queue) { - queue.push(entry); - } else { - queues.set(ck, [entry]); - } + map.set(compositeKey(entry.key, entry.origin), entry); } return (key: string, origin?: string): DrilldownsApplicability | undefined => { - const queue = queues.get(compositeKey(key, origin)); - if (!queue || queue.length === 0) { - return undefined; - } - return queue.shift(); + return map.get(compositeKey(key, origin)); }; } @@ -44,14 +33,19 @@ function normalizeQuery(q: SceneDataQuery): { refId: string; expr?: unknown } { } export function buildApplicabilityCacheKey(parts: { - filters?: Array<{ origin?: string; key: string; operator: string; value: string }>; + filters?: Array<{ origin?: string; key: string; operator: string; value: string; values?: string[] }>; groupByKeys?: string[]; value?: string[]; queries: SceneDataQuery[]; scopes: Scope[] | undefined; }): string { return JSON.stringify({ - filters: parts.filters, + filters: parts.filters?.map((f) => ({ + origin: f.origin, + key: f.key, + operator: f.operator, + value: f.values?.length ? f.values.join(',') : f.value, + })), groupByKeys: parts.groupByKeys, value: parts.value, queries: parts.queries.map(normalizeQuery), From cfbfa102a08d78511a823fba59a74bfb2b87d040 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 9 Mar 2026 12:20:33 +0200 Subject: [PATCH 15/25] fix test --- .../src/variables/DrilldownDependenciesManager.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index 4957afcef..27c50300e 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -388,15 +388,15 @@ describe('DrilldownDependenciesManager', () => { manager['_applicabilityResults'] = { filters: [ - { key: 'env', applicable: false, reason: 'user filter overridden' }, - { key: 'env', applicable: true, origin: 'dashboard' }, + { key: 'env', applicable: false, reason: 'overridden by user filter', origin: 'dashboard' }, + { key: 'env', applicable: true }, ], groupBy: [], }; const result = manager.getFilters() ?? []; expect(result).toHaveLength(1); - expect(result[0].origin).toBe('dashboard'); + expect(result[0].origin).toBeUndefined(); }); }); From 78beba4125a72a61446ff5d9923e065f16e60e84 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 9 Mar 2026 12:53:15 +0200 Subject: [PATCH 16/25] fix --- .../DrilldownDependenciesManager.test.ts | 28 ++++++++++++++++--- .../variables/DrilldownDependenciesManager.ts | 12 ++++---- .../variables/adhoc/AdHocFiltersVariable.tsx | 8 +++--- .../src/variables/applicabilityUtils.ts | 27 ++++++++++++------ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index 27c50300e..a90c9b434 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -312,7 +312,7 @@ describe('DrilldownDependenciesManager', () => { expect(result[0].key).toBe('env'); }); - it('should exclude per-panel nonApplicable filters using key+origin matching', () => { + it('should exclude per-panel nonApplicable filters using index-based matching', () => { const filters: AdHocFilterWithLabels[] = [ { key: 'env', value: 'prod', operator: '=' }, { key: 'cluster', value: 'us', operator: '=' }, @@ -325,9 +325,9 @@ describe('DrilldownDependenciesManager', () => { manager['_applicabilityResults'] = { filters: [ + { key: 'region', applicable: true, origin: 'dashboard' }, { key: 'env', applicable: true }, { key: 'cluster', applicable: false, reason: 'not found' }, - { key: 'region', applicable: true, origin: 'dashboard' }, ], groupBy: [], }; @@ -357,7 +357,7 @@ describe('DrilldownDependenciesManager', () => { expect(result[1].key).toBe('extra'); }); - it('should use last entry for duplicate keys (last wins)', () => { + it('should exclude overridden duplicate-key filter and keep the active one', () => { const filters: AdHocFilterWithLabels[] = [ { key: 'env', value: 'prod', operator: '=' }, { key: 'env', value: 'staging', operator: '=' }, @@ -367,13 +367,33 @@ describe('DrilldownDependenciesManager', () => { manager['_applicabilityResults'] = { filters: [ + { key: 'env', applicable: false, reason: 'overridden by duplicate' }, { key: 'env', applicable: true }, + ], + groupBy: [], + }; + + const result = manager.getFilters() ?? []; + expect(result).toHaveLength(1); + expect(result[0].value).toBe('staging'); + }); + + it('should exclude all duplicate-key filters when none are applicable', () => { + const filters: AdHocFilterWithLabels[] = [ + { key: 'env', value: 'prod', operator: '=' }, + { key: 'env', value: 'nonexistent', operator: '=' }, + ]; + + const manager = createManager({ adhocVar: createAdhocVar(filters) }); + + manager['_applicabilityResults'] = { + filters: [ + { key: 'env', applicable: false, reason: 'overridden by duplicate' }, { key: 'env', applicable: false, reason: 'value not found' }, ], groupBy: [], }; - // Last entry wins → applicable: false → both excluded const result = manager.getFilters() ?? []; expect(result).toHaveLength(0); }); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 2ad8b93b0..1f705fed3 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -204,17 +204,19 @@ export class DrilldownDependenciesManager { const stateFilters = this._adhocFiltersVar.state.filters; const originFilters = this._adhocFiltersVar.state.originFilters ?? []; - - const applicable = [...originFilters, ...stateFilters].filter((f) => isFilterComplete(f) && isFilterApplicable(f)); + const allFilters = [...originFilters, ...stateFilters]; if (!this._applicabilityResults) { - return applicable; + return allFilters.filter((f) => isFilterComplete(f) && isFilterApplicable(f)); } const matchResult = buildApplicabilityMatcher(this._applicabilityResults.filters); - return applicable.filter((f) => { - const result = matchResult(f.key, f.origin); + return allFilters.filter((f, i) => { + if (!isFilterComplete(f) || !isFilterApplicable(f)) { + return false; + } + const result = matchResult(f.key, f.origin, i); return !result || result.applicable; }); } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 70c301d00..f37e089ff 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -777,16 +777,16 @@ export class AdHocFiltersVariable originFilters: [...(this.state.originFilters ?? [])], }; - update.filters.forEach((f) => { - const result = matchResult(f.key); + update.filters.forEach((f, i) => { + const result = matchResult(f.key, undefined, i); if (result) { f.nonApplicable = !result.applicable; f.nonApplicableReason = result.reason; } }); - update.originFilters?.forEach((f) => { - const result = matchResult(f.key, f.origin); + update.originFilters?.forEach((f, i) => { + const result = matchResult(f.key, f.origin, update.filters.length + i); if (result) { if (!f.matchAllFilter) { f.nonApplicable = !result.applicable; diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index fbcc68a64..95147818e 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -7,24 +7,35 @@ import { SceneDataQuery } from '../core/types'; const COMPOSITE_KEY_SEPARATOR = '|'; -function compositeKey(key: string, origin?: string): string { - return origin ? `${key}${COMPOSITE_KEY_SEPARATOR}${origin}` : key; +function compositeKey(key: string, origin?: string, index?: number): string { + let result = origin ? `${key}${COMPOSITE_KEY_SEPARATOR}${origin}` : key; + if (index !== undefined) { + result += `${COMPOSITE_KEY_SEPARATOR}${index}`; + } + return result; } /** - * Builds a stateless lookup from a DS applicability response. - * Maps each key+origin to its result. For duplicate keys the last entry wins, - * which is the "active" one (the DS marks earlier duplicates as overridden). + * Builds a lookup from a DS applicability response. + * + * Each entry is stored under both an index-aware key (`key|origin|index`) + * and a plain key (`key|origin`). Pass `index` when the response can + * contain duplicate keys (e.g. two user filters with the same label) to + * get precise positional matching. Without `index`, duplicate keys + * collapse to last-wins which is safe for inherently-unique sets like + * groupBy keys. */ export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { const map = new Map(); - for (const entry of response) { + for (let i = 0; i < response.length; i++) { + const entry = response[i]; + map.set(compositeKey(entry.key, entry.origin, i), entry); map.set(compositeKey(entry.key, entry.origin), entry); } - return (key: string, origin?: string): DrilldownsApplicability | undefined => { - return map.get(compositeKey(key, origin)); + return (key: string, origin?: string, index?: number): DrilldownsApplicability | undefined => { + return map.get(compositeKey(key, origin, index)); }; } From d7f4cb5b32e384d90f53c063bd89fefd550b728d Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 9 Mar 2026 13:04:40 +0200 Subject: [PATCH 17/25] fix tests, order of filters must be maintained --- .../variables/adhoc/AdHocFiltersVariable.test.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx index fe630681e..3cdfb8ec8 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx @@ -3320,12 +3320,13 @@ function setup( ...(useGetDrilldownsApplicability && { getDrilldownsApplicability(options: any) { getDrilldownsApplicabilitySpy(options); - return [ - { key: 'cluster', applicable: true }, - { key: 'container', applicable: true }, - { key: 'pod', applicable: false, reason: 'reason' }, - { key: 'static', applicable: false, origin: 'dashboard' }, - ]; + const nonApplicableKeys = new Set(['pod', 'static']); + return (options.filters ?? []).map((f: any) => ({ + key: f.key, + applicable: !nonApplicableKeys.has(f.key), + ...(nonApplicableKeys.has(f.key) && { reason: 'reason' }), + ...(f.origin && { origin: f.origin }), + })); }, }), }; From dc5fd3c2e2cac140828a6c3fd3c74b650e04a7c1 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Wed, 11 Mar 2026 13:08:33 +0200 Subject: [PATCH 18/25] fix --- .../scenes/src/variables/DrilldownDependenciesManager.ts | 8 ++++++-- .../scenes/src/variables/adhoc/AdHocFiltersVariable.tsx | 4 +++- packages/scenes/src/variables/groupby/GroupByVariable.tsx | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 1f705fed3..71b135dba 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -109,8 +109,12 @@ export class DrilldownDependenciesManager { return; } - const filtersApplicabilityEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; - const groupByApplicabilityEnabled = this._groupByVar?.state.applicabilityEnabled; + const filtersApplicabilityEnabled = this._adhocFiltersVar + ? this._adhocFiltersVar.state.applicabilityEnabled !== false + : false; + const groupByApplicabilityEnabled = this._groupByVar + ? this._groupByVar.state.applicabilityEnabled !== false + : false; if (!filtersApplicabilityEnabled && !groupByApplicabilityEnabled) { return; diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index f37e089ff..39ec538c2 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -499,6 +499,8 @@ export class AdHocFiltersVariable if ((filterExpressionChanged && options?.skipPublish !== true) || options?.forcePublish) { this.publishEvent(new SceneVariableValueChangedEvent(this), true); } + + this._debouncedVerifyApplicability(); } public restoreOriginalFilter(filter: AdHocFilterWithLabels) { @@ -738,7 +740,7 @@ export class AdHocFiltersVariable } public async _verifyApplicability() { - if (!this.state.applicabilityEnabled) { + if (this.state.applicabilityEnabled === false) { return; } diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 537c31d61..cd51108b8 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -287,7 +287,7 @@ export class GroupByVariable extends MultiValueVariable { } public async _verifyApplicability() { - if (!this.state.applicabilityEnabled) { + if (this.state.applicabilityEnabled === false) { return; } From cdae7e7537a5115fdfef1f3c5a1c418b8e2f5ed5 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Wed, 11 Mar 2026 13:31:12 +0200 Subject: [PATCH 19/25] fix --- .../scenes/src/variables/DrilldownDependenciesManager.ts | 8 ++------ .../scenes/src/variables/adhoc/AdHocFiltersVariable.tsx | 2 +- packages/scenes/src/variables/groupby/GroupByVariable.tsx | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 71b135dba..1f705fed3 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -109,12 +109,8 @@ export class DrilldownDependenciesManager { return; } - const filtersApplicabilityEnabled = this._adhocFiltersVar - ? this._adhocFiltersVar.state.applicabilityEnabled !== false - : false; - const groupByApplicabilityEnabled = this._groupByVar - ? this._groupByVar.state.applicabilityEnabled !== false - : false; + const filtersApplicabilityEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; + const groupByApplicabilityEnabled = this._groupByVar?.state.applicabilityEnabled; if (!filtersApplicabilityEnabled && !groupByApplicabilityEnabled) { return; diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 39ec538c2..c5e91ea97 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -740,7 +740,7 @@ export class AdHocFiltersVariable } public async _verifyApplicability() { - if (this.state.applicabilityEnabled === false) { + if (!this.state.applicabilityEnabled) { return; } diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index cd51108b8..537c31d61 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -287,7 +287,7 @@ export class GroupByVariable extends MultiValueVariable { } public async _verifyApplicability() { - if (this.state.applicabilityEnabled === false) { + if (!this.state.applicabilityEnabled) { return; } From 6723f29c6430c3d6a9d7f8c9e771bf13a9955cfd Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 16 Mar 2026 15:43:21 +0200 Subject: [PATCH 20/25] fixes --- .../variables/adhoc/AdHocFiltersVariable.tsx | 20 +++++++++---------- .../src/variables/applicabilityUtils.ts | 2 -- .../src/variables/groupby/GroupByVariable.tsx | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index c5e91ea97..2c1d4bf9d 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -744,7 +744,7 @@ export class AdHocFiltersVariable return; } - const filters = [...this.state.filters, ...(this.state.originFilters ?? [])]; + const filters = [...(this.state.originFilters ?? []), ...this.state.filters]; const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; const scopes = sceneGraph.getScopes(this); @@ -779,16 +779,8 @@ export class AdHocFiltersVariable originFilters: [...(this.state.originFilters ?? [])], }; - update.filters.forEach((f, i) => { - const result = matchResult(f.key, undefined, i); - if (result) { - f.nonApplicable = !result.applicable; - f.nonApplicableReason = result.reason; - } - }); - update.originFilters?.forEach((f, i) => { - const result = matchResult(f.key, f.origin, update.filters.length + i); + const result = matchResult(f.key, f.origin, i); if (result) { if (!f.matchAllFilter) { f.nonApplicable = !result.applicable; @@ -803,6 +795,14 @@ export class AdHocFiltersVariable } }); + update.filters.forEach((f, i) => { + const result = matchResult(f.key, undefined, (update.originFilters?.length ?? 0) + i); + if (result) { + f.nonApplicable = !result.applicable; + f.nonApplicableReason = result.reason; + } + }); + this.setState(update); } diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index 95147818e..1d003e511 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -46,7 +46,6 @@ function normalizeQuery(q: SceneDataQuery): { refId: string; expr?: unknown } { export function buildApplicabilityCacheKey(parts: { filters?: Array<{ origin?: string; key: string; operator: string; value: string; values?: string[] }>; groupByKeys?: string[]; - value?: string[]; queries: SceneDataQuery[]; scopes: Scope[] | undefined; }): string { @@ -58,7 +57,6 @@ export function buildApplicabilityCacheKey(parts: { value: f.values?.length ? f.values.join(',') : f.value, })), groupByKeys: parts.groupByKeys, - value: parts.value, queries: parts.queries.map(normalizeQuery), scopes: parts.scopes?.map((s) => s.metadata.name), }); diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 537c31d61..b6bd04d6c 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -296,7 +296,7 @@ export class GroupByVariable extends MultiValueVariable { const scopes = sceneGraph.getScopes(this); const cacheKey = buildApplicabilityCacheKey({ - value: Array.isArray(value) ? value.map(String) : [String(value ?? '')], + groupByKeys: Array.isArray(value) ? value.map(String) : [String(value ?? '')], queries, scopes, }); From e32af32e800e0675dfea1e91b4a0532cff139b42 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Mon, 16 Mar 2026 17:03:17 +0200 Subject: [PATCH 21/25] remove query caching -- keep it only for filters/groupBys/scopes --- .../scenes/src/variables/DrilldownDependenciesManager.ts | 1 - .../scenes/src/variables/adhoc/AdHocFiltersVariable.tsx | 1 - packages/scenes/src/variables/applicabilityUtils.ts | 7 ------- packages/scenes/src/variables/groupby/GroupByVariable.tsx | 1 - 4 files changed, 10 deletions(-) diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 1f705fed3..33b64a884 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -147,7 +147,6 @@ export class DrilldownDependenciesManager { values: f.values, })), groupByKeys, - queries, scopes, }); if (cacheKey === this._lastApplicabilityCacheKey && this._applicabilityResults) { diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 2c1d4bf9d..6c35960f7 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -756,7 +756,6 @@ export class AdHocFiltersVariable value: f.value, values: f.values, })), - queries: queries ?? [], scopes, }); if (cacheKey === this._lastApplicabilityCacheKey) { diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index 1d003e511..6bab52406 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -3,7 +3,6 @@ import { DrilldownsApplicability, Scope, } from '@grafana/data'; -import { SceneDataQuery } from '../core/types'; const COMPOSITE_KEY_SEPARATOR = '|'; @@ -39,14 +38,9 @@ export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { }; } -function normalizeQuery(q: SceneDataQuery): { refId: string; expr?: unknown } { - return { refId: q.refId, expr: q.expr ?? q.expression ?? q.query }; -} - export function buildApplicabilityCacheKey(parts: { filters?: Array<{ origin?: string; key: string; operator: string; value: string; values?: string[] }>; groupByKeys?: string[]; - queries: SceneDataQuery[]; scopes: Scope[] | undefined; }): string { return JSON.stringify({ @@ -57,7 +51,6 @@ export function buildApplicabilityCacheKey(parts: { value: f.values?.length ? f.values.join(',') : f.value, })), groupByKeys: parts.groupByKeys, - queries: parts.queries.map(normalizeQuery), scopes: parts.scopes?.map((s) => s.metadata.name), }); } diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index b6bd04d6c..948430b93 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -297,7 +297,6 @@ export class GroupByVariable extends MultiValueVariable { const cacheKey = buildApplicabilityCacheKey({ groupByKeys: Array.isArray(value) ? value.map(String) : [String(value ?? '')], - queries, scopes, }); if (cacheKey === this._lastApplicabilityCacheKey) { From 65b52e43ed2af3f1fa9915e93975f17dcbebf011 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Wed, 18 Mar 2026 14:39:42 +0200 Subject: [PATCH 22/25] refactor applicability, remove some groupBy logic --- packages/scenes/src/index.ts | 1 + .../scenes/src/querying/SceneQueryRunner.ts | 15 - .../DrilldownDependenciesManager.test.ts | 489 +----------------- .../variables/DrilldownDependenciesManager.ts | 148 +----- .../AdHocFiltersRecommendations.test.tsx | 2 +- .../adhoc/AdHocFiltersRecommendations.tsx | 10 +- .../adhoc/AdHocFiltersVariable.test.tsx | 3 +- .../variables/adhoc/AdHocFiltersVariable.tsx | 47 +- .../src/variables/applicabilityUtils.ts | 18 - .../groupby/GroupByRecommendations.test.tsx | 12 +- .../groupby/GroupByRecommendations.tsx | 44 +- .../groupby/GroupByVariable.test.tsx | 97 +++- .../src/variables/groupby/GroupByVariable.tsx | 77 +-- 13 files changed, 119 insertions(+), 844 deletions(-) diff --git a/packages/scenes/src/index.ts b/packages/scenes/src/index.ts index 7058460aa..bdd2a780d 100644 --- a/packages/scenes/src/index.ts +++ b/packages/scenes/src/index.ts @@ -42,6 +42,7 @@ export { SceneTimeZoneOverride } from './core/SceneTimeZoneOverride'; export { SceneQueryRunner, type QueryRunnerState } from './querying/SceneQueryRunner'; export { DataProviderProxy } from './querying/DataProviderProxy'; export { buildApplicabilityMatcher } from './variables/applicabilityUtils'; +export { findClosestAdHocFilterInHierarchy } from './variables/adhoc/patchGetAdhocFilters'; export { type ExtraQueryDescriptor, type ExtraQueryProvider, diff --git a/packages/scenes/src/querying/SceneQueryRunner.ts b/packages/scenes/src/querying/SceneQueryRunner.ts index 2133e6494..d027c5b97 100644 --- a/packages/scenes/src/querying/SceneQueryRunner.ts +++ b/packages/scenes/src/querying/SceneQueryRunner.ts @@ -426,14 +426,6 @@ export class SceneQueryRunner extends SceneObjectBase implemen }); } - /** - * Returns the per-panel applicability results computed before the last query run. - * Used by UI components (e.g. the panel sub-header) to display non-applicable filters. - */ - public getNonApplicableFilters() { - return this._drilldownDependenciesManager.getApplicabilityResults(); - } - private async runWithTimeRange(timeRange: SceneTimeRangeLike) { // If no maxDataPoints specified we might need to wait for container width to be set from the outside if (!this.state.maxDataPoints && this.state.maxDataPointsFromWidth && !this._containerWidth) { @@ -478,13 +470,6 @@ export class SceneQueryRunner extends SceneObjectBase implemen this._drilldownDependenciesManager.findAndSubscribeToDrilldowns(ds.uid, this); - await this._drilldownDependenciesManager.resolveApplicability( - ds, - queries, - timeRange.state.value, - sceneGraph.getScopes(this) - ); - const runRequest = getRunRequest(); const { primary, secondaries, processors } = this.prepareRequests(timeRange, ds); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts index a90c9b434..8d9e50573 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.test.ts @@ -1,5 +1,3 @@ -import { DataSourceApi, getDefaultTimeRange } from '@grafana/data'; - import { AdHocFiltersVariable, AdHocFilterWithLabels } from './adhoc/AdHocFiltersVariable'; import { DrilldownDependenciesManager } from './DrilldownDependenciesManager'; import { GroupByVariable } from './groupby/GroupByVariable'; @@ -46,226 +44,6 @@ function createGroupByVar(value: string[], keysApplicability?: any[], applicabil } describe('DrilldownDependenciesManager', () => { - describe('resolveApplicability', () => { - it('should split response into filter and groupBy portions by count', async () => { - // DS returns results in input order: originFilters first, then filters, then groupByKeys - const getDrilldownsApplicability = jest.fn().mockResolvedValue([ - { key: 'region', applicable: true, origin: 'dashboard' }, - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'not found' }, - { key: 'namespace', applicable: true }, - { key: 'pod', applicable: false, reason: 'label not found' }, - ]); - - const manager = createManager({ - adhocVar: createAdhocVar( - [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us-east', operator: '=' }, - ], - [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }], - true - ), - groupByVar: createGroupByVar(['namespace', 'pod'], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - const results = manager.getApplicabilityResults(); - expect(results?.filters).toEqual([ - { key: 'region', applicable: true, origin: 'dashboard' }, - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'not found' }, - ]); - expect(results?.groupBy).toEqual([ - { key: 'namespace', applicable: true }, - { key: 'pod', applicable: false, reason: 'label not found' }, - ]); - }); - - it('should clear results when there are no filters or groupBy keys', async () => { - const getDrilldownsApplicability = jest.fn(); - - const manager = createManager({ - adhocVar: createAdhocVar([], undefined, true), - groupByVar: createGroupByVar([], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - expect(manager.getApplicabilityResults()).toBeUndefined(); - expect(getDrilldownsApplicability).not.toHaveBeenCalled(); - }); - - it('should clear results when ds call throws', async () => { - const getDrilldownsApplicability = jest.fn().mockRejectedValue(new Error('fail')); - - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - expect(manager.getApplicabilityResults()).toBeUndefined(); - }); - - it('should skip when applicabilityEnabled is not set on any variable', async () => { - const getDrilldownsApplicability = jest.fn(); - - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), - groupByVar: createGroupByVar(['namespace']), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - expect(manager.getApplicabilityResults()).toBeUndefined(); - expect(getDrilldownsApplicability).not.toHaveBeenCalled(); - }); - - it('should skip when ds does not support getDrilldownsApplicability', async () => { - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - }); - - const ds = {} as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - expect(manager.getApplicabilityResults()).toBeUndefined(); - }); - - it('should use cached results when filters, groupBy, queries, and scopes are unchanged', async () => { - const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'env', applicable: true }]); - - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - const queries = [{ refId: 'A', expr: 'up{job="test"}' }] as any[]; - - await manager.resolveApplicability(ds, queries, getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); - - // Second call with same inputs should use cache - await manager.resolveApplicability(ds, queries, getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); - }); - - it('should invalidate cache when filters change', async () => { - const getDrilldownsApplicability = jest - .fn() - .mockResolvedValueOnce([{ key: 'env', applicable: true }]) - .mockResolvedValueOnce([ - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'not found' }, - ]); - - const adhocVar = createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true); - const manager = createManager({ adhocVar }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - - // Add a new filter - adhocVar.setState({ - filters: [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ], - }); - - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); - }); - - it('should invalidate cache when queries change', async () => { - const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'env', applicable: true }]); - - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - - await manager.resolveApplicability(ds, [{ refId: 'A', expr: 'up' }] as any[], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - - // Different query expression - await manager.resolveApplicability( - ds, - [{ refId: 'A', expr: 'node_cpu_seconds_total' }] as any[], - getDefaultTimeRange(), - undefined - ); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); - }); - - it('should invalidate cache when groupBy keys change', async () => { - const getDrilldownsApplicability = jest.fn().mockResolvedValue([{ key: 'ns', applicable: true }]); - - const groupByVar = createGroupByVar(['ns'], undefined, true); - const manager = createManager({ groupByVar }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - - // Change groupBy value - groupByVar.changeValueTo(['ns', 'pod']); - - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); - }); - - it('should clear cache when resolveApplicability encounters an error and re-fetch on next call', async () => { - const getDrilldownsApplicability = jest - .fn() - .mockResolvedValueOnce([{ key: 'env', applicable: true }]) - .mockRejectedValueOnce(new Error('fail')) - .mockResolvedValueOnce([ - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'gone' }, - ]); - - const adhocVar = createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true); - const manager = createManager({ adhocVar }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - - // First call succeeds and caches - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(1); - expect(manager.getApplicabilityResults()?.filters).toEqual([{ key: 'env', applicable: true }]); - - // Change inputs so the cache key differs, then make the DS call fail - adhocVar.setState({ - filters: [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ], - }); - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(2); - expect(manager.getApplicabilityResults()).toBeUndefined(); - - // Third call with same inputs should re-fetch since error cleared the cache - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - expect(getDrilldownsApplicability).toHaveBeenCalledTimes(3); - expect(manager.getApplicabilityResults()?.filters).toEqual([ - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'gone' }, - ]); - }); - }); - describe('getFilters', () => { it('should return undefined when no adhocFiltersVar', () => { const manager = createManager({}); @@ -311,113 +89,6 @@ describe('DrilldownDependenciesManager', () => { expect(result).toHaveLength(1); expect(result[0].key).toBe('env'); }); - - it('should exclude per-panel nonApplicable filters using index-based matching', () => { - const filters: AdHocFilterWithLabels[] = [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ]; - const originFilters: AdHocFilterWithLabels[] = [ - { key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }, - ]; - - const manager = createManager({ adhocVar: createAdhocVar(filters, originFilters) }); - - manager['_applicabilityResults'] = { - filters: [ - { key: 'region', applicable: true, origin: 'dashboard' }, - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'not found' }, - ], - groupBy: [], - }; - - const result = manager.getFilters() ?? []; - expect(result).toHaveLength(2); - expect(result[0].key).toBe('region'); - expect(result[1].key).toBe('env'); - }); - - it('should keep filters when no matching response entry exists', () => { - const filters: AdHocFilterWithLabels[] = [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'extra', value: 'val', operator: '=' }, - ]; - - const manager = createManager({ adhocVar: createAdhocVar(filters) }); - - manager['_applicabilityResults'] = { - filters: [{ key: 'env', applicable: true }], - groupBy: [], - }; - - const result = manager.getFilters() ?? []; - expect(result).toHaveLength(2); - expect(result[0].key).toBe('env'); - expect(result[1].key).toBe('extra'); - }); - - it('should exclude overridden duplicate-key filter and keep the active one', () => { - const filters: AdHocFilterWithLabels[] = [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'env', value: 'staging', operator: '=' }, - ]; - - const manager = createManager({ adhocVar: createAdhocVar(filters) }); - - manager['_applicabilityResults'] = { - filters: [ - { key: 'env', applicable: false, reason: 'overridden by duplicate' }, - { key: 'env', applicable: true }, - ], - groupBy: [], - }; - - const result = manager.getFilters() ?? []; - expect(result).toHaveLength(1); - expect(result[0].value).toBe('staging'); - }); - - it('should exclude all duplicate-key filters when none are applicable', () => { - const filters: AdHocFilterWithLabels[] = [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'env', value: 'nonexistent', operator: '=' }, - ]; - - const manager = createManager({ adhocVar: createAdhocVar(filters) }); - - manager['_applicabilityResults'] = { - filters: [ - { key: 'env', applicable: false, reason: 'overridden by duplicate' }, - { key: 'env', applicable: false, reason: 'value not found' }, - ], - groupBy: [], - }; - - const result = manager.getFilters() ?? []; - expect(result).toHaveLength(0); - }); - - it('should separate user filters from origin filters with same key via origin', () => { - const filters: AdHocFilterWithLabels[] = [{ key: 'env', value: 'prod', operator: '=' }]; - const originFilters: AdHocFilterWithLabels[] = [ - { key: 'env', value: 'staging', operator: '=', origin: 'dashboard' }, - ]; - - const manager = createManager({ adhocVar: createAdhocVar(filters, originFilters) }); - - manager['_applicabilityResults'] = { - filters: [ - { key: 'env', applicable: false, reason: 'overridden by user filter', origin: 'dashboard' }, - { key: 'env', applicable: true }, - ], - groupBy: [], - }; - - const result = manager.getFilters() ?? []; - expect(result).toHaveLength(1); - expect(result[0].origin).toBeUndefined(); - }); }); describe('getGroupByKeys', () => { @@ -426,167 +97,9 @@ describe('DrilldownDependenciesManager', () => { expect(manager.getGroupByKeys()).toBeUndefined(); }); - it('should return all keys when no applicability results', () => { - const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod']) }); - expect(manager.getGroupByKeys()).toEqual(['ns', 'pod']); - }); - - it('should exclude per-panel nonApplicable groupBy keys', () => { - const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod', 'container']) }); - - manager['_applicabilityResults'] = { - filters: [], - groupBy: [ - { key: 'ns', applicable: true }, - { key: 'pod', applicable: false, reason: 'label not found' }, - { key: 'container', applicable: true }, - ], - }; - - expect(manager.getGroupByKeys()).toEqual(['ns', 'container']); - }); - - it('should keep keys with no matching response entry', () => { + it('should return all applicable keys', () => { const manager = createManager({ groupByVar: createGroupByVar(['ns', 'pod']) }); - - manager['_applicabilityResults'] = { - filters: [], - groupBy: [{ key: 'ns', applicable: true }], - }; - expect(manager.getGroupByKeys()).toEqual(['ns', 'pod']); }); - - it('should respect variable-level keysApplicability then per-panel results', () => { - const manager = createManager({ - groupByVar: createGroupByVar( - ['ns', 'pod', 'container'], - [ - { key: 'ns', applicable: true }, - { key: 'pod', applicable: false }, - { key: 'container', applicable: true }, - ] - ), - }); - - manager['_applicabilityResults'] = { - filters: [], - groupBy: [ - { key: 'ns', applicable: false, reason: 'per-panel not found' }, - { key: 'container', applicable: true }, - ], - }; - - // 'pod' excluded by variable-level keysApplicability - // 'ns' further excluded by per-panel result - expect(manager.getGroupByKeys()).toEqual(['container']); - }); - - it('should not cross-contaminate filter and groupBy results for same key', () => { - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }]), - groupByVar: createGroupByVar(['env']), - }); - - manager['_applicabilityResults'] = { - filters: [{ key: 'env', applicable: true }], - groupBy: [{ key: 'env', applicable: false, reason: 'groupBy env not found' }], - }; - - const filters = manager.getFilters() ?? []; - expect(filters).toHaveLength(1); - expect(filters[0].key).toBe('env'); - expect(manager.getGroupByKeys()).toEqual([]); - }); - }); - - describe('combined filters and groupBy', () => { - it('should resolve and return both filters and groupBy keys through the full flow', async () => { - // DS returns results in input order: originFilters, then filters, then groupByKeys - const getDrilldownsApplicability = jest.fn().mockResolvedValue([ - { key: 'region', applicable: true, origin: 'dashboard' }, - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'label not found' }, - { key: 'namespace', applicable: true }, - { key: 'pod', applicable: false, reason: 'label not found' }, - ]); - - const manager = createManager({ - adhocVar: createAdhocVar( - [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ], - [{ key: 'region', value: 'eu', operator: '=', origin: 'dashboard' }], - true - ), - groupByVar: createGroupByVar(['namespace', 'pod'], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - const filters = manager.getFilters() ?? []; - expect(filters).toHaveLength(2); - expect(filters[0].key).toBe('region'); - expect(filters[1].key).toBe('env'); - - expect(manager.getGroupByKeys()).toEqual(['namespace']); - }); - - it('should correctly split results when filter and groupBy share the same key name', async () => { - const getDrilldownsApplicability = jest.fn().mockResolvedValue([ - // filter result for 'env' (applicable) - { key: 'env', applicable: true }, - // groupBy result for 'env' (not applicable) - { key: 'env', applicable: false, reason: 'label not found for groupBy' }, - ]); - - const manager = createManager({ - adhocVar: createAdhocVar([{ key: 'env', value: 'prod', operator: '=' }], undefined, true), - groupByVar: createGroupByVar(['env'], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - const results = manager.getApplicabilityResults(); - expect(results?.filters).toEqual([{ key: 'env', applicable: true }]); - expect(results?.groupBy).toEqual([{ key: 'env', applicable: false, reason: 'label not found for groupBy' }]); - - const filters = manager.getFilters() ?? []; - expect(filters).toHaveLength(1); - expect(filters[0].key).toBe('env'); - - expect(manager.getGroupByKeys()).toEqual([]); - }); - - it('should handle filters-only response when both variables exist', async () => { - const getDrilldownsApplicability = jest.fn().mockResolvedValue([ - { key: 'env', applicable: true }, - { key: 'cluster', applicable: false, reason: 'overridden' }, - ]); - - const manager = createManager({ - adhocVar: createAdhocVar( - [ - { key: 'env', value: 'prod', operator: '=' }, - { key: 'cluster', value: 'us', operator: '=' }, - ], - undefined, - true - ), - groupByVar: createGroupByVar([], undefined, true), - }); - - const ds = { getDrilldownsApplicability } as unknown as DataSourceApi; - await manager.resolveApplicability(ds, [], getDefaultTimeRange(), undefined); - - const filters = manager.getFilters() ?? []; - expect(filters).toHaveLength(1); - expect(filters[0].key).toBe('env'); - - expect(manager.getGroupByKeys()).toEqual([]); - }); }); }); diff --git a/packages/scenes/src/variables/DrilldownDependenciesManager.ts b/packages/scenes/src/variables/DrilldownDependenciesManager.ts index 33b64a884..200e12adc 100644 --- a/packages/scenes/src/variables/DrilldownDependenciesManager.ts +++ b/packages/scenes/src/variables/DrilldownDependenciesManager.ts @@ -1,10 +1,3 @@ -import { - DataSourceApi, - // @ts-expect-error (temporary till we update grafana/data) - DrilldownsApplicability, - Scope, - TimeRange, -} from '@grafana/data'; import { findClosestAdHocFilterInHierarchy, findGlobalAdHocFilterVariableByUid, @@ -21,13 +14,7 @@ import { isFilterComplete, } from '../variables/adhoc/AdHocFiltersVariable'; import { VariableDependencyConfig } from '../variables/VariableDependencyConfig'; -import { SceneDataQuery, SceneObject, SceneObjectState } from '../core/types'; -import { buildApplicabilityMatcher, buildApplicabilityCacheKey } from '../variables/applicabilityUtils'; - -export interface ApplicabilityResults { - filters: DrilldownsApplicability[]; - groupBy: DrilldownsApplicability[]; -} +import { SceneObject, SceneObjectState } from '../core/types'; /** * Manages ad-hoc filters and group-by variables for data providers @@ -36,8 +23,6 @@ export class DrilldownDependenciesManager { private _adhocFiltersVar?: AdHocFiltersVariable; private _groupByVar?: GroupByVariable; private _variableDependency: VariableDependencyConfig; - private _applicabilityResults?: ApplicabilityResults; - private _lastApplicabilityCacheKey?: string; public constructor(variableDependency: VariableDependencyConfig) { this._variableDependency = variableDependency; @@ -95,107 +80,6 @@ export class DrilldownDependenciesManager { return this._groupByVar; } - /** - * Resolves per-panel drilldown applicability before the data query runs. - * All gating logic lives here so SceneQueryRunner stays thin. - */ - public async resolveApplicability( - ds: DataSourceApi, - queries: SceneDataQuery[], - timeRange: TimeRange, - scopes: Scope[] | undefined - ): Promise { - if (!this._adhocFiltersVar && !this._groupByVar) { - return; - } - - const filtersApplicabilityEnabled = this._adhocFiltersVar?.state.applicabilityEnabled; - const groupByApplicabilityEnabled = this._groupByVar?.state.applicabilityEnabled; - - if (!filtersApplicabilityEnabled && !groupByApplicabilityEnabled) { - return; - } - - // @ts-expect-error (temporary till we update grafana/data) - if (!ds.getDrilldownsApplicability) { - return; - } - - const filters = this._adhocFiltersVar - ? [...(this._adhocFiltersVar.state.originFilters ?? []), ...this._adhocFiltersVar.state.filters] - : []; - const groupByKeys = this._groupByVar - ? Array.isArray(this._groupByVar.state.value) - ? this._groupByVar.state.value.map((v) => String(v)) - : this._groupByVar.state.value - ? [String(this._groupByVar.state.value)] - : [] - : []; - - if (filters.length === 0 && groupByKeys.length === 0) { - this._clearPerPanelApplicability(); - this._lastApplicabilityCacheKey = undefined; - return; - } - - const cacheKey = buildApplicabilityCacheKey({ - filters: filters.map((f) => ({ - origin: f.origin, - key: f.key, - operator: f.operator, - value: f.value, - values: f.values, - })), - groupByKeys, - scopes, - }); - if (cacheKey === this._lastApplicabilityCacheKey && this._applicabilityResults) { - return; - } - - try { - // @ts-expect-error (temporary till we update grafana/data) - const results: DrilldownsApplicability[] = await ds.getDrilldownsApplicability({ - filters, - groupByKeys, - queries, - timeRange, - scopes, - }); - - // The DS returns results in order: one entry per filter, then one per groupBy key. - // Split by count so filters and groupBy keys with the same name don't share a queue. - const filterResults: DrilldownsApplicability[] = results.slice(0, filters.length); - const groupByResults: DrilldownsApplicability[] = results.slice( - filters.length, - filters.length + groupByKeys.length - ); - - this._setPerPanelApplicability({ filters: filterResults, groupBy: groupByResults }); - this._lastApplicabilityCacheKey = cacheKey; - } catch { - this._clearPerPanelApplicability(); - this._lastApplicabilityCacheKey = undefined; - } - } - - /** - * Returns the applicability results from the last resolveApplicability() call, - * split into filter and groupBy portions. - * Used by UI components to display which filters are non-applicable for this panel. - */ - public getApplicabilityResults(): ApplicabilityResults | undefined { - return this._applicabilityResults; - } - - private _clearPerPanelApplicability(): void { - this._applicabilityResults = undefined; - } - - private _setPerPanelApplicability(results: ApplicabilityResults): void { - this._applicabilityResults = results; - } - public getFilters(): AdHocFilterWithLabels[] | undefined { if (!this._adhocFiltersVar) { return undefined; @@ -204,44 +88,18 @@ export class DrilldownDependenciesManager { const stateFilters = this._adhocFiltersVar.state.filters; const originFilters = this._adhocFiltersVar.state.originFilters ?? []; const allFilters = [...originFilters, ...stateFilters]; - - if (!this._applicabilityResults) { - return allFilters.filter((f) => isFilterComplete(f) && isFilterApplicable(f)); - } - - const matchResult = buildApplicabilityMatcher(this._applicabilityResults.filters); - - return allFilters.filter((f, i) => { - if (!isFilterComplete(f) || !isFilterApplicable(f)) { - return false; - } - const result = matchResult(f.key, f.origin, i); - return !result || result.applicable; - }); + return allFilters.filter((f) => isFilterComplete(f) && isFilterApplicable(f)); } public getGroupByKeys(): string[] | undefined { if (!this._groupByVar) { return undefined; } - - const keys = this._groupByVar.getApplicableKeys(); - - if (!this._applicabilityResults) { - return keys; - } - - const matchResult = buildApplicabilityMatcher(this._applicabilityResults.groupBy); - - return keys.filter((k) => { - const result = matchResult(k); - return !result || result.applicable; - }); + return this._groupByVar.getApplicableKeys(); } public cleanup(): void { this._adhocFiltersVar = undefined; this._groupByVar = undefined; - this._lastApplicabilityCacheKey = undefined; } } diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx index 77ed1aa2e..2d4735d8f 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx @@ -234,7 +234,7 @@ let runRequestSet = false; function setup(overrides?: Partial) { const getTagKeysSpy = jest.fn(); const getTagValuesSpy = jest.fn(); - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue(new Map([['_default_', []]])); const getRecommendedDrilldownsSpy = jest.fn().mockResolvedValue({ filters: [] }); setDataSourceSrv({ diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx index 0565cbc7e..ca845c976 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.tsx @@ -3,14 +3,14 @@ import { store } from '@grafana/data'; import { sceneGraph } from '../../core/sceneGraph'; import { getEnrichedDataRequest } from '../../querying/getEnrichedDataRequest'; import { getQueriesForVariables } from '../utils'; -import { getDataSource } from '../../utils/getDataSource'; + import { DrilldownRecommendations, DrilldownPill } from '../components/DrilldownRecommendations'; import { ScopesVariable } from '../variants/ScopesVariable'; import { SCOPES_VARIABLE_NAME } from '../constants'; import { AdHocFilterWithLabels, AdHocFiltersVariable } from './AdHocFiltersVariable'; import { SceneObjectBase } from '../../core/SceneObjectBase'; import { SceneComponentProps, SceneObjectState } from '../../core/types'; -import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; + import { Unsubscribable } from 'rxjs'; import { buildApplicabilityMatcher } from '../applicabilityUtils'; @@ -42,10 +42,6 @@ export class AdHocFiltersRecommendations extends SceneObjectBase { const json = store.get(this._getStorageKey()); const storedFilters = json ? JSON.parse(json) : []; @@ -107,7 +103,7 @@ export class AdHocFiltersRecommendations extends SceneObjectBase ({ + const results = (options.filters ?? []).map((f: any) => ({ key: f.key, applicable: !nonApplicableKeys.has(f.key), ...(nonApplicableKeys.has(f.key) && { reason: 'reason' }), ...(f.origin && { origin: f.origin }), })); + return new Map([['_default_', results]]); }, }), }; diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 6c35960f7..3023cb8a6 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -1,6 +1,7 @@ import React, { useMemo } from 'react'; import { AdHocVariableFilter, + DataSourceApi, GetTagResponse, GrafanaTheme2, MetricFindValue, @@ -32,7 +33,7 @@ import { getEnrichedFiltersRequest } from '../getEnrichedFiltersRequest'; import { AdHocFiltersComboboxRenderer } from './AdHocFiltersCombobox/AdHocFiltersComboboxRenderer'; import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { debounce, isEqual } from 'lodash'; -import { buildApplicabilityMatcher, buildApplicabilityCacheKey } from '../applicabilityUtils'; +import { buildApplicabilityMatcher } from '../applicabilityUtils'; import { getAdHocFiltersFromScopes } from './getAdHocFiltersFromScopes'; import { VariableDependencyConfig } from '../VariableDependencyConfig'; import { getQueryController } from '../../core/sceneGraph/getQueryController'; @@ -274,7 +275,9 @@ export class AdHocFiltersVariable protected _urlSync = new AdHocFiltersVariableUrlSyncHandler(this); private _debouncedVerifyApplicability = debounce(this._verifyApplicability, 100); - private _lastApplicabilityCacheKey?: string; + + private _resolvedDs?: DataSourceApi; + private _resolvedDsUid?: string; private _recommendations: AdHocFiltersRecommendations | undefined; @@ -319,7 +322,8 @@ export class AdHocFiltersVariable this._debouncedVerifyApplicability(); return () => { - this._lastApplicabilityCacheKey = undefined; + this._resolvedDs = undefined; + this._resolvedDsUid = undefined; this.state.originFilters?.forEach((filter) => { if (filter.restorable) { this.restoreOriginalFilter(filter); @@ -717,11 +721,25 @@ export class AdHocFiltersVariable } } + public async getResolvedDataSource(): Promise { + const currentUid = this.state.datasource?.uid; + if (this._resolvedDs && this._resolvedDsUid === currentUid) { + return this._resolvedDs; + } + + const ds = await this._dataSourceSrv.get(this.state.datasource, this._scopedVars); + if (ds) { + this._resolvedDs = ds; + this._resolvedDsUid = currentUid; + } + return ds; + } + public async getFiltersApplicabilityForQueries( filters: AdHocFilterWithLabels[], queries: SceneDataQuery[] ): Promise { - const ds = await this._dataSourceSrv.get(this.state.datasource, this._scopedVars); + const ds = await this.getResolvedDataSource(); // @ts-expect-error (temporary till we update grafana/data) if (!ds || !ds.getDrilldownsApplicability) { return; @@ -730,13 +748,15 @@ export class AdHocFiltersVariable const timeRange = sceneGraph.getTimeRange(this).state.value; // @ts-expect-error (temporary till we update grafana/data) - return await ds.getDrilldownsApplicability({ + const result: Map = await ds.getDrilldownsApplicability({ filters, queries, timeRange, scopes: sceneGraph.getScopes(this), ...getEnrichedFiltersRequest(this), }); + + return result?.get('_default_'); } public async _verifyApplicability() { @@ -746,21 +766,6 @@ export class AdHocFiltersVariable const filters = [...(this.state.originFilters ?? []), ...this.state.filters]; const queries = this.state.useQueriesAsFilterForOptions ? getQueriesForVariables(this) : undefined; - const scopes = sceneGraph.getScopes(this); - - const cacheKey = buildApplicabilityCacheKey({ - filters: filters.map((f) => ({ - origin: f.origin, - key: f.key, - operator: f.operator, - value: f.value, - values: f.values, - })), - scopes, - }); - if (cacheKey === this._lastApplicabilityCacheKey) { - return; - } const response = await this.getFiltersApplicabilityForQueries(filters, queries ?? []); @@ -768,8 +773,6 @@ export class AdHocFiltersVariable return; } - this._lastApplicabilityCacheKey = cacheKey; - const matchResult = buildApplicabilityMatcher(response); const update = { diff --git a/packages/scenes/src/variables/applicabilityUtils.ts b/packages/scenes/src/variables/applicabilityUtils.ts index 6bab52406..986e90bfd 100644 --- a/packages/scenes/src/variables/applicabilityUtils.ts +++ b/packages/scenes/src/variables/applicabilityUtils.ts @@ -1,7 +1,6 @@ import { // @ts-expect-error (temporary till we update grafana/data) DrilldownsApplicability, - Scope, } from '@grafana/data'; const COMPOSITE_KEY_SEPARATOR = '|'; @@ -37,20 +36,3 @@ export function buildApplicabilityMatcher(response: DrilldownsApplicability[]) { return map.get(compositeKey(key, origin, index)); }; } - -export function buildApplicabilityCacheKey(parts: { - filters?: Array<{ origin?: string; key: string; operator: string; value: string; values?: string[] }>; - groupByKeys?: string[]; - scopes: Scope[] | undefined; -}): string { - return JSON.stringify({ - filters: parts.filters?.map((f) => ({ - origin: f.origin, - key: f.key, - operator: f.operator, - value: f.values?.length ? f.values.join(',') : f.value, - })), - groupByKeys: parts.groupByKeys, - scopes: parts.scopes?.map((s) => s.metadata.name), - }); -} diff --git a/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx b/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx index 7d4dbce09..8c30cb2ea 100644 --- a/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx +++ b/packages/scenes/src/variables/groupby/GroupByRecommendations.test.tsx @@ -189,7 +189,7 @@ describe('GroupByRecommendations', () => { describe('addValueToParent', () => { it('should add value to parent variable', async () => { const getRecommendedDrilldowns = jest.fn().mockResolvedValue(undefined); - const getDrilldownsApplicability = jest.fn().mockResolvedValue(undefined); + const getDrilldownsApplicability = jest.fn().mockResolvedValue(new Map([['_default_', []]])); const { variable } = setupTest( { @@ -261,7 +261,9 @@ describe('GroupByRecommendations', () => { describe('integration with parent variable', () => { it('should store recent grouping when parent calls _verifyApplicabilityAndStoreRecentGrouping', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'value1', applicable: true }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'value1', applicable: true }]]])); const { variable } = setupTest( { @@ -294,7 +296,9 @@ describe('GroupByRecommendations', () => { }); it('should not store non-applicable values', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'value1', applicable: false }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'value1', applicable: false }]]])); const { variable } = setupTest( { @@ -369,7 +373,7 @@ interface DsOverrides { function setupTest(overrides?: Partial, dsOverrides?: DsOverrides) { const getGroupByKeysSpy = jest.fn().mockResolvedValue([{ text: 'key3', value: 'key3' }]); const getDrilldownsApplicabilitySpy = - dsOverrides?.getDrilldownsApplicability ?? jest.fn().mockResolvedValue(undefined); + dsOverrides?.getDrilldownsApplicability ?? jest.fn().mockResolvedValue(new Map([['_default_', []]])); const getRecommendedDrilldownsSpy = dsOverrides?.getRecommendedDrilldowns ?? jest.fn().mockResolvedValue(undefined); setDataSourceSrv({ diff --git a/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx b/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx index 5ea401046..7689db80a 100644 --- a/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx +++ b/packages/scenes/src/variables/groupby/GroupByRecommendations.tsx @@ -13,7 +13,7 @@ import { VariableValueSingle } from '../types'; import { isArray } from 'lodash'; import { SceneObjectBase } from '../../core/SceneObjectBase'; import { SceneComponentProps, SceneObjectState } from '../../core/types'; -import { buildApplicabilityMatcher } from '../applicabilityUtils'; + import { wrapInSafeSerializableSceneObject } from '../../utils/wrapInSafeSerializableSceneObject'; import { Unsubscribable } from 'rxjs'; @@ -50,11 +50,7 @@ export class GroupByRecommendations extends SceneObjectBase 0) { - this._verifyRecentGroupingsApplicability(storedGroupings); - } else { - this.setState({ recentGrouping: [] }); - } + this.setState({ recentGrouping: storedGroupings.slice(-MAX_RECENT_DRILLDOWNS) }); this._fetchRecommendedDrilldowns(); @@ -67,13 +63,6 @@ export class GroupByRecommendations extends SceneObjectBase { if (newState.scopes !== prevState.scopes) { - const json = store.get(this._getStorageKey()); - const storedGroupings = json ? JSON.parse(json) : []; - - if (storedGroupings.length > 0) { - this._verifyRecentGroupingsApplicability(storedGroupings); - } - this._fetchRecommendedDrilldowns(); } })) @@ -83,13 +72,6 @@ export class GroupByRecommendations extends SceneObjectBase { if (newState.value !== prevState.value) { - const json = store.get(this._getStorageKey()); - const storedGroupings = json ? JSON.parse(json) : []; - - if (storedGroupings.length > 0) { - this._verifyRecentGroupingsApplicability(storedGroupings); - } - this._fetchRecommendedDrilldowns(); } })) @@ -145,28 +127,6 @@ export class GroupByRecommendations extends SceneObjectBase>) { - const queries = getQueriesForVariables(this._groupBy); - const keys = storedGroupings.map((g) => String(g.value)); - const response = await this._groupBy.getGroupByApplicabilityForQueries(keys, queries); - - if (!response) { - this.setState({ recentGrouping: storedGroupings.slice(-MAX_RECENT_DRILLDOWNS) }); - return; - } - - const matcher = buildApplicabilityMatcher(response); - - const applicableGroupings = storedGroupings - .filter((g) => { - const result = matcher(String(g.value)); - return !result || result.applicable; - }) - .slice(-MAX_RECENT_DRILLDOWNS); - - this.setState({ recentGrouping: applicableGroupings }); - } - /** * Stores recent groupings in localStorage and updates state. * Should be called by the parent variable when a grouping is added/updated. diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx index 0b596aa04..836183dc3 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.test.tsx @@ -567,10 +567,17 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { describe('_verifyApplicability', () => { it('should call getDrilldownsApplicability and update keysApplicability state', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([ - { key: 'key1', applicable: true }, - { key: 'key2', applicable: false }, - ]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue( + new Map([ + [ + '_default_', + [ + { key: 'key1', applicable: true }, + { key: 'key2', applicable: false }, + ], + ], + ]) + ); const { variable } = setupTest({ value: ['key1', 'key2'], applicabilityEnabled: true }, undefined, undefined, { // @ts-expect-error (temporary till we update grafana/data) @@ -626,7 +633,9 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should be called during activation handler', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'key1', applicable: true }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'key1', applicable: true }]]])); const { variable } = setupTest({ value: ['key1'], applicabilityEnabled: true }, undefined, undefined, { // @ts-expect-error (temporary till we update grafana/data) @@ -643,10 +652,17 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should pass values to verifyApplicabilitySpy on blur', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([ - { key: 'existingKey', applicable: true }, - { key: 'newTypedKey', applicable: false }, - ]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue( + new Map([ + [ + '_default_', + [ + { key: 'existingKey', applicable: true }, + { key: 'newTypedKey', applicable: false }, + ], + ], + ]) + ); const { variable } = setupTest( { @@ -815,7 +831,9 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should add applicable keys to recentGrouping and store in localStorage after verifyApplicabilityAndStoreRecentGrouping', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'value1', applicable: true }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'value1', applicable: true }]]])); const { variable } = setupTest( { @@ -845,7 +863,9 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should not store non-applicable keys in recentGrouping', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'value1', applicable: false }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'value1', applicable: false }]]])); const { variable } = setupTest( { @@ -873,11 +893,18 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should only store applicable keys when some are non-applicable', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([ - { key: 'value1', applicable: true }, - { key: 'value2', applicable: false }, - { key: 'value3', applicable: true }, - ]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue( + new Map([ + [ + '_default_', + [ + { key: 'value1', applicable: true }, + { key: 'value2', applicable: false }, + { key: 'value3', applicable: true }, + ], + ], + ]) + ); const { variable } = setupTest( { @@ -914,12 +941,19 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { } localStorage.setItem(RECENT_GROUPING_KEY, JSON.stringify(existingGroupings)); - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([ - { key: 'newValue1', applicable: true }, - { key: 'newValue2', applicable: true }, - { key: 'newValue3', applicable: true }, - { key: 'newValue4', applicable: true }, - ]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue( + new Map([ + [ + '_default_', + [ + { key: 'newValue1', applicable: true }, + { key: 'newValue2', applicable: true }, + { key: 'newValue3', applicable: true }, + { key: 'newValue4', applicable: true }, + ], + ], + ]) + ); const { variable } = setupTest( { @@ -947,10 +981,17 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should set in browser storage with applicable values', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([ - { key: 'value1', applicable: true }, - { key: 'value2', applicable: true }, - ]); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue( + new Map([ + [ + '_default_', + [ + { key: 'value1', applicable: true }, + { key: 'value2', applicable: true }, + ], + ], + ]) + ); const { variable } = setupTest( { @@ -976,7 +1017,9 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => { }); it('should not store anything if drilldownRecommendationsEnabled is false', async () => { - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue([{ key: 'value1', applicable: true }]); + const getDrilldownsApplicabilitySpy = jest + .fn() + .mockResolvedValue(new Map([['_default_', [{ key: 'value1', applicable: true }]]])); const { variable } = setupTest( { diff --git a/packages/scenes/src/variables/groupby/GroupByVariable.tsx b/packages/scenes/src/variables/groupby/GroupByVariable.tsx index 948430b93..699755118 100644 --- a/packages/scenes/src/variables/groupby/GroupByVariable.tsx +++ b/packages/scenes/src/variables/groupby/GroupByVariable.tsx @@ -12,15 +12,9 @@ import { } from '@grafana/data'; import { allActiveGroupByVariables } from './findActiveGroupByVariablesByUid'; import { DataSourceRef, VariableType } from '@grafana/schema'; -import { SceneComponentProps, ControlsLayout, SceneObjectUrlSyncHandler, SceneDataQuery } from '../../core/types'; +import { SceneComponentProps, ControlsLayout, SceneObjectUrlSyncHandler } from '../../core/types'; import { sceneGraph } from '../../core/sceneGraph'; -import { - SceneVariableValueChangedEvent, - ValidateAndUpdateResult, - VariableValue, - VariableValueOption, - VariableValueSingle, -} from '../types'; +import { ValidateAndUpdateResult, VariableValue, VariableValueOption, VariableValueSingle } from '../types'; import { MultiValueVariable, MultiValueVariableState, VariableGetOptionsArgs } from '../variants/MultiValueVariable'; import { from, lastValueFrom, map, mergeMap, Observable, of, take, tap } from 'rxjs'; import { getDataSource } from '../../utils/getDataSource'; @@ -39,7 +33,6 @@ import { getInteractionTracker } from '../../core/sceneGraph/getInteractionTrack import { GROUPBY_DIMENSIONS_INTERACTION } from '../../performance/interactionConstants'; import { css, cx } from '@emotion/css'; import { GroupByRecommendations } from './GroupByRecommendations'; -import { buildApplicabilityCacheKey } from '../applicabilityUtils'; export interface GroupByVariableState extends MultiValueVariableState { /** Defaults to "Group" */ @@ -112,7 +105,6 @@ export class GroupByVariable extends MultiValueVariable { private _scopedVars = { __sceneObject: wrapInSafeSerializableSceneObject(this) }; private _recommendations: GroupByRecommendations | undefined; - private _lastApplicabilityCacheKey?: string; public validateAndUpdate(): Observable { return this.getValueOptions({}).pipe( @@ -222,8 +214,6 @@ export class GroupByVariable extends MultiValueVariable { } private _activationHandler = () => { - this._verifyApplicability(); - if (this.state.defaultValue) { if (this.checkIfRestorable(this.state.value)) { this.setState({ restorable: true }); @@ -231,8 +221,6 @@ export class GroupByVariable extends MultiValueVariable { } return () => { - this._lastApplicabilityCacheKey = undefined; - if (this.state.defaultValue) { this.restoreDefaultValues(); } @@ -263,63 +251,6 @@ export class GroupByVariable extends MultiValueVariable { return applicableValues; } - public async getGroupByApplicabilityForQueries( - value: VariableValue, - queries: SceneDataQuery[] - ): Promise { - const ds = await getDataSource(this.state.datasource, this._scopedVars); - - // @ts-expect-error (temporary till we update grafana/data) - if (!ds.getDrilldownsApplicability) { - return; - } - - const timeRange = sceneGraph.getTimeRange(this).state.value; - - // @ts-expect-error (temporary till we update grafana/data) - return await ds.getDrilldownsApplicability({ - groupByKeys: Array.isArray(value) ? value.map((v) => String(v)) : value ? [String(value)] : [], - queries, - timeRange, - scopes: sceneGraph.getScopes(this), - ...getEnrichedFiltersRequest(this), - }); - } - - public async _verifyApplicability() { - if (!this.state.applicabilityEnabled) { - return; - } - - const queries = getQueriesForVariables(this); - const value = this.state.value; - const scopes = sceneGraph.getScopes(this); - - const cacheKey = buildApplicabilityCacheKey({ - groupByKeys: Array.isArray(value) ? value.map(String) : [String(value ?? '')], - scopes, - }); - if (cacheKey === this._lastApplicabilityCacheKey) { - return; - } - - const response = await this.getGroupByApplicabilityForQueries(value, queries); - - if (!response) { - return; - } - - this._lastApplicabilityCacheKey = cacheKey; - - if (!isEqual(response, this.state.keysApplicability)) { - this.setState({ keysApplicability: response ?? undefined, applicabilityEnabled: true }); - - this.publishEvent(new SceneVariableValueChangedEvent(this), true); - } else { - this.setState({ applicabilityEnabled: true }); - } - } - // This method is related to the defaultValue property. We check if the current value // is different from the default value. If it is, the groupBy will show a button // allowing the user to restore the default values. @@ -399,9 +330,7 @@ export class GroupByVariable extends MultiValueVariable { return keys; }; - public async _verifyApplicabilityAndStoreRecentGrouping() { - await this._verifyApplicability(); - + public _verifyApplicabilityAndStoreRecentGrouping() { if (!this._recommendations) { return; } From 200f5fcafdb5574d9a95f5cab99e815272c576b0 Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Thu, 19 Mar 2026 15:28:21 +0200 Subject: [PATCH 23/25] fixes --- packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 3023cb8a6..166561233 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -7,6 +7,8 @@ import { MetricFindValue, // @ts-expect-error (temporary till we update grafana/data) DrilldownsApplicability, + // @ts-expect-error (temporary till we update grafana/data) + DEFAULT_APPLICABILITY_KEY, Scope, SelectableValue, } from '@grafana/data'; @@ -756,7 +758,7 @@ export class AdHocFiltersVariable ...getEnrichedFiltersRequest(this), }); - return result?.get('_default_'); + return result?.get(DEFAULT_APPLICABILITY_KEY); } public async _verifyApplicability() { From 5769a1f335ffb7e5bb5ae5ab0eaa930b34511fbb Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Fri, 20 Mar 2026 11:21:29 +0200 Subject: [PATCH 24/25] fixes --- .../adhoc/AdHocFiltersRecommendations.test.tsx | 12 ++++++++++-- .../variables/adhoc/AdHocFiltersVariable.test.tsx | 4 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx index 2d4735d8f..a20ed69bb 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersRecommendations.test.tsx @@ -1,7 +1,15 @@ import React from 'react'; import { render, waitFor } from '@testing-library/react'; import { DataSourceSrv, locationService, setDataSourceSrv, setRunRequest, setTemplateSrv } from '@grafana/runtime'; -import { DataQueryRequest, DataSourceApi, getDefaultTimeRange, LoadingState, PanelData } from '@grafana/data'; +import { + DataQueryRequest, + DataSourceApi, + getDefaultTimeRange, + LoadingState, + PanelData, + // @ts-expect-error (temporary till we update grafana/data) + DEFAULT_APPLICABILITY_KEY, +} from '@grafana/data'; import { Observable, of } from 'rxjs'; import { EmbeddedScene } from '../../components/EmbeddedScene'; import { SceneFlexLayout, SceneFlexItem } from '../../components/layout/SceneFlexLayout'; @@ -234,7 +242,7 @@ let runRequestSet = false; function setup(overrides?: Partial) { const getTagKeysSpy = jest.fn(); const getTagValuesSpy = jest.fn(); - const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue(new Map([['_default_', []]])); + const getDrilldownsApplicabilitySpy = jest.fn().mockResolvedValue(new Map([[DEFAULT_APPLICABILITY_KEY, []]])); const getRecommendedDrilldownsSpy = jest.fn().mockResolvedValue({ filters: [] }); setDataSourceSrv({ diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx index 56643ded6..827b25af0 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.test.tsx @@ -26,6 +26,8 @@ import { PanelData, Scope, ScopeSpecFilter, + // @ts-expect-error (temporary till we update grafana/data) + DEFAULT_APPLICABILITY_KEY, } from '@grafana/data'; import { Observable, of } from 'rxjs'; import userEvent from '@testing-library/user-event'; @@ -3327,7 +3329,7 @@ function setup( ...(nonApplicableKeys.has(f.key) && { reason: 'reason' }), ...(f.origin && { origin: f.origin }), })); - return new Map([['_default_', results]]); + return new Map([[DEFAULT_APPLICABILITY_KEY, results]]); }, }), }; From 7543750c723d5e1190c9aa18574781a529c128ce Mon Sep 17 00:00:00 2001 From: Victor Marin Date: Tue, 24 Mar 2026 13:22:50 +0200 Subject: [PATCH 25/25] fix --- .../variables/adhoc/AdHocFiltersVariable.tsx | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx index 166561233..27782dd89 100644 --- a/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx +++ b/packages/scenes/src/variables/adhoc/AdHocFiltersVariable.tsx @@ -1,7 +1,6 @@ import React, { useMemo } from 'react'; import { AdHocVariableFilter, - DataSourceApi, GetTagResponse, GrafanaTheme2, MetricFindValue, @@ -278,9 +277,6 @@ export class AdHocFiltersVariable private _debouncedVerifyApplicability = debounce(this._verifyApplicability, 100); - private _resolvedDs?: DataSourceApi; - private _resolvedDsUid?: string; - private _recommendations: AdHocFiltersRecommendations | undefined; public constructor(state: Partial) { @@ -324,8 +320,6 @@ export class AdHocFiltersVariable this._debouncedVerifyApplicability(); return () => { - this._resolvedDs = undefined; - this._resolvedDsUid = undefined; this.state.originFilters?.forEach((filter) => { if (filter.restorable) { this.restoreOriginalFilter(filter); @@ -723,18 +717,8 @@ export class AdHocFiltersVariable } } - public async getResolvedDataSource(): Promise { - const currentUid = this.state.datasource?.uid; - if (this._resolvedDs && this._resolvedDsUid === currentUid) { - return this._resolvedDs; - } - - const ds = await this._dataSourceSrv.get(this.state.datasource, this._scopedVars); - if (ds) { - this._resolvedDs = ds; - this._resolvedDsUid = currentUid; - } - return ds; + public async getResolvedDataSource() { + return this._dataSourceSrv.get(this.state.datasource, this._scopedVars); } public async getFiltersApplicabilityForQueries(