From 2bc30daa49dd88fcf986e2e046c99542241c5bb3 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 9 Nov 2021 18:20:36 +0100 Subject: [PATCH] Alerting: allows clearing the datasource selection in rule list (#41264) --- packages/grafana-runtime/package.json | 2 ++ .../src/components/DataSourcePicker.test.tsx | 27 +++++++++++++++++++ .../src/components/DataSourcePicker.tsx | 15 ++++++++--- .../src/services/__mocks__/dataSourceSrv.ts | 22 +++++++++++++++ .../src/components/Select/SelectBase.test.tsx | 8 +++--- .../src/components/Select/SelectBase.tsx | 8 +++--- .../grafana-ui/src/components/Select/types.ts | 4 ++- packages/grafana-ui/src/types/index.ts | 1 + packages/grafana-ui/src/types/select.ts | 1 + .../components/Select/MetricSelect.test.tsx | 2 +- .../unified/components/rules/RulesFilter.tsx | 8 +++++- ...CreatableSelectPersistedBehaviour.test.tsx | 5 +++- .../configuration/ElasticDetails.test.tsx | 11 +++++--- yarn.lock | 15 ++++++++++- 14 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 packages/grafana-runtime/src/components/DataSourcePicker.test.tsx create mode 100644 packages/grafana-runtime/src/services/__mocks__/dataSourceSrv.ts create mode 100644 packages/grafana-ui/src/types/select.ts diff --git a/packages/grafana-runtime/package.json b/packages/grafana-runtime/package.json index d183ba848d8..bec195852c0 100644 --- a/packages/grafana-runtime/package.json +++ b/packages/grafana-runtime/package.json @@ -39,6 +39,8 @@ "@grafana/tsconfig": "^1.0.0-rc1", "@rollup/plugin-commonjs": "21.0.1", "@rollup/plugin-node-resolve": "13.0.6", + "@testing-library/react": "^12.1.2", + "@testing-library/user-event": "^13.5.0", "@types/angular": "1.6.56", "@types/history": "^4.7.8", "@types/jest": "27.0.2", diff --git a/packages/grafana-runtime/src/components/DataSourcePicker.test.tsx b/packages/grafana-runtime/src/components/DataSourcePicker.test.tsx new file mode 100644 index 00000000000..6f18fe2141a --- /dev/null +++ b/packages/grafana-runtime/src/components/DataSourcePicker.test.tsx @@ -0,0 +1,27 @@ +import React from 'react'; +import { DataSourcePicker } from './DataSourcePicker'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +jest.mock('../services/dataSourceSrv'); + +describe('DataSourcePicker', () => { + describe('onClear', () => { + it('should call onClear when function is passed', async () => { + const onClear = jest.fn(); + const select = render(); + + const clearButton = select.getByLabelText('select-clear-value'); + userEvent.click(clearButton); + expect(onClear).toHaveBeenCalled(); + }); + + it('should not render clear button when no onClear function is passed', async () => { + const select = render(); + + expect(() => { + select.getByLabelText('select-clear-value'); + }).toThrowError(); + }); + }); +}); diff --git a/packages/grafana-runtime/src/components/DataSourcePicker.tsx b/packages/grafana-runtime/src/components/DataSourcePicker.tsx index 68384b34f3d..8cba8f45f70 100644 --- a/packages/grafana-runtime/src/components/DataSourcePicker.tsx +++ b/packages/grafana-runtime/src/components/DataSourcePicker.tsx @@ -2,7 +2,7 @@ import React, { PureComponent } from 'react'; // Components -import { HorizontalGroup, PluginSignatureBadge, Select, stylesFactory } from '@grafana/ui'; +import { ActionMeta, HorizontalGroup, PluginSignatureBadge, Select, stylesFactory } from '@grafana/ui'; import { DataSourceInstanceSettings, DataSourceRef, @@ -40,6 +40,7 @@ export interface DataSourcePickerProps { noDefault?: boolean; width?: number; filter?: (dataSource: DataSourceInstanceSettings) => boolean; + onClear?: () => void; } /** @@ -80,7 +81,12 @@ export class DataSourcePicker extends PureComponent) => { + onChange = (item: SelectableValue, actionMeta: ActionMeta) => { + if (actionMeta.action === 'clear' && this.props.onClear) { + this.props.onClear(); + return; + } + const dsSettings = this.dataSourceSrv.getInstanceSettings(item.value); if (dsSettings) { @@ -142,11 +148,12 @@ export class DataSourcePicker extends PureComponent @@ -156,7 +163,7 @@ export class DataSourcePicker extends PureComponent [ds1], + getInstanceSettings: () => ds1, + }; +} diff --git a/packages/grafana-ui/src/components/Select/SelectBase.test.tsx b/packages/grafana-ui/src/components/Select/SelectBase.test.tsx index f2768833941..9530fa31657 100644 --- a/packages/grafana-ui/src/components/Select/SelectBase.test.tsx +++ b/packages/grafana-ui/src/components/Select/SelectBase.test.tsx @@ -202,10 +202,10 @@ describe('SelectBase', () => { expect(selectEl).toBeInTheDocument(); await selectOptionInTest(selectEl, 'Option 2'); - expect(spy).toHaveBeenCalledWith({ - label: 'Option 2', - value: 2, - }); + expect(spy).toHaveBeenCalledWith( + { label: 'Option 2', value: 2 }, + { action: 'select-option', name: undefined, option: undefined } + ); }); }); }); diff --git a/packages/grafana-ui/src/components/Select/SelectBase.tsx b/packages/grafana-ui/src/components/Select/SelectBase.tsx index 99406e8bae3..18c5d853fe6 100644 --- a/packages/grafana-ui/src/components/Select/SelectBase.tsx +++ b/packages/grafana-ui/src/components/Select/SelectBase.tsx @@ -20,7 +20,7 @@ import { MultiValueContainer, MultiValueRemove } from './MultiValue'; import { useTheme2 } from '../../themes'; import { getSelectStyles } from './getSelectStyles'; import { cleanValue, findSelectedValue } from './utils'; -import { SelectBaseProps, SelectValue } from './types'; +import { ActionMeta, SelectBaseProps, SelectValue } from './types'; import { deprecationWarning } from '@grafana/data'; interface ExtraValuesIndicatorProps { @@ -146,11 +146,11 @@ export function SelectBase({ const theme = useTheme2(); const styles = getSelectStyles(theme); const onChangeWithEmpty = useCallback( - (value: SelectValue) => { + (value: SelectValue, action: ActionMeta) => { if (isMulti && (value === undefined || value === null)) { - return onChange([]); + return onChange([], action); } - onChange(value); + onChange(value, action); }, [isMulti, onChange] ); diff --git a/packages/grafana-ui/src/components/Select/types.ts b/packages/grafana-ui/src/components/Select/types.ts index cf56c2c57f4..e9ca7b9a5f1 100644 --- a/packages/grafana-ui/src/components/Select/types.ts +++ b/packages/grafana-ui/src/components/Select/types.ts @@ -1,7 +1,9 @@ import { SelectableValue } from '@grafana/data'; import React from 'react'; +import { ActionMeta as SelectActionMeta } from 'react-select'; export type SelectValue = T | SelectableValue | T[] | Array>; +export type ActionMeta = SelectActionMeta<{}>; export type InputActionMeta = { action: 'set-value' | 'input-change' | 'input-blur' | 'menu-close'; }; @@ -52,7 +54,7 @@ export interface SelectCommonProps { /** The message to display when no options could be found */ noOptionsMessage?: string; onBlur?: () => void; - onChange: (value: SelectableValue) => {} | void; + onChange: (value: SelectableValue, actionMeta: ActionMeta) => {} | void; onCloseMenu?: () => void; /** allowCustomValue must be enabled. Function decides what to do with that custom value. */ onCreateOption?: (value: string) => void; diff --git a/packages/grafana-ui/src/types/index.ts b/packages/grafana-ui/src/types/index.ts index 3914ae5b02c..fc81391c049 100644 --- a/packages/grafana-ui/src/types/index.ts +++ b/packages/grafana-ui/src/types/index.ts @@ -4,3 +4,4 @@ export * from './completion'; export * from './storybook'; export * from './forms'; export * from './icon'; +export * from './select'; diff --git a/packages/grafana-ui/src/types/select.ts b/packages/grafana-ui/src/types/select.ts new file mode 100644 index 00000000000..2186bd0ea7c --- /dev/null +++ b/packages/grafana-ui/src/types/select.ts @@ -0,0 +1 @@ +export { ActionMeta } from '../components/Select/types'; diff --git a/public/app/core/components/Select/MetricSelect.test.tsx b/public/app/core/components/Select/MetricSelect.test.tsx index 252837d4cd6..fef81075285 100644 --- a/public/app/core/components/Select/MetricSelect.test.tsx +++ b/public/app/core/components/Select/MetricSelect.test.tsx @@ -36,7 +36,7 @@ describe('MetricSelect', () => { const wrapper = shallow(); const select = wrapper.find(Select); - select.props().onChange({ value: 'foo' }); + select.props().onChange({ value: 'foo' }, { action: 'select-option', option: undefined }); expect(select.props().noOptionsMessage).toBeDefined(); diff --git a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx index e110f4eff02..c6fbf27ef10 100644 --- a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx @@ -53,6 +53,10 @@ const RulesFilter = () => { setQueryParams({ dataSource: dataSourceValue.name }); }; + const clearDataSource = () => { + setQueryParams({ dataSource: null }); + }; + const handleQueryStringChange = debounce((e: FormEvent) => { const target = e.target as HTMLInputElement; setQueryParams({ queryString: target.value || null }); @@ -84,13 +88,15 @@ const RulesFilter = () => { return (
- +
diff --git a/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx b/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx index 26aea76ad40..8b5f9cb5ede 100644 --- a/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx @@ -74,7 +74,10 @@ describe('useCreatableSelectPersistedBehaviour', () => { // Should call onChange when selecting an already existing option userEvent.click(option1); - expect(onChange).toHaveBeenLastCalledWith({ value: 'Option 1', label: 'Option 1' }); + expect(onChange).toHaveBeenLastCalledWith( + { value: 'Option 1', label: 'Option 1' }, + { action: 'select-option', name: undefined, option: undefined } + ); userEvent.click(input); diff --git a/public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.test.tsx b/public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.test.tsx index fa6044ea076..3b5da1fe4b4 100644 --- a/public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.test.tsx @@ -27,7 +27,7 @@ describe('ElasticDetails', () => { const onChangeMock = jest.fn(); const wrapper = mount(); const selectEl = wrapper.find({ label: 'Pattern' }).find(Select); - selectEl.props().onChange({ value: 'Daily', label: 'Daily' }); + selectEl.props().onChange({ value: 'Daily', label: 'Daily' }, { action: 'select-option', option: undefined }); expect(onChangeMock.mock.calls[0][0].jsonData.interval).toBe('Daily'); expect(onChangeMock.mock.calls[0][0].database).toBe('[logstash-]YYYY.MM.DD'); @@ -40,7 +40,7 @@ describe('ElasticDetails', () => { const wrapper = mount(); const selectEl = wrapper.find({ label: 'Pattern' }).find(Select); - selectEl.props().onChange({ value: 'Monthly', label: 'Monthly' }); + selectEl.props().onChange({ value: 'Monthly', label: 'Monthly' }, { action: 'select-option', option: undefined }); expect(onChangeMock.mock.calls[0][0].jsonData.interval).toBe('Monthly'); expect(onChangeMock.mock.calls[0][0].database).toBe('[logstash-]YYYY.MM'); @@ -78,7 +78,12 @@ describe('ElasticDetails', () => { }); const selectEl = wrapper.find({ label: 'Version' }).find(Select); - selectEl.props().onChange({ value: tc.version, label: tc.version.toString() }); + selectEl + .props() + .onChange( + { value: tc.version, label: tc.version.toString() }, + { action: 'select-option', option: undefined } + ); expect(last(onChangeMock.mock.calls)[0].jsonData.maxConcurrentShardRequests).toBe( tc.expectedMaxConcurrentShardRequests diff --git a/yarn.lock b/yarn.lock index 8817270a48c..e9ca85d928e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2508,6 +2508,8 @@ __metadata: "@rollup/plugin-commonjs": 21.0.1 "@rollup/plugin-node-resolve": 13.0.6 "@sentry/browser": 5.25.0 + "@testing-library/react": ^12.1.2 + "@testing-library/user-event": ^13.5.0 "@types/angular": 1.6.56 "@types/history": ^4.7.8 "@types/jest": 27.0.2 @@ -6845,7 +6847,7 @@ __metadata: languageName: node linkType: hard -"@testing-library/react@npm:12.1.2": +"@testing-library/react@npm:12.1.2, @testing-library/react@npm:^12.1.2": version: 12.1.2 resolution: "@testing-library/react@npm:12.1.2" dependencies: @@ -6869,6 +6871,17 @@ __metadata: languageName: node linkType: hard +"@testing-library/user-event@npm:^13.5.0": + version: 13.5.0 + resolution: "@testing-library/user-event@npm:13.5.0" + dependencies: + "@babel/runtime": ^7.12.5 + peerDependencies: + "@testing-library/dom": ">=7.21.4" + checksum: 16319de685fbb7008f1ba667928f458b2d08196918002daca56996de80ef35e6d9de26e9e1ece7d00a004692b95a597cf9142fff0dc53f2f51606a776584f549 + languageName: node + linkType: hard + "@tootallnate/once@npm:1": version: 1.1.2 resolution: "@tootallnate/once@npm:1.1.2"