Variables: Fixes Textbox current value persistence (#29481)

* Variables: Fixes savequery for Constant and TextBox variables

* Refactor: reverts textbox changes

* Refactor: Fixes dashboard export and tests

* Refactor: hides or migrates Constant variables

* Tests: fixes snapshots

* Variables: Fixes Textbox current value persistance

* Refactor: fixes PR comments and adds e2e tests

public/app/features/dashboard/components/DashboardSettings/template.html

@@ -10,10 +10,10 @@
 
   <div class="dashboard-settings__aside-actions">
     <div ng-show="ctrl.canSave">
-      <save-dashboard-button getDashboard="ctrl.getDashboard" aria-label="Dashboard settings aside actions Save button" />
+      <save-dashboard-button getDashboard="ctrl.getDashboard" />
     </div>
     <div ng-show="ctrl.canSaveAs">
-      <save-dashboard-as-button getDashboard="ctrl.getDashboard" aria-label="Dashboard settings aside actions Save as button" variant="'secondary'" />
+      <save-dashboard-as-button getDashboard="ctrl.getDashboard" variant="'secondary'" />
     </div>
   </div>
 </aside>

public/app/features/dashboard/components/SaveDashboard/SaveDashboardButton.tsx

@@ -5,6 +5,7 @@ import { connectWithProvider } from 'app/core/utils/connectWithReduxStore';
 import { provideModalsContext } from 'app/routes/ReactContainer';
 import { SaveDashboardAsModal } from './SaveDashboardAsModal';
 import { SaveDashboardModalProxy } from './SaveDashboardModalProxy';
+import { selectors } from '@grafana/e2e-selectors';
 
 interface SaveDashboardButtonProps {
   dashboard: DashboardModel;
@@ -30,6 +31,7 @@ export const SaveDashboardButton: React.FC<SaveDashboardButtonProps> = ({ dashbo
                 onDismiss: hideModal,
               });
             }}
+            aria-label={selectors.pages.Dashboard.Settings.General.saveDashBoard}
           >
             Save dashboard
           </Button>
@@ -63,6 +65,7 @@ export const SaveDashboardAsButton: React.FC<SaveDashboardButtonProps & { varian
               // In Dashboard Settings in sidebar we need to use new form but with inverse variant to make it look like it should
               // Everywhere else we use old button component :(
               variant={variant as ButtonVariant}
+              aria-label={selectors.pages.Dashboard.Settings.General.saveAsDashBoard}
             >
               Save As...
             </Button>

public/app/features/dashboard/state/DashboardMigrator.test.ts

@@ -857,7 +857,7 @@ describe('DashboardModel', () => {
       });
     });
 
