Schema: Change how origin filters are added to AdHocVariable (#106086)

* refactor origin filters schema

* add canary version

* update schema

* fix failing tests

* refactor

* remove autogenerated import

* tests

* maintain only dashboard originated filters

* refactor

* refactor

* Schema: Persist defaultValue for groupBy in schema (#106542)

* persist defaultValue for groupBy in schema

* fix

* type

* refactor

* refactor

* bump scenes
This commit is contained in:
Victor Marin
2025-06-16 11:15:00 +03:00
committed by GitHub
parent f974cb12b5
commit ffc16ee072
14 changed files with 118 additions and 102 deletions
@@ -692,8 +692,7 @@ VariableRefresh: *"never" | "onDashboardLoad" | "onTimeRangeChanged"
VariableHide: *"dontHide" | "hideLabel" | "hideVariable"
// Determine the origin of the adhoc variable filter
// Accepted values are `dashboard` (filter originated from dashboard), or `scope` (filter originated from scope).
FilterOrigin: "dashboard" | "scope"
FilterOrigin: "dashboard"
// FIXME: should we introduce this? --- Variable value option
VariableValueOption: {
@@ -696,8 +696,7 @@ VariableRefresh: *"never" | "onDashboardLoad" | "onTimeRangeChanged"
VariableHide: *"dontHide" | "hideLabel" | "hideVariable"
// Determine the origin of the adhoc variable filter
// Accepted values are `dashboard` (filter originated from dashboard), or `scope` (filter originated from scope).
FilterOrigin: "dashboard" | "scope"
FilterOrigin: "dashboard"
// FIXME: should we introduce this? --- Variable value option
VariableValueOption: {
@@ -1673,14 +1673,14 @@ func NewDashboardAdhocVariableSpec() *DashboardAdhocVariableSpec {
// Define the AdHocFilterWithLabels type
// +k8s:openapi-gen=true
type DashboardAdHocFilterWithLabels struct {
Key string `json:"key"`
Operator string `json:"operator"`
Value string `json:"value"`
Values []string `json:"values,omitempty"`
KeyLabel *string `json:"keyLabel,omitempty"`
ValueLabels []string `json:"valueLabels,omitempty"`
ForceEdit *bool `json:"forceEdit,omitempty"`
Origin *DashboardFilterOrigin `json:"origin,omitempty"`
Key string `json:"key"`
Operator string `json:"operator"`
Value string `json:"value"`
Values []string `json:"values,omitempty"`
KeyLabel *string `json:"keyLabel,omitempty"`
ValueLabels []string `json:"valueLabels,omitempty"`
ForceEdit *bool `json:"forceEdit,omitempty"`
Origin string `json:"origin,omitempty"`
// @deprecated
Condition *string `json:"condition,omitempty"`
}
@@ -1691,14 +1691,8 @@ func NewDashboardAdHocFilterWithLabels() *DashboardAdHocFilterWithLabels {
}
// Determine the origin of the adhoc variable filter
// Accepted values are `dashboard` (filter originated from dashboard), or `scope` (filter originated from scope).
// +k8s:openapi-gen=true
type DashboardFilterOrigin string
const (
DashboardFilterOriginDashboard DashboardFilterOrigin = "dashboard"
DashboardFilterOriginScope DashboardFilterOrigin = "scope"
)
const DashboardFilterOrigin = "dashboard"
// Define the MetricFindValue type
// +k8s:openapi-gen=true
@@ -7,13 +7,17 @@ package apis
import (
"fmt"
"github.com/grafana/grafana-app-sdk/app"
"github.com/grafana/grafana-app-sdk/resource"
v0alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
v1beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1"
v2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1"
)
var ()
var appManifestData = app.ManifestData{
AppName: "dashboard",
Group: "dashboard.grafana.app",
+2 -2
View File
@@ -286,8 +286,8 @@
"@grafana/plugin-ui": "0.10.6",
"@grafana/prometheus": "workspace:*",
"@grafana/runtime": "workspace:*",
"@grafana/scenes": "^6.19.0",
"@grafana/scenes-react": "^6.19.0",
"@grafana/scenes": "^6.20.1",
"@grafana/scenes-react": "^6.20.1",
"@grafana/schema": "workspace:*",
"@grafana/sql": "workspace:*",
"@grafana/ui": "workspace:*",
@@ -54,6 +54,7 @@ export interface AdHocVariableFilter {
operator: string;
value: string;
values?: string[];
origin?: 'dashboard' | string;
/** @deprecated */
condition?: string;
}
@@ -1327,7 +1327,7 @@ export interface AdHocFilterWithLabels {
keyLabel?: string;
valueLabels?: string[];
forceEdit?: boolean;
origin?: FilterOrigin;
origin?: "dashboard";
// @deprecated
condition?: string;
}
@@ -1339,10 +1339,7 @@ export const defaultAdHocFilterWithLabels = (): AdHocFilterWithLabels => ({
});
// Determine the origin of the adhoc variable filter
// Accepted values are `dashboard` (filter originated from dashboard), or `scope` (filter originated from scope).
export type FilterOrigin = "dashboard" | "scope";
export const defaultFilterOrigin = (): FilterOrigin => ("dashboard");
export const FilterOrigin = "dashboard";
// Define the MetricFindValue type
export interface MetricFindValue {
@@ -12,6 +12,7 @@ import { ResponseTransformers } from 'app/features/dashboard/api/ResponseTransfo
import { isDashboardV2Spec } from 'app/features/dashboard/api/utils';
import { DashboardDataDTO, DashboardDTO } from 'app/types';
import { validateFiltersOrigin } from '../serialization/sceneVariablesSetToVariables';
import { jsonDiff } from '../settings/version-history/utils';
export function get(obj: any, keys: string[]) {
@@ -252,8 +253,8 @@ export function applyVariableChanges(saveModel: Dashboard, originalSaveModel: Da
} else if (
variable.type === 'adhoc' &&
!adHocVariableFiltersEqual(
(variable as AdHocVariableModel | undefined)?.filters,
(original as AdHocVariableModel | undefined)?.filters
validateFiltersOrigin((variable as AdHocVariableModel | undefined)?.filters),
validateFiltersOrigin((original as AdHocVariableModel | undefined)?.filters)
)
) {
hasVariableValueChanges = true;
@@ -553,14 +553,14 @@ describe('sceneVariablesSetToVariables', () => {
});
describe('should adapt AdHocFiltersVariable filters', () => {
it('should remove origin from filter if its not dashboard or scope', () => {
it('should remove non dashboard originated filters from schema', () => {
const variable = new AdHocFiltersVariable({
name: 'test',
allowCustomValue: true,
label: 'test-label',
description: 'test-desc',
datasource: { uid: 'fake-std', type: 'fake-std' },
filters: [
originFilters: [
{
key: 'filterTest',
operator: '=',
@@ -568,14 +568,7 @@ describe('sceneVariablesSetToVariables', () => {
origin: 'asserts',
},
],
baseFilters: [
{
key: 'baseFilterTest',
operator: '=',
value: 'test',
origin: 'asserts',
},
],
filters: [],
});
const set = new SceneVariableSet({
variables: [variable],
@@ -587,26 +580,14 @@ describe('sceneVariablesSetToVariables', () => {
expect(result[0]).toMatchInlineSnapshot(`
{
"allowCustomValue": true,
"baseFilters": [
{
"key": "baseFilterTest",
"operator": "=",
"value": "test",
},
],
"baseFilters": [],
"datasource": {
"type": "fake-std",
"uid": "fake-std",
},
"defaultKeys": undefined,
"description": "test-desc",
"filters": [
{
"key": "filterTest",
"operator": "=",
"value": "test",
},
],
"filters": [],
"label": "test-label",
"name": "test",
"type": "adhoc",
@@ -614,14 +595,14 @@ describe('sceneVariablesSetToVariables', () => {
`);
});
it('should maintain dashboard or scope origin', () => {
it('should not save origin filters in model', () => {
const variable = new AdHocFiltersVariable({
name: 'test',
allowCustomValue: true,
label: 'test-label',
description: 'test-desc',
datasource: { uid: 'fake-std', type: 'fake-std' },
filters: [
originFilters: [
{
key: 'filterTest',
operator: '=',
@@ -629,12 +610,11 @@ describe('sceneVariablesSetToVariables', () => {
origin: 'dashboard',
},
],
baseFilters: [
filters: [
{
key: 'baseFilterTest',
key: 'filterTest2',
operator: '=',
value: 'test',
origin: 'scope',
value: 'test2',
},
],
});
@@ -648,14 +628,7 @@ describe('sceneVariablesSetToVariables', () => {
expect(result[0]).toMatchInlineSnapshot(`
{
"allowCustomValue": true,
"baseFilters": [
{
"key": "baseFilterTest",
"operator": "=",
"origin": "scope",
"value": "test",
},
],
"baseFilters": [],
"datasource": {
"type": "fake-std",
"uid": "fake-std",
@@ -669,6 +642,11 @@ describe('sceneVariablesSetToVariables', () => {
"origin": "dashboard",
"value": "test",
},
{
"key": "filterTest2",
"operator": "=",
"value": "test2",
},
],
"label": "test-label",
"name": "test",
@@ -809,6 +787,7 @@ describe('sceneVariablesSetToVariables', () => {
"type": "fake-std",
"uid": "fake-std",
},
"defaultValue": undefined,
"description": "test-desc",
"label": "test-label",
"name": "test",
@@ -1364,6 +1343,7 @@ describe('sceneVariablesSetToVariables', () => {
"type": "fake-std",
"uid": "fake-std",
},
"defaultValue": undefined,
"description": "test-desc",
"hide": "dontHide",
"label": "test-label",
@@ -167,6 +167,14 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio
query: variable.state.value,
});
} else if (sceneUtils.isGroupByVariable(variable) && config.featureToggles.groupByVariable) {
// @ts-expect-error
const defaultVariableOption: VariableOption | undefined = variable.state.defaultValue
? {
value: variable.state.defaultValue.value,
text: variable.state.defaultValue.text,
}
: undefined;
variables.push({
...commonProperties,
datasource: variable.state.datasource,
@@ -181,6 +189,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio
// @ts-expect-error
value: variable.state.value,
},
defaultValue: defaultVariableOption,
allowCustomValue: variable.state.allowCustomValue,
});
} else if (sceneUtils.isAdHocVariable(variable)) {
@@ -189,8 +198,8 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio
datasource: variable.state.datasource,
allowCustomValue: variable.state.allowCustomValue,
// @ts-expect-error
baseFilters: validateFiltersOrigin(variable.state.baseFilters),
filters: validateFiltersOrigin(variable.state.filters),
baseFilters: variable.state.baseFilters || [],
filters: [...validateFiltersOrigin(variable.state.originFilters), ...variable.state.filters],
defaultKeys: variable.state.defaultKeys,
});
} else if (variable.state.type === 'system') {
@@ -413,6 +422,14 @@ export function sceneVariablesSetToSchemaV2Variables(
} else if (sceneUtils.isGroupByVariable(variable) && config.featureToggles.groupByVariable) {
options = variableValueOptionsToVariableOptions(variable.state);
// @ts-expect-error
const defaultVariableOption: VariableOption | undefined = variable.state.defaultValue
? {
value: variable.state.defaultValue.value,
text: variable.state.defaultValue.text,
}
: undefined;
const groupVariable: GroupByVariableKind = {
kind: 'GroupByVariable',
spec: {
@@ -425,6 +442,7 @@ export function sceneVariablesSetToSchemaV2Variables(
value: String(option.value),
})) || [],
current: currentVariableOption,
defaultValue: defaultVariableOption,
multi: variable.state.isMulti || false,
},
};
@@ -436,8 +454,11 @@ export function sceneVariablesSetToSchemaV2Variables(
...commonProperties,
name: variable.state.name,
datasource: variable.state.datasource || {}, //FIXME what is the default value?
baseFilters: validateFiltersOrigin(variable.state.baseFilters),
filters: validateFiltersOrigin(variable.state.filters),
baseFilters: validateFiltersOrigin(variable.state.baseFilters) || [],
filters: [
...validateFiltersOrigin(variable.state.originFilters),
...validateFiltersOrigin(variable.state.filters),
],
defaultKeys: variable.state.defaultKeys || [], //FIXME what is the default value?
allowCustomValue: variable.state.allowCustomValue ?? true,
},
@@ -453,21 +474,9 @@ export function sceneVariablesSetToSchemaV2Variables(
return variables;
}
function validateFiltersOrigin(filters?: SceneAdHocFilterWithLabels[]): AdHocFilterWithLabels[] {
return (
filters?.map((filter) => {
const { origin: initialOrigin, ...restOfFilter } = filter;
if (initialOrigin === 'dashboard' || initialOrigin === 'scope') {
return {
...restOfFilter,
origin: initialOrigin,
};
}
return restOfFilter;
}) || []
);
export function validateFiltersOrigin(filters?: SceneAdHocFilterWithLabels[]): AdHocFilterWithLabels[] {
// Only keep dashboard originated filters in the schema
return filters?.filter((f): f is AdHocFilterWithLabels => !f.origin || f.origin === 'dashboard') || [];
}
export function isVariableEditable(variable: SceneVariable) {
@@ -411,6 +411,12 @@ describe('when creating variables objects', () => {
operator: '=',
value: 'test',
},
{
key: 'originFilter',
operator: '=',
value: 'val',
origin: 'dashboard',
},
],
baseFilters: [
{
@@ -434,9 +440,17 @@ describe('when creating variables objects', () => {
hide: 0,
label: 'Adhoc Label',
name: 'adhoc',
originFilters: [
{
key: 'originFilter',
operator: '=',
value: 'val',
origin: 'dashboard',
},
],
skipUrlSync: false,
type: 'adhoc',
filterExpression: 'filterTest="test"',
filterExpression: 'originFilter="val",filterTest="test"',
filters: [{ key: 'filterTest', operator: '=', value: 'test' }],
baseFilters: [{ key: 'baseFilterTest', operator: '=', value: 'test' }],
datasource: { uid: 'gdev-prometheus', type: 'prometheus' },
@@ -504,6 +518,7 @@ describe('when creating variables objects', () => {
hide: 0,
label: 'Adhoc Label',
name: 'adhoc',
originFilters: [],
skipUrlSync: false,
type: 'adhoc',
filterExpression: 'filterTest="test"',
@@ -1,4 +1,4 @@
import { TypedVariableModel } from '@grafana/data';
import { AdHocVariableFilter, TypedVariableModel } from '@grafana/data';
import { config, getDataSourceSrv } from '@grafana/runtime';
import {
AdHocFiltersVariable,
@@ -133,6 +133,10 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode
description: variable.description,
};
if (variable.type === 'adhoc') {
const originFilters: AdHocVariableFilter[] = [];
const filters: AdHocVariableFilter[] = [];
variable.filters?.forEach((filter) => (filter.origin ? originFilters.push(filter) : filters.push(filter)));
return new AdHocFiltersVariable({
...commonProperties,
description: variable.description,
@@ -140,7 +144,8 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode
hide: variable.hide,
datasource: variable.datasource,
applyMode: 'auto',
filters: variable.filters ?? [],
originFilters,
filters,
baseFilters: variable.baseFilters ?? [],
defaultKeys: variable.defaultKeys,
allowCustomValue: variable.allowCustomValue,
@@ -56,6 +56,7 @@ import {
ObjectMeta,
} from 'app/features/apiserver/types';
import { GRID_ROW_HEIGHT } from 'app/features/dashboard-scene/serialization/const';
import { validateFiltersOrigin } from 'app/features/dashboard-scene/serialization/sceneVariablesSetToVariables';
import { TypedVariableModelV2 } from 'app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene';
import { getDefaultDataSourceRef } from 'app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2';
import {
@@ -627,8 +628,8 @@ function getVariables(vars: TypedVariableModel[]): DashboardV2Spec['variables']
spec: {
...commonProperties,
datasource: v.datasource || getDefaultDatasource(),
baseFilters: v.baseFilters || [],
filters: v.filters || [],
baseFilters: validateFiltersOrigin(v.baseFilters) || [],
filters: validateFiltersOrigin(v.filters) || [],
defaultKeys: v.defaultKeys || [],
allowCustomValue: v.allowCustomValue ?? true,
},
+23 -12
View File
@@ -3489,11 +3489,11 @@ __metadata:
languageName: unknown
linkType: soft
"@grafana/scenes-react@npm:^6.19.0":
version: 6.19.0
resolution: "@grafana/scenes-react@npm:6.19.0"
"@grafana/scenes-react@npm:^6.20.1":
version: 6.20.1
resolution: "@grafana/scenes-react@npm:6.20.1"
dependencies:
"@grafana/scenes": "npm:6.19.0"
"@grafana/scenes": "npm:6.20.1"
lru-cache: "npm:^10.2.2"
react-use: "npm:^17.4.0"
peerDependencies:
@@ -3505,13 +3505,13 @@ __metadata:
react: ^18.0.0
react-dom: ^18.0.0
react-router-dom: ^6.28.0
checksum: 10/3743e14282f0d8b07913bd1de9470031db1dc6930b2d1f0b864dc2a96934e11e5044eb37d7a27fca2c9f4678bf5dbb9551a6d3b0c7056acfe4024606375f8151
checksum: 10/55b828054328c202046f3ac808a15fe353075098ef7ce361c7308b0e244dc59929115983795bcaf89bdd549022331e3fd08fae5db12e57aabceb6254712f5745
languageName: node
linkType: hard
"@grafana/scenes@npm:6.19.0, @grafana/scenes@npm:^6.19.0":
version: 6.19.0
resolution: "@grafana/scenes@npm:6.19.0"
"@grafana/scenes@npm:6.20.1, @grafana/scenes@npm:^6.20.1":
version: 6.20.1
resolution: "@grafana/scenes@npm:6.20.1"
dependencies:
"@floating-ui/react": "npm:^0.26.16"
"@leeoniya/ufuzzy": "npm:^1.0.16"
@@ -3529,7 +3529,7 @@ __metadata:
react: ^18.0.0
react-dom: ^18.0.0
react-router-dom: ^6.28.0
checksum: 10/ed51646f7dbda3598198d65106ed12f45c1dce5cd6bc7217606f99f291d6166a25a19bd530beb0a2e502b2db2f4433e66dad44495d522ddb4dfe743e41bc2ec1
checksum: 10/fd8fa7a17bfd59e495e8102148b678832c974ce9d9e7ea825ccc027d7ec544cd5393f97295306e94cee04511be9258c030fabf3bb6be04f221571227ce1706db
languageName: node
linkType: hard
@@ -9233,7 +9233,18 @@ __metadata:
languageName: node
linkType: hard
"@types/express-serve-static-core@npm:*, @types/express-serve-static-core@npm:^4.17.21, @types/express-serve-static-core@npm:^4.17.33":
"@types/express-serve-static-core@npm:*, @types/express-serve-static-core@npm:^4.17.33":
version: 4.17.33
resolution: "@types/express-serve-static-core@npm:4.17.33"
dependencies:
"@types/node": "npm:*"
"@types/qs": "npm:*"
"@types/range-parser": "npm:*"
checksum: 10/47ee1b46be710ae6451a2e658e2eab75f4affe874b0d156a31e792db0ddb35184ac7b35be926eb23424cc45f6e0d3dbacc86ac5d63a3c988d8235aedb1143841
languageName: node
linkType: hard
"@types/express-serve-static-core@npm:^4.17.21":
version: 4.19.6
resolution: "@types/express-serve-static-core@npm:4.19.6"
dependencies:
@@ -18002,8 +18013,8 @@ __metadata:
"@grafana/plugin-ui": "npm:0.10.6"
"@grafana/prometheus": "workspace:*"
"@grafana/runtime": "workspace:*"
"@grafana/scenes": "npm:^6.19.0"
"@grafana/scenes-react": "npm:^6.19.0"
"@grafana/scenes": "npm:^6.20.1"
"@grafana/scenes-react": "npm:^6.20.1"
"@grafana/schema": "workspace:*"
"@grafana/sql": "workspace:*"
"@grafana/test-utils": "workspace:*"