Variables: Confirms selection before opening new picker (#32586)

* Variables: Confirms selection before opening new picker

* Tests: fixes e2e tests

e2e/suite1/specs/variables/load-options-from-url.ts

@@ -150,14 +150,7 @@ describe('Variables - Load options from Url', () => {
     e2e.pages.Dashboard.SubMenu.submenuItemValueDropDownDropDown()
       .should('be.visible')
       .within(() => {
-        e2e().get('.variable-option').should('have.length', 0);
-      });
-
-    e2e.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts('All').should('be.visible').click();
-    e2e.pages.Dashboard.SubMenu.submenuItemValueDropDownDropDown()
-      .should('be.visible')
-      .within(() => {
-        e2e().get('.variable-option').should('have.length', 0);
+        e2e().get('.variable-option').should('have.length', 10);
       });
   });
 });

public/app/features/variables/pickers/OptionsPicker/OptionPicker.test.tsx

@@ -0,0 +1,140 @@
+import React from 'react';
+import { Provider } from 'react-redux';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { selectors } from '@grafana/e2e-selectors';
+import { LoadingState } from '@grafana/data';
+
+import { VariablePickerProps } from '../types';
+import { QueryVariableModel } from '../../types';
+import { queryBuilder } from '../../shared/testing/builders';
+import { optionPickerFactory } from './OptionsPicker';
+import { initialState, OptionsPickerState } from './reducer';
+
+interface Args {
+  pickerState?: Partial<OptionsPickerState>;
+  variable?: Partial<QueryVariableModel>;
+}
+
+const defaultVariable = queryBuilder()
+  .withId('query0')
+  .withName('query0')
+  .withMulti()
+  .withCurrent(['A', 'C'])
+  .withOptions('A', 'B', 'C')
+  .build();
+
+function setupTestContext({ pickerState = {}, variable = {} }: Args = {}) {
+  const v = {
+    ...defaultVariable,
+    ...variable,
+  };
+  const onVariableChange = jest.fn();
+  const props: VariablePickerProps<QueryVariableModel> = {
+    variable: v,
+    onVariableChange,
+  };
+  const Picker = optionPickerFactory();
+  const optionsPicker: OptionsPickerState = { ...initialState, ...pickerState };
+  const dispatch = jest.fn();
+  const subscribe = jest.fn();
+  const getState = jest.fn().mockReturnValue({
+    templating: {
+      variables: {
+        [v.id]: { ...v },
+      },
+      optionsPicker,
+    },
+  });
+  const store: any = { getState, dispatch, subscribe };
+  const { rerender } = render(
+    <Provider store={store}>
+      <Picker {...props} />
+    </Provider>
+  );
+  return { onVariableChange, variable, rerender, dispatch };
+}
+
+function getSubMenu(text: string) {
+  return screen.getByLabelText(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownValueLinkTexts(text));
+}
+
+function getOption(text: string) {
+  return screen.getByLabelText(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownOptionTexts('A'));
+}
+
+describe('OptionPicker', () => {
+  describe('when mounted and picker id is not set', () => {
+    it('should render link with correct text', () => {
+      setupTestContext();
+      expect(getSubMenu('A + C')).toBeInTheDocument();
+    });
+
+    it('link text should be clickable', () => {
+      const { dispatch } = setupTestContext();
+
+      dispatch.mockClear();
+      userEvent.click(getSubMenu('A + C'));
+      expect(dispatch).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('when mounted and picker id differs from variable id', () => {
+    it('should render link with correct text', () => {
+      setupTestContext({
+        variable: defaultVariable,
+        pickerState: { id: 'Other' },
+      });
+      expect(getSubMenu('A + C')).toBeInTheDocument();
+    });
+
+    it('link text should be clickable', () => {
+      const { dispatch } = setupTestContext({
+        variable: defaultVariable,
+        pickerState: { id: 'Other' },
+      });
+
+      dispatch.mockClear();
+      userEvent.click(getSubMenu('A + C'));
+      expect(dispatch).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('when mounted and variable is loading', () => {
+    it('should render link with correct text and loading indicator should be visible', () => {
+      setupTestContext({
+        variable: { ...defaultVariable, state: LoadingState.Loading },
+      });
+      expect(getSubMenu('A + C')).toBeInTheDocument();
+      expect(screen.getByLabelText(selectors.components.LoadingIndicator.icon)).toBeInTheDocument();
+    });
+
+    it('link text should not be clickable', () => {
+      const { dispatch } = setupTestContext({
+        variable: { ...defaultVariable, state: LoadingState.Loading },
+      });
+
+      dispatch.mockClear();
+      userEvent.click(getSubMenu('A + C'));
+      expect(dispatch).toHaveBeenCalledTimes(0);
+    });
+  });
+
+  describe('when mounted and picker id equals the variable id', () => {
+    it('should render input, drop down list with correct options', () => {
+      setupTestContext({
+        variable: defaultVariable,
+        pickerState: { id: defaultVariable.id, options: defaultVariable.options, multi: defaultVariable.multi },
+      });
+
+      expect(screen.getByRole('textbox')).toBeInTheDocument();
+      expect(screen.getByRole('textbox')).toHaveValue('');
+      expect(
+        screen.getByLabelText(selectors.pages.Dashboard.SubMenu.submenuItemValueDropDownDropDown)
+      ).toBeInTheDocument();
+      expect(getOption('A')).toBeInTheDocument();
+      expect(getOption('B')).toBeInTheDocument();
+      expect(getOption('C')).toBeInTheDocument();
+    });
+  });
+});

public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx

@@ -5,8 +5,14 @@ import { LoadingState } from '@grafana/data';
 
 import { StoreState } from 'app/types';
 import { VariableInput } from '../shared/VariableInput';
-import { commitChangesToVariable, filterOrSearchOptions, navigateOptions, toggleAndFetchTag } from './actions';
-import { OptionsPickerState, showOptions, toggleAllOptions, toggleOption } from './reducer';
+import {
+  commitChangesToVariable,
+  filterOrSearchOptions,
+  navigateOptions,
+  openOptions,
+  toggleAndFetchTag,
+} from './actions';
+import { OptionsPickerState, toggleAllOptions, toggleOption } from './reducer';
 import { VariableOption, VariableTag, VariableWithMultiSupport, VariableWithOptions } from '../../types';
 import { VariableOptions } from '../shared/VariableOptions';
 import { isMulti, isQuery } from '../../guard';
@@ -20,7 +26,7 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
   VariablePickerProps<Model>
 > => {
   const mapDispatchToProps = {
-    showOptions,
+    openOptions,
     commitChangesToVariable,
     filterOrSearchOptions,
     toggleAllOptions,
@@ -40,7 +46,8 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
   type Props = OwnProps & ConnectedProps<typeof connector>;
 
   class OptionsPickerUnconnected extends PureComponent<Props> {
-    onShowOptions = () => this.props.showOptions(this.props.variable);
+    onShowOptions = () =>
+      this.props.openOptions(toVariableIdentifier(this.props.variable), this.props.onVariableChange);
     onHideOptions = () => this.props.commitChangesToVariable(this.props.onVariableChange);
 
     onToggleOption = (option: VariableOption, clearOthers: boolean) => {

public/app/features/variables/pickers/OptionsPicker/actions.test.ts

@@ -3,6 +3,7 @@ import { getRootReducer, RootReducerType } from '../../state/helpers';
 import { initialVariableModelState, QueryVariableModel, VariableRefresh, VariableSort } from '../../types';
 import {
   hideOptions,
+  initialState,
   moveOptionsHighlight,
   showOptions,
   toggleOption,
@@ -14,6 +15,7 @@ import {
   commitChangesToVariable,
   filterOrSearchOptions,
   navigateOptions,
+  openOptions,
   toggleAndFetchTag,
   toggleOptionByHighlight,
 } from './actions';
@@ -23,6 +25,7 @@ import { addVariable, changeVariableProp, setCurrentVariableValue } from '../../
 import { variableAdapters } from '../../adapters';
 import { createQueryVariableAdapter } from '../../query/adapter';
 import { locationService } from '@grafana/runtime';
+import { queryBuilder } from '../../shared/testing/builders';
 
 const datasource = {
   metricFindQuery: jest.fn(() => Promise.resolve([])),
@@ -218,6 +221,101 @@ describe('options picker actions', () => {
     });
   });
 
+  describe('when openOptions is dispatched and there is no picker state yet', () => {
+    it('then correct actions are dispatched', async () => {
+      const variable = queryBuilder()
+        .withId('query0')
+        .withName('query0')
+        .withMulti()
+        .withCurrent(['A', 'C'])
+        .withOptions('A', 'B', 'C')
+        .build();
+
+      const preloadedState: any = {
+        templating: {
+          variables: {
+            [variable.id]: { ...variable },
+          },
+          optionsPicker: { ...initialState },
+        },
+      };
+
+      const tester = await reduxTester<RootReducerType>({ preloadedState })
+        .givenRootReducer(getRootReducer())
+        .whenAsyncActionIsDispatched(openOptions(variable, undefined));
+
+      tester.thenDispatchedActionsShouldEqual(showOptions(variable));
+    });
+  });
+
+  describe('when openOptions is dispatched and picker.id is same as variable.id', () => {
+    it('then correct actions are dispatched', async () => {
+      const variable = queryBuilder()
+        .withId('query0')
+        .withName('query0')
+        .withMulti()
+        .withCurrent(['A', 'C'])
+        .withOptions('A', 'B', 'C')
+        .build();
+
+      const preloadedState: any = {
+        templating: {
+          variables: {
+            [variable.id]: { ...variable },
+          },
+          optionsPicker: { ...initialState, id: variable.id },
+        },
+      };
+
+      const tester = await reduxTester<RootReducerType>({ preloadedState })
+        .givenRootReducer(getRootReducer())
+        .whenAsyncActionIsDispatched(openOptions(variable, undefined));
+
+      tester.thenDispatchedActionsShouldEqual(showOptions(variable));
+    });
+  });
+
+  describe('when openOptions is dispatched and picker.id is not the same as variable.id', () => {
+    it('then correct actions are dispatched', async () => {
+      const variableInPickerState = queryBuilder()
+        .withId('query1')
+        .withName('query1')
+        .withMulti()
+        .withCurrent(['A', 'C'])
+        .withOptions('A', 'B', 'C')
+        .build();
+
+      const variable = queryBuilder()
+        .withId('query0')
+        .withName('query0')
+        .withMulti()
+        .withCurrent(['A'])
+        .withOptions('A', 'B', 'C')
+        .build();
+
+      const preloadedState: any = {
+        templating: {
+          variables: {
+            [variable.id]: { ...variable },
+            [variableInPickerState.id]: { ...variableInPickerState },
+          },
+          optionsPicker: { ...initialState, id: variableInPickerState.id },
+        },
+      };
+
+      const tester = await reduxTester<RootReducerType>({ preloadedState })
+        .givenRootReducer(getRootReducer())
+        .whenAsyncActionIsDispatched(openOptions(variable, undefined));
+
+      tester.thenDispatchedActionsShouldEqual(
+        setCurrentVariableValue({ type: 'query', id: 'query1', data: { option: undefined } }),
+        changeVariableProp({ type: 'query', id: 'query1', data: { propName: 'queryValue', propValue: '' } }),
+        hideOptions(),
+        showOptions(variable)
+      );
+    });
+  });
+
   describe('when commitChangesToVariable is dispatched with no changes', () => {
     it('then correct actions are dispatched', async () => {
       const options = [createOption('A', 'A', true), createOption('B'), createOption('C')];

public/app/features/variables/pickers/OptionsPicker/actions.ts

@@ -16,6 +16,7 @@ import {
   hideOptions,
   moveOptionsHighlight,
   OptionsPickerState,
+  showOptions,
   toggleOption,
   toggleTag,
   updateOptionsAndFilter,
@@ -25,7 +26,7 @@ import {
 import { getDataSourceSrv } from '@grafana/runtime';
 import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv';
 import { changeVariableProp, setCurrentVariableValue } from '../../state/sharedReducer';
-import { toVariablePayload } from '../../state/types';
+import { toVariablePayload, VariableIdentifier } from '../../state/types';
 import { containsSearchFilter, getCurrentText } from '../../utils';
 
 export const navigateOptions = (key: NavigationKey, clearOthers: boolean): ThunkResult<void> => {
@@ -98,10 +99,24 @@ export const commitChangesToVariable = (callback?: (updated: any) => void): Thun
       return callback(updated);
     }
 
-    return setVariable(updated);
+    return await setVariable(updated);
   };
 };
 
+export const openOptions = ({ id }: VariableIdentifier, callback?: (updated: any) => void): ThunkResult<void> => async (
+  dispatch,
+  getState
+) => {
+  const picker = getState().templating.optionsPicker;
+
+  if (picker.id && picker.id !== id) {
+    await dispatch(commitChangesToVariable(callback));
+  }
+
+  const variable = getVariable<VariableWithMultiSupport>(id, getState());
+  dispatch(showOptions(variable));
+};
+
 export const toggleOptionByHighlight = (clearOthers: boolean, forceSelect = false): ThunkResult<void> => {
   return (dispatch, getState) => {
     const { highlightIndex, options } = getState().templating.optionsPicker;