From de4cd984bd9f6065b36d639f26c68a8d8953dbf7 Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Thu, 25 Aug 2022 01:41:14 -0700 Subject: [PATCH] [v9.1.x] AdHoc variable: Correctly preselect datasource when provisioning (#54205) * AdHoc variable: Correctly preselect datasource when provisioning (#54088) * Adhoc variable: Correctly preselect datasource when provisioning * Fix test * Remove data sources from ad hoc variable state in favor of DataSourcePicker (cherry picked from commit 8eac5706fd46719463f9251a6b96aae90416db56) * Fix merge --- .../adhoc/AdHocVariableEditor.test.tsx | 41 ++++++++++++++++--- .../variables/adhoc/AdHocVariableEditor.tsx | 28 ++++--------- .../features/variables/adhoc/actions.test.ts | 35 +--------------- .../app/features/variables/adhoc/actions.ts | 35 +--------------- .../app/features/variables/editor/reducer.ts | 3 +- .../variables/editor/selectors.test.ts | 5 +-- .../features/variables/editor/selectors.ts | 2 +- 7 files changed, 51 insertions(+), 98 deletions(-) diff --git a/public/app/features/variables/adhoc/AdHocVariableEditor.test.tsx b/public/app/features/variables/adhoc/AdHocVariableEditor.test.tsx index aefc96375a8..66caf9b4cbc 100644 --- a/public/app/features/variables/adhoc/AdHocVariableEditor.test.tsx +++ b/public/app/features/variables/adhoc/AdHocVariableEditor.test.tsx @@ -1,11 +1,42 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; -import { selectOptionInTest, getSelectParent } from 'test/helpers/selectOptionInTest'; +import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; + +import { selectors } from '@grafana/e2e-selectors'; +import { mockDataSource } from 'app/features/alerting/unified/mocks'; +import { DataSourceType } from 'app/features/alerting/unified/utils/datasource'; import { adHocBuilder } from '../shared/testing/builders'; import { AdHocVariableEditorUnConnected as AdHocVariableEditor } from './AdHocVariableEditor'; +const promDsMock = mockDataSource({ + name: 'Prometheus', + type: DataSourceType.Prometheus, +}); + +const lokiDsMock = mockDataSource({ + name: 'Loki', + type: DataSourceType.Loki, +}); + +jest.mock('@grafana/runtime/src/services/dataSourceSrv', () => { + return { + getDataSourceSrv: () => ({ + get: () => { + return Promise.resolve(promDsMock); + }, + getList: () => [promDsMock, lokiDsMock], + getInstanceSettings: (v: string) => { + if (v === 'Prometheus') { + return promDsMock; + } + return lokiDsMock; + }, + }), + }; +}); + const props = { extended: { dataSources: [ @@ -29,17 +60,17 @@ describe('AdHocVariableEditor', () => { it('has a datasource select menu', async () => { render(); - const selectContainer = getSelectParent(screen.getByLabelText('Data source')); - expect(selectContainer).toHaveTextContent('Prometheus'); + expect(await screen.findByLabelText(selectors.components.DataSourcePicker.inputV2)).toBeInTheDocument(); }); it('calls the callback when changing the datasource', async () => { render(); - await selectOptionInTest(screen.getByLabelText('Data source'), 'Loki'); + const selectEl = screen.getByLabelText(selectors.components.DataSourcePicker.inputV2); + await selectOptionInTest(selectEl, 'Loki'); expect(props.changeVariableDatasource).toBeCalledWith( { type: 'adhoc', id: 'adhoc', rootStateKey: 'key' }, - { type: 'loki-ds', uid: 'abc' } + { type: 'loki', uid: 'mock-ds-3' } ); }); diff --git a/public/app/features/variables/adhoc/AdHocVariableEditor.tsx b/public/app/features/variables/adhoc/AdHocVariableEditor.tsx index 0aa81cd7d9b..c82c671d6c9 100644 --- a/public/app/features/variables/adhoc/AdHocVariableEditor.tsx +++ b/public/app/features/variables/adhoc/AdHocVariableEditor.tsx @@ -1,12 +1,12 @@ import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; -import { DataSourceRef, SelectableValue } from '@grafana/data'; -import { Alert, InlineFieldRow, VerticalGroup } from '@grafana/ui'; +import { DataSourceInstanceSettings, getDataSourceRef } from '@grafana/data'; +import { DataSourcePicker } from '@grafana/runtime'; +import { Alert, InlineField, InlineFieldRow, VerticalGroup } from '@grafana/ui'; import { StoreState } from 'app/types'; import { VariableSectionHeader } from '../editor/VariableSectionHeader'; -import { VariableSelectField } from '../editor/VariableSelectField'; import { initialVariableEditorState } from '../editor/reducer'; import { getAdhocVariableEditorState } from '../editor/selectors'; import { VariableEditorProps } from '../editor/types'; @@ -14,7 +14,7 @@ import { getVariablesState } from '../state/selectors'; import { AdHocVariableModel } from '../types'; import { toKeyedVariableIdentifier } from '../utils'; -import { changeVariableDatasource, initAdHocVariableEditor } from './actions'; +import { changeVariableDatasource } from './actions'; const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { const { rootStateKey } = ownProps.variable; @@ -34,7 +34,6 @@ const mapStateToProps = (state: StoreState, ownProps: OwnProps) => { }; const mapDispatchToProps = { - initAdHocVariableEditor, changeVariableDatasource, }; @@ -51,33 +50,24 @@ export class AdHocVariableEditorUnConnected extends PureComponent { console.error('AdHocVariableEditor: variable has no rootStateKey'); return; } - - this.props.initAdHocVariableEditor(rootStateKey); } - onDatasourceChanged = (option: SelectableValue) => { - this.props.changeVariableDatasource(toKeyedVariableIdentifier(this.props.variable), option.value); + onDatasourceChanged = (ds: DataSourceInstanceSettings) => { + this.props.changeVariableDatasource(toKeyedVariableIdentifier(this.props.variable), getDataSourceRef(ds)); }; render() { const { variable, extended } = this.props; - const dataSources = extended?.dataSources ?? []; const infoText = extended?.infoText ?? null; - const options = dataSources.map((ds) => ({ label: ds.text, value: ds.value })); - const value = options.find((o) => o.value?.uid === variable.datasource?.uid) ?? options[0]; return ( - + + + {infoText ? : null} diff --git a/public/app/features/variables/adhoc/actions.test.ts b/public/app/features/variables/adhoc/actions.test.ts index 22c3d49b1e0..60a96513253 100644 --- a/public/app/features/variables/adhoc/actions.test.ts +++ b/public/app/features/variables/adhoc/actions.test.ts @@ -4,7 +4,7 @@ import { VariableModel } from 'app/features/variables/types'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { variableAdapters } from '../adapters'; -import { changeVariableEditorExtended, setIdInEditor } from '../editor/reducer'; +import { changeVariableEditorExtended } from '../editor/reducer'; import { adHocBuilder } from '../shared/testing/builders'; import { getPreloadedState, getRootReducer, RootReducerType } from '../state/helpers'; import { toKeyedAction } from '../state/keyedVariablesReducer'; @@ -17,7 +17,6 @@ import { applyFilterFromTable, changeFilter, changeVariableDatasource, - initAdHocVariableEditor, removeFilter, setFiltersFromUrl, } from './actions'; @@ -46,14 +45,6 @@ const datasources = [ createDatasource('elasticsearch-v7'), ]; -const expectedDatasources = [ - { text: '', value: {} }, - { text: 'default (default)', value: { uid: 'default', type: 'default' } }, - { text: 'elasticsearch-v1', value: { uid: 'elasticsearch-v1', type: 'elasticsearch-v1' } }, - { text: 'influx', value: { uid: 'influx', type: 'influx' } }, - { text: 'elasticsearch-v7', value: { uid: 'elasticsearch-v7', type: 'elasticsearch-v7' } }, -]; - describe('adhoc actions', () => { describe('when applyFilterFromTable is dispatched and filter already exist', () => { it('then correct actions are dispatched', async () => { @@ -406,23 +397,6 @@ describe('adhoc actions', () => { }); }); - describe('when initAdHocVariableEditor is dispatched', () => { - it('then correct actions are dispatched', async () => { - const key = 'key'; - - getList.mockRestore(); - getList.mockReturnValue(datasources); - - const tester = reduxTester() - .givenRootReducer(getRootReducer()) - .whenActionIsDispatched(initAdHocVariableEditor(key)); - - tester.thenDispatchedActionsShouldEqual( - toKeyedAction(key, changeVariableEditorExtended({ dataSources: expectedDatasources })) - ); - }); - }); - describe('when changeVariableDatasource is dispatched with unsupported datasource', () => { it('then correct actions are dispatched', async () => { const key = 'key'; @@ -442,8 +416,6 @@ describe('adhoc actions', () => { const tester = await reduxTester() .givenRootReducer(getRootReducer()) .whenActionIsDispatched(createAddVariableAction(variable)) - .whenActionIsDispatched(toKeyedAction(key, setIdInEditor({ id: variable.id }))) - .whenActionIsDispatched(initAdHocVariableEditor(key)) .whenAsyncActionIsDispatched(changeVariableDatasource(toKeyedVariableIdentifier(variable), datasource), true); tester.thenDispatchedActionsShouldEqual( @@ -455,7 +427,6 @@ describe('adhoc actions', () => { key, changeVariableEditorExtended({ infoText: 'This data source does not support ad hoc filters yet.', - dataSources: expectedDatasources, }) ) ); @@ -484,8 +455,6 @@ describe('adhoc actions', () => { const tester = await reduxTester() .givenRootReducer(getRootReducer()) .whenActionIsDispatched(createAddVariableAction(variable)) - .whenActionIsDispatched(toKeyedAction(key, setIdInEditor({ id: variable.id }))) - .whenActionIsDispatched(initAdHocVariableEditor(key)) .whenAsyncActionIsDispatched(changeVariableDatasource(toKeyedVariableIdentifier(variable), datasource), true); tester.thenDispatchedActionsShouldEqual( @@ -493,7 +462,7 @@ describe('adhoc actions', () => { key, changeVariableProp(toVariablePayload(variable, { propName: 'datasource', propValue: datasource })) ), - toKeyedAction(key, changeVariableEditorExtended({ infoText: loadingText, dataSources: expectedDatasources })) + toKeyedAction(key, changeVariableEditorExtended({ infoText: loadingText })) ); }); }); diff --git a/public/app/features/variables/adhoc/actions.ts b/public/app/features/variables/adhoc/actions.ts index 596c60cdc49..c436e6f96e8 100644 --- a/public/app/features/variables/adhoc/actions.ts +++ b/public/app/features/variables/adhoc/actions.ts @@ -1,12 +1,11 @@ import { cloneDeep } from 'lodash'; -import { DataSourceRef, getDataSourceRef } from '@grafana/data'; +import { DataSourceRef } from '@grafana/data'; import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { AdHocVariableFilter, AdHocVariableModel } from 'app/features/variables/types'; import { StoreState, ThunkResult } from 'app/types'; import { changeVariableEditorExtended } from '../editor/reducer'; -import { getAdhocVariableEditorState } from '../editor/selectors'; import { isAdHoc } from '../guard'; import { variableUpdated } from '../state/actions'; import { toKeyedAction } from '../state/keyedVariablesReducer'; @@ -101,8 +100,6 @@ export const changeVariableDatasource = ( datasource?: DataSourceRef ): ThunkResult => { return async (dispatch, getState) => { - const { editor } = getVariablesState(identifier.rootStateKey, getState()); - const extended = getAdhocVariableEditorState(editor); const variable = getVariable(identifier, getState()); dispatch( toKeyedAction( @@ -123,42 +120,12 @@ export const changeVariableDatasource = ( identifier.rootStateKey, changeVariableEditorExtended({ infoText: message, - dataSources: extended?.dataSources ?? [], }) ) ); }; }; -export const initAdHocVariableEditor = - (key: string): ThunkResult => - (dispatch) => { - const dataSources = getDatasourceSrv().getList({ metrics: true, variables: true }); - const selectable = dataSources.reduce( - (all: Array<{ text: string; value: DataSourceRef | null }>, ds) => { - if (ds.meta.mixed) { - return all; - } - - const text = ds.isDefault ? `${ds.name} (default)` : ds.name; - const value = getDataSourceRef(ds); - all.push({ text, value }); - - return all; - }, - [{ text: '', value: {} }] - ); - - dispatch( - toKeyedAction( - key, - changeVariableEditorExtended({ - dataSources: selectable, - }) - ) - ); - }; - const createAdHocVariable = (options: AdHocTableOptions): ThunkResult => { return (dispatch, getState) => { const key = getLastKey(getState()); diff --git a/public/app/features/variables/editor/reducer.ts b/public/app/features/variables/editor/reducer.ts index 530a486bd6c..bd65ee56450 100644 --- a/public/app/features/variables/editor/reducer.ts +++ b/public/app/features/variables/editor/reducer.ts @@ -1,13 +1,12 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { DataSourceApi, DataSourceRef } from '@grafana/data'; +import { DataSourceApi } from '@grafana/data'; import { VariablePayload } from '../state/types'; import { VariableQueryEditorType } from '../types'; export interface AdHocVariableEditorState { infoText?: string; - dataSources: Array<{ text: string; value: DataSourceRef | null }>; } export interface DataSourceVariableEditorState { diff --git a/public/app/features/variables/editor/selectors.test.ts b/public/app/features/variables/editor/selectors.test.ts index 7281ec468b3..71c13e1e44a 100644 --- a/public/app/features/variables/editor/selectors.test.ts +++ b/public/app/features/variables/editor/selectors.test.ts @@ -14,10 +14,7 @@ import { } from './selectors'; const adhocExtended: AdHocVariableEditorState = { - dataSources: [ - { text: 'Prometheus', value: null }, // default datasource - { text: 'Loki', value: { type: 'loki-ds', uid: 'abc' } }, - ], + infoText: 'infoText', }; const datasourceExtended: DataSourceVariableEditorState = { diff --git a/public/app/features/variables/editor/selectors.ts b/public/app/features/variables/editor/selectors.ts index 2152d4ed18f..e9d4553628b 100644 --- a/public/app/features/variables/editor/selectors.ts +++ b/public/app/features/variables/editor/selectors.ts @@ -9,7 +9,7 @@ import { * Narrows generic variable editor state down to specific Adhoc variable extended editor state */ export function getAdhocVariableEditorState(editorState: VariableEditorState): AdHocVariableEditorState | null { - if (editorState.extended && 'dataSources' in editorState.extended) { + if (editorState.extended && 'infoText' in editorState.extended) { return editorState.extended; }