From 2774e0d023abd182c426831538ed09e9f72b9579 Mon Sep 17 00:00:00 2001 From: Alexa V <239999+axelavargas@users.noreply.github.com> Date: Thu, 25 Jan 2024 12:56:37 +0100 Subject: [PATCH] Dashboard: Migration - EditVariable Settings: Implement DataSource Variable (#80885) * Extract DatasourceVariableForm logic and use it in core grafana * Implement DataSourceVariable editor in scenes * Refactor VariableSelect and add unit test for DataSourceVariableEditor * Refactor old unit test to use userEvent and mock getDataSourceSrv --- .../components/DataSourceVariableForm.tsx | 79 +++++++++ .../components/VariableSelectField.tsx | 19 +-- .../editors/DataSourceVariableEditor.test.tsx | 157 ++++++++++++++++++ .../editors/DataSourceVariableEditor.tsx | 58 ++++++- .../settings/variables/utils.test.ts | 51 +++++- .../settings/variables/utils.ts | 20 ++- .../DataSourceVariableEditor.test.tsx | 22 ++- .../datasource/DataSourceVariableEditor.tsx | 76 +++------ 8 files changed, 412 insertions(+), 70 deletions(-) create mode 100644 public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx create mode 100644 public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx diff --git a/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx b/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx new file mode 100644 index 00000000000..ac77b8c747c --- /dev/null +++ b/public/app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm.tsx @@ -0,0 +1,79 @@ +import React, { FormEvent } from 'react'; + +import { SelectableValue } from '@grafana/data'; +import { selectors } from '@grafana/e2e-selectors'; + +import { SelectionOptionsForm } from './SelectionOptionsForm'; +import { VariableLegend } from './VariableLegend'; +import { VariableSelectField } from './VariableSelectField'; +import { VariableTextField } from './VariableTextField'; + +interface DataSourceVariableFormProps { + query: string; + regex: string; + multi: boolean; + allValue?: string | null; + includeAll: boolean; + onChange: (option: SelectableValue) => void; + optionTypes: Array<{ value: string; label: string }>; + onRegExBlur: (event: FormEvent) => void; + onMultiChange: (event: FormEvent) => void; + onIncludeAllChange: (event: FormEvent) => void; + onAllValueChange: (event: FormEvent) => void; + onQueryBlur?: (event: FormEvent) => void; + onAllValueBlur?: (event: FormEvent) => void; +} + +export function DataSourceVariableForm({ + query, + regex, + optionTypes, + onChange, + onRegExBlur, + multi, + includeAll, + allValue, + onMultiChange, + onIncludeAllChange, + onAllValueChange, +}: DataSourceVariableFormProps) { + const typeValue = optionTypes.find((o) => o.value === query) ?? optionTypes[0]; + + return ( + <> + Data source options + + + + Regex filter for which data source instances to choose from in the variable value list. Leave empty for all. +
+
+ Example: /^prod/ + + } + /> + + Selection options + + + ); +} diff --git a/public/app/features/dashboard-scene/settings/variables/components/VariableSelectField.tsx b/public/app/features/dashboard-scene/settings/variables/components/VariableSelectField.tsx index 88c53385747..17f273dab46 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/VariableSelectField.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/VariableSelectField.tsx @@ -29,16 +29,15 @@ export function VariableSelectField({ 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 new file mode 100644 index 00000000000..0b1e4c333a9 --- /dev/null +++ b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.test.tsx @@ -0,0 +1,157 @@ +// add unit test for the DataSourceVariableEditor component + +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import { selectors } from '@grafana/e2e-selectors'; +import { DataSourceVariable } from '@grafana/scenes'; + +import { DataSourceVariableEditor } from './DataSourceVariableEditor'; + +//mock getDataSorceSrv.getList() to return a list of datasources +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getDataSourceSrv: () => ({ + getList: () => { + return [ + { + name: 'DataSourceInstance1', + uid: 'ds1', + meta: { + name: 'ds1', + id: 'dsTestDataSource', + }, + }, + { + name: 'DataSourceInstance2', + uid: 'ds2', + meta: { + name: 'ds1', + id: 'dsTestDataSource', + }, + }, + { + name: 'ABCDataSourceInstance', + uid: 'ds3', + meta: { + name: 'abDS', + id: 'ABCDS', + }, + }, + ]; + }, + }), +})); + +describe('DataSourceVariableEditor', () => { + it('shoud render correctly with multi and all not checked', () => { + const variable = new DataSourceVariable({ + name: 'dsVariable', + type: 'datasource', + label: 'Datasource', + pluginId: 'dsTestDataSource', + }); + const onRunQuery = jest.fn(); + + const { getByTestId } = render(); + + const multiCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsMultiSwitch + ); + const includeAllCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsIncludeAllSwitch + ); + + const typeSelect = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.DatasourceVariable.datasourceSelect + ); + expect(typeSelect).toBeInTheDocument(); + expect(typeSelect.textContent).toBe('ds1'); + expect(multiCheckbox).toBeInTheDocument(); + expect(multiCheckbox).not.toBeChecked(); + expect(includeAllCheckbox).toBeInTheDocument(); + expect(includeAllCheckbox).not.toBeChecked(); + }); + + it('shoud render correctly with multi and includeAll checked', () => { + const variable = new DataSourceVariable({ + name: 'dsVariable', + type: 'datasource', + label: 'Datasource', + pluginId: 'dsTestDataSource', + isMulti: true, + includeAll: true, + }); + const onRunQuery = jest.fn(); + + const { getByTestId } = render(); + + const multiCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsMultiSwitch + ); + const includeAllCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsIncludeAllSwitch + ); + + const typeSelect = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.DatasourceVariable.datasourceSelect + ); + expect(typeSelect).toBeInTheDocument(); + expect(typeSelect.textContent).toBe('ds1'); + expect(multiCheckbox).toBeInTheDocument(); + expect(multiCheckbox).toBeChecked(); + expect(includeAllCheckbox).toBeInTheDocument(); + expect(includeAllCheckbox).toBeChecked(); + }); + + it('Should change type option when users select a different datasource type', async () => { + const variable = new DataSourceVariable({ + name: 'dsVariable', + type: 'datasource', + label: 'Datasource', + pluginId: 'dsTestDataSource', + isMulti: false, + includeAll: false, + }); + const onRunQuery = jest.fn(); + + const { getByTestId, user } = setup(); + + const typeSelect = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.DatasourceVariable.datasourceSelect + ); + // when user change type datasource + await user.click(typeSelect); + await user.type(typeSelect, 'abDS'); + await user.keyboard('{enter}'); + expect(typeSelect).toBeInTheDocument(); + expect(typeSelect.textContent).toBe('abDS'); + expect(onRunQuery).toHaveBeenCalledTimes(1); + + // when user change checkbox multi + + const multiCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsMultiSwitch + ); + const includeAllCheckbox = getByTestId( + selectors.pages.Dashboard.Settings.Variables.Edit.General.selectionOptionsIncludeAllSwitch + ); + + await user.click(multiCheckbox); + expect(multiCheckbox).toBeChecked(); + + // when user include all there is a new call to onRunQuery + await user.click(includeAllCheckbox); + expect(includeAllCheckbox).toBeChecked(); + expect(onRunQuery).toHaveBeenCalledTimes(1); + }); +}); + +// based on styleguide recomendation +function setup(jsx: JSX.Element) { + return { + user: userEvent.setup(), + ...render(jsx), + }; +} 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 6dc5099dc28..6c36f94e3ef 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/DataSourceVariableEditor.tsx @@ -1,12 +1,62 @@ -import React from 'react'; +import React, { FormEvent } from 'react'; +import { SelectableValue } from '@grafana/data'; import { DataSourceVariable } from '@grafana/scenes'; +import { DataSourceVariableForm } from '../components/DataSourceVariableForm'; +import { getOptionDataSourceTypes } from '../utils'; + interface DataSourceVariableEditorProps { variable: DataSourceVariable; - onChange: (variable: DataSourceVariable) => void; + onRunQuery: () => void; } -export function DataSourceVariableEditor(props: DataSourceVariableEditorProps) { - return
DataSourceVariableEditor
; +export function DataSourceVariableEditor({ variable, onRunQuery }: DataSourceVariableEditorProps) { + const { pluginId, regex, isMulti, allValue, includeAll } = variable.useState(); + + const optionTypes = getOptionDataSourceTypes(); + + const onChangeType = (option: SelectableValue) => { + variable.setState({ + pluginId: option.value, + }); + onRunQuery(); + }; + + const onRegExChange = (event: FormEvent) => { + variable.setState({ + regex: event.currentTarget.value, + }); + onRunQuery(); + }; + + const onMultiChange = (event: FormEvent) => { + variable.setState({ + isMulti: event.currentTarget.checked, + }); + }; + + const onIncludeAllChange = (event: FormEvent) => { + variable.setState({ includeAll: event.currentTarget.checked }); + }; + + const onAllValueChange = (event: FormEvent) => { + variable.setState({ allValue: event.currentTarget.value }); + }; + + return ( + + ); } diff --git a/public/app/features/dashboard-scene/settings/variables/utils.test.ts b/public/app/features/dashboard-scene/settings/variables/utils.test.ts index a216213f117..6d43b34891a 100644 --- a/public/app/features/dashboard-scene/settings/variables/utils.test.ts +++ b/public/app/features/dashboard-scene/settings/variables/utils.test.ts @@ -1,3 +1,4 @@ +import { DataSourceApi } from '@grafana/data'; import { setTemplateSrv, TemplateSrv } from '@grafana/runtime'; import { CustomVariable, @@ -8,7 +9,9 @@ import { AdHocFiltersVariable, TextBoxVariable, } from '@grafana/scenes'; -import { VariableType } from '@grafana/schema'; +import { DataQuery, DataSourceJsonData, VariableType } from '@grafana/schema'; +import { SHARED_DASHBOARD_QUERY } from 'app/plugins/datasource/dashboard'; +import { DASHBOARD_DATASOURCE_PLUGIN_ID } from 'app/plugins/datasource/dashboard/types'; import { AdHocFiltersVariableEditor } from './editors/AdHocFiltersVariableEditor'; import { ConstantVariableEditor } from './editors/ConstantVariableEditor'; @@ -27,12 +30,44 @@ import { hasVariableOptions, EditableVariableType, getDefinition, + getOptionDataSourceTypes, } from './utils'; const templateSrv = { getAdhocFilters: jest.fn().mockReturnValue([{ key: 'origKey', operator: '=', value: '' }]), } as unknown as TemplateSrv; +const dsMock: DataSourceApi = { + meta: { + id: DASHBOARD_DATASOURCE_PLUGIN_ID, + }, + name: SHARED_DASHBOARD_QUERY, + type: SHARED_DASHBOARD_QUERY, + uid: SHARED_DASHBOARD_QUERY, + getRef: () => { + return { type: SHARED_DASHBOARD_QUERY, uid: SHARED_DASHBOARD_QUERY }; + }, +} as DataSourceApi; + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getDataSourceSrv: () => ({ + get: async () => dsMock, + getList: () => { + return [ + { + name: 'DataSourceInstance1', + uid: 'ds1', + meta: { + name: 'ds1', + id: 'dsTestDataSource', + }, + }, + ]; + }, + }), +})); + describe('isEditableVariableType', () => { it('should return true for editable variable types', () => { const editableTypes: VariableType[] = ['custom', 'query', 'constant', 'interval', 'datasource', 'adhoc', 'textbox']; @@ -75,6 +110,10 @@ describe('getVariableTypeSelectOptions', () => { }); describe('getVariableEditor', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it.each(Object.keys(EDITABLE_VARIABLES) as EditableVariableType[])( 'should define an editor for every variable type', (type) => { @@ -200,3 +239,13 @@ describe('getDefinition', () => { expect(getDefinition(model)).toBe('Constant Value'); }); }); + +describe('getOptionDataSourceTypes', () => { + it('should return all data source types when no data source types are specified', () => { + const optionTypes = getOptionDataSourceTypes(); + expect(optionTypes).toHaveLength(2); + // in the old code we always had an empty option + expect(optionTypes[0].value).toBe(''); + expect(optionTypes[1].label).toBe('ds1'); + }); +}); diff --git a/public/app/features/dashboard-scene/settings/variables/utils.ts b/public/app/features/dashboard-scene/settings/variables/utils.ts index addb6f45823..1a3076fbd75 100644 --- a/public/app/features/dashboard-scene/settings/variables/utils.ts +++ b/public/app/features/dashboard-scene/settings/variables/utils.ts @@ -1,4 +1,7 @@ -import { SelectableValue } from '@grafana/data'; +import { chain } from 'lodash'; + +import { DataSourceInstanceSettings, SelectableValue } from '@grafana/data'; +import { getDataSourceSrv } from '@grafana/runtime'; import { ConstantVariable, CustomVariable, @@ -140,3 +143,18 @@ export function getDefinition(model: SceneVariable): string { return definition; } + +export function getOptionDataSourceTypes() { + const datasources = getDataSourceSrv().getList({ metrics: true, variables: true }); + + const optionTypes = chain(datasources) + .uniqBy('meta.id') + .map((ds: DataSourceInstanceSettings) => { + return { label: ds.meta.name, value: ds.meta.id }; + }) + .value(); + + optionTypes.unshift({ label: '', value: '' }); + + return optionTypes; +} diff --git a/public/app/features/variables/datasource/DataSourceVariableEditor.test.tsx b/public/app/features/variables/datasource/DataSourceVariableEditor.test.tsx index 661a8671669..ec4c34245ea 100644 --- a/public/app/features/variables/datasource/DataSourceVariableEditor.test.tsx +++ b/public/app/features/variables/datasource/DataSourceVariableEditor.test.tsx @@ -1,4 +1,5 @@ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { selectOptionInTest, getSelectParent } from 'test/helpers/selectOptionInTest'; @@ -45,10 +46,21 @@ describe('DataSourceVariableEditor', () => { expect(field).toBeInTheDocument(); }); - it('calls the handler when the regex filter is changed', () => { - render(); + it('calls the handler when the regex filter is changed in onBlur', async () => { + const { user } = setup(); const field = screen.getByLabelText(/Instance name filter/); - fireEvent.change(field, { target: { value: '/prod/' } }); - expect(props.onPropChange).toBeCalledWith({ propName: 'regex', propValue: '/prod/' }); + await user.click(field); + await user.type(field, '/prod/'); + expect(field).toHaveValue('/prod/'); + await user.tab(); + expect(props.onPropChange).toHaveBeenCalledWith({ propName: 'regex', propValue: '/prod/', updateOptions: true }); }); }); + +// based on styleguide recomendation +function setup(jsx: JSX.Element) { + return { + user: userEvent.setup(), + ...render(jsx), + }; +} diff --git a/public/app/features/variables/datasource/DataSourceVariableEditor.tsx b/public/app/features/variables/datasource/DataSourceVariableEditor.tsx index a7ac33bfd93..3fb6996aed3 100644 --- a/public/app/features/variables/datasource/DataSourceVariableEditor.tsx +++ b/public/app/features/variables/datasource/DataSourceVariableEditor.tsx @@ -2,19 +2,16 @@ import React, { FormEvent, PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { SelectableValue } from '@grafana/data'; -import { selectors } from '@grafana/e2e-selectors'; +import { DataSourceVariableForm } from 'app/features/dashboard-scene/settings/variables/components/DataSourceVariableForm'; import { StoreState } from '../../../types'; -import { VariableLegend } from '../../dashboard-scene/settings/variables/components/VariableLegend'; -import { VariableSelectField } from '../../dashboard-scene/settings/variables/components/VariableSelectField'; -import { VariableTextField } from '../../dashboard-scene/settings/variables/components/VariableTextField'; -import { SelectionOptionsEditor } from '../editor/SelectionOptionsEditor'; import { initialVariableEditorState } from '../editor/reducer'; import { getDatasourceVariableEditorState } from '../editor/selectors'; import { OnPropChangeArguments, VariableEditorProps } from '../editor/types'; import { changeVariableMultiValue } from '../state/actions'; import { getVariablesState } from '../state/selectors'; import { DataSourceVariableModel, VariableWithMultiSupport } from '../types'; +import { toKeyedVariableIdentifier } from '../utils'; import { initDataSourceVariableEditor } from './actions'; @@ -57,13 +54,6 @@ export class DataSourceVariableEditorUnConnected extends PureComponent { this.props.initDataSourceVariableEditor(rootStateKey); } - onRegExChange = (event: FormEvent) => { - this.props.onPropChange({ - propName: 'regex', - propValue: event.currentTarget.value, - }); - }; - onRegExBlur = (event: FormEvent) => { this.props.onPropChange({ propName: 'regex', @@ -76,6 +66,18 @@ export class DataSourceVariableEditorUnConnected extends PureComponent { this.props.onPropChange({ propName, propValue, updateOptions: true }); }; + onMultiChanged = (event: FormEvent) => { + this.props.changeVariableMultiValue(toKeyedVariableIdentifier(this.props.variable), event.currentTarget.checked); + }; + + onIncludeAllChanged = (event: FormEvent) => { + this.onSelectionOptionsChange({ propName: 'includeAll', propValue: event.currentTarget.checked }); + }; + + onAllValueChanged = (event: FormEvent) => { + this.onSelectionOptionsChange({ propName: 'allValue', propValue: event.currentTarget.value }); + }; + getSelectedDataSourceTypeValue = (): string => { const { extended } = this.props; @@ -93,49 +95,25 @@ export class DataSourceVariableEditorUnConnected extends PureComponent { }; render() { - const { variable, extended, changeVariableMultiValue } = this.props; + const { variable, extended } = this.props; const typeOptions = extended?.dataSourceTypes?.length ? extended.dataSourceTypes?.map((ds) => ({ value: ds.value ?? '', label: ds.text })) : []; - const typeValue = typeOptions.find((o) => o.value === variable.query) ?? typeOptions[0]; - return ( - <> - Data source options - - - - Regex filter for which data source instances to choose from in the variable value list. Leave empty for - all. -
-
- Example: /^prod/ -
- } - /> - - Selection options - - + ); } }