-    it('should have three variables after migration', () => {
+    it('should have six variables after migration', () => {
       expect(model.templating.list.length).toBe(6);
     });
 

public/app/features/dashboard/state/DashboardModel.ts

@@ -14,12 +14,12 @@ import {
   dateTimeFormat,
   dateTimeFormatTimeAgo,
   DateTimeInput,
+  EventBusExtended,
+  EventBusSrv,
   PanelEvents,
   TimeRange,
   TimeZone,
   UrlQueryValue,
-  EventBusSrv,
-  EventBusExtended,
 } from '@grafana/data';
 import { CoreEvents, DashboardMeta, KIOSK_MODE_TV } from 'app/types';
 import { GetVariables, getVariables } from 'app/features/variables/state/selectors';
@@ -236,7 +236,9 @@ export class DashboardModel {
     const currentVariables = this.getVariablesFromState();
 
     copy.templating = {
-      list: currentVariables.map(variable => variableAdapters.get(variable.type).getSaveModel(variable)),
+      list: currentVariables.map(variable =>
+        variableAdapters.get(variable.type).getSaveModel(variable, defaults.saveVariables)
+      ),
     };
 
     if (!defaults.saveVariables) {

public/app/features/variables/adapters.ts

@@ -34,7 +34,7 @@ export interface VariableAdapter<Model extends VariableModel> {
   setValue: (variable: Model, option: VariableOption, emitChanges?: boolean) => Promise<void>;
   setValueFromUrl: (variable: Model, urlValue: UrlQueryValue) => Promise<void>;
   updateOptions: (variable: Model, searchFilter?: string) => Promise<void>;
-  getSaveModel: (variable: Model) => Partial<Model>;
+  getSaveModel: (variable: Model, saveCurrentAsDefault?: boolean) => Partial<Model>;
   getValueForUrl: (variable: Model) => string | string[];
   picker: ComponentType<VariablePickerProps>;
   editor: ComponentType<VariableEditorProps>;

public/app/features/variables/shared/testing/builders.ts

@@ -11,11 +11,12 @@ import { initialCustomVariableModelState } from '../../custom/reducer';
 import { MultiVariableBuilder } from './multiVariableBuilder';
 import { initialConstantVariableModelState } from '../../constant/reducer';
 import { QueryVariableBuilder } from './queryVariableBuilder';
+import { TextBoxVariableBuilder } from './textboxVariableBuilder';
 
 export const adHocBuilder = () => new AdHocVariableBuilder(initialAdHocVariableModelState);
 export const intervalBuilder = () => new IntervalVariableBuilder(initialIntervalVariableModelState);
 export const datasourceBuilder = () => new DatasourceVariableBuilder(initialDataSourceVariableModelState);
 export const queryBuilder = () => new QueryVariableBuilder(initialQueryVariableModelState);
-export const textboxBuilder = () => new OptionsVariableBuilder(initialTextBoxVariableModelState);
+export const textboxBuilder = () => new TextBoxVariableBuilder(initialTextBoxVariableModelState);
 export const customBuilder = () => new MultiVariableBuilder(initialCustomVariableModelState);
 export const constantBuilder = () => new OptionsVariableBuilder(initialConstantVariableModelState);

public/app/features/variables/shared/testing/textboxVariableBuilder.ts

@@ -0,0 +1,9 @@
+import { TextBoxVariableModel } from 'app/features/variables/types';
+import { OptionsVariableBuilder } from './optionsVariableBuilder';
+
+export class TextBoxVariableBuilder<T extends TextBoxVariableModel> extends OptionsVariableBuilder<T> {
+  withOriginalQuery(original: string) {
+    this.variable.originalQuery = original;
+    return this;
+  }
+}

public/app/features/variables/textbox/TextBoxVariableEditor.tsx

@@ -1,36 +1,40 @@
-import React, { ChangeEvent, PureComponent } from 'react';
+import React, { ChangeEvent, ReactElement, useCallback } from 'react';
 import { VerticalGroup } from '@grafana/ui';
 
 import { TextBoxVariableModel } from '../types';
 import { VariableEditorProps } from '../editor/types';
 import { VariableSectionHeader } from '../editor/VariableSectionHeader';
 import { VariableTextField } from '../editor/VariableTextField';
+import { selectors } from '@grafana/e2e-selectors';
 
 export interface Props extends VariableEditorProps<TextBoxVariableModel> {}
-export class TextBoxVariableEditor extends PureComponent<Props> {
-  onQueryChange = (event: ChangeEvent<HTMLInputElement>) => {
-    event.preventDefault();
-    this.props.onPropChange({ propName: 'query', propValue: event.target.value, updateOptions: false });
-  };
-  onQueryBlur = (event: ChangeEvent<HTMLInputElement>) => {
-    event.preventDefault();
-    this.props.onPropChange({ propName: 'query', propValue: event.target.value, updateOptions: true });
-  };
-  render() {
-    const { query } = this.props.variable;
-    return (
-      <VerticalGroup spacing="xs">
-        <VariableSectionHeader name="Text Options" />
-        <VariableTextField
-          value={query}
-          name="Default value"
-          placeholder="default value, if any"
-          onChange={this.onQueryChange}
-          onBlur={this.onQueryBlur}
-          labelWidth={20}
-          grow
-        />
-      </VerticalGroup>
-    );
-  }
+
+export function TextBoxVariableEditor({ onPropChange, variable: { query } }: Props): ReactElement {
+  const updateVariable = useCallback(
+    (event: ChangeEvent<HTMLInputElement>, updateOptions: boolean) => {
+      event.preventDefault();
+      onPropChange({ propName: 'originalQuery', propValue: event.target.value, updateOptions: false });
+      onPropChange({ propName: 'query', propValue: event.target.value, updateOptions });
+    },
+    [onPropChange]
+  );
+
+  const onChange = useCallback((e: ChangeEvent<HTMLInputElement>) => updateVariable(e, false), [updateVariable]);
+  const onBlur = useCallback((e: ChangeEvent<HTMLInputElement>) => updateVariable(e, true), [updateVariable]);
+
+  return (
+    <VerticalGroup spacing="xs">
+      <VariableSectionHeader name="Text Options" />
+      <VariableTextField
+        value={query}
+        name="Default value"
+        placeholder="default value, if any"
+        onChange={onChange}
+        onBlur={onBlur}
+        labelWidth={20}
+        grow
+        ariaLabel={selectors.pages.Dashboard.Settings.Variables.Edit.TextBoxVariable.textBoxOptionsQueryInput}
+      />
+    </VerticalGroup>
+  );
 }

public/app/features/variables/textbox/TextBoxVariablePicker.tsx

@@ -1,43 +1,45 @@
-import React, { ChangeEvent, FocusEvent, KeyboardEvent, PureComponent } from 'react';
+import React, { ChangeEvent, FocusEvent, KeyboardEvent, ReactElement, useCallback, useEffect, useState } from 'react';
 
 import { TextBoxVariableModel } from '../types';
-import { toVariableIdentifier, toVariablePayload } from '../state/types';
-import { dispatch } from '../../../store/store';
+import { toVariablePayload } from '../state/types';
 import { changeVariableProp } from '../state/sharedReducer';
 import { VariablePickerProps } from '../pickers/types';
-import { updateOptions } from '../state/actions';
+import { Input } from '@grafana/ui';
+import { variableAdapters } from '../adapters';
+import { useDispatch } from 'react-redux';
 
 export interface Props extends VariablePickerProps<TextBoxVariableModel> {}
 
-export class TextBoxVariablePicker extends PureComponent<Props> {
-  onQueryChange = (event: ChangeEvent<HTMLInputElement>) => {
+export function TextBoxVariablePicker({ variable }: Props): ReactElement {
+  const dispatch = useDispatch();
+  const [updatedValue, setUpdatedValue] = useState(variable.current.value);
+  useEffect(() => {
+    setUpdatedValue(variable.current.value);
+  }, [variable]);
+
+  const updateVariable = useCallback(() => {
+    if (variable.current.value === updatedValue) {
+      return;
+    }
+
     dispatch(
-      changeVariableProp(toVariablePayload(this.props.variable, { propName: 'query', propValue: event.target.value }))
+      changeVariableProp(
+        toVariablePayload({ id: variable.id, type: variable.type }, { propName: 'query', propValue: updatedValue })
+      )
     );
-  };
+    variableAdapters.get(variable.type).updateOptions(variable);
+  }, [dispatch, variable, updatedValue]);
 
-  onQueryBlur = (event: FocusEvent<HTMLInputElement>) => {
-    if (this.props.variable.current.value !== this.props.variable.query) {
-      dispatch(updateOptions(toVariableIdentifier(this.props.variable)));
+  const onChange = useCallback((event: ChangeEvent<HTMLInputElement>) => setUpdatedValue(event.target.value), [
+    setUpdatedValue,
+  ]);
+
+  const onBlur = (e: FocusEvent<HTMLInputElement>) => updateVariable();
+  const onKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
+    if (event.keyCode === 13) {
+      updateVariable();
     }
   };
 
-  onQueryKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
-    if (event.keyCode === 13 && this.props.variable.current.value !== this.props.variable.query) {
-      dispatch(updateOptions(toVariableIdentifier(this.props.variable)));
-    }
-  };
-
-  render() {
-    return (
-      <input
-        type="text"
-        value={this.props.variable.query}
-        className="gf-form-input width-12"
-        onChange={this.onQueryChange}
-        onBlur={this.onQueryBlur}
-        onKeyDown={this.onQueryKeyDown}
-      />
-    );
-  }
+  return <Input type="text" value={updatedValue} onChange={onChange} onBlur={onBlur} onKeyDown={onKeyDown} />;
 }

public/app/features/variables/textbox/adapter.test.ts

@@ -0,0 +1,84 @@
+import { variableAdapters } from '../adapters';
+import { createTextBoxVariableAdapter } from './adapter';
+import { textboxBuilder } from '../shared/testing/builders';
+import { VariableHide } from '../types';
+
+variableAdapters.setInit(() => [createTextBoxVariableAdapter()]);
+
+describe('createTextBoxVariableAdapter', () => {
+  describe('getSaveModel', () => {
+    describe('when called and query differs from the original query and not saving current as default', () => {
+      it('then the model should be correct', () => {
+        const text = textboxBuilder()
+          .withId('text')
+          .withName('text')
+          .withQuery('query')
+          .withOriginalQuery('original')
+          .withCurrent('query')
+          .withOptions('query')
+          .build();
+
+        const adapter = variableAdapters.get('textbox');
+
+        const result = adapter.getSaveModel(text, false);
+
+        expect(result).toEqual({
+          name: 'text',
+          query: 'original',
+          current: { selected: false, text: 'original', value: 'original' },
+          options: [{ selected: false, text: 'original', value: 'original' }],
+          type: 'textbox',
+          label: null,
+          hide: VariableHide.dontHide,
+          skipUrlSync: false,
+          error: null,
+          description: null,
+        });
+      });
+    });
+
+    describe('when called and query differs from the original query and saving current as default', () => {
+      it('then the model should be correct', () => {
+        const text = textboxBuilder()
+          .withId('text')
+          .withName('text')
+          .withQuery('query')
+          .withOriginalQuery('original')
+          .withCurrent('query')
+          .withOptions('query')
+          .build();
+
+        const adapter = variableAdapters.get('textbox');
+
+        const result = adapter.getSaveModel(text, true);
+
+        expect(result).toEqual({
+          name: 'text',
+          query: 'query',
+          current: { selected: true, text: 'query', value: 'query' },
+          options: [{ selected: false, text: 'query', value: 'query' }],
+          type: 'textbox',
+          label: null,
+          hide: VariableHide.dontHide,
+          skipUrlSync: false,
+          error: null,
+          description: null,
+        });
+      });
+    });
+  });
+
+  describe('beforeAdding', () => {
+    describe('when called', () => {
+      it('then originalQuery should be same added', () => {
+        const model = { name: 'text', query: 'a query' };
+
+        const adapter = variableAdapters.get('textbox');
+
+        const result = adapter.beforeAdding!(model);
+
+        expect(result).toEqual({ name: 'text', query: 'a query', originalQuery: 'a query' });
+      });
+    });
+  });
+});

public/app/features/variables/textbox/adapter.ts

@@ -31,12 +31,22 @@ export const createTextBoxVariableAdapter = (): VariableAdapter<TextBoxVariableM
     updateOptions: async variable => {
       await dispatch(updateTextBoxVariableOptions(toVariableIdentifier(variable)));
     },
-    getSaveModel: variable => {
-      const { index, id, state, global, ...rest } = cloneDeep(variable);
+    getSaveModel: (variable, saveCurrentAsDefault) => {
+      const { index, id, state, global, originalQuery, ...rest } = cloneDeep(variable);
+
+      if (variable.query !== originalQuery && !saveCurrentAsDefault) {
+        const origQuery = originalQuery ?? '';
+        const current = { selected: false, text: origQuery, value: origQuery };
+        return { ...rest, query: origQuery, current, options: [current] };
+      }
+
       return rest;
     },
     getValueForUrl: variable => {
       return variable.current.value;
     },
+    beforeAdding: model => {
+      return { ...cloneDeep(model), originalQuery: model.query };
+    },
   };
 };

public/app/features/variables/textbox/reducer.ts

@@ -10,6 +10,7 @@ export const initialTextBoxVariableModelState: TextBoxVariableModel = {
   query: '',
   current: {} as VariableOption,
   options: [],
+  originalQuery: null,
 };
 
 export const textBoxVariableSlice = createSlice({

public/app/features/variables/types.ts

@@ -86,7 +86,9 @@ export interface QueryVariableModel extends DataSourceVariableModel {
   query: any;
 }
 
-export interface TextBoxVariableModel extends VariableWithOptions {}
+export interface TextBoxVariableModel extends VariableWithOptions {
+  originalQuery: string | null;
+}
 
 export interface ConstantVariableModel extends VariableWithOptions {}