From d5f404d082ce33a966f5d44f51a7ad3452edf0d1 Mon Sep 17 00:00:00 2001 From: Victor Marin <36818606+mdvictor@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:01:54 +0200 Subject: [PATCH] Dashboards: Add possibility to lock multi value variables option list (#95949) * add new option for multi variables to lock value list wip * WIP - lock option list * tests * fix * fixes + canary scenes * wip * wip * fix snapshot * bump scenes * Dashboards: Add possibility to lock adhoc variables options list (#96077) * Lock list of options flag for ad hoc * refactor * fix snapshot --- kinds/dashboard/dashboard_kind.cue | 2 ++ .../grafana-data/src/types/templateVars.ts | 2 ++ .../src/selectors/pages.ts | 3 +++ .../raw/dashboard/x/dashboard_types.gen.ts | 5 +++++ pkg/kinds/dashboard/dashboard_spec_gen.go | 3 +++ .../transformSceneToSaveModel.test.ts.snap | 10 +++++++++ .../sceneVariablesSetToVariables.test.ts | 12 ++++++++++ .../sceneVariablesSetToVariables.ts | 4 ++++ .../components/AdHocVariableForm.test.tsx | 16 ++++++++++++++ .../components/AdHocVariableForm.tsx | 17 +++++++++++++- .../components/CustomVariableForm.test.tsx | 18 +++++++++++++++ .../components/CustomVariableForm.tsx | 6 +++++ .../components/DataSourceVariableForm.tsx | 6 +++++ .../components/QueryVariableForm.test.tsx | 22 +++++++++++++++++++ .../components/QueryVariableForm.tsx | 6 +++++ .../components/SelectionOptionsForm.tsx | 13 +++++++++++ .../AdHocFiltersVariableEditor.test.tsx | 6 +++++ .../editors/AdHocFiltersVariableEditor.tsx | 9 +++++++- .../editors/CustomVariableEditor.test.tsx | 7 ++++++ .../editors/CustomVariableEditor.tsx | 7 +++++- .../editors/DataSourceVariableEditor.test.tsx | 5 +++++ .../editors/DataSourceVariableEditor.tsx | 8 ++++++- .../editors/QueryVariableEditor.test.tsx | 6 +++++ .../variables/editors/QueryVariableEditor.tsx | 8 ++++++- .../dashboard-scene/utils/variables.test.ts | 11 ++++++++++ .../dashboard-scene/utils/variables.ts | 4 ++++ 26 files changed, 211 insertions(+), 5 deletions(-) diff --git a/kinds/dashboard/dashboard_kind.cue b/kinds/dashboard/dashboard_kind.cue index 6a24fc92266..9ed6b5cc0c1 100644 --- a/kinds/dashboard/dashboard_kind.cue +++ b/kinds/dashboard/dashboard_kind.cue @@ -200,6 +200,8 @@ lineage: schemas: [{ current?: #VariableOption // Whether multiple values can be selected or not from variable value list multi?: bool | *false + // Allow custom values to be entered in the variable + allowCustomValue?: bool | *true // Options that can be selected for a variable. options?: [...#VariableOption] // Options to config when to refresh a variable diff --git a/packages/grafana-data/src/types/templateVars.ts b/packages/grafana-data/src/types/templateVars.ts index b8d0de9092a..e31319b9675 100644 --- a/packages/grafana-data/src/types/templateVars.ts +++ b/packages/grafana-data/src/types/templateVars.ts @@ -70,6 +70,7 @@ export interface AdHocVariableModel extends BaseVariableModel { * Static keys that override any dynamic keys from the datasource. */ defaultKeys?: MetricFindValue[]; + allowCustomValue?: boolean; } export interface GroupByVariableModel extends VariableWithOptions { @@ -127,6 +128,7 @@ export interface VariableWithMultiSupport extends VariableWithOptions { multi: boolean; includeAll: boolean; allValue?: string | null; + allowCustomValue?: boolean; } export interface VariableWithOptions extends BaseVariableModel { diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 92f73b85303..148f4fd56e2 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -414,6 +414,9 @@ export const versionedPages = { generalHideSelectV2: { [MIN_GRAFANA_VERSION]: 'data-testid Variable editor Form Hide select', }, + selectionOptionsAllowCustomValueSwitch: { + [MIN_GRAFANA_VERSION]: 'data-testid Variable editor Form Allow Custom Value switch', + }, selectionOptionsMultiSwitch: { '10.4.0': 'data-testid Variable editor Form Multi switch', [MIN_GRAFANA_VERSION]: 'Variable editor Form Multi switch', diff --git a/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts b/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts index a67ab34ae38..2a0a2de17ba 100644 --- a/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts +++ b/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts @@ -130,6 +130,10 @@ export interface VariableModel { * Custom all value */ allValue?: string; + /** + * Allow custom values to be entered in the variable + */ + allowCustomValue?: boolean; /** * Shows current selected variable text/value on the dashboard */ @@ -194,6 +198,7 @@ export interface VariableModel { } export const defaultVariableModel: Partial = { + allowCustomValue: true, includeAll: false, multi: false, options: [], diff --git a/pkg/kinds/dashboard/dashboard_spec_gen.go b/pkg/kinds/dashboard/dashboard_spec_gen.go index de16129aa20..2a26d5eb998 100644 --- a/pkg/kinds/dashboard/dashboard_spec_gen.go +++ b/pkg/kinds/dashboard/dashboard_spec_gen.go @@ -924,6 +924,9 @@ type VariableModel struct { // Custom all value AllValue *string `json:"allValue,omitempty"` + // Allow custom values to be entered in the variable + AllowCustomValue *bool `json:"allowCustomValue,omitempty"` + // Option to be selected in a variable. Current *VariableOption `json:"current,omitempty"` diff --git a/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap b/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap index 8ab27fc7a70..ab41276f8ef 100644 --- a/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap +++ b/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap @@ -288,6 +288,7 @@ exports[`transformSceneToSaveModel Given a scene with rows Should transform back "templating": { "list": [ { + "allowCustomValue": true, "current": { "text": [ "A", @@ -306,6 +307,7 @@ exports[`transformSceneToSaveModel Given a scene with rows Should transform back "type": "custom", }, { + "allowCustomValue": true, "current": { "text": [ "1", @@ -556,6 +558,7 @@ exports[`transformSceneToSaveModel Given a simple scene with custom settings Sho "templating": { "list": [ { + "allowCustomValue": true, "baseFilters": [], "datasource": { "type": "prometheus", @@ -631,6 +634,7 @@ exports[`transformSceneToSaveModel Given a simple scene with custom settings Sho "type": "interval", }, { + "allowCustomValue": true, "current": { "text": [ "a", @@ -647,6 +651,7 @@ exports[`transformSceneToSaveModel Given a simple scene with custom settings Sho "type": "custom", }, { + "allowCustomValue": true, "current": { "text": "gdev-testdata", "value": "PD8C576611E62080A", @@ -660,6 +665,7 @@ exports[`transformSceneToSaveModel Given a simple scene with custom settings Sho "type": "datasource", }, { + "allowCustomValue": true, "current": { "text": "A", "value": "A", @@ -915,6 +921,7 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr "templating": { "list": [ { + "allowCustomValue": true, "baseFilters": [], "datasource": { "type": "prometheus", @@ -990,6 +997,7 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr "type": "interval", }, { + "allowCustomValue": true, "current": { "text": [ "a", @@ -1006,6 +1014,7 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr "type": "custom", }, { + "allowCustomValue": true, "current": { "text": "gdev-testdata", "value": "PD8C576611E62080A", @@ -1019,6 +1028,7 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr "type": "datasource", }, { + "allowCustomValue": true, "current": { "text": "A", "value": "A", diff --git a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts index 293b17a1143..b6d4a0ee672 100644 --- a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts +++ b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.test.ts @@ -98,6 +98,7 @@ describe('sceneVariablesSetToVariables', () => { datasource: { uid: 'fake-std', type: 'fake-std' }, query: 'query', includeAll: true, + allowCustomValue: true, allValue: 'test-all', isMulti: true, }); @@ -112,6 +113,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result[0]).toMatchInlineSnapshot(` { "allValue": "test-all", + "allowCustomValue": true, "current": { "text": [ "selected-value-text", @@ -151,6 +153,7 @@ describe('sceneVariablesSetToVariables', () => { definition: 'query', includeAll: true, allValue: 'test-all', + allowCustomValue: false, isMulti: true, }); const set = new SceneVariableSet({ @@ -163,6 +166,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result[0]).toMatchInlineSnapshot(` { "allValue": "test-all", + "allowCustomValue": false, "current": { "text": [ "selected-value-text", @@ -256,6 +260,7 @@ describe('sceneVariablesSetToVariables', () => { pluginId: 'fake-std', includeAll: true, allValue: 'test-all', + allowCustomValue: true, isMulti: true, }); const set = new SceneVariableSet({ @@ -268,6 +273,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result[0]).toMatchInlineSnapshot(` { "allValue": "test-all", + "allowCustomValue": true, "current": { "text": [ "selected-ds-1-text", @@ -307,6 +313,7 @@ describe('sceneVariablesSetToVariables', () => { ], includeAll: true, allValue: 'test-all', + allowCustomValue: true, isMulti: true, }); const set = new SceneVariableSet({ @@ -319,6 +326,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result[0]).toMatchInlineSnapshot(` { "allValue": "test-all", + "allowCustomValue": true, "current": { "text": [ "test", @@ -488,6 +496,7 @@ describe('sceneVariablesSetToVariables', () => { it('should handle AdHocFiltersVariable', () => { const variable = new AdHocFiltersVariable({ name: 'test', + allowCustomValue: true, label: 'test-label', description: 'test-desc', datasource: { uid: 'fake-std', type: 'fake-std' }, @@ -515,6 +524,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result).toHaveLength(1); expect(result[0]).toMatchInlineSnapshot(` { + "allowCustomValue": true, "baseFilters": [ { "key": "baseFilterTest", @@ -545,6 +555,7 @@ describe('sceneVariablesSetToVariables', () => { it('should handle AdHocFiltersVariable with defaultKeys', () => { const variable = new AdHocFiltersVariable({ name: 'test', + allowCustomValue: true, label: 'test-label', description: 'test-desc', datasource: { uid: 'fake-std', type: 'fake-std' }, @@ -586,6 +597,7 @@ describe('sceneVariablesSetToVariables', () => { expect(result).toHaveLength(1); expect(result[0]).toMatchInlineSnapshot(` { + "allowCustomValue": true, "baseFilters": [ { "key": "baseFilterTest", diff --git a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts index 4aff03af4ec..5c8dd721129 100644 --- a/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts +++ b/public/app/features/dashboard-scene/serialization/sceneVariablesSetToVariables.ts @@ -74,6 +74,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio allValue: variable.state.allValue, includeAll: variable.state.includeAll, multi: variable.state.isMulti, + allowCustomValue: variable.state.allowCustomValue, skipUrlSync: variable.state.skipUrlSync, }); } else if (sceneUtils.isCustomVariable(variable)) { @@ -90,6 +91,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio multi: variable.state.isMulti, allValue: variable.state.allValue, includeAll: variable.state.includeAll, + allowCustomValue: variable.state.allowCustomValue, }); } else if (sceneUtils.isDataSourceVariable(variable)) { variables.push({ @@ -107,6 +109,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio multi: variable.state.isMulti, allValue: variable.state.allValue, includeAll: variable.state.includeAll, + allowCustomValue: variable.state.allowCustomValue, }); } else if (sceneUtils.isConstantVariable(variable)) { variables.push({ @@ -175,6 +178,7 @@ export function sceneVariablesSetToVariables(set: SceneVariables, keepQueryOptio name: variable.state.name, type: 'adhoc', datasource: variable.state.datasource, + allowCustomValue: variable.state.allowCustomValue, // @ts-expect-error baseFilters: variable.state.baseFilters, filters: variable.state.filters, diff --git a/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.test.tsx b/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.test.tsx index 3aae0735cce..c07db30ba55 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.test.tsx @@ -63,6 +63,22 @@ describe('AdHocVariableForm', () => { expect(onDataSourceChange).toHaveBeenCalledWith(promDatasource, undefined); }); + it('should render the form with allow custom value true', async () => { + const mockOnAllowCustomValueChange = jest.fn(); + const { renderer } = await setup({ + ...defaultProps, + allowCustomValue: true, + onAllowCustomValueChange: mockOnAllowCustomValueChange, + }); + + const allowCustomValueCheckbox = renderer.getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + + expect(allowCustomValueCheckbox).toBeInTheDocument(); + expect(allowCustomValueCheckbox).toBeChecked(); + }); + it('should not render code editor when no default keys provided', async () => { await setup(defaultProps); diff --git a/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.tsx index e683e1ea981..9f25276c86f 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/AdHocVariableForm.tsx @@ -1,4 +1,4 @@ -import { useCallback } from 'react'; +import { FormEvent, useCallback } from 'react'; import { DataSourceInstanceSettings, MetricFindValue, readCSV } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; @@ -6,21 +6,26 @@ import { DataSourceRef } from '@grafana/schema'; import { Alert, CodeEditor, Field, Switch } from '@grafana/ui'; import { DataSourcePicker } from 'app/features/datasources/components/picker/DataSourcePicker'; +import { VariableCheckboxField } from './VariableCheckboxField'; import { VariableLegend } from './VariableLegend'; export interface AdHocVariableFormProps { datasource?: DataSourceRef; onDataSourceChange: (dsSettings: DataSourceInstanceSettings) => void; + allowCustomValue?: boolean; infoText?: string; defaultKeys?: MetricFindValue[]; onDefaultKeysChange?: (keys?: MetricFindValue[]) => void; + onAllowCustomValueChange?: (event: FormEvent) => void; } export function AdHocVariableForm({ datasource, infoText, + allowCustomValue, onDataSourceChange, onDefaultKeysChange, + onAllowCustomValueChange, defaultKeys, }: AdHocVariableFormProps) { const updateStaticKeys = useCallback( @@ -80,6 +85,16 @@ export function AdHocVariableForm({ )} )} + + {onAllowCustomValueChange && ( + + )} ); } diff --git a/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.test.tsx b/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.test.tsx index 334383998df..924f9fa5702 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.test.tsx @@ -9,6 +9,7 @@ describe('CustomVariableForm', () => { const onMultiChange = jest.fn(); const onIncludeAllChange = jest.fn(); const onAllValueChange = jest.fn(); + const onAllowCustomValueChange = jest.fn(); beforeEach(() => { jest.clearAllMocks(); @@ -21,10 +22,12 @@ describe('CustomVariableForm', () => { multi={true} allValue="custom value" includeAll={true} + allowCustomValue={true} onQueryChange={onQueryChange} onMultiChange={onMultiChange} onIncludeAllChange={onIncludeAllChange} onAllValueChange={onAllValueChange} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); @@ -39,12 +42,18 @@ describe('CustomVariableForm', () => { selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsCustomAllInput ); + const allowCustomValueCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + expect(queryInput).toBeInTheDocument(); expect(queryInput).toHaveValue('query'); expect(multiCheckbox).toBeInTheDocument(); expect(multiCheckbox).toBeChecked(); expect(includeAllCheckbox).toBeInTheDocument(); expect(includeAllCheckbox).toBeChecked(); + expect(allowCustomValueCheckbox).toBeInTheDocument(); + expect(allowCustomValueCheckbox).toBeChecked(); expect(allValueInput).toBeInTheDocument(); expect(allValueInput).toHaveValue('custom value'); }); @@ -56,10 +65,12 @@ describe('CustomVariableForm', () => { multi={true} allValue="" includeAll={true} + allowCustomValue={true} onQueryChange={onQueryChange} onMultiChange={onMultiChange} onIncludeAllChange={onIncludeAllChange} onAllValueChange={onAllValueChange} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); @@ -73,13 +84,18 @@ describe('CustomVariableForm', () => { const allValueInput = getByTestId( selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsCustomAllInput ); + const allowCustomValueCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); fireEvent.click(multiCheckbox); fireEvent.click(includeAllCheckbox); + fireEvent.click(allowCustomValueCheckbox); fireEvent.change(queryInput, { currentTarget: { value: 'test query' } }); fireEvent.change(allValueInput, { currentTarget: { value: 'test value' } }); expect(onMultiChange).toHaveBeenCalledTimes(1); + expect(onAllowCustomValueChange).toHaveBeenCalledTimes(1); expect(onIncludeAllChange).toHaveBeenCalledTimes(1); expect(onQueryChange).not.toHaveBeenCalledTimes(1); expect(onAllValueChange).not.toHaveBeenCalledTimes(1); @@ -92,10 +108,12 @@ describe('CustomVariableForm', () => { multi={true} allValue="custom all value" includeAll={true} + allowCustomValue={true} onQueryChange={onQueryChange} onMultiChange={onMultiChange} onIncludeAllChange={onIncludeAllChange} onAllValueChange={onAllValueChange} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); diff --git a/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.tsx index 5c11178141a..5236e9e3f1e 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/CustomVariableForm.tsx @@ -12,12 +12,14 @@ interface CustomVariableFormProps { multi: boolean; allValue?: string | null; includeAll: boolean; + allowCustomValue?: boolean; onQueryChange: (event: FormEvent) => void; onMultiChange: (event: FormEvent) => void; onIncludeAllChange: (event: FormEvent) => void; onAllValueChange: (event: FormEvent) => void; onQueryBlur?: (event: FormEvent) => void; onAllValueBlur?: (event: FormEvent) => void; + onAllowCustomValueChange?: (event: FormEvent) => void; } export function CustomVariableForm({ @@ -25,10 +27,12 @@ export function CustomVariableForm({ multi, allValue, includeAll, + allowCustomValue, onQueryChange, onMultiChange, onIncludeAllChange, onAllValueChange, + onAllowCustomValueChange, }: CustomVariableFormProps) { return ( <> @@ -48,9 +52,11 @@ export function CustomVariableForm({ multi={multi} includeAll={includeAll} allValue={allValue} + allowCustomValue={allowCustomValue} onMultiChange={onMultiChange} onIncludeAllChange={onIncludeAllChange} onAllValueChange={onAllValueChange} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); diff --git a/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx index 52300ca4c5d..bb6432a5b2a 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx @@ -13,6 +13,7 @@ interface DataSourceVariableFormProps { regex: string; multi: boolean; allValue?: string | null; + allowCustomValue?: boolean; includeAll: boolean; onChange: (option: SelectableValue) => void; optionTypes: Array<{ value: string; label: string }>; @@ -20,6 +21,7 @@ interface DataSourceVariableFormProps { onMultiChange: (event: FormEvent) => void; onIncludeAllChange: (event: FormEvent) => void; onAllValueChange: (event: FormEvent) => void; + onAllowCustomValueChange?: (event: FormEvent) => void; onQueryBlur?: (event: FormEvent) => void; onAllValueBlur?: (event: FormEvent) => void; } @@ -28,6 +30,7 @@ export function DataSourceVariableForm({ query, regex, optionTypes, + allowCustomValue, onChange, onRegExBlur, multi, @@ -36,6 +39,7 @@ export function DataSourceVariableForm({ onMultiChange, onIncludeAllChange, onAllValueChange, + onAllowCustomValueChange, }: DataSourceVariableFormProps) { const typeValue = optionTypes.find((o) => o.value === query) ?? optionTypes[0]; @@ -70,9 +74,11 @@ export function DataSourceVariableForm({ multi={multi} includeAll={includeAll} allValue={allValue} + allowCustomValue={allowCustomValue} onMultiChange={onMultiChange} onIncludeAllChange={onIncludeAllChange} onAllValueChange={onAllValueChange} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); diff --git a/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.test.tsx b/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.test.tsx index 8000aebdd9a..12fdb76f103 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.test.tsx @@ -76,6 +76,7 @@ describe('QueryVariableEditorForm', () => { const mockOnMultiChange = jest.fn(); const mockOnIncludeAllChange = jest.fn(); const mockOnAllValueChange = jest.fn(); + const mockOnAllowCustomValueChange = jest.fn(); const defaultProps: React.ComponentProps = { datasource: { uid: defaultDatasource.uid, type: defaultDatasource.type }, @@ -90,12 +91,14 @@ describe('QueryVariableEditorForm', () => { onSortChange: mockOnSortChange, refresh: VariableRefresh.onDashboardLoad, onRefreshChange: mockOnRefreshChange, + allowCustomValue: true, isMulti: true, onMultiChange: mockOnMultiChange, includeAll: true, onIncludeAllChange: mockOnIncludeAllChange, allValue: 'custom all value', onAllValueChange: mockOnAllValueChange, + onAllowCustomValueChange: mockOnAllowCustomValueChange, }; async function setup(props?: React.ComponentProps) { @@ -135,6 +138,9 @@ describe('QueryVariableEditorForm', () => { const allValueInput = getByTestId( selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsCustomAllInput ); + const allowCustomValueSwitch = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); expect(dataSourcePicker).toBeInTheDocument(); expect(dataSourcePicker.getAttribute('placeholder')).toBe('Default Test Data Source'); @@ -146,6 +152,8 @@ describe('QueryVariableEditorForm', () => { expect(getByRole('radio', { name: 'On dashboard load' })).toBeChecked(); expect(multiSwitch).toBeInTheDocument(); expect(multiSwitch).toBeChecked(); + expect(allowCustomValueSwitch).toBeInTheDocument(); + expect(allowCustomValueSwitch).toBeChecked(); expect(includeAllSwitch).toBeInTheDocument(); expect(includeAllSwitch).toBeChecked(); expect(allValueInput).toBeInTheDocument(); @@ -257,6 +265,20 @@ describe('QueryVariableEditorForm', () => { ).toBeChecked(); }); + it('should call onAllowCustomValue when changing the allow custom value switch', async () => { + const { + renderer: { getByTestId }, + } = await setup(); + const allowCustomValue = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + await userEvent.click(allowCustomValue); + expect(mockOnAllowCustomValueChange).toHaveBeenCalledTimes(1); + expect( + (mockOnAllowCustomValueChange.mock.calls[0][0] as FormEvent).target as HTMLInputElement + ).toBeChecked(); + }); + it('should call onAllValueChange when changing the all value', async () => { const { renderer: { getByTestId }, diff --git a/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.tsx index 9a8e0c73078..5185cfb1287 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/QueryVariableForm.tsx @@ -34,6 +34,8 @@ interface QueryVariableEditorFormProps { onRefreshChange: (option: VariableRefresh) => void; isMulti: boolean; onMultiChange: (event: FormEvent) => void; + allowCustomValue?: boolean; + onAllowCustomValueChange?: (event: FormEvent) => void; includeAll: boolean; onIncludeAllChange: (event: FormEvent) => void; allValue: string; @@ -55,6 +57,8 @@ export function QueryVariableEditorForm({ onRefreshChange, isMulti, onMultiChange, + allowCustomValue, + onAllowCustomValueChange, includeAll, onIncludeAllChange, allValue, @@ -126,10 +130,12 @@ export function QueryVariableEditorForm({ ); diff --git a/public/app/features/dashboard-scene/settings/variables/components/SelectionOptionsForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/SelectionOptionsForm.tsx index de2ba28a91f..9e9d6a15c5c 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/SelectionOptionsForm.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/SelectionOptionsForm.tsx @@ -8,17 +8,21 @@ import { VariableTextField } from 'app/features/dashboard-scene/settings/variabl interface SelectionOptionsFormProps { multi: boolean; includeAll: boolean; + allowCustomValue?: boolean; allValue?: string | null; onMultiChange: (event: ChangeEvent) => void; + onAllowCustomValueChange?: (event: ChangeEvent) => void; onIncludeAllChange: (event: ChangeEvent) => void; onAllValueChange: (event: FormEvent) => void; } export function SelectionOptionsForm({ multi, + allowCustomValue, includeAll, allValue, onMultiChange, + onAllowCustomValueChange, onIncludeAllChange, onAllValueChange, }: SelectionOptionsFormProps) { @@ -31,6 +35,15 @@ export function SelectionOptionsForm({ onChange={onMultiChange} testId={selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsMultiSwitch} /> + {onAllowCustomValueChange && ( // backwards compat with old arch, remove on cleanup + + )} { const infoText = renderer.getByTestId( selectors.pages.Dashboard.Settings.Variables.Edit.AdHocFiltersVariable.infoText ); + const allowCustomValueCheckbox = renderer.getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + expect(allowCustomValueCheckbox).toBeInTheDocument(); + expect(allowCustomValueCheckbox).toBeChecked(); expect(dataSourcePicker).toBeInTheDocument(); expect(dataSourcePicker.getAttribute('placeholder')).toBe('Default Test Data Source'); expect(infoText).toBeInTheDocument(); @@ -132,6 +137,7 @@ async function setup(props?: React.ComponentProps { return await getDataSourceSrv().get(datasourceRef); @@ -38,13 +39,19 @@ export function AdHocFiltersVariableEditor(props: AdHocFiltersVariableEditorProp }); }; + const onAllowCustomValueChange = (event: FormEvent) => { + variable.setState({ allowCustomValue: event.currentTarget.checked }); + }; + return ( ); } diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.test.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.test.tsx index 90292024c02..a7315d37a76 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.test.tsx @@ -55,11 +55,17 @@ describe('CustomVariableEditor', () => { selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsIncludeAllSwitch ); + const allowCustomValueCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + // It include-all-custom input appears after include-all checkbox is checked only expect(() => getByTestId(selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsCustomAllInput) ).toThrow('Unable to find an element'); + fireEvent.click(allowCustomValueCheckbox); + fireEvent.click(multiCheckbox); fireEvent.click(includeAllCheckbox); @@ -69,6 +75,7 @@ describe('CustomVariableEditor', () => { expect(variable.state.isMulti).toBe(true); expect(variable.state.includeAll).toBe(true); + expect(variable.state.allowCustomValue).toBe(false); expect(allValueInput).toBeInTheDocument(); }); diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.tsx index 9b507c34a66..9ec8289a074 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor.tsx @@ -10,7 +10,7 @@ interface CustomVariableEditorProps { } export function CustomVariableEditor({ variable, onRunQuery }: CustomVariableEditorProps) { - const { query, isMulti, allValue, includeAll } = variable.useState(); + const { query, isMulti, allValue, includeAll, allowCustomValue } = variable.useState(); const onMultiChange = (event: FormEvent) => { variable.setState({ isMulti: event.currentTarget.checked }); @@ -25,6 +25,9 @@ export function CustomVariableEditor({ variable, onRunQuery }: CustomVariableEdi const onAllValueChange = (event: FormEvent) => { variable.setState({ allValue: event.currentTarget.value }); }; + const onAllowCustomValueChange = (event: FormEvent) => { + variable.setState({ allowCustomValue: event.currentTarget.checked }); + }; return ( ); } diff --git a/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx index 8dd102b49f3..44e0cb9a5e2 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx @@ -61,6 +61,9 @@ describe('DataSourceVariableEditor', () => { const includeAllCheckbox = getByTestId( selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsIncludeAllSwitch ); + const allowCustomValueCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); const typeSelect = getByTestId( selectors.pages.Dashboard.Settings.Variables.Edit.DatasourceVariable.datasourceSelect @@ -69,6 +72,8 @@ describe('DataSourceVariableEditor', () => { expect(typeSelect.textContent).toBe('ds1'); expect(multiCheckbox).toBeInTheDocument(); expect(multiCheckbox).not.toBeChecked(); + expect(allowCustomValueCheckbox).toBeInTheDocument(); + expect(allowCustomValueCheckbox).toBeChecked(); expect(includeAllCheckbox).toBeInTheDocument(); expect(includeAllCheckbox).not.toBeChecked(); }); diff --git a/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx index 82deb3965fd..63a2a63238e 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx @@ -12,7 +12,7 @@ interface DataSourceVariableEditorProps { } export function DataSourceVariableEditor({ variable, onRunQuery }: DataSourceVariableEditorProps) { - const { pluginId, regex, isMulti, allValue, includeAll } = variable.useState(); + const { pluginId, regex, isMulti, allValue, includeAll, allowCustomValue } = variable.useState(); const optionTypes = getOptionDataSourceTypes(); @@ -44,6 +44,10 @@ export function DataSourceVariableEditor({ variable, onRunQuery }: DataSourceVar variable.setState({ allValue: event.currentTarget.value }); }; + const onAllowCustomValueChange = (event: FormEvent) => { + variable.setState({ allowCustomValue: event.currentTarget.checked }); + }; + return ( ); } diff --git a/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.test.tsx b/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.test.tsx index d96bb9516ca..920744e53ec 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.test.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.test.tsx @@ -117,6 +117,10 @@ describe('QueryVariableEditor', () => { selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsCustomAllInput ); + const allowCustomValueCheckbox = renderer.getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsAllowCustomValueSwitch + ); + expect(dataSourcePicker).toBeInTheDocument(); expect(dataSourcePicker.getAttribute('placeholder')).toBe('Default Test Data Source'); expect(queryEditor).toBeInTheDocument(); @@ -129,6 +133,8 @@ describe('QueryVariableEditor', () => { expect(getByRole(refreshSelect, 'radio', { name: 'On dashboard load' })).toBeChecked(); expect(multiSwitch).toBeInTheDocument(); expect(multiSwitch).toBeChecked(); + expect(allowCustomValueCheckbox).toBeInTheDocument(); + expect(allowCustomValueCheckbox).toBeChecked(); expect(includeAllSwitch).toBeInTheDocument(); expect(includeAllSwitch).toBeChecked(); expect(allValueInput).toBeInTheDocument(); diff --git a/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.tsx b/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.tsx index 05ea0873e91..f1ec139eb38 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/QueryVariableEditor.tsx @@ -14,7 +14,8 @@ interface QueryVariableEditorProps { type VariableQueryType = QueryVariable['state']['query']; export function QueryVariableEditor({ variable, onRunQuery }: QueryVariableEditorProps) { - const { datasource, regex, sort, refresh, isMulti, includeAll, allValue, query } = variable.useState(); + const { datasource, regex, sort, refresh, isMulti, includeAll, allValue, query, allowCustomValue } = + variable.useState(); const { value: timeRange } = sceneGraph.getTimeRange(variable).useState(); const onRegExChange = (event: React.FormEvent) => { @@ -35,6 +36,9 @@ export function QueryVariableEditor({ variable, onRunQuery }: QueryVariableEdito const onAllValueChange = (event: FormEvent) => { variable.setState({ allValue: event.currentTarget.value }); }; + const onAllowCustomValueChange = (event: FormEvent) => { + variable.setState({ allowCustomValue: event.currentTarget.checked }); + }; const onDataSourceChange = (dsInstanceSettings: DataSourceInstanceSettings) => { const datasource = getDataSourceRef(dsInstanceSettings); @@ -78,6 +82,8 @@ export function QueryVariableEditor({ variable, onRunQuery }: QueryVariableEdito onIncludeAllChange={onIncludeAllChange} allValue={allValue ?? ''} onAllValueChange={onAllValueChange} + allowCustomValue={allowCustomValue} + onAllowCustomValueChange={onAllowCustomValueChange} /> ); } diff --git a/public/app/features/dashboard-scene/utils/variables.test.ts b/public/app/features/dashboard-scene/utils/variables.test.ts index 4c6e7bc3165..9bdd9c6c5be 100644 --- a/public/app/features/dashboard-scene/utils/variables.test.ts +++ b/public/app/features/dashboard-scene/utils/variables.test.ts @@ -45,6 +45,7 @@ describe('when creating variables objects', () => { hide: 0, includeAll: false, multi: false, + allowCustomValue: true, name: 'query0', options: [ { @@ -91,6 +92,7 @@ describe('when creating variables objects', () => { description: null, includeAll: false, isMulti: false, + allowCustomValue: true, label: undefined, name: 'query0', options: [], @@ -106,6 +108,7 @@ describe('when creating variables objects', () => { it('should migrate query variable with definition', () => { const variable: QueryVariableModel = { allValue: null, + allowCustomValue: false, current: { text: 'America', value: 'America', @@ -164,6 +167,7 @@ describe('when creating variables objects', () => { expect(migrated).toBeInstanceOf(QueryVariable); expect(rest).toEqual({ allValue: undefined, + allowCustomValue: false, datasource: { type: 'influxdb', uid: 'P15396BDD62B2BE29', @@ -191,6 +195,7 @@ describe('when creating variables objects', () => { it('should migrate datasource variable', () => { const variable: DataSourceVariableModel = { id: 'query1', + allowCustomValue: true, rootStateKey: 'N4XLmH5Vz', name: 'query1', type: 'datasource', @@ -237,6 +242,7 @@ describe('when creating variables objects', () => { expect(migrated).toBeInstanceOf(DataSourceVariable); expect(rest).toEqual({ allValue: 'Custom all', + allowCustomValue: true, defaultToAll: true, includeAll: true, label: undefined, @@ -385,6 +391,7 @@ describe('when creating variables objects', () => { it('should migrate adhoc variable', () => { const variable: TypedVariableModel = { id: 'adhoc', + allowCustomValue: false, global: false, index: 0, state: LoadingState.Done, @@ -423,6 +430,7 @@ describe('when creating variables objects', () => { expect(filterVarState).toEqual({ key: expect.any(String), description: 'Adhoc Description', + allowCustomValue: false, hide: 0, label: 'Adhoc Label', name: 'adhoc', @@ -493,6 +501,7 @@ describe('when creating variables objects', () => { expect(filterVarState).toEqual({ key: expect.any(String), description: 'Adhoc Description', + allowCustomValue: true, hide: 0, label: 'Adhoc Label', name: 'adhoc', @@ -663,6 +672,7 @@ describe('when creating variables objects', () => { expect(migrated).toBeInstanceOf(DataSourceVariable); expect(rest).toEqual({ allValue: 'Custom all', + allowCustomValue: true, defaultToAll: true, includeAll: true, label: undefined, @@ -705,6 +715,7 @@ describe('when creating variables objects', () => { expect(migrated).toBeInstanceOf(DataSourceVariable); expect(rest).toEqual({ allValue: 'Custom all', + allowCustomValue: true, defaultToAll: true, includeAll: true, label: undefined, diff --git a/public/app/features/dashboard-scene/utils/variables.ts b/public/app/features/dashboard-scene/utils/variables.ts index 48ab1641003..4b6c1bdcb6d 100644 --- a/public/app/features/dashboard-scene/utils/variables.ts +++ b/public/app/features/dashboard-scene/utils/variables.ts @@ -138,6 +138,7 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode filters: variable.filters ?? [], baseFilters: variable.baseFilters ?? [], defaultKeys: variable.defaultKeys, + allowCustomValue: variable.allowCustomValue ?? true, useQueriesAsFilterForOptions: true, layout: config.featureToggles.newFiltersUI ? 'combobox' : undefined, supportsMultiValueOperators: Boolean( @@ -158,6 +159,7 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode defaultToAll: Boolean(variable.includeAll), skipUrlSync: variable.skipUrlSync, hide: variable.hide, + allowCustomValue: variable.allowCustomValue ?? true, }); } else if (variable.type === 'query') { return new QueryVariable({ @@ -177,6 +179,7 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode skipUrlSync: variable.skipUrlSync, hide: variable.hide, definition: variable.definition, + allowCustomValue: variable.allowCustomValue ?? true, }); } else if (variable.type === 'datasource') { return new DataSourceVariable({ @@ -192,6 +195,7 @@ export function createSceneVariableFromVariableModel(variable: TypedVariableMode isMulti: variable.multi, hide: variable.hide, defaultOptionEnabled: variable.current?.value === DEFAULT_DATASOURCE && variable.current?.text === 'default', + allowCustomValue: variable.allowCustomValue ?? true, }); } else if (variable.type === 'interval') { const intervals = getIntervalsFromQueryString(variable.query);