diff --git a/e2e/suite1/specs/queryVariableCrud.spec.ts b/e2e/suite1/specs/queryVariableCrud.spec.ts index 8b76d542a07..808de6adbeb 100644 --- a/e2e/suite1/specs/queryVariableCrud.spec.ts +++ b/e2e/suite1/specs/queryVariableCrud.spec.ts @@ -30,15 +30,11 @@ describe.skip('Variables', () => { if (!lastUid || !lastData) { e2e.flows.addDataSource(); e2e.flows.addDashboard(); + lastUid = 'test'; + lastData = 'test'; } else { - e2e.setScenarioContext({ lastAddedDataSource: lastData, lastAddedDashboardUid: lastUid }); + e2e.flows.openDashboard(); } - - e2e.getScenarioContext().then(({ lastAddedDashboardUid, lastAddedDataSource }: any) => { - e2e.flows.openDashboard({ uid: lastAddedDashboardUid }); - lastUid = lastAddedDashboardUid; - lastData = lastAddedDataSource; - }); }); it(`asserts defaults`, () => { @@ -254,7 +250,7 @@ const createQueryVariable = ({ name, label, dataSourceName, query }: CreateQuery expect(input.attr('placeholder')).equals('blank = auto'); expect(input.val()).equals(''); }); - e2e.pages.Dashboard.Settings.Variables.Edit.General.addButton().click(); + e2e.pages.Dashboard.Settings.Variables.Edit.General.submitButton().click(); }; const assertVariableLabelAndComponent = ({ label, options, selectedOption }: VariablesData) => { diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 17e2a5adf9c..9d2558ecd46 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -85,8 +85,7 @@ export const Pages = { selectionOptionsIncludeAllSwitch: 'Variable editor Form IncludeAll switch', selectionOptionsCustomAllInput: 'Variable editor Form IncludeAll field', previewOfValuesOption: 'Variable editor Preview of Values option', - addButton: 'Variable editor Add button', - updateButton: 'Variable editor Update button', + submitButton: 'Variable editor Submit button', }, QueryVariable: { queryOptionsDataSourceSelect: 'Variable editor Form Query DataSource select', diff --git a/packages/grafana-e2e/src/flows/addDashboard.ts b/packages/grafana-e2e/src/flows/addDashboard.ts index aa2c1edcaf1..b8b50a70ac9 100644 --- a/packages/grafana-e2e/src/flows/addDashboard.ts +++ b/packages/grafana-e2e/src/flows/addDashboard.ts @@ -217,7 +217,7 @@ const addVariable = (config: PartialAddVariableConfig, isFirst: boolean): AddVar } }); - e2e.pages.Dashboard.Settings.Variables.Edit.General.addButton().click(); + e2e.pages.Dashboard.Settings.Variables.Edit.General.submitButton().click(); return fullConfig; }; diff --git a/public/app/features/variables/adhoc/adapter.ts b/public/app/features/variables/adhoc/adapter.ts index 33e1e162e8a..ade84604f5e 100644 --- a/public/app/features/variables/adhoc/adapter.ts +++ b/public/app/features/variables/adhoc/adapter.ts @@ -28,7 +28,7 @@ export const createAdHocVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return rest; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/adhoc/reducer.ts b/public/app/features/variables/adhoc/reducer.ts index 5a02aebadec..4b57c8dc666 100644 --- a/public/app/features/variables/adhoc/reducer.ts +++ b/public/app/features/variables/adhoc/reducer.ts @@ -1,5 +1,5 @@ -import { AdHocVariableFilter, AdHocVariableModel, VariableHide } from 'app/features/variables/types'; -import { getInstanceState, NEW_VARIABLE_ID, VariablePayload } from '../state/types'; +import { AdHocVariableFilter, AdHocVariableModel, initialVariableModelState } from 'app/features/variables/types'; +import { getInstanceState, VariablePayload } from '../state/types'; import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; @@ -13,15 +13,8 @@ export interface AdHocVariableEditorState { } export const initialAdHocVariableModelState: AdHocVariableModel = { - id: NEW_VARIABLE_ID, - global: false, + ...initialVariableModelState, type: 'adhoc', - name: '', - hide: VariableHide.dontHide, - label: '', - skipUrlSync: false, - index: -1, - initLock: null, datasource: null, filters: [], }; diff --git a/public/app/features/variables/constant/actions.test.ts b/public/app/features/variables/constant/actions.test.ts index 277b2271baf..dd71d341e2e 100644 --- a/public/app/features/variables/constant/actions.test.ts +++ b/public/app/features/variables/constant/actions.test.ts @@ -4,7 +4,7 @@ import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { TemplatingState } from 'app/features/variables/state/reducers'; import { updateConstantVariableOptions } from './actions'; import { getRootReducer } from '../state/helpers'; -import { ConstantVariableModel, VariableHide, VariableOption } from '../types'; +import { ConstantVariableModel, initialVariableModelState, VariableOption } from '../types'; import { toVariablePayload } from '../state/types'; import { createConstantOptionsFromQuery } from './reducer'; import { addVariable, setCurrentVariableValue } from '../state/sharedReducer'; @@ -21,9 +21,11 @@ describe('constant actions', () => { }; const variable: ConstantVariableModel = { - type: 'constant', + ...initialVariableModelState, id: '0', - global: false, + index: 0, + type: 'constant', + name: 'Constant', current: { value: '', text: '', @@ -31,11 +33,6 @@ describe('constant actions', () => { }, options: [], query: 'A', - name: 'Constant', - label: '', - hide: VariableHide.dontHide, - skipUrlSync: false, - index: 0, }; const tester = await reduxTester<{ templating: TemplatingState }>() diff --git a/public/app/features/variables/constant/adapter.ts b/public/app/features/variables/constant/adapter.ts index 4c2d90c6eba..57b89df4f25 100644 --- a/public/app/features/variables/constant/adapter.ts +++ b/public/app/features/variables/constant/adapter.ts @@ -31,7 +31,7 @@ export const createConstantVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return rest; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/constant/reducer.ts b/public/app/features/variables/constant/reducer.ts index 6b228c79876..3de34403577 100644 --- a/public/app/features/variables/constant/reducer.ts +++ b/public/app/features/variables/constant/reducer.ts @@ -1,21 +1,15 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { ConstantVariableModel, VariableHide, VariableOption } from '../types'; -import { getInstanceState, NEW_VARIABLE_ID, VariablePayload } from '../state/types'; +import { ConstantVariableModel, initialVariableModelState, VariableHide, VariableOption } from '../types'; +import { getInstanceState, VariablePayload } from '../state/types'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; export const initialConstantVariableModelState: ConstantVariableModel = { - id: NEW_VARIABLE_ID, - global: false, + ...initialVariableModelState, type: 'constant', - name: '', hide: VariableHide.hideVariable, - label: '', query: '', current: {} as VariableOption, options: [], - skipUrlSync: false, - index: -1, - initLock: null, }; export const constantVariableSlice = createSlice({ diff --git a/public/app/features/variables/custom/actions.test.ts b/public/app/features/variables/custom/actions.test.ts index be73b78f627..2f2c0e13990 100644 --- a/public/app/features/variables/custom/actions.test.ts +++ b/public/app/features/variables/custom/actions.test.ts @@ -3,7 +3,7 @@ import { updateCustomVariableOptions } from './actions'; import { createCustomVariableAdapter } from './adapter'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { getRootReducer } from '../state/helpers'; -import { CustomVariableModel, VariableHide, VariableOption } from '../types'; +import { CustomVariableModel, initialVariableModelState, VariableOption } from '../types'; import { toVariablePayload } from '../state/types'; import { addVariable, setCurrentVariableValue } from '../state/sharedReducer'; import { TemplatingState } from '../state/reducers'; @@ -21,9 +21,11 @@ describe('custom actions', () => { }; const variable: CustomVariableModel = { - type: 'custom', + ...initialVariableModelState, id: '0', - global: false, + index: 0, + type: 'custom', + name: 'Custom', current: { value: '', text: '', @@ -42,11 +44,6 @@ describe('custom actions', () => { }, ], query: 'A,B', - name: 'Custom', - label: '', - hide: VariableHide.dontHide, - skipUrlSync: false, - index: 0, multi: true, includeAll: false, }; diff --git a/public/app/features/variables/custom/adapter.ts b/public/app/features/variables/custom/adapter.ts index 1858f8a21cd..02043577cf4 100644 --- a/public/app/features/variables/custom/adapter.ts +++ b/public/app/features/variables/custom/adapter.ts @@ -32,7 +32,7 @@ export const createCustomVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return rest; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/custom/reducer.ts b/public/app/features/variables/custom/reducer.ts index f433f595516..8d4d396ded7 100644 --- a/public/app/features/variables/custom/reducer.ts +++ b/public/app/features/variables/custom/reducer.ts @@ -1,31 +1,18 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { CustomVariableModel, VariableHide, VariableOption } from '../types'; -import { - ALL_VARIABLE_TEXT, - ALL_VARIABLE_VALUE, - getInstanceState, - NEW_VARIABLE_ID, - VariablePayload, -} from '../state/types'; +import { CustomVariableModel, initialVariableModelState, VariableOption } from '../types'; +import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, getInstanceState, VariablePayload } from '../state/types'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; export const initialCustomVariableModelState: CustomVariableModel = { - id: NEW_VARIABLE_ID, - global: false, + ...initialVariableModelState, + type: 'custom', multi: false, includeAll: false, allValue: null, query: '', options: [], current: {} as VariableOption, - name: '', - type: 'custom', - label: null, - hide: VariableHide.dontHide, - skipUrlSync: false, - index: -1, - initLock: null, }; export const customVariableSlice = createSlice({ diff --git a/public/app/features/variables/datasource/adapter.ts b/public/app/features/variables/datasource/adapter.ts index 40fb9d29fb0..41993fe50b2 100644 --- a/public/app/features/variables/datasource/adapter.ts +++ b/public/app/features/variables/datasource/adapter.ts @@ -35,7 +35,7 @@ export const createDataSourceVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return { ...rest, options: [] }; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/datasource/reducer.ts b/public/app/features/variables/datasource/reducer.ts index e4e50f581a2..8918991fea4 100644 --- a/public/app/features/variables/datasource/reducer.ts +++ b/public/app/features/variables/datasource/reducer.ts @@ -1,12 +1,6 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { DataSourceVariableModel, VariableHide, VariableOption, VariableRefresh } from '../types'; -import { - ALL_VARIABLE_TEXT, - ALL_VARIABLE_VALUE, - getInstanceState, - NEW_VARIABLE_ID, - VariablePayload, -} from '../state/types'; +import { DataSourceVariableModel, initialVariableModelState, VariableOption, VariableRefresh } from '../types'; +import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, getInstanceState, VariablePayload } from '../state/types'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; import { DataSourceSelectItem } from '@grafana/data'; @@ -15,12 +9,8 @@ export interface DataSourceVariableEditorState { } export const initialDataSourceVariableModelState: DataSourceVariableModel = { - id: NEW_VARIABLE_ID, - global: false, + ...initialVariableModelState, type: 'datasource', - name: '', - hide: VariableHide.dontHide, - label: '', current: {} as VariableOption, regex: '', options: [], @@ -28,9 +18,6 @@ export const initialDataSourceVariableModelState: DataSourceVariableModel = { multi: false, includeAll: false, refresh: VariableRefresh.onDashboardLoad, - skipUrlSync: false, - index: -1, - initLock: null, }; export const dataSourceVariableSlice = createSlice({ diff --git a/public/app/features/variables/editor/VariableEditorEditor.tsx b/public/app/features/variables/editor/VariableEditorEditor.tsx index 749af6dcfa6..4843278ea7d 100644 --- a/public/app/features/variables/editor/VariableEditorEditor.tsx +++ b/public/app/features/variables/editor/VariableEditorEditor.tsx @@ -1,11 +1,11 @@ import React, { ChangeEvent, FormEvent, PureComponent } from 'react'; import isEqual from 'lodash/isEqual'; -import { AppEvents, VariableType } from '@grafana/data'; -import { InlineFormLabel } from '@grafana/ui'; +import { AppEvents, LoadingState, VariableType } from '@grafana/data'; +import { Icon, InlineFormLabel } from '@grafana/ui'; import { selectors } from '@grafana/e2e-selectors'; import { variableAdapters } from '../adapters'; -import { NEW_VARIABLE_ID, toVariablePayload, VariableIdentifier } from '../state/types'; +import { NEW_VARIABLE_ID, toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types'; import { VariableHide, VariableModel } from '../types'; import { appEvents } from '../../../core/core'; import { VariableValuesPreview } from './VariableValuesPreview'; @@ -17,6 +17,7 @@ import { getVariable } from '../state/selectors'; import { connectWithStore } from '../../../core/utils/connectWithReduxStore'; import { OnPropChangeArguments } from './types'; import { changeVariableProp, changeVariableType } from '../state/sharedReducer'; +import { updateOptions } from '../state/actions'; export interface OwnProps { identifier: VariableIdentifier; @@ -35,6 +36,7 @@ interface DispatchProps { onEditorUpdate: typeof onEditorUpdate; onEditorAdd: typeof onEditorAdd; changeVariableType: typeof changeVariableType; + updateOptions: typeof updateOptions; } type Props = OwnProps & ConnectedProps & DispatchProps; @@ -88,7 +90,7 @@ export class VariableEditorEditorUnConnected extends PureComponent { onPropChanged = async ({ propName, propValue, updateOptions = false }: OnPropChangeArguments) => { this.props.changeVariableProp(toVariablePayload(this.props.identifier, { propName, propValue })); if (updateOptions) { - await variableAdapters.get(this.props.variable.type).updateOptions(this.props.variable); + await this.props.updateOptions(toVariableIdentifier(this.props.variable)); } }; @@ -108,11 +110,13 @@ export class VariableEditorEditorUnConnected extends PureComponent { }; render() { + const { variable } = this.props; const EditorToRender = variableAdapters.get(this.props.variable.type).editor; if (!EditorToRender) { return null; } const newVariable = this.props.variable.id && this.props.variable.id === NEW_VARIABLE_ID; + const loading = variable.state === LoadingState.Loading; return (
@@ -201,24 +205,15 @@ export class VariableEditorEditorUnConnected extends PureComponent {
- {!newVariable && ( - - )} - {newVariable && ( - - )} +
@@ -239,6 +234,7 @@ const mapDispatchToProps: MapDispatchToProps = { onEditorUpdate, onEditorAdd, changeVariableType, + updateOptions, }; export const VariableEditorEditor = connectWithStore( diff --git a/public/app/features/variables/editor/actions.ts b/public/app/features/variables/editor/actions.ts index 150d1e83f54..593055abebf 100644 --- a/public/app/features/variables/editor/actions.ts +++ b/public/app/features/variables/editor/actions.ts @@ -19,6 +19,7 @@ import { import cloneDeep from 'lodash/cloneDeep'; import { VariableType } from '@grafana/data'; import { addVariable, removeVariable, storeNewVariable } from '../state/sharedReducer'; +import { updateOptions } from '../state/actions'; export const variableEditorMount = (identifier: VariableIdentifier): ThunkResult => { return async dispatch => { @@ -36,9 +37,8 @@ export const variableEditorUnMount = (identifier: VariableIdentifier): ThunkResu }; export const onEditorUpdate = (identifier: VariableIdentifier): ThunkResult => { - return async (dispatch, getState) => { - const variableInState = getVariable(identifier.id, getState()); - await variableAdapters.get(variableInState.type).updateOptions(variableInState); + return async dispatch => { + await dispatch(updateOptions(identifier)); dispatch(switchToListMode()); }; }; @@ -48,8 +48,7 @@ export const onEditorAdd = (identifier: VariableIdentifier): ThunkResult = const newVariableInState = getVariable(NEW_VARIABLE_ID, getState()); const id = newVariableInState.name; dispatch(storeNewVariable(toVariablePayload({ type: identifier.type, id }))); - const variableInState = getVariable(id, getState()); - await variableAdapters.get(variableInState.type).updateOptions(variableInState); + await dispatch(updateOptions(identifier)); dispatch(switchToListMode()); dispatch(removeVariable(toVariablePayload({ type: identifier.type, id: NEW_VARIABLE_ID }, { reIndex: false }))); }; diff --git a/public/app/features/variables/interval/actions.test.ts b/public/app/features/variables/interval/actions.test.ts index ca9782020c6..bdac2a2045b 100644 --- a/public/app/features/variables/interval/actions.test.ts +++ b/public/app/features/variables/interval/actions.test.ts @@ -2,22 +2,23 @@ import { getRootReducer } from '../state/helpers'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { TemplatingState } from '../state/reducers'; import { toVariableIdentifier, toVariablePayload } from '../state/types'; -import { - updateAutoValue, - UpdateAutoValueDependencies, - updateIntervalVariableOptions, - UpdateIntervalVariableOptionsDependencies, -} from './actions'; +import { updateAutoValue, UpdateAutoValueDependencies, updateIntervalVariableOptions } from './actions'; import { createIntervalOptions } from './reducer'; -import { setCurrentVariableValue, addVariable } from '../state/sharedReducer'; +import { + addVariable, + setCurrentVariableValue, + variableStateFailed, + variableStateFetching, +} from '../state/sharedReducer'; import { variableAdapters } from '../adapters'; import { createIntervalVariableAdapter } from './adapter'; -import { Emitter } from 'app/core/core'; -import { AppEvents, dateTime } from '@grafana/data'; +import { dateTime } from '@grafana/data'; import { getTimeSrv, setTimeSrv, TimeSrv } from '../../dashboard/services/TimeSrv'; import { TemplateSrv } from '../../templating/template_srv'; import { intervalBuilder } from '../shared/testing/builders'; -import kbn from 'app/core/utils/kbn'; +import { updateOptions } from '../state/actions'; +import { notifyApp } from '../../../core/actions'; +import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput'; describe('interval actions', () => { variableAdapters.setInit(() => [createIntervalVariableAdapter()]); @@ -45,8 +46,9 @@ describe('interval actions', () => { }); }); - describe('when updateIntervalVariableOptions is dispatched but something throws', () => { - it('then an app event should be emitted', async () => { + describe('when updateOptions is dispatched but something throws', () => { + silenceConsoleOutput(); + it('then an notifyApp action should be dispatched', async () => { const timeSrvMock = ({ timeRange: jest.fn().mockReturnValue({ from: dateTime(new Date()) @@ -67,23 +69,36 @@ describe('interval actions', () => { .withAuto(true) .withAutoMin('1xyz') // illegal interval string .build(); - const appEventMock = ({ - emit: jest.fn(), - } as unknown) as Emitter; - const dependencies: UpdateIntervalVariableOptionsDependencies = { appEvents: appEventMock }; - await reduxTester<{ templating: TemplatingState }>() + const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getRootReducer()) .whenActionIsDispatched(addVariable(toVariablePayload(interval, { global: false, index: 0, model: interval }))) - .whenAsyncActionIsDispatched(updateIntervalVariableOptions(toVariableIdentifier(interval), dependencies), true); + .whenAsyncActionIsDispatched(updateOptions(toVariableIdentifier(interval)), true); + + tester.thenDispatchedActionsPredicateShouldEqual(dispatchedActions => { + const expectedNumberOfActions = 4; + expect(dispatchedActions[0]).toEqual(variableStateFetching(toVariablePayload(interval))); + expect(dispatchedActions[1]).toEqual(createIntervalOptions(toVariablePayload(interval))); + expect(dispatchedActions[2]).toEqual( + variableStateFailed( + toVariablePayload(interval, { + error: new Error( + 'Invalid interval string, has to be either unit-less or end with one of the following units: "y, M, w, d, h, m, s, ms"' + ), + }) + ) + ); + + expect(dispatchedActions[3].type).toEqual(notifyApp.type); + expect(dispatchedActions[3].payload.title).toEqual('Templating [0]'); + expect(dispatchedActions[3].payload.text).toEqual( + 'Error updating options: Invalid interval string, has to be either unit-less or end with one of the following units: "y, M, w, d, h, m, s, ms"' + ); + expect(dispatchedActions[3].payload.severity).toEqual('error'); + + return dispatchedActions.length === expectedNumberOfActions; + }); - expect(appEventMock.emit).toHaveBeenCalledTimes(1); - expect(appEventMock.emit).toHaveBeenCalledWith(AppEvents.alertError, [ - 'Templating', - `Invalid interval string, has to be either unit-less or end with one of the following units: "${Object.keys( - kbn.intervalsInSeconds - ).join(', ')}"`, - ]); setTimeSrv(originalTimeSrv); }); }); diff --git a/public/app/features/variables/interval/actions.ts b/public/app/features/variables/interval/actions.ts index 05975ad9358..e63d0e18d3b 100644 --- a/public/app/features/variables/interval/actions.ts +++ b/public/app/features/variables/interval/actions.ts @@ -1,4 +1,4 @@ -import { AppEvents, rangeUtil } from '@grafana/data'; +import { rangeUtil } from '@grafana/data'; import { toVariablePayload, VariableIdentifier } from '../state/types'; import { ThunkResult } from '../../../types'; @@ -8,23 +8,11 @@ import { getVariable } from '../state/selectors'; import { IntervalVariableModel } from '../types'; import { getTimeSrv } from '../../dashboard/services/TimeSrv'; import { getTemplateSrv, TemplateSrv } from '../../templating/template_srv'; -import appEvents from '../../../core/app_events'; -export interface UpdateIntervalVariableOptionsDependencies { - appEvents: typeof appEvents; -} - -export const updateIntervalVariableOptions = ( - identifier: VariableIdentifier, - dependencies: UpdateIntervalVariableOptionsDependencies = { appEvents: appEvents } -): ThunkResult => async dispatch => { - try { - await dispatch(createIntervalOptions(toVariablePayload(identifier))); - await dispatch(updateAutoValue(identifier)); - await dispatch(validateVariableSelectionState(identifier)); - } catch (error) { - dependencies.appEvents.emit(AppEvents.alertError, ['Templating', error.message]); - } +export const updateIntervalVariableOptions = (identifier: VariableIdentifier): ThunkResult => async dispatch => { + await dispatch(createIntervalOptions(toVariablePayload(identifier))); + await dispatch(updateAutoValue(identifier)); + await dispatch(validateVariableSelectionState(identifier)); }; export interface UpdateAutoValueDependencies { diff --git a/public/app/features/variables/interval/adapter.ts b/public/app/features/variables/interval/adapter.ts index b40da6768e5..26b9f34130d 100644 --- a/public/app/features/variables/interval/adapter.ts +++ b/public/app/features/variables/interval/adapter.ts @@ -33,7 +33,7 @@ export const createIntervalVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return rest; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/interval/reducer.ts b/public/app/features/variables/interval/reducer.ts index e643ba14f62..87884bdb700 100644 --- a/public/app/features/variables/interval/reducer.ts +++ b/public/app/features/variables/interval/reducer.ts @@ -1,17 +1,12 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { IntervalVariableModel, VariableHide, VariableOption, VariableRefresh } from '../types'; -import { getInstanceState, NEW_VARIABLE_ID, VariablePayload } from '../state/types'; +import { initialVariableModelState, IntervalVariableModel, VariableOption, VariableRefresh } from '../types'; +import { getInstanceState, VariablePayload } from '../state/types'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; import _ from 'lodash'; export const initialIntervalVariableModelState: IntervalVariableModel = { - id: NEW_VARIABLE_ID, - global: false, + ...initialVariableModelState, type: 'interval', - name: '', - label: '', - hide: VariableHide.dontHide, - skipUrlSync: false, auto_count: 30, auto_min: '10s', options: [], @@ -19,8 +14,6 @@ export const initialIntervalVariableModelState: IntervalVariableModel = { query: '1m,10m,30m,1h,6h,12h,1d,7d,14d,30d', refresh: VariableRefresh.onTimeRangeChanged, current: {} as VariableOption, - index: -1, - initLock: null, }; export const intervalVariableSlice = createSlice({ diff --git a/public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx b/public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx index a5901e0ace6..eb790b72f25 100644 --- a/public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx +++ b/public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx @@ -11,6 +11,7 @@ import { VariableOptions } from '../shared/VariableOptions'; import { isQuery } from '../../guard'; import { VariablePickerProps } from '../types'; import { formatVariableLabel } from '../../shared/formatVariable'; +import { LoadingState } from '@grafana/data'; interface OwnProps extends VariablePickerProps {} @@ -67,8 +68,9 @@ export class OptionsPickerUnconnected extends PureComponent { const linkText = formatVariableLabel(variable); const tags = getSelectedTags(variable); + const loading = variable.state === LoadingState.Loading; - return ; + return ; } renderOptions(showOptions: boolean, picker: OptionsPickerState) { diff --git a/public/app/features/variables/pickers/OptionsPicker/actions.test.ts b/public/app/features/variables/pickers/OptionsPicker/actions.test.ts index 3daf82e7f28..6172b34174e 100644 --- a/public/app/features/variables/pickers/OptionsPicker/actions.test.ts +++ b/public/app/features/variables/pickers/OptionsPicker/actions.test.ts @@ -1,7 +1,7 @@ import { reduxTester } from '../../../../../test/core/redux/reduxTester'; import { getRootReducer } from '../../state/helpers'; import { TemplatingState } from '../../state/reducers'; -import { QueryVariableModel, VariableHide, VariableRefresh, VariableSort } from '../../types'; +import { initialVariableModelState, QueryVariableModel, VariableRefresh, VariableSort } from '../../types'; import { hideOptions, moveOptionsHighlight, @@ -404,17 +404,14 @@ describe('options picker actions', () => { function createMultiVariable(extend?: Partial): QueryVariableModel { return { + ...initialVariableModelState, type: 'query', id: '0', - global: false, + index: 0, current: createOption([]), options: [], query: 'options-query', name: 'Constant', - label: '', - hide: VariableHide.dontHide, - skipUrlSync: false, - index: 0, datasource: 'datasource', definition: '', sort: VariableSort.alphabeticalAsc, diff --git a/public/app/features/variables/pickers/shared/VariableLink.tsx b/public/app/features/variables/pickers/shared/VariableLink.tsx index 621da63504c..99a76d16c85 100644 --- a/public/app/features/variables/pickers/shared/VariableLink.tsx +++ b/public/app/features/variables/pickers/shared/VariableLink.tsx @@ -1,55 +1,98 @@ -import React, { PureComponent } from 'react'; -import { getTagColorsFromName, Icon } from '@grafana/ui'; +import React, { FC, MouseEvent, useCallback } from 'react'; +import { css } from 'emotion'; +import { getTagColorsFromName, Icon, useStyles } from '@grafana/ui'; import { selectors } from '@grafana/e2e-selectors'; +import { GrafanaTheme } from '@grafana/data'; import { VariableTag } from '../../types'; -import { css } from 'emotion'; interface Props { onClick: () => void; text: string; tags: VariableTag[]; + loading: boolean; } -export class VariableLink extends PureComponent { - onClick = (event: React.MouseEvent) => { - event.stopPropagation(); - event.preventDefault(); - this.props.onClick(); - }; - render() { - const { tags = [], text } = this.props; +export const VariableLink: FC = ({ loading, onClick: propsOnClick, tags, text }) => { + const styles = useStyles(getStyles); + const onClick = useCallback( + (event: MouseEvent) => { + event.stopPropagation(); + event.preventDefault(); + propsOnClick(); + }, + [propsOnClick] + ); + if (loading) { return ( - - - {text} - {tags.map(tag => { - const { color, borderColor } = getTagColorsFromName(tag.text.toString()); - return ( - - -    - -   {tag.text} - - - ); - })} - - - + + + ); } -} + + return ( + + + + + ); +}; + +const VariableLinkText: FC> = ({ tags, text }) => { + const styles = useStyles(getStyles); + return ( + + {text} + {tags.map(tag => { + const { color, borderColor } = getTagColorsFromName(tag.text.toString()); + return ( + + +    + +   {tag.text} + + + ); + })} + + ); +}; + +const getStyles = (theme: GrafanaTheme) => ({ + container: css` + max-width: 500px; + padding-right: 10px; + padding: 0 ${theme.spacing.sm}; + background-color: ${theme.colors.formInputBg}; + border: 1px solid ${theme.colors.formInputBorder}; + border-radius: ${theme.border.radius.sm}; + display: flex; + align-items: center; + color: ${theme.colors.text}; + height: ${theme.height.md}px; + + .label-tag { + margin: 0 5px; + } + `, + textAndTags: css` + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + margin-right: ${theme.spacing.xxs}; + user-select: none; + `, +}); diff --git a/public/app/features/variables/query/actions.test.ts b/public/app/features/variables/query/actions.test.ts index 2ccb6dc3c38..33bf44b3510 100644 --- a/public/app/features/variables/query/actions.test.ts +++ b/public/app/features/variables/query/actions.test.ts @@ -1,10 +1,19 @@ +import { LoadingState } from '@grafana/data'; + import { variableAdapters } from '../adapters'; import { createQueryVariableAdapter } from './adapter'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { getRootReducer } from '../state/helpers'; import { QueryVariableModel, VariableHide, VariableRefresh, VariableSort } from '../types'; import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, toVariablePayload } from '../state/types'; -import { addVariable, changeVariableProp, setCurrentVariableValue } from '../state/sharedReducer'; +import { + addVariable, + changeVariableProp, + setCurrentVariableValue, + variableStateCompleted, + variableStateFailed, + variableStateFetching, +} from '../state/sharedReducer'; import { TemplatingState } from '../state/reducers'; import { changeQueryVariableDataSource, @@ -21,6 +30,9 @@ import { } from '../editor/reducer'; import DefaultVariableQueryEditor from '../editor/DefaultVariableQueryEditor'; import { expect } from 'test/lib/common'; +import { updateOptions } from '../state/actions'; +import { notifyApp } from '../../../core/reducers/appNotification'; +import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput'; const mocks: Record = { datasource: { @@ -215,6 +227,7 @@ describe('query actions', () => { }); describe('when updateQueryVariableOptions is dispatched and fails for variable open in editor', () => { + silenceConsoleOutput(); it('then correct actions are dispatched', async () => { const variable = createVariable({ includeAll: true, useTags: false }); const error = { message: 'failed to fetch metrics' }; @@ -225,15 +238,23 @@ describe('query actions', () => { .givenRootReducer(getRootReducer()) .whenActionIsDispatched(addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable }))) .whenActionIsDispatched(setIdInEditor({ id: variable.id })) - .whenAsyncActionIsDispatched(updateQueryVariableOptions(toVariablePayload(variable)), true); + .whenAsyncActionIsDispatched(updateOptions(toVariablePayload(variable)), true); - tester.thenDispatchedActionsPredicateShouldEqual(actions => { - const [clearErrors, errorOccurred] = actions; - const expectedNumberOfActions = 2; + tester.thenDispatchedActionsPredicateShouldEqual(dispatchedActions => { + const expectedNumberOfActions = 5; - expect(errorOccurred).toEqual(addVariableEditorError({ errorProp: 'update', errorText: error.message })); - expect(clearErrors).toEqual(removeVariableEditorError({ errorProp: 'update' })); - return actions.length === expectedNumberOfActions; + expect(dispatchedActions[0]).toEqual(variableStateFetching(toVariablePayload(variable))); + expect(dispatchedActions[1]).toEqual(removeVariableEditorError({ errorProp: 'update' })); + expect(dispatchedActions[2]).toEqual(addVariableEditorError({ errorProp: 'update', errorText: error.message })); + expect(dispatchedActions[3]).toEqual( + variableStateFailed(toVariablePayload(variable, { error: { message: 'failed to fetch metrics' } })) + ); + expect(dispatchedActions[4].type).toEqual(notifyApp.type); + expect(dispatchedActions[4].payload.title).toEqual('Templating [0]'); + expect(dispatchedActions[4].payload.text).toEqual('Error updating options: failed to fetch metrics'); + expect(dispatchedActions[4].payload.severity).toEqual('error'); + + return dispatchedActions.length === expectedNumberOfActions; }); }); }); @@ -435,23 +456,16 @@ describe('query actions', () => { const option = createOption(ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE); const update = { results: optionsMetrics, templatedRegex: '' }; - tester.thenDispatchedActionsPredicateShouldEqual(actions => { - const [clearError, changeQuery, changeDefinition, updateOptions, updateTags, setOption] = actions; - const expectedNumberOfActions = 6; - - expect(clearError).toEqual(removeVariableEditorError({ errorProp: 'query' })); - expect(changeQuery).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })) - ); - expect(changeDefinition).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })) - ); - expect(updateOptions).toEqual(updateVariableOptions(toVariablePayload(variable, update))); - expect(updateTags).toEqual(updateVariableTags(toVariablePayload(variable, tagsMetrics))); - expect(setOption).toEqual(setCurrentVariableValue(toVariablePayload(variable, { option }))); - - return actions.length === expectedNumberOfActions; - }); + tester.thenDispatchedActionsShouldEqual( + removeVariableEditorError({ errorProp: 'query' }), + changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })), + changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })), + variableStateFetching(toVariablePayload(variable)), + updateVariableOptions(toVariablePayload(variable, update)), + updateVariableTags(toVariablePayload(variable, tagsMetrics)), + setCurrentVariableValue(toVariablePayload(variable, { option })), + variableStateCompleted(toVariablePayload(variable)) + ); }); }); @@ -473,22 +487,15 @@ describe('query actions', () => { const option = createOption(ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE); const update = { results: optionsMetrics, templatedRegex: '' }; - tester.thenDispatchedActionsPredicateShouldEqual(actions => { - const [clearError, changeQuery, changeDefinition, updateOptions, setOption] = actions; - const expectedNumberOfActions = 5; - - expect(clearError).toEqual(removeVariableEditorError({ errorProp: 'query' })); - expect(changeQuery).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })) - ); - expect(changeDefinition).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })) - ); - expect(updateOptions).toEqual(updateVariableOptions(toVariablePayload(variable, update))); - expect(setOption).toEqual(setCurrentVariableValue(toVariablePayload(variable, { option }))); - - return actions.length === expectedNumberOfActions; - }); + tester.thenDispatchedActionsShouldEqual( + removeVariableEditorError({ errorProp: 'query' }), + changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })), + changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })), + variableStateFetching(toVariablePayload(variable)), + updateVariableOptions(toVariablePayload(variable, update)), + setCurrentVariableValue(toVariablePayload(variable, { option })), + variableStateCompleted(toVariablePayload(variable)) + ); }); }); @@ -509,22 +516,15 @@ describe('query actions', () => { const option = createOption('A'); const update = { results: optionsMetrics, templatedRegex: '' }; - tester.thenDispatchedActionsPredicateShouldEqual(actions => { - const [clearError, changeQuery, changeDefinition, updateOptions, setOption] = actions; - const expectedNumberOfActions = 5; - - expect(clearError).toEqual(removeVariableEditorError({ errorProp: 'query' })); - expect(changeQuery).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })) - ); - expect(changeDefinition).toEqual( - changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })) - ); - expect(updateOptions).toEqual(updateVariableOptions(toVariablePayload(variable, update))); - expect(setOption).toEqual(setCurrentVariableValue(toVariablePayload(variable, { option }))); - - return actions.length === expectedNumberOfActions; - }); + tester.thenDispatchedActionsShouldEqual( + removeVariableEditorError({ errorProp: 'query' }), + changeVariableProp(toVariablePayload(variable, { propName: 'query', propValue: query })), + changeVariableProp(toVariablePayload(variable, { propName: 'definition', propValue: definition })), + variableStateFetching(toVariablePayload(variable)), + updateVariableOptions(toVariablePayload(variable, update)), + setCurrentVariableValue(toVariablePayload(variable, { option })), + variableStateCompleted(toVariablePayload(variable)) + ); }); }); @@ -588,6 +588,8 @@ function createVariable(extend?: Partial): QueryVariableMode regex: '', multi: true, includeAll: true, + state: LoadingState.NotStarted, + error: null, ...(extend ?? {}), }; } diff --git a/public/app/features/variables/query/actions.ts b/public/app/features/variables/query/actions.ts index 5572e91ac89..5de8c8abb57 100644 --- a/public/app/features/variables/query/actions.ts +++ b/public/app/features/variables/query/actions.ts @@ -1,16 +1,15 @@ -import { AppEvents, DataSourcePluginMeta, DataSourceSelectItem } from '@grafana/data'; -import { getTemplateSrv } from '@grafana/runtime'; -import { validateVariableSelectionState } from '../state/actions'; +import { DataSourcePluginMeta, DataSourceSelectItem } from '@grafana/data'; +import { toDataQueryError, getTemplateSrv } from '@grafana/runtime'; + +import { updateOptions, validateVariableSelectionState } from '../state/actions'; import { QueryVariableModel, VariableRefresh } from '../types'; import { ThunkResult } from '../../../types'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; import { getTimeSrv } from '../../dashboard/services/TimeSrv'; -import appEvents from '../../../core/app_events'; import { importDataSourcePlugin } from '../../plugins/plugin_loader'; import DefaultVariableQueryEditor from '../editor/DefaultVariableQueryEditor'; import { getVariable } from '../state/selectors'; import { addVariableEditorError, changeVariableEditorExtended, removeVariableEditorError } from '../editor/reducer'; -import { variableAdapters } from '../adapters'; import { changeVariableProp } from '../state/sharedReducer'; import { updateVariableOptions, updateVariableTags } from './reducer'; import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from '../state/types'; @@ -60,17 +59,12 @@ export const updateQueryVariableOptions = ( await dispatch(validateVariableSelectionState(toVariableIdentifier(variableInState))); } } catch (err) { - console.error(err); - if (err.data && err.data.message) { - err.message = err.data.message; - } + const error = toDataQueryError(err); if (getState().templating.editor.id === variableInState.id) { - dispatch(addVariableEditorError({ errorProp: 'update', errorText: err.message })); + dispatch(addVariableEditorError({ errorProp: 'update', errorText: error.message })); } - appEvents.emit(AppEvents.alertError, [ - 'Templating', - 'Template variables could not be initialized: ' + err.message, - ]); + + throw error; } }; }; @@ -126,7 +120,7 @@ export const changeQueryVariableQuery = ( dispatch(removeVariableEditorError({ errorProp: 'query' })); dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'query', propValue: query }))); dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'definition', propValue: definition }))); - await variableAdapters.get(identifier.type).updateOptions(variableInState); + await dispatch(updateOptions(identifier)); }; const getTemplatedRegex = (variable: QueryVariableModel): string => { diff --git a/public/app/features/variables/query/adapter.ts b/public/app/features/variables/query/adapter.ts index c6f6a01696f..b2a8d220323 100644 --- a/public/app/features/variables/query/adapter.ts +++ b/public/app/features/variables/query/adapter.ts @@ -33,7 +33,7 @@ export const createQueryVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, queryValue, ...rest } = cloneDeep(variable); + const { index, id, state, global, queryValue, ...rest } = cloneDeep(variable); // remove options if (variable.refresh !== VariableRefresh.never) { return { ...rest, options: [] }; diff --git a/public/app/features/variables/query/reducer.ts b/public/app/features/variables/query/reducer.ts index 1d607b0e5b6..ad77b03ce6b 100644 --- a/public/app/features/variables/query/reducer.ts +++ b/public/app/features/variables/query/reducer.ts @@ -2,13 +2,19 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import _ from 'lodash'; import { DataSourceApi, DataSourceSelectItem, MetricFindValue, stringToJsRegex } from '@grafana/data'; -import { QueryVariableModel, VariableHide, VariableOption, VariableRefresh, VariableSort, VariableTag } from '../types'; +import { + initialVariableModelState, + QueryVariableModel, + VariableOption, + VariableRefresh, + VariableSort, + VariableTag, +} from '../types'; import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, getInstanceState, - NEW_VARIABLE_ID, NONE_VARIABLE_TEXT, NONE_VARIABLE_VALUE, VariablePayload, @@ -29,14 +35,8 @@ export interface QueryVariableEditorState { } export const initialQueryVariableModelState: QueryVariableModel = { - id: NEW_VARIABLE_ID, - global: false, - index: -1, + ...initialVariableModelState, type: 'query', - name: '', - label: null, - hide: VariableHide.dontHide, - skipUrlSync: false, datasource: null, query: '', regex: '', @@ -52,7 +52,6 @@ export const initialQueryVariableModelState: QueryVariableModel = { tagsQuery: '', tagValuesQuery: '', definition: '', - initLock: null, }; const sortVariableValues = (options: any[], sortOrder: VariableSort) => { diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index 5ca68d71dd4..5f373882bd7 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -20,13 +20,12 @@ import { validateVariableSelectionState, } from './actions'; import { - addInitLock, addVariable, changeVariableProp, - removeInitLock, removeVariable, - resolveInitLock, setCurrentVariableValue, + variableStateCompleted, + variableStateNotStarted, } from './sharedReducer'; import { NEW_VARIABLE_ID, toVariableIdentifier, toVariablePayload } from './types'; import { @@ -98,16 +97,16 @@ describe('shared actions', () => { // because uuid are dynamic we need to get the uuid from the resulting state // an alternative would be to add our own uuids in the model above instead expect(dispatchedActions[4]).toEqual( - addInitLock(toVariablePayload({ ...query, id: dispatchedActions[4].payload.id })) + variableStateNotStarted(toVariablePayload({ ...query, id: dispatchedActions[4].payload.id })) ); expect(dispatchedActions[5]).toEqual( - addInitLock(toVariablePayload({ ...constant, id: dispatchedActions[5].payload.id })) + variableStateNotStarted(toVariablePayload({ ...constant, id: dispatchedActions[5].payload.id })) ); expect(dispatchedActions[6]).toEqual( - addInitLock(toVariablePayload({ ...custom, id: dispatchedActions[6].payload.id })) + variableStateNotStarted(toVariablePayload({ ...custom, id: dispatchedActions[6].payload.id })) ); expect(dispatchedActions[7]).toEqual( - addInitLock(toVariablePayload({ ...textbox, id: dispatchedActions[7].payload.id })) + variableStateNotStarted(toVariablePayload({ ...textbox, id: dispatchedActions[7].payload.id })) ); return true; @@ -128,36 +127,27 @@ describe('shared actions', () => { preloadedState: { templating: ({} as unknown) as TemplatingState, location: { query: {} } }, }) .givenRootReducer(getTemplatingAndLocationRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariables(), true); await tester.thenDispatchedActionsPredicateShouldEqual(dispatchedActions => { - expect(dispatchedActions.length).toEqual(8); + expect(dispatchedActions.length).toEqual(4); expect(dispatchedActions[0]).toEqual( - resolveInitLock(toVariablePayload({ ...query, id: dispatchedActions[0].payload.id })) - ); - expect(dispatchedActions[1]).toEqual( - resolveInitLock(toVariablePayload({ ...constant, id: dispatchedActions[1].payload.id })) - ); - expect(dispatchedActions[2]).toEqual( - resolveInitLock(toVariablePayload({ ...custom, id: dispatchedActions[2].payload.id })) - ); - expect(dispatchedActions[3]).toEqual( - resolveInitLock(toVariablePayload({ ...textbox, id: dispatchedActions[3].payload.id })) + variableStateCompleted(toVariablePayload({ ...query, id: dispatchedActions[0].payload.id })) ); - expect(dispatchedActions[4]).toEqual( - removeInitLock(toVariablePayload({ ...query, id: dispatchedActions[4].payload.id })) + expect(dispatchedActions[1]).toEqual( + variableStateCompleted(toVariablePayload({ ...constant, id: dispatchedActions[1].payload.id })) ); - expect(dispatchedActions[5]).toEqual( - removeInitLock(toVariablePayload({ ...constant, id: dispatchedActions[5].payload.id })) + + expect(dispatchedActions[2]).toEqual( + variableStateCompleted(toVariablePayload({ ...custom, id: dispatchedActions[2].payload.id })) ); - expect(dispatchedActions[6]).toEqual( - removeInitLock(toVariablePayload({ ...custom, id: dispatchedActions[6].payload.id })) - ); - expect(dispatchedActions[7]).toEqual( - removeInitLock(toVariablePayload({ ...textbox, id: dispatchedActions[7].payload.id })) + + expect(dispatchedActions[3]).toEqual( + variableStateCompleted(toVariablePayload({ ...textbox, id: dispatchedActions[3].payload.id })) ); return true; @@ -578,12 +568,11 @@ describe('shared actions', () => { expect(dispatchedActions[4]).toEqual( addVariable(toVariablePayload(constant, { global: false, index: 0, model: constant })) ); - expect(dispatchedActions[5]).toEqual(addInitLock(toVariablePayload(constant))); - expect(dispatchedActions[6]).toEqual(resolveInitLock(toVariablePayload(constant))); - expect(dispatchedActions[7]).toEqual(removeInitLock(toVariablePayload(constant))); + expect(dispatchedActions[5]).toEqual(variableStateNotStarted(toVariablePayload(constant))); + expect(dispatchedActions[6]).toEqual(variableStateCompleted(toVariablePayload(constant))); - expect(dispatchedActions[8]).toEqual(variablesCompleteTransaction({ uid })); - return dispatchedActions.length === 9; + expect(dispatchedActions[7]).toEqual(variablesCompleteTransaction({ uid })); + return dispatchedActions.length === 8; }); }); }); @@ -618,11 +607,10 @@ describe('shared actions', () => { expect(dispatchedActions[6]).toEqual( addVariable(toVariablePayload(constant, { global: false, index: 0, model: constant })) ); - expect(dispatchedActions[7]).toEqual(addInitLock(toVariablePayload(constant))); - expect(dispatchedActions[8]).toEqual(resolveInitLock(toVariablePayload(constant))); - expect(dispatchedActions[9]).toEqual(removeInitLock(toVariablePayload(constant))); - expect(dispatchedActions[10]).toEqual(variablesCompleteTransaction({ uid })); - return dispatchedActions.length === 11; + expect(dispatchedActions[7]).toEqual(variableStateNotStarted(toVariablePayload(constant))); + expect(dispatchedActions[8]).toEqual(variableStateCompleted(toVariablePayload(constant))); + expect(dispatchedActions[9]).toEqual(variablesCompleteTransaction({ uid })); + return dispatchedActions.length === 10; }); }); }); diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index e845ff5eea9..8866c489d55 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -1,9 +1,10 @@ -import castArray from 'lodash/castArray'; -import { AppEvents, TimeRange, UrlQueryMap, UrlQueryValue } from '@grafana/data'; import angular from 'angular'; +import castArray from 'lodash/castArray'; +import { LoadingState, TimeRange, UrlQueryMap, UrlQueryValue } from '@grafana/data'; import { DashboardVariableModel, + initialVariableModelState, OrgVariableModel, QueryVariableModel, UserVariableModel, @@ -14,21 +15,21 @@ import { VariableWithMultiSupport, VariableWithOptions, } from '../types'; -import { StoreState, ThunkResult } from '../../../types'; +import { AppNotification, StoreState, ThunkResult } from '../../../types'; import { getVariable, getVariables } from './selectors'; import { variableAdapters } from '../adapters'; import { Graph } from '../../../core/utils/dag'; import { notifyApp, updateLocation } from 'app/core/actions'; import { - addInitLock, addVariable, changeVariableProp, - removeInitLock, - resolveInitLock, setCurrentVariableValue, + variableStateCompleted, + variableStateFailed, + variableStateFetching, + variableStateNotStarted, } from './sharedReducer'; import { toVariableIdentifier, toVariablePayload, VariableIdentifier } from './types'; -import { appEvents } from 'app/core/core'; import { contextSrv } from 'app/core/services/context_srv'; import { getTemplateSrv, TemplateSrv } from '../../templating/template_srv'; import { alignCurrentWithMulti } from '../shared/multiOptions'; @@ -46,6 +47,7 @@ import { getBackendSrv } from '../../../core/services/backend_srv'; import { cleanVariables } from './variablesReducer'; import isEqual from 'lodash/isEqual'; import { getCurrentText } from '../utils'; +import { store } from 'app/store/store'; // process flow queryVariable // thunk => processVariables @@ -92,7 +94,7 @@ export const initDashboardTemplating = (list: VariableModel[]): ThunkResult => { return (dispatch, getState) => { const dashboardModel: DashboardVariableModel = { + ...initialVariableModelState, id: '__dashboard', name: '__dashboard', - label: null, type: 'system', index: -3, skipUrlSync: true, hide: VariableHide.hideVariable, - global: false, current: { value: { name: dashboard.title, @@ -128,14 +129,13 @@ export const addSystemTemplateVariables = (dashboard: DashboardModel): ThunkResu ); const orgModel: OrgVariableModel = { + ...initialVariableModelState, id: '__org', name: '__org', - label: null, type: 'system', index: -2, skipUrlSync: true, hide: VariableHide.hideVariable, - global: false, current: { value: { name: contextSrv.user.orgName, @@ -150,14 +150,13 @@ export const addSystemTemplateVariables = (dashboard: DashboardModel): ThunkResu ); const userModel: UserVariableModel = { + ...initialVariableModelState, id: '__user', name: '__user', - label: null, type: 'system', index: -1, skipUrlSync: true, hide: VariableHide.hideVariable, - global: false, current: { value: { login: contextSrv.user.login, @@ -184,7 +183,7 @@ export const changeVariableMultiValue = (identifier: VariableIdentifier, multi: }; export const processVariableDependencies = async (variable: VariableModel, state: StoreState) => { - let dependencies: Array> = []; + const dependencies: VariableModel[] = []; for (const otherVariable of getVariables(state)) { if (variable === otherVariable) { @@ -193,12 +192,36 @@ export const processVariableDependencies = async (variable: VariableModel, state if (variableAdapters.getIfExists(variable.type)) { if (variableAdapters.get(variable.type).dependsOn(variable, otherVariable)) { - dependencies.push(otherVariable.initLock!.promise); + dependencies.push(otherVariable); } } } - await Promise.all(dependencies); + if (!isWaitingForDependencies(dependencies, state)) { + return; + } + + await new Promise(resolve => { + const unsubscribe = store.subscribe(() => { + if (!isWaitingForDependencies(dependencies, store.getState())) { + unsubscribe(); + resolve(); + } + }); + }); +}; + +const isWaitingForDependencies = (dependencies: VariableModel[], state: StoreState): boolean => { + if (dependencies.length === 0) { + return false; + } + + const variables = getVariables(state); + const notCompletedDependencies = dependencies.filter(d => + variables.some(v => v.id === d.id && (v.state === LoadingState.NotStarted || v.state === LoadingState.Loading)) + ); + + return notCompletedDependencies.length > 0; }; export const processVariable = ( @@ -212,7 +235,6 @@ export const processVariable = ( const urlValue = queryParams['var-' + variable.name]; if (urlValue !== void 0) { await variableAdapters.get(variable.type).setValueFromUrl(variable, urlValue ?? ''); - dispatch(resolveInitLock(toVariablePayload(variable))); return; } @@ -222,13 +244,13 @@ export const processVariable = ( refreshableVariable.refresh === VariableRefresh.onDashboardLoad || refreshableVariable.refresh === VariableRefresh.onTimeRangeChanged ) { - await variableAdapters.get(variable.type).updateOptions(refreshableVariable); - dispatch(resolveInitLock(toVariablePayload(variable))); + await dispatch(updateOptions(toVariableIdentifier(refreshableVariable))); return; } } - dispatch(resolveInitLock(toVariablePayload(variable))); + // for variables that aren't updated via url or refresh let's simulate the same state changes + dispatch(variableStateCompleted(toVariablePayload(variable))); }; }; @@ -240,10 +262,6 @@ export const processVariables = (): ThunkResult> => { ); await Promise.all(promises); - - for (let index = 0; index < getVariables(getState()).length; index++) { - dispatch(removeInitLock(toVariablePayload(getVariables(getState())[index]))); - } }; }; @@ -255,7 +273,12 @@ export const setOptionFromUrl = ( const variable = getVariable(identifier.id, getState()); if (variable.hasOwnProperty('refresh') && (variable as QueryVariableModel).refresh !== VariableRefresh.never) { // updates options - await variableAdapters.get(variable.type).updateOptions(variable); + await dispatch(updateOptions(toVariableIdentifier(variable))); + } + + if (variable.hasOwnProperty('refresh') && (variable as QueryVariableModel).refresh === VariableRefresh.never) { + // for variables that have refresh to never simulate the same state changes + dispatch(variableStateCompleted(toVariablePayload(variable))); } // get variable from state @@ -425,16 +448,17 @@ export const variableUpdated = ( emitChangeEvents: boolean ): ThunkResult> => { return (dispatch, getState) => { - // if there is a variable lock ignore cascading update because we are in a boot up scenario - const variable = getVariable(identifier.id, getState()); - if (variable.initLock) { + const variableInState = getVariable(identifier.id, getState()); + + // if we're initializing variables ignore cascading update because we are in a boot up scenario + if (getState().templating.transaction.status === TransactionStatus.Fetching) { return Promise.resolve(); } const variables = getVariables(getState()); const g = createGraph(variables); - const node = g.getNode(variable.name); + const node = g.getNode(variableInState.name); let promises: Array> = []; if (node) { promises = node.getOptimizedInputEdges().map(e => { @@ -442,7 +466,8 @@ export const variableUpdated = ( if (!variable) { return Promise.resolve(); } - return variableAdapters.get(variable.type).updateOptions(variable); + + return dispatch(updateOptions(toVariableIdentifier(variable))); }); } @@ -459,12 +484,11 @@ export const variableUpdated = ( export interface OnTimeRangeUpdatedDependencies { templateSrv: TemplateSrv; - appEvents: typeof appEvents; } export const onTimeRangeUpdated = ( timeRange: TimeRange, - dependencies: OnTimeRangeUpdatedDependencies = { templateSrv: getTemplateSrv(), appEvents: appEvents } + dependencies: OnTimeRangeUpdatedDependencies = { templateSrv: getTemplateSrv() } ): ThunkResult> => async (dispatch, getState) => { dependencies.templateSrv.updateTimeRange(timeRange); const variablesThatNeedRefresh = getVariables(getState()).filter(variable => { @@ -476,15 +500,9 @@ export const onTimeRangeUpdated = ( return false; }); - const promises = variablesThatNeedRefresh.map(async (variable: VariableWithOptions) => { - const previousOptions = variable.options.slice(); - await variableAdapters.get(variable.type).updateOptions(variable); - const updatedVariable = getVariable(variable.id, getState()); - if (angular.toJson(previousOptions) !== angular.toJson(updatedVariable.options)) { - const dashboard = getState().dashboard.getModel(); - dashboard?.templateVariableValueUpdated(); - } - }); + const promises = variablesThatNeedRefresh.map((variable: VariableWithOptions) => + dispatch(timeRangeUpdated(toVariableIdentifier(variable))) + ); try { await Promise.all(promises); @@ -492,7 +510,22 @@ export const onTimeRangeUpdated = ( dashboard?.startRefresh(); } catch (error) { console.error(error); - dependencies.appEvents.emit(AppEvents.alertError, ['Template variable service failed', error.message]); + dispatch(notifyApp(createVariableErrorNotification('Template variable service failed', error))); + } +}; + +const timeRangeUpdated = (identifier: VariableIdentifier): ThunkResult> => async (dispatch, getState) => { + const variableInState = getVariable(identifier.id); + const previousOptions = variableInState.options.slice(); + + await dispatch(updateOptions(toVariableIdentifier(variableInState), true)); + + const updatedVariable = getVariable(identifier.id, getState()); + const updatedOptions = updatedVariable.options; + + if (angular.toJson(previousOptions) !== angular.toJson(updatedOptions)) { + const dashboard = getState().dashboard.getModel(); + dashboard?.templateVariableValueUpdated(); } }; @@ -565,7 +598,7 @@ export const initVariablesTransaction = (dashboardUid: string, dashboard: Dashbo // Mark update as complete dispatch(variablesCompleteTransaction({ uid: dashboardUid })); } catch (err) { - dispatch(notifyApp(createErrorNotification('Templating init failed', err))); + dispatch(notifyApp(createVariableErrorNotification('Templating init failed', err))); console.error(err); } }; @@ -582,3 +615,36 @@ export const cancelVariables = ( dependencies.getBackendSrv().cancelAllInFlightRequests(); dispatch(cleanUpVariables()); }; + +export const updateOptions = (identifier: VariableIdentifier, rethrow = false): ThunkResult> => async ( + dispatch, + getState +) => { + const variableInState = getVariable(identifier.id, getState()); + try { + dispatch(variableStateFetching(toVariablePayload(variableInState))); + await variableAdapters.get(variableInState.type).updateOptions(variableInState); + dispatch(variableStateCompleted(toVariablePayload(variableInState))); + } catch (error) { + dispatch(variableStateFailed(toVariablePayload(variableInState, { error }))); + + if (!rethrow) { + console.error(error); + dispatch(notifyApp(createVariableErrorNotification('Error updating options:', error, identifier))); + } + + if (rethrow) { + throw error; + } + } +}; + +export const createVariableErrorNotification = ( + message: string, + error: any, + identifier?: VariableIdentifier +): AppNotification => + createErrorNotification( + `${identifier ? `Templating [${identifier.id}]` : 'Templating'}`, + `${message} ${error.message}` + ); diff --git a/public/app/features/variables/state/helpers.ts b/public/app/features/variables/state/helpers.ts index 0bb31ee4551..f8a7a2327b3 100644 --- a/public/app/features/variables/state/helpers.ts +++ b/public/app/features/variables/state/helpers.ts @@ -1,4 +1,5 @@ import { combineReducers } from '@reduxjs/toolkit'; +import { LoadingState } from '@grafana/data'; import { NEW_VARIABLE_ID } from './types'; import { VariableHide, VariableModel } from '../types'; @@ -25,6 +26,8 @@ export const getVariableState = ( label: `Label-${index}`, skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }; } @@ -38,6 +41,8 @@ export const getVariableState = ( label: `Label-${NEW_VARIABLE_ID}`, skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }; } diff --git a/public/app/features/variables/state/onTimeRangeUpdated.test.ts b/public/app/features/variables/state/onTimeRangeUpdated.test.ts index 7f4c055333a..1e74481d461 100644 --- a/public/app/features/variables/state/onTimeRangeUpdated.test.ts +++ b/public/app/features/variables/state/onTimeRangeUpdated.test.ts @@ -1,8 +1,7 @@ import { dateTime, TimeRange } from '@grafana/data'; import { TemplateSrv } from '../../templating/template_srv'; -import { Emitter } from '../../../core/utils/emitter'; -import { onTimeRangeUpdated, OnTimeRangeUpdatedDependencies } from './actions'; +import { onTimeRangeUpdated, OnTimeRangeUpdatedDependencies, setOptionAsCurrent } from './actions'; import { DashboardModel } from '../../dashboard/state'; import { DashboardState } from '../../../types'; import { createIntervalVariableAdapter } from '../interval/adapter'; @@ -10,10 +9,39 @@ import { variableAdapters } from '../adapters'; import { createConstantVariableAdapter } from '../constant/adapter'; import { VariableRefresh } from '../types'; import { constantBuilder, intervalBuilder } from '../shared/testing/builders'; +import { reduxTester } from '../../../../test/core/redux/reduxTester'; +import { TemplatingState } from './reducers'; +import { getRootReducer } from './helpers'; +import { toVariableIdentifier, toVariablePayload } from './types'; +import { + setCurrentVariableValue, + variableStateCompleted, + variableStateFailed, + variableStateFetching, +} from './sharedReducer'; +import { createIntervalOptions } from '../interval/reducer'; +import { silenceConsoleOutput } from '../../../../test/core/utils/silenceConsoleOutput'; +import { notifyApp } from '../../../core/reducers/appNotification'; +import { expect } from '../../../../test/lib/common'; variableAdapters.setInit(() => [createIntervalVariableAdapter(), createConstantVariableAdapter()]); -const getOnTimeRangeUpdatedContext = (args: { update?: boolean; throw?: boolean }) => { +const getTestContext = () => { + const interval = intervalBuilder() + .withId('interval-0') + .withName('interval-0') + .withOptions('1m', '10m', '30m', '1h', '6h', '12h', '1d', '7d', '14d', '30d') + .withCurrent('1m') + .withRefresh(VariableRefresh.onTimeRangeChanged) + .build(); + + const constant = constantBuilder() + .withId('constant-1') + .withName('constant-1') + .withOptions('a constant') + .withCurrent('a constant') + .build(); + const range: TimeRange = { from: dateTime(new Date().getTime()).subtract(1, 'minutes'), to: dateTime(new Date().getTime()), @@ -24,9 +52,7 @@ const getOnTimeRangeUpdatedContext = (args: { update?: boolean; throw?: boolean }; const updateTimeRangeMock = jest.fn(); const templateSrvMock = ({ updateTimeRange: updateTimeRangeMock } as unknown) as TemplateSrv; - const emitMock = jest.fn(); - const appEventsMock = ({ emit: emitMock } as unknown) as Emitter; - const dependencies: OnTimeRangeUpdatedDependencies = { templateSrv: templateSrvMock, appEvents: appEventsMock }; + const dependencies: OnTimeRangeUpdatedDependencies = { templateSrv: templateSrvMock }; const templateVariableValueUpdatedMock = jest.fn(); const dashboard = ({ getModel: () => @@ -37,131 +63,139 @@ const getOnTimeRangeUpdatedContext = (args: { update?: boolean; throw?: boolean } as unknown) as DashboardState; const startRefreshMock = jest.fn(); const adapter = variableAdapters.get('interval'); - adapter.updateOptions = args.throw ? jest.fn().mockRejectedValue('Something broke') : jest.fn().mockResolvedValue({}); - - // initial variable state - const initialVariable = intervalBuilder() - .withId('interval-0') - .withName('interval-0') - .withOptions('1m', '10m', '30m', '1h', '6h', '12h', '1d', '7d', '14d', '30d') - .withCurrent('1m') - .withRefresh(VariableRefresh.onTimeRangeChanged) - .build(); - - // the constant variable should be filtered out - const constant = constantBuilder() - .withId('constant-1') - .withName('constant-1') - .withOptions('a constant') - .withCurrent('a constant') - .build(); - const initialState = { - templating: { variables: { '0': { ...initialVariable }, '1': { ...constant } } }, + const preloadedState = { dashboard, + location: { query: '' }, + templating: ({ + variables: { + 'interval-0': { ...interval }, + 'constant-1': { ...constant }, + }, + } as unknown) as TemplatingState, }; - // updated variable state - const updatedVariable = intervalBuilder() - .withId('interval-0') - .withName('interval-0') - .withOptions('1m') - .withCurrent('1m') - .withRefresh(VariableRefresh.onTimeRangeChanged) - .build(); - - const variable = args.update ? { ...updatedVariable } : { ...initialVariable }; - const state = { templating: { variables: { 'interval-0': variable, 'constant-1': { ...constant } } }, dashboard }; - const getStateMock = jest - .fn() - .mockReturnValueOnce(initialState) - .mockReturnValue(state); - const dispatchMock = jest.fn(); - return { + interval, range, dependencies, - dispatchMock, - getStateMock, + adapter, + preloadedState, updateTimeRangeMock, templateVariableValueUpdatedMock, startRefreshMock, - emitMock, }; }; describe('when onTimeRangeUpdated is dispatched', () => { describe('and options are changed by update', () => { - it('then correct dependencies are called', async () => { + it('then correct actions are dispatched and correct dependencies are called', async () => { const { + preloadedState, range, dependencies, - dispatchMock, - getStateMock, updateTimeRangeMock, templateVariableValueUpdatedMock, startRefreshMock, - emitMock, - } = getOnTimeRangeUpdatedContext({ update: true }); + } = getTestContext(); - await onTimeRangeUpdated(range, dependencies)(dispatchMock, getStateMock, undefined); + const tester = await reduxTester<{ templating: TemplatingState }>({ preloadedState }) + .givenRootReducer(getRootReducer()) + .whenAsyncActionIsDispatched(onTimeRangeUpdated(range, dependencies)); + + tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'interval', id: 'interval-0' })), + createIntervalOptions(toVariablePayload({ type: 'interval', id: 'interval-0' })), + setCurrentVariableValue( + toVariablePayload( + { type: 'interval', id: 'interval-0' }, + { option: { text: '1m', value: '1m', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'interval', id: 'interval-0' })) + ); - expect(dispatchMock).toHaveBeenCalledTimes(0); - expect(getStateMock).toHaveBeenCalledTimes(4); expect(updateTimeRangeMock).toHaveBeenCalledTimes(1); expect(updateTimeRangeMock).toHaveBeenCalledWith(range); expect(templateVariableValueUpdatedMock).toHaveBeenCalledTimes(1); expect(startRefreshMock).toHaveBeenCalledTimes(1); - expect(emitMock).toHaveBeenCalledTimes(0); }); }); describe('and options are not changed by update', () => { - it('then correct dependencies are called', async () => { + it('then correct actions are dispatched and correct dependencies are called', async () => { const { + interval, + preloadedState, range, dependencies, - dispatchMock, - getStateMock, updateTimeRangeMock, templateVariableValueUpdatedMock, startRefreshMock, - emitMock, - } = getOnTimeRangeUpdatedContext({ update: false }); + } = getTestContext(); - await onTimeRangeUpdated(range, dependencies)(dispatchMock, getStateMock, undefined); + const tester = await reduxTester<{ templating: TemplatingState }>({ preloadedState }) + .givenRootReducer(getRootReducer()) + .whenActionIsDispatched(setOptionAsCurrent(toVariableIdentifier(interval), interval.options[0], false)) + .whenAsyncActionIsDispatched(onTimeRangeUpdated(range, dependencies), true); + + tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'interval', id: 'interval-0' })), + createIntervalOptions(toVariablePayload({ type: 'interval', id: 'interval-0' })), + setCurrentVariableValue( + toVariablePayload( + { type: 'interval', id: 'interval-0' }, + { option: { text: '1m', value: '1m', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'interval', id: 'interval-0' })) + ); - expect(dispatchMock).toHaveBeenCalledTimes(0); - expect(getStateMock).toHaveBeenCalledTimes(3); expect(updateTimeRangeMock).toHaveBeenCalledTimes(1); expect(updateTimeRangeMock).toHaveBeenCalledWith(range); expect(templateVariableValueUpdatedMock).toHaveBeenCalledTimes(0); expect(startRefreshMock).toHaveBeenCalledTimes(1); - expect(emitMock).toHaveBeenCalledTimes(0); }); }); describe('and updateOptions throws', () => { - it('then correct dependencies are called', async () => { + silenceConsoleOutput(); + it('then correct actions are dispatched and correct dependencies are called', async () => { const { + adapter, + preloadedState, range, dependencies, - dispatchMock, - getStateMock, updateTimeRangeMock, templateVariableValueUpdatedMock, startRefreshMock, - emitMock, - } = getOnTimeRangeUpdatedContext({ update: false, throw: true }); + } = getTestContext(); - await onTimeRangeUpdated(range, dependencies)(dispatchMock, getStateMock, undefined); + adapter.updateOptions = jest.fn().mockRejectedValue(new Error('Something broke')); + + const tester = await reduxTester<{ templating: TemplatingState }>({ preloadedState, debug: true }) + .givenRootReducer(getRootReducer()) + .whenAsyncActionIsDispatched(onTimeRangeUpdated(range, dependencies), true); + + tester.thenDispatchedActionsPredicateShouldEqual(dispatchedActions => { + expect(dispatchedActions[0]).toEqual( + variableStateFetching(toVariablePayload({ type: 'interval', id: 'interval-0' })) + ); + expect(dispatchedActions[1]).toEqual( + variableStateFailed( + toVariablePayload({ type: 'interval', id: 'interval-0' }, { error: new Error('Something broke') }) + ) + ); + expect(dispatchedActions[2].type).toEqual(notifyApp.type); + expect(dispatchedActions[2].payload.title).toEqual('Templating'); + expect(dispatchedActions[2].payload.text).toEqual('Template variable service failed Something broke'); + expect(dispatchedActions[2].payload.severity).toEqual('error'); + return dispatchedActions.length === 3; + }); - expect(dispatchMock).toHaveBeenCalledTimes(0); - expect(getStateMock).toHaveBeenCalledTimes(1); expect(updateTimeRangeMock).toHaveBeenCalledTimes(1); expect(updateTimeRangeMock).toHaveBeenCalledWith(range); expect(templateVariableValueUpdatedMock).toHaveBeenCalledTimes(0); expect(startRefreshMock).toHaveBeenCalledTimes(0); - expect(emitMock).toHaveBeenCalledTimes(1); }); }); }); diff --git a/public/app/features/variables/state/processVariable.test.ts b/public/app/features/variables/state/processVariable.test.ts index f9b7ddbf2e8..9f611a64da8 100644 --- a/public/app/features/variables/state/processVariable.test.ts +++ b/public/app/features/variables/state/processVariable.test.ts @@ -7,11 +7,12 @@ import { createCustomVariableAdapter } from '../custom/adapter'; import { reduxTester } from '../../../../test/core/redux/reduxTester'; import { TemplatingState } from 'app/features/variables/state/reducers'; import { initDashboardTemplating, processVariable } from './actions'; -import { resolveInitLock, setCurrentVariableValue } from './sharedReducer'; +import { setCurrentVariableValue, variableStateCompleted, variableStateFetching } from './sharedReducer'; import { toVariableIdentifier, toVariablePayload } from './types'; import { VariableRefresh } from '../types'; import { updateVariableOptions } from '../query/reducer'; import { customBuilder, queryBuilder } from '../shared/testing/builders'; +import { variablesInitTransaction } from './transactionReducer'; jest.mock('app/features/dashboard/services/TimeSrv', () => ({ getTimeSrv: jest.fn().mockReturnValue({ @@ -110,12 +111,11 @@ describe('processVariable', () => { const queryParams: UrlQueryMap = {}; const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams), true); - await tester.thenDispatchedActionsShouldEqual( - resolveInitLock(toVariablePayload({ type: 'custom', id: 'custom' })) - ); + await tester.thenDispatchedActionsShouldEqual(variableStateCompleted(toVariablePayload(custom))); }); }); @@ -125,14 +125,14 @@ describe('processVariable', () => { const queryParams: UrlQueryMap = { 'var-custom': 'B' }; const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams), true); await tester.thenDispatchedActionsShouldEqual( setCurrentVariableValue( toVariablePayload({ type: 'custom', id: 'custom' }, { option: { text: 'B', value: 'B', selected: false } }) - ), - resolveInitLock(toVariablePayload({ type: 'custom', id: 'custom' })) + ) ); }); }); @@ -150,49 +150,50 @@ describe('processVariable', () => { queryNoDepends.refresh = refresh; const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); - await tester.thenDispatchedActionsShouldEqual( - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryNoDepends' })) - ); + await tester.thenDispatchedActionsShouldEqual(variableStateCompleted(toVariablePayload(queryNoDepends))); }); }); - [VariableRefresh.onDashboardLoad, VariableRefresh.onTimeRangeChanged].forEach(refresh => { - describe(`and refresh is ${refresh}`, () => { - it('then correct actions are dispatched', async () => { - const { list, queryNoDepends } = getAndSetupProcessVariableContext(); - queryNoDepends.refresh = refresh; - const tester = await reduxTester<{ templating: TemplatingState }>() - .givenRootReducer(getTemplatingRootReducer()) - .whenActionIsDispatched(initDashboardTemplating(list)) - .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); + it.each` + refresh + ${VariableRefresh.onDashboardLoad} + ${VariableRefresh.onTimeRangeChanged} + `('and refresh is $refresh then correct actions are dispatched', async ({ refresh }) => { + const { list, queryNoDepends } = getAndSetupProcessVariableContext(); + queryNoDepends.refresh = refresh; + const tester = await reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) + .whenActionIsDispatched(initDashboardTemplating(list)) + .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); - await tester.thenDispatchedActionsShouldEqual( - updateVariableOptions( - toVariablePayload( - { type: 'query', id: 'queryNoDepends' }, - { - results: [ - { value: 'A', text: 'A' }, - { value: 'B', text: 'B' }, - { value: 'C', text: 'C' }, - ], - templatedRegex: '', - } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryNoDepends' }, - { option: { text: 'A', value: 'A', selected: false } } - ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryNoDepends' })) - ); - }); - }); + await tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'query', id: 'queryNoDepends' })), + updateVariableOptions( + toVariablePayload( + { type: 'query', id: 'queryNoDepends' }, + { + results: [ + { value: 'A', text: 'A' }, + { value: 'B', text: 'B' }, + { value: 'C', text: 'C' }, + ], + templatedRegex: '', + } + ) + ), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryNoDepends' }, + { option: { text: 'A', value: 'A', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryNoDepends' })) + ); }); }); @@ -206,65 +207,64 @@ describe('processVariable', () => { queryNoDepends.refresh = refresh; const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); await tester.thenDispatchedActionsShouldEqual( + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryNoDepends' })), setCurrentVariableValue( toVariablePayload( { type: 'query', id: 'queryNoDepends' }, { option: { text: 'B', value: 'B', selected: false } } ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryNoDepends' })) + ) ); }); }); - [VariableRefresh.onDashboardLoad, VariableRefresh.onTimeRangeChanged].forEach(refresh => { - describe(`and refresh is ${ - refresh === VariableRefresh.onDashboardLoad - ? 'VariableRefresh.onDashboardLoad' - : 'VariableRefresh.onTimeRangeChanged' - }`, () => { - it('then correct actions are dispatched', async () => { - const { list, queryNoDepends } = getAndSetupProcessVariableContext(); - queryNoDepends.refresh = refresh; - const tester = await reduxTester<{ templating: TemplatingState }>() - .givenRootReducer(getTemplatingRootReducer()) - .whenActionIsDispatched(initDashboardTemplating(list)) - .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); + it.each` + refresh + ${VariableRefresh.onDashboardLoad} + ${VariableRefresh.onTimeRangeChanged} + `('and refresh is $refresh then correct actions are dispatched', async ({ refresh }) => { + const { list, queryNoDepends } = getAndSetupProcessVariableContext(); + queryNoDepends.refresh = refresh; + const tester = await reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) + .whenActionIsDispatched(initDashboardTemplating(list)) + .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(queryNoDepends), queryParams), true); - await tester.thenDispatchedActionsShouldEqual( - updateVariableOptions( - toVariablePayload( - { type: 'query', id: 'queryNoDepends' }, - { - results: [ - { value: 'A', text: 'A' }, - { value: 'B', text: 'B' }, - { value: 'C', text: 'C' }, - ], - templatedRegex: '', - } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryNoDepends' }, - { option: { text: 'A', value: 'A', selected: false } } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryNoDepends' }, - { option: { text: 'B', value: 'B', selected: false } } - ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryNoDepends' })) - ); - }); - }); + await tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'query', id: 'queryNoDepends' })), + updateVariableOptions( + toVariablePayload( + { type: 'query', id: 'queryNoDepends' }, + { + results: [ + { value: 'A', text: 'A' }, + { value: 'B', text: 'B' }, + { value: 'C', text: 'C' }, + ], + templatedRegex: '', + } + ) + ), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryNoDepends' }, + { option: { text: 'A', value: 'A', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryNoDepends' })), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryNoDepends' }, + { option: { text: 'B', value: 'B', selected: false } } + ) + ) + ); }); }); }); @@ -281,6 +281,7 @@ describe('processVariable', () => { queryDependsOnCustom.refresh = refresh; const customProcessed = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain @@ -290,50 +291,52 @@ describe('processVariable', () => { ); await tester.thenDispatchedActionsShouldEqual( - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) ); }); }); - [VariableRefresh.onDashboardLoad, VariableRefresh.onTimeRangeChanged].forEach(refresh => { - describe(`and refresh is ${refresh}`, () => { - it('then correct actions are dispatched', async () => { - const { list, custom, queryDependsOnCustom } = getAndSetupProcessVariableContext(); - queryDependsOnCustom.refresh = refresh; - const customProcessed = await reduxTester<{ templating: TemplatingState }>() - .givenRootReducer(getTemplatingRootReducer()) - .whenActionIsDispatched(initDashboardTemplating(list)) - .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain + it.each` + refresh + ${VariableRefresh.onDashboardLoad} + ${VariableRefresh.onTimeRangeChanged} + `('and refresh is $refresh then correct actions are dispatched', async ({ refresh }) => { + const { list, custom, queryDependsOnCustom } = getAndSetupProcessVariableContext(); + queryDependsOnCustom.refresh = refresh; + const customProcessed = await reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) + .whenActionIsDispatched(initDashboardTemplating(list)) + .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain - const tester = await customProcessed.whenAsyncActionIsDispatched( - processVariable(toVariableIdentifier(queryDependsOnCustom), queryParams), - true - ); + const tester = await customProcessed.whenAsyncActionIsDispatched( + processVariable(toVariableIdentifier(queryDependsOnCustom), queryParams), + true + ); - await tester.thenDispatchedActionsShouldEqual( - updateVariableOptions( - toVariablePayload( - { type: 'query', id: 'queryDependsOnCustom' }, - { - results: [ - { value: 'AA', text: 'AA' }, - { value: 'AB', text: 'AB' }, - { value: 'AC', text: 'AC' }, - ], - templatedRegex: '', - } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryDependsOnCustom' }, - { option: { text: 'AA', value: 'AA', selected: false } } - ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) - ); - }); - }); + await tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })), + updateVariableOptions( + toVariablePayload( + { type: 'query', id: 'queryDependsOnCustom' }, + { + results: [ + { value: 'AA', text: 'AA' }, + { value: 'AB', text: 'AB' }, + { value: 'AC', text: 'AC' }, + ], + templatedRegex: '', + } + ) + ), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryDependsOnCustom' }, + { option: { text: 'AA', value: 'AA', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) + ); }); }); @@ -347,6 +350,7 @@ describe('processVariable', () => { queryDependsOnCustom.refresh = refresh; const customProcessed = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) .whenActionIsDispatched(initDashboardTemplating(list)) .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain @@ -356,66 +360,64 @@ describe('processVariable', () => { ); await tester.thenDispatchedActionsShouldEqual( + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })), setCurrentVariableValue( toVariablePayload( { type: 'query', id: 'queryDependsOnCustom' }, { option: { text: 'AB', value: 'AB', selected: false } } ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) + ) ); }); }); - [VariableRefresh.onDashboardLoad, VariableRefresh.onTimeRangeChanged].forEach(refresh => { - describe(`and refresh is ${ - refresh === VariableRefresh.onDashboardLoad - ? 'VariableRefresh.onDashboardLoad' - : 'VariableRefresh.onTimeRangeChanged' - }`, () => { - it('then correct actions are dispatched', async () => { - const { list, custom, queryDependsOnCustom } = getAndSetupProcessVariableContext(); - queryDependsOnCustom.refresh = refresh; - const customProcessed = await reduxTester<{ templating: TemplatingState }>() - .givenRootReducer(getTemplatingRootReducer()) - .whenActionIsDispatched(initDashboardTemplating(list)) - .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain + it.each` + refresh + ${VariableRefresh.onDashboardLoad} + ${VariableRefresh.onTimeRangeChanged} + `('and refresh is $refresh then correct actions are dispatched', async ({ refresh }) => { + const { list, custom, queryDependsOnCustom } = getAndSetupProcessVariableContext(); + queryDependsOnCustom.refresh = refresh; + const customProcessed = await reduxTester<{ templating: TemplatingState }>() + .givenRootReducer(getTemplatingRootReducer()) + .whenActionIsDispatched(variablesInitTransaction({ uid: '' })) + .whenActionIsDispatched(initDashboardTemplating(list)) + .whenAsyncActionIsDispatched(processVariable(toVariableIdentifier(custom), queryParams)); // Need to process this dependency otherwise we never complete the promise chain - const tester = await customProcessed.whenAsyncActionIsDispatched( - processVariable(toVariableIdentifier(queryDependsOnCustom), queryParams), - true - ); + const tester = await customProcessed.whenAsyncActionIsDispatched( + processVariable(toVariableIdentifier(queryDependsOnCustom), queryParams), + true + ); - await tester.thenDispatchedActionsShouldEqual( - updateVariableOptions( - toVariablePayload( - { type: 'query', id: 'queryDependsOnCustom' }, - { - results: [ - { value: 'AA', text: 'AA' }, - { value: 'AB', text: 'AB' }, - { value: 'AC', text: 'AC' }, - ], - templatedRegex: '', - } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryDependsOnCustom' }, - { option: { text: 'AA', value: 'AA', selected: false } } - ) - ), - setCurrentVariableValue( - toVariablePayload( - { type: 'query', id: 'queryDependsOnCustom' }, - { option: { text: 'AB', value: 'AB', selected: false } } - ) - ), - resolveInitLock(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })) - ); - }); - }); + await tester.thenDispatchedActionsShouldEqual( + variableStateFetching(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })), + updateVariableOptions( + toVariablePayload( + { type: 'query', id: 'queryDependsOnCustom' }, + { + results: [ + { value: 'AA', text: 'AA' }, + { value: 'AB', text: 'AB' }, + { value: 'AC', text: 'AC' }, + ], + templatedRegex: '', + } + ) + ), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryDependsOnCustom' }, + { option: { text: 'AA', value: 'AA', selected: false } } + ) + ), + variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })), + setCurrentVariableValue( + toVariablePayload( + { type: 'query', id: 'queryDependsOnCustom' }, + { option: { text: 'AB', value: 'AB', selected: false } } + ) + ) + ); }); }); }); diff --git a/public/app/features/variables/state/reducers.test.ts b/public/app/features/variables/state/reducers.test.ts index 2b10e687c23..dc79d7f740e 100644 --- a/public/app/features/variables/state/reducers.test.ts +++ b/public/app/features/variables/state/reducers.test.ts @@ -1,5 +1,5 @@ import { reducerTester } from '../../../../test/core/redux/reducerTester'; -import { QueryVariableModel, VariableHide } from '../types'; +import { initialVariableModelState, QueryVariableModel } from '../types'; import { VariableAdapter, variableAdapters } from '../adapters'; import { createAction } from '@reduxjs/toolkit'; import { cleanVariables, variablesReducer, VariablesState } from './variablesReducer'; @@ -29,43 +29,37 @@ describe('variablesReducer', () => { it('then all variables except global variables should be removed', () => { const initialState: VariablesState = { '0': { + ...initialVariableModelState, id: '0', + index: 0, type: 'query', name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, label: 'Label-0', - skipUrlSync: false, - global: false, }, '1': { + ...initialVariableModelState, id: '1', + index: 1, type: 'query', name: 'Name-1', - hide: VariableHide.dontHide, - index: 1, label: 'Label-1', - skipUrlSync: false, global: true, }, '2': { + ...initialVariableModelState, id: '2', + index: 2, type: 'query', name: 'Name-2', - hide: VariableHide.dontHide, - index: 2, label: 'Label-2', - skipUrlSync: false, - global: false, }, '3': { + ...initialVariableModelState, id: '3', + index: 3, type: 'query', name: 'Name-3', - hide: VariableHide.dontHide, - index: 3, label: 'Label-3', - skipUrlSync: false, global: true, }, }; @@ -75,23 +69,21 @@ describe('variablesReducer', () => { .whenActionIsDispatched(cleanVariables()) .thenStateShouldEqual({ '1': { + ...initialVariableModelState, id: '1', + index: 1, type: 'query', name: 'Name-1', - hide: VariableHide.dontHide, - index: 1, label: 'Label-1', - skipUrlSync: false, global: true, }, '3': { + ...initialVariableModelState, id: '3', + index: 3, type: 'query', name: 'Name-3', - hide: VariableHide.dontHide, - index: 3, label: 'Label-3', - skipUrlSync: false, global: true, }, }); @@ -102,14 +94,12 @@ describe('variablesReducer', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { '0': { + ...initialVariableModelState, id: '0', + index: 0, type: 'query', name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, label: 'Label-0', - skipUrlSync: false, - global: false, }, }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); @@ -130,14 +120,12 @@ describe('variablesReducer', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { '0': { + ...initialVariableModelState, id: '0', + index: 0, type: 'query', name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, label: 'Label-0', - skipUrlSync: false, - global: false, }, }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); @@ -154,14 +142,12 @@ describe('variablesReducer', () => { it('then the reducer for that variableAdapter should be invoked', () => { const initialState: VariablesState = { '0': { + ...initialVariableModelState, id: '0', + index: 0, type: 'query', name: 'Name-0', - hide: VariableHide.dontHide, - index: 0, label: 'Label-0', - skipUrlSync: false, - global: false, }, }; variableAdapters.get('mock').reducer = jest.fn().mockReturnValue(initialState); diff --git a/public/app/features/variables/state/sharedReducer.test.ts b/public/app/features/variables/state/sharedReducer.test.ts index 26e977356b8..8ef98e08ea7 100644 --- a/public/app/features/variables/state/sharedReducer.test.ts +++ b/public/app/features/variables/state/sharedReducer.test.ts @@ -1,26 +1,27 @@ import cloneDeep from 'lodash/cloneDeep'; import { default as lodashDefaults } from 'lodash/defaults'; +import { LoadingState } from '@grafana/data'; import { reducerTester } from '../../../../test/core/redux/reducerTester'; import { - addInitLock, addVariable, changeVariableOrder, changeVariableProp, duplicateVariable, - removeInitLock, removeVariable, - resolveInitLock, setCurrentVariableValue, sharedReducer, storeNewVariable, + variableStateCompleted, + variableStateFailed, + variableStateFetching, + variableStateNotStarted, } from './sharedReducer'; import { QueryVariableModel, VariableHide } from '../types'; import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, NEW_VARIABLE_ID, toVariablePayload } from './types'; import { variableAdapters } from '../adapters'; import { createQueryVariableAdapter } from '../query/adapter'; import { initialQueryVariableModelState } from '../query/reducer'; -import { Deferred } from '../../../core/utils/deferred'; import { getVariableState, getVariableTestContext } from './helpers'; import { initialVariablesState, VariablesState } from './variablesReducer'; import { changeVariableNameSucceeded } from '../editor/reducer'; @@ -69,6 +70,8 @@ describe('sharedReducer', () => { label: 'Label-0', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '2': { id: '2', @@ -79,6 +82,8 @@ describe('sharedReducer', () => { label: 'Label-2', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, }); }); @@ -101,6 +106,8 @@ describe('sharedReducer', () => { label: 'Label-0', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '2': { id: '2', @@ -111,6 +118,8 @@ describe('sharedReducer', () => { label: 'Label-2', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, }); }); @@ -133,6 +142,8 @@ describe('sharedReducer', () => { label: 'Label-0', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '1': { id: '1', @@ -143,6 +154,8 @@ describe('sharedReducer', () => { label: 'Label-1', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '2': { id: '2', @@ -153,6 +166,8 @@ describe('sharedReducer', () => { label: 'Label-2', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '11': { ...initialQueryVariableModelState, @@ -182,6 +197,8 @@ describe('sharedReducer', () => { label: 'Label-0', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '1': { id: '1', @@ -192,6 +209,8 @@ describe('sharedReducer', () => { label: 'Label-1', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '2': { id: '2', @@ -202,6 +221,8 @@ describe('sharedReducer', () => { label: 'Label-2', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, }); }); @@ -224,6 +245,8 @@ describe('sharedReducer', () => { label: 'Label-0', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '1': { id: '1', @@ -234,6 +257,8 @@ describe('sharedReducer', () => { label: 'Label-1', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, '2': { id: '2', @@ -244,6 +269,8 @@ describe('sharedReducer', () => { label: 'Label-2', skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, [NEW_VARIABLE_ID]: { id: NEW_VARIABLE_ID, @@ -254,6 +281,8 @@ describe('sharedReducer', () => { label: `Label-${NEW_VARIABLE_ID}`, skipUrlSync: false, global: false, + state: LoadingState.NotStarted, + error: null, }, [11]: { ...initialQueryVariableModelState, @@ -356,79 +385,87 @@ describe('sharedReducer', () => { }); }); - describe('when addInitLock is dispatched', () => { + describe('when variableStateNotStarted is dispatched', () => { it('then state should be correct', () => { const adapter = createQueryVariableAdapter(); - const { initialState } = getVariableTestContext(adapter, {}); + const { initialState } = getVariableTestContext(adapter, { + state: LoadingState.Done, + error: 'Some error', + }); const payload = toVariablePayload({ id: '0', type: 'query' }); reducerTester() .givenReducer(sharedReducer, cloneDeep(initialState)) - .whenActionIsDispatched(addInitLock(payload)) - .thenStatePredicateShouldEqual(resultingState => { - // we need to remove initLock because instances will no be reference equal - const { initLock, ...resultingRest } = resultingState[0]; - const expectedState = cloneDeep(initialState); - delete expectedState[0].initLock; - expect(resultingRest).toEqual(expectedState[0]); - // make sure that initLock is defined - expect(resultingState[0].initLock!).toBeDefined(); - expect(resultingState[0].initLock!.promise).toBeDefined(); - expect(resultingState[0].initLock!.resolve).toBeDefined(); - expect(resultingState[0].initLock!.reject).toBeDefined(); - return true; - }); - }); - }); - - describe('when resolveInitLock is dispatched', () => { - it('then state should be correct', () => { - const initLock = ({ - resolve: jest.fn(), - reject: jest.fn(), - promise: jest.fn(), - } as unknown) as Deferred; - const adapter = createQueryVariableAdapter(); - const { initialState } = getVariableTestContext(adapter, { initLock }); - const payload = toVariablePayload({ id: '0', type: 'query' }); - reducerTester() - .givenReducer(sharedReducer, cloneDeep(initialState)) - .whenActionIsDispatched(resolveInitLock(payload)) - .thenStatePredicateShouldEqual(resultingState => { - // we need to remove initLock because instances will no be reference equal - const { initLock, ...resultingRest } = resultingState[0]; - const expectedState = cloneDeep(initialState); - delete expectedState[0].initLock; - expect(resultingRest).toEqual(expectedState[0]); - // make sure that initLock is defined - expect(resultingState[0].initLock!).toBeDefined(); - expect(resultingState[0].initLock!.promise).toBeDefined(); - expect(resultingState[0].initLock!.resolve).toBeDefined(); - expect(resultingState[0].initLock!.resolve).toHaveBeenCalledTimes(1); - expect(resultingState[0].initLock!.reject).toBeDefined(); - return true; - }); - }); - }); - - describe('when removeInitLock is dispatched', () => { - it('then state should be correct', () => { - const initLock = ({ - resolve: jest.fn(), - reject: jest.fn(), - promise: jest.fn(), - } as unknown) as Deferred; - const adapter = createQueryVariableAdapter(); - const { initialState } = getVariableTestContext(adapter, { initLock }); - const payload = toVariablePayload({ id: '0', type: 'query' }); - reducerTester() - .givenReducer(sharedReducer, cloneDeep(initialState)) - .whenActionIsDispatched(removeInitLock(payload)) + .whenActionIsDispatched(variableStateNotStarted(payload)) .thenStateShouldEqual({ ...initialState, - '0': { + '0': ({ ...initialState[0], - initLock: null, - }, + state: LoadingState.NotStarted, + error: null, + } as unknown) as QueryVariableModel, + }); + }); + }); + + describe('when variableStateFetching is dispatched', () => { + it('then state should be correct', () => { + const adapter = createQueryVariableAdapter(); + const { initialState } = getVariableTestContext(adapter, { + state: LoadingState.Done, + error: 'Some error', + }); + const payload = toVariablePayload({ id: '0', type: 'query' }); + reducerTester() + .givenReducer(sharedReducer, cloneDeep(initialState)) + .whenActionIsDispatched(variableStateFetching(payload)) + .thenStateShouldEqual({ + ...initialState, + '0': ({ + ...initialState[0], + state: LoadingState.Loading, + error: null, + } as unknown) as QueryVariableModel, + }); + }); + }); + + describe('when variableStateCompleted is dispatched', () => { + it('then state should be correct', () => { + const adapter = createQueryVariableAdapter(); + const { initialState } = getVariableTestContext(adapter, { + state: LoadingState.Loading, + error: 'Some error', + }); + const payload = toVariablePayload({ id: '0', type: 'query' }); + reducerTester() + .givenReducer(sharedReducer, cloneDeep(initialState)) + .whenActionIsDispatched(variableStateCompleted(payload)) + .thenStateShouldEqual({ + ...initialState, + '0': ({ + ...initialState[0], + state: LoadingState.Done, + error: null, + } as unknown) as QueryVariableModel, + }); + }); + }); + + describe('when variableStateFailed is dispatched', () => { + it('then state should be correct', () => { + const adapter = createQueryVariableAdapter(); + const { initialState } = getVariableTestContext(adapter, { state: LoadingState.Loading }); + const payload = toVariablePayload({ id: '0', type: 'query' }, { error: 'Some error' }); + reducerTester() + .givenReducer(sharedReducer, cloneDeep(initialState)) + .whenActionIsDispatched(variableStateFailed(payload)) + .thenStateShouldEqual({ + ...initialState, + '0': ({ + ...initialState[0], + state: LoadingState.Error, + error: 'Some error', + } as unknown) as QueryVariableModel, }); }); }); diff --git a/public/app/features/variables/state/sharedReducer.ts b/public/app/features/variables/state/sharedReducer.ts index 0ac0e7c88b3..d5b8bada462 100644 --- a/public/app/features/variables/state/sharedReducer.ts +++ b/public/app/features/variables/state/sharedReducer.ts @@ -2,12 +2,11 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import cloneDeep from 'lodash/cloneDeep'; import { default as lodashDefaults } from 'lodash/defaults'; -import { VariableType } from '@grafana/data'; +import { LoadingState, VariableType } from '@grafana/data'; import { VariableModel, VariableOption, VariableWithOptions } from '../types'; import { AddVariable, getInstanceState, NEW_VARIABLE_ID, VariablePayload } from './types'; import { variableAdapters } from '../adapters'; import { changeVariableNameSucceeded } from '../editor/reducer'; -import { Deferred } from '../../../core/utils/deferred'; import { initialVariablesState, VariablesState } from './variablesReducer'; import { isQuery } from '../guard'; @@ -29,27 +28,33 @@ const sharedReducerSlice = createSlice({ state[id] = variable; }, - addInitLock: (state: VariablesState, action: PayloadAction) => { + variableStateNotStarted: (state: VariablesState, action: PayloadAction) => { const instanceState = getInstanceState(state, action.payload.id); - instanceState.initLock = new Deferred(); + instanceState.state = LoadingState.NotStarted; + instanceState.error = null; }, - resolveInitLock: (state: VariablesState, action: PayloadAction) => { + variableStateFetching: (state: VariablesState, action: PayloadAction) => { + const instanceState = getInstanceState(state, action.payload.id); + instanceState.state = LoadingState.Loading; + instanceState.error = null; + }, + variableStateCompleted: (state: VariablesState, action: PayloadAction) => { const instanceState = getInstanceState(state, action.payload.id); - if (!instanceState) { // we might have cancelled a batch so then this state has been removed return; } - instanceState.initLock?.resolve(); + instanceState.state = LoadingState.Done; + instanceState.error = null; }, - removeInitLock: (state: VariablesState, action: PayloadAction) => { + variableStateFailed: (state: VariablesState, action: PayloadAction>) => { const instanceState = getInstanceState(state, action.payload.id); - if (!instanceState) { // we might have cancelled a batch so then this state has been removed return; } - instanceState.initLock = null; + instanceState.state = LoadingState.Error; + instanceState.error = action.payload.data.error; }, removeVariable: (state: VariablesState, action: PayloadAction>) => { delete state[action.payload.id]; @@ -173,7 +178,6 @@ const sharedReducerSlice = createSlice({ export const sharedReducer = sharedReducerSlice.reducer; export const { - addInitLock, removeVariable, addVariable, changeVariableProp, @@ -182,8 +186,10 @@ export const { duplicateVariable, setCurrentVariableValue, changeVariableType, - removeInitLock, - resolveInitLock, + variableStateNotStarted, + variableStateFetching, + variableStateCompleted, + variableStateFailed, } = sharedReducerSlice.actions; const hasTags = (option: VariableOption): boolean => { diff --git a/public/app/features/variables/system/adapter.ts b/public/app/features/variables/system/adapter.ts index bae30da4680..9ef8a6bab91 100644 --- a/public/app/features/variables/system/adapter.ts +++ b/public/app/features/variables/system/adapter.ts @@ -1,8 +1,8 @@ import { ComponentType } from 'react'; -import { SystemVariable, VariableHide } from '../types'; +import { LoadingState } from '@grafana/data'; + +import { initialVariableModelState, SystemVariable, VariableHide } from '../types'; import { VariableAdapter } from '../adapters'; -import { NEW_VARIABLE_ID } from '../state/types'; -import { Deferred } from '../../../core/utils/deferred'; import { VariablePickerProps } from '../pickers/types'; import { VariableEditorProps } from '../editor/types'; @@ -12,16 +12,12 @@ export const createSystemVariableAdapter = (): VariableAdapter '' } }, - index: -1, - initLock: (null as unknown) as Deferred, + state: LoadingState.Done, }, reducer: (state: any, action: any) => state, picker: (null as unknown) as ComponentType, diff --git a/public/app/features/variables/textbox/TextBoxVariablePicker.tsx b/public/app/features/variables/textbox/TextBoxVariablePicker.tsx index 515cccb7892..10632773665 100644 --- a/public/app/features/variables/textbox/TextBoxVariablePicker.tsx +++ b/public/app/features/variables/textbox/TextBoxVariablePicker.tsx @@ -1,11 +1,11 @@ import React, { ChangeEvent, FocusEvent, KeyboardEvent, PureComponent } from 'react'; import { TextBoxVariableModel } from '../types'; -import { toVariablePayload } from '../state/types'; +import { toVariableIdentifier, toVariablePayload } from '../state/types'; import { dispatch } from '../../../store/store'; -import { variableAdapters } from '../adapters'; import { changeVariableProp } from '../state/sharedReducer'; import { VariablePickerProps } from '../pickers/types'; +import { updateOptions } from '../state/actions'; export interface Props extends VariablePickerProps {} @@ -18,13 +18,13 @@ export class TextBoxVariablePicker extends PureComponent { onQueryBlur = (event: FocusEvent) => { if (this.props.variable.current.value !== this.props.variable.query) { - variableAdapters.get(this.props.variable.type).updateOptions(this.props.variable); + dispatch(updateOptions(toVariableIdentifier(this.props.variable))); } }; onQueryKeyDown = (event: KeyboardEvent) => { if (event.keyCode === 13 && this.props.variable.current.value !== this.props.variable.query) { - variableAdapters.get(this.props.variable.type).updateOptions(this.props.variable); + dispatch(updateOptions(toVariableIdentifier(this.props.variable))); } }; diff --git a/public/app/features/variables/textbox/adapter.ts b/public/app/features/variables/textbox/adapter.ts index a6e4b5bc3d6..801983d4c54 100644 --- a/public/app/features/variables/textbox/adapter.ts +++ b/public/app/features/variables/textbox/adapter.ts @@ -32,7 +32,7 @@ export const createTextBoxVariableAdapter = (): VariableAdapter { - const { index, id, initLock, global, ...rest } = cloneDeep(variable); + const { index, id, state, global, ...rest } = cloneDeep(variable); return rest; }, getValueForUrl: variable => { diff --git a/public/app/features/variables/textbox/reducer.ts b/public/app/features/variables/textbox/reducer.ts index b371010e05c..ca962e87b08 100644 --- a/public/app/features/variables/textbox/reducer.ts +++ b/public/app/features/variables/textbox/reducer.ts @@ -1,22 +1,15 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { TextBoxVariableModel, VariableHide, VariableOption } from '../types'; -import { getInstanceState, NEW_VARIABLE_ID, VariablePayload } from '../state/types'; +import { initialVariableModelState, TextBoxVariableModel, VariableOption } from '../types'; +import { getInstanceState, VariablePayload } from '../state/types'; import { initialVariablesState, VariablesState } from '../state/variablesReducer'; export const initialTextBoxVariableModelState: TextBoxVariableModel = { - id: NEW_VARIABLE_ID, - global: false, - index: -1, + ...initialVariableModelState, type: 'textbox', - name: '', - label: '', - hide: VariableHide.dontHide, query: '', current: {} as VariableOption, options: [], - skipUrlSync: false, - initLock: null, }; export const textBoxVariableSlice = createSlice({ diff --git a/public/app/features/variables/types.ts b/public/app/features/variables/types.ts index 6d1fc1b1cb5..357c283f167 100644 --- a/public/app/features/variables/types.ts +++ b/public/app/features/variables/types.ts @@ -1,5 +1,5 @@ -import { Deferred } from '../../core/utils/deferred'; -import { VariableModel as BaseVariableModel } from '@grafana/data'; +import { LoadingState, VariableModel as BaseVariableModel, VariableType } from '@grafana/data'; +import { NEW_VARIABLE_ID } from './state/types'; export enum VariableRefresh { never, @@ -125,5 +125,19 @@ export interface VariableModel extends BaseVariableModel { hide: VariableHide; skipUrlSync: boolean; index: number; - initLock?: Deferred | null; + state: LoadingState; + error: any | null; } + +export const initialVariableModelState: VariableModel = { + id: NEW_VARIABLE_ID, + name: '', + label: null, + type: ('' as unknown) as VariableType, + global: false, + index: -1, + hide: VariableHide.dontHide, + skipUrlSync: false, + state: LoadingState.NotStarted, + error: null, +}; diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.test.tsx index 855b1feb8e8..0d805d98354 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor.test.tsx @@ -6,7 +6,7 @@ import { DataSourceInstanceSettings } from '@grafana/data'; import { TemplateSrv } from 'app/features/templating/template_srv'; import { MetricsQueryEditor, normalizeQuery, Props } from './MetricsQueryEditor'; import { CloudWatchDatasource } from '../datasource'; -import { CustomVariableModel, VariableHide } from '../../../../features/variables/types'; +import { CustomVariableModel, initialVariableModelState } from '../../../../features/variables/types'; const setup = () => { const instanceSettings = { @@ -15,6 +15,7 @@ const setup = () => { const templateSrv = new TemplateSrv(); const variable: CustomVariableModel = { + ...initialVariableModelState, id: 'var3', index: 0, name: 'var3', @@ -27,11 +28,7 @@ const setup = () => { multi: true, includeAll: false, query: '', - hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; templateSrv.init([variable]); diff --git a/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts index ea65a7c5a40..94c1991daec 100644 --- a/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/specs/datasource.test.ts @@ -3,26 +3,26 @@ import { CloudWatchDatasource, MAX_ATTEMPTS } from '../datasource'; import * as redux from 'app/store/store'; import { DataFrame, + DataQueryErrorType, DataQueryResponse, DataSourceInstanceSettings, dateMath, getFrameDisplayName, - DataQueryErrorType, } from '@grafana/data'; import { TemplateSrv } from 'app/features/templating/template_srv'; import { + CloudWatchLogsQuery, CloudWatchLogsQueryStatus, CloudWatchMetricsQuery, CloudWatchQuery, LogAction, - CloudWatchLogsQuery, } from '../types'; import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__ import { TimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { convertToStoreState } from '../../../../../test/helpers/convertToStoreState'; import { getTemplateSrvDependencies } from 'test/helpers/getTemplateSrvDependencies'; -import { of, interval } from 'rxjs'; -import { CustomVariableModel, VariableHide } from '../../../../features/variables/types'; +import { interval, of } from 'rxjs'; +import { CustomVariableModel, initialVariableModelState, VariableHide } from '../../../../features/variables/types'; import { TimeSrvStub } from '../../../../../test/specs/helpers'; import * as rxjsUtils from '../utils/rxjs/increasingInterval'; @@ -376,6 +376,7 @@ describe('CloudWatchDatasource', () => { it('should generate the correct query with interval variable', async () => { const period: CustomVariableModel = { + ...initialVariableModelState, id: 'period', name: 'period', index: 0, @@ -386,9 +387,6 @@ describe('CloudWatchDatasource', () => { query: '', hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; templateSrv.init([period]); @@ -821,6 +819,7 @@ describe('CloudWatchDatasource', () => { let requestParams: { queries: CloudWatchMetricsQuery[] }; beforeEach(() => { const var1: CustomVariableModel = { + ...initialVariableModelState, id: 'var1', name: 'var1', index: 0, @@ -831,11 +830,9 @@ describe('CloudWatchDatasource', () => { query: '', hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; const var2: CustomVariableModel = { + ...initialVariableModelState, id: 'var2', name: 'var2', index: 1, @@ -846,11 +843,9 @@ describe('CloudWatchDatasource', () => { query: '', hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; const var3: CustomVariableModel = { + ...initialVariableModelState, id: 'var3', name: 'var3', index: 2, @@ -865,11 +860,9 @@ describe('CloudWatchDatasource', () => { query: '', hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; const var4: CustomVariableModel = { + ...initialVariableModelState, id: 'var4', name: 'var4', index: 3, @@ -884,9 +877,6 @@ describe('CloudWatchDatasource', () => { query: '', hide: VariableHide.dontHide, type: 'custom', - label: null, - skipUrlSync: false, - global: false, }; const variables = [var1, var2, var3, var4]; const state = convertToStoreState(variables); diff --git a/public/test/core/utils/silenceConsoleOutput.ts b/public/test/core/utils/silenceConsoleOutput.ts new file mode 100644 index 00000000000..3205c7db7aa --- /dev/null +++ b/public/test/core/utils/silenceConsoleOutput.ts @@ -0,0 +1,15 @@ +export const silenceConsoleOutput = () => { + beforeEach(() => { + jest.spyOn(console, 'log').mockImplementation(jest.fn()); + jest.spyOn(console, 'error').mockImplementation(jest.fn()); + jest.spyOn(console, 'debug').mockImplementation(jest.fn()); + jest.spyOn(console, 'info').mockImplementation(jest.fn()); + }); + + afterEach(() => { + jest.spyOn(console, 'log').mockRestore(); + jest.spyOn(console, 'error').mockRestore(); + jest.spyOn(console, 'debug').mockRestore(); + jest.spyOn(console, 'info').mockRestore(); + }); +};