Variables: Removes the never refresh option (#33533)

* Variables: Removes the never refresh option

* Tests: fixes DashboardModel repeat tests

* Tests: fixs snapshots

* Tests: fixes processVariable test

* Tests: fixes DashboardModel tests

docs/sources/variables/variable-types/add-query-variable.md

@@ -30,7 +30,6 @@ Query expressions are different for each data source. For more information, refe
 
 1. In the **Data source** list, select the target data source for the query. For more information about data sources, refer to [Add a data source]({{< relref "../../datasources/add-a-data-source.md" >}}).
 1. In the **Refresh** list, select when the variable should update options.
-   - **Never -** Variables queries are cached and values are not updated. This is fine if the values never change, but problematic if they are dynamic and change a lot.
    - **On Dashboard Load -** Queries the data source every time the dashboard loads. This slows down dashboard loading, because the variable query needs to be completed before dashboard can be initialized.
    - **On Time Range Change -** Queries the data source when the dashboard time range changes. Only use this option if your variable options query contains a time range filter or is dependent on the dashboard time range.
 1. In the **Query** field, enter a query.

e2e/suite1/specs/variables/new-query-variable.ts

@@ -48,7 +48,7 @@ describe('Variables - Add variable', () => {
     e2e.pages.Dashboard.Settings.Variables.Edit.QueryVariable.queryOptionsRefreshSelect()
       .should('be.visible')
       .within((select) => {
-        e2e.components.Select.singleValue().should('have.text', 'Never');
+        e2e.components.Select.singleValue().should('have.text', 'On dashboard load');
       });
     e2e.pages.Dashboard.Settings.Variables.Edit.QueryVariable.queryOptionsRegExInput()
       .should('be.visible')

public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap

@@ -92,7 +92,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],
@@ -228,7 +228,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
                 ],
                 "refresh": undefined,
                 "revision": undefined,
-                "schemaVersion": 28,
+                "schemaVersion": 29,
                 "snapshot": undefined,
                 "style": "dark",
                 "tags": Array [],
@@ -334,7 +334,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
               ],
               "refresh": undefined,
               "revision": undefined,
-              "schemaVersion": 28,
+              "schemaVersion": 29,
               "snapshot": undefined,
               "style": "dark",
               "tags": Array [],
@@ -462,7 +462,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],
@@ -598,7 +598,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
                 ],
                 "refresh": undefined,
                 "revision": undefined,
-                "schemaVersion": 28,
+                "schemaVersion": 29,
                 "snapshot": undefined,
                 "style": "dark",
                 "tags": Array [],
@@ -704,7 +704,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
               ],
               "refresh": undefined,
               "revision": undefined,
-              "schemaVersion": 28,
+              "schemaVersion": 29,
               "snapshot": undefined,
               "style": "dark",
               "tags": Array [],
@@ -814,7 +814,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
         ],
         "refresh": undefined,
         "revision": undefined,
-        "schemaVersion": 28,
+        "schemaVersion": 29,
         "snapshot": undefined,
         "style": "dark",
         "tags": Array [],

public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap

@@ -244,7 +244,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],
@@ -505,7 +505,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],
@@ -766,7 +766,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],
@@ -1027,7 +1027,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
           ],
           "refresh": undefined,
           "revision": undefined,
-          "schemaVersion": 28,
+          "schemaVersion": 29,
           "snapshot": undefined,
           "style": "dark",
           "tags": Array [],

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

@@ -162,7 +162,7 @@ describe('DashboardModel', () => {
     });
 
     it('dashboard schema version should be set to latest', () => {
-      expect(model.schemaVersion).toBe(28);
+      expect(model.schemaVersion).toBe(29);
     });
 
     it('graph thresholds should be migrated', () => {
@@ -925,6 +925,149 @@ describe('DashboardModel', () => {
       });
     });
   });
+
+  describe('when migrating variable refresh to on dashboard load', () => {
+    let model: DashboardModel;
+
+    beforeEach(() => {
+      model = new DashboardModel({
+        templating: {
+          list: [
+            {
+              type: 'query',
+              name: 'variable_with_never_refresh_with_options',
+              options: [{ text: 'A', value: 'A' }],
+              refresh: 0,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_never_refresh_without_options',
+              options: [],
+              refresh: 0,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_dashboard_refresh_with_options',
+              options: [{ text: 'A', value: 'A' }],
+              refresh: 1,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_dashboard_refresh_without_options',
+              options: [],
+              refresh: 1,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_timerange_refresh_with_options',
+              options: [{ text: 'A', value: 'A' }],
+              refresh: 2,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_timerange_refresh_without_options',
+              options: [],
+              refresh: 2,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_no_refresh_with_options',
+              options: [{ text: 'A', value: 'A' }],
+            },
+            {
+              type: 'query',
+              name: 'variable_with_no_refresh_without_options',
+              options: [],
+            },
+            {
+              type: 'query',
+              name: 'variable_with_unknown_refresh_with_options',
+              options: [{ text: 'A', value: 'A' }],
+              refresh: 2001,
+            },
+            {
+              type: 'query',
+              name: 'variable_with_unknown_refresh_without_options',
+              options: [],
+              refresh: 2001,
+            },
+            {
+              type: 'custom',
+              name: 'custom',
+              options: [{ text: 'custom', value: 'custom' }],
+            },
+            {
+              type: 'textbox',
+              name: 'textbox',
+              options: [{ text: 'Hello', value: 'World' }],
+            },
+            {
+              type: 'datasource',
+              name: 'datasource',
+              options: [{ text: 'ds', value: 'ds' }], // fake example doesn't exist
+            },
+            {
+              type: 'interval',
+              name: 'interval',
+              options: [{ text: '1m', value: '1m' }],
+            },
+          ],
+        },
+      });
+    });
+
+    it('should have 11 variables after migration', () => {
+      expect(model.templating.list.length).toBe(14);
+    });
+
+    it('should not affect custom variable types', () => {
+      const custom = model.templating.list[10];
+      expect(custom.type).toEqual('custom');
+      expect(custom.options).toEqual([{ text: 'custom', value: 'custom' }]);
+    });
+
+    it('should not affect textbox variable types', () => {
+      const textbox = model.templating.list[11];
+      expect(textbox.type).toEqual('textbox');
+      expect(textbox.options).toEqual([{ text: 'Hello', value: 'World' }]);
+    });
+
+    it('should not affect datasource variable types', () => {
+      const datasource = model.templating.list[12];
+      expect(datasource.type).toEqual('datasource');
+      expect(datasource.options).toEqual([{ text: 'ds', value: 'ds' }]);
+    });
+
+    it('should not affect interval variable types', () => {
+      const interval = model.templating.list[13];
+      expect(interval.type).toEqual('interval');
+      expect(interval.options).toEqual([{ text: '1m', value: '1m' }]);
+    });
+
+    it('should removed options from all query variables', () => {
+      const queryVariables = model.templating.list.filter((v) => v.type === 'query');
+      expect(queryVariables).toHaveLength(10);
+      const noOfOptions = queryVariables.reduce((all, variable) => all + variable.options.length, 0);
+      expect(noOfOptions).toBe(0);
+    });
+
+    it('should set the refresh prop to on dashboard load for all query variables that have never or unknown', () => {
+      expect(model.templating.list[0].refresh).toBe(1);
+      expect(model.templating.list[1].refresh).toBe(1);
+      expect(model.templating.list[2].refresh).toBe(1);
+      expect(model.templating.list[3].refresh).toBe(1);
+      expect(model.templating.list[4].refresh).toBe(2);
+      expect(model.templating.list[5].refresh).toBe(2);
+      expect(model.templating.list[6].refresh).toBe(1);
+      expect(model.templating.list[7].refresh).toBe(1);
+      expect(model.templating.list[8].refresh).toBe(1);
+      expect(model.templating.list[9].refresh).toBe(1);
+      expect(model.templating.list[10].refresh).toBeUndefined();
+      expect(model.templating.list[11].refresh).toBeUndefined();
+      expect(model.templating.list[12].refresh).toBeUndefined();
+      expect(model.templating.list[13].refresh).toBeUndefined();
+    });
+  });
 });
 
 function createRow(options: any, panelDescriptions: any[]) {

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

@@ -45,7 +45,7 @@ export class DashboardMigrator {
     let i, j, k, n;
     const oldVersion = this.dashboard.schemaVersion;
     const panelUpgrades = [];
-    this.dashboard.schemaVersion = 28;
+    this.dashboard.schemaVersion = 29;
 
     if (oldVersion === this.dashboard.schemaVersion) {
       return;
@@ -594,6 +594,22 @@ export class DashboardMigrator {
       }
     }
 
+    if (oldVersion < 29) {
+      for (const variable of this.dashboard.templating.list) {
+        if (variable.type !== 'query') {
+          continue;
+        }
+
+        if (variable.refresh !== 1 && variable.refresh !== 2) {
+          variable.refresh = 1;
+        }
+
+        if (variable.options?.length) {
+          variable.options = [];
+        }
+      }
+    }
+
     if (panelUpgrades.length === 0) {
       return;
     }

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

@@ -19,6 +19,7 @@ describe('given dashboard with panel repeat', () => {
         list: [
           {
             name: 'apps',
+            type: 'custom',
             current: {
               text: 'se1, se2, se3',
               value: ['se1', 'se2', 'se3'],
@@ -74,6 +75,7 @@ describe('given dashboard with panel repeat in horizontal direction', () => {
         list: [
           {
             name: 'apps',
+            type: 'custom',
             current: {
               text: 'se1, se2, se3',
               value: ['se1', 'se2', 'se3'],
@@ -204,6 +206,7 @@ describe('given dashboard with panel repeat in vertical direction', () => {
         list: [
           {
             name: 'apps',
+            type: 'custom',
             current: {
               text: 'se1, se2, se3',
               value: ['se1', 'se2', 'se3'],
@@ -246,6 +249,7 @@ describe('given dashboard with row repeat and panel repeat in horizontal directi
         list: [
           {
             name: 'region',
+            type: 'custom',
             current: {
               text: 'reg1, reg2',
               value: ['reg1', 'reg2'],
@@ -257,6 +261,7 @@ describe('given dashboard with row repeat and panel repeat in horizontal directi
           },
           {
             name: 'app',
+            type: 'custom',
             current: {
               text: 'se1, se2, se3, se4, se5, se6',
               value: ['se1', 'se2', 'se3', 'se4', 'se5', 'se6'],
@@ -339,6 +344,7 @@ describe('given dashboard with row repeat', () => {
         list: [
           {
             name: 'apps',
+            type: 'custom',
             current: {
               text: 'se1, se2',
               value: ['se1', 'se2'],
@@ -435,6 +441,7 @@ describe('given dashboard with row repeat', () => {
     ];
     dashboardJSON.templating.list.push({
       name: 'hosts',
+      type: 'custom',
       current: {
         text: 'backend01, backend02',
         value: ['backend01', 'backend02'],
@@ -543,6 +550,7 @@ describe('given dashboard with row and panel repeat', () => {
         list: [
           {
             name: 'region',
+            type: 'custom',
             current: {
               text: 'reg1, reg2',
               value: ['reg1', 'reg2'],
@@ -555,6 +563,7 @@ describe('given dashboard with row and panel repeat', () => {
           },
           {
             name: 'app',
+            type: 'custom',
             current: {
               text: 'se1, se2',
               value: ['se1', 'se2'],
@@ -690,6 +699,7 @@ describe('given panel is in view mode', () => {
         list: [
           {
             name: 'apps',
+            type: 'custom',
             current: {
               text: 'se1, se2, se3',
               value: ['se1', 'se2', 'se3'],

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

@@ -5,9 +5,14 @@ import { getDashboardModel } from '../../../../test/helpers/getDashboardModel';
 import { variableAdapters } from '../../variables/adapters';
 import { createAdHocVariableAdapter } from '../../variables/adhoc/adapter';
 import { createQueryVariableAdapter } from '../../variables/query/adapter';
+import { createCustomVariableAdapter } from '../../variables/custom/adapter';
 
 jest.mock('app/core/services/context_srv', () => ({}));
-variableAdapters.setInit(() => [createQueryVariableAdapter(), createAdHocVariableAdapter()]);
+variableAdapters.setInit(() => [
+  createQueryVariableAdapter(),
+  createAdHocVariableAdapter(),
+  createCustomVariableAdapter(),
+]);
 
 describe('DashboardModel', () => {
   describe('when creating new dashboard model defaults only', () => {
@@ -522,6 +527,7 @@ describe('DashboardModel', () => {
           list: [
             {
               name: 'dc',
+              type: 'custom',
               current: {
                 text: 'dc1 + dc2',
                 value: ['dc1', 'dc2'],
@@ -533,6 +539,7 @@ describe('DashboardModel', () => {
             },
             {
               name: 'app',
+              type: 'custom',
               current: {
                 text: 'se1 + se2',
                 value: ['se1', 'se2'],

public/app/features/variables/query/QueryVariableRefreshSelect.tsx

@@ -10,7 +10,6 @@ interface Props {
 }
 
 const REFRESH_OPTIONS = [
-  { label: 'Never', value: VariableRefresh.never },
   { label: 'On dashboard load', value: VariableRefresh.onDashboardLoad },
   { label: 'On time range change', value: VariableRefresh.onTimeRangeChanged },
 ];

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

@@ -39,7 +39,7 @@ export const initialQueryVariableModelState: QueryVariableModel = {
   query: '',
   regex: '',
   sort: VariableSort.disabled,
-  refresh: VariableRefresh.never,
+  refresh: VariableRefresh.onDashboardLoad,
   multi: false,
   includeAll: false,
   allValue: null,

public/app/features/variables/state/actions.test.ts

@@ -61,6 +61,8 @@ import { ConstantVariableModel, VariableRefresh } from '../types';
 import { updateVariableOptions } from '../query/reducer';
 import { setVariableQueryRunner, VariableQueryRunner } from '../query/VariableQueryRunner';
 import { setDataSourceSrv, setLocationService } from '@grafana/runtime';
+import { LoadingState } from '@grafana/data';
+import { toAsyncOfResult } from '../../query/state/DashboardQueryRunner/testHelpers';
 
 variableAdapters.setInit(() => [
   createQueryVariableAdapter(),
@@ -148,6 +150,13 @@ describe('shared actions', () => {
       };
       const locationService: any = { getSearchObject: () => ({}) };
       setLocationService(locationService);
+      const variableQueryRunner: any = {
+        cancelRequest: jest.fn(),
+        queueRequest: jest.fn(),
+        getResponse: () => toAsyncOfResult({ state: LoadingState.Done, identifier: toVariableIdentifier(query) }),
+        destroy: jest.fn(),
+      };
+      setVariableQueryRunner(variableQueryRunner);
 
       const tester = await reduxTester<TemplatingReducerType>({ preloadedState })
         .givenRootReducer(getTemplatingRootReducer())
@@ -156,10 +165,10 @@ describe('shared actions', () => {
         .whenAsyncActionIsDispatched(processVariables(), true);
 
       await tester.thenDispatchedActionsPredicateShouldEqual((dispatchedActions) => {
-        expect(dispatchedActions.length).toEqual(4);
+        expect(dispatchedActions.length).toEqual(5);
 
         expect(dispatchedActions[0]).toEqual(
-          variableStateCompleted(toVariablePayload({ ...query, id: dispatchedActions[0].payload.id }))
+          variableStateFetching(toVariablePayload({ ...query, id: dispatchedActions[0].payload.id }))
         );
 
         expect(dispatchedActions[1]).toEqual(
@@ -174,6 +183,10 @@ describe('shared actions', () => {
           variableStateCompleted(toVariablePayload({ ...textbox, id: dispatchedActions[3].payload.id }))
         );
 
+        expect(dispatchedActions[4]).toEqual(
+          variableStateCompleted(toVariablePayload({ ...query, id: dispatchedActions[4].payload.id }))
+        );
+
         return true;
       });
     });

public/app/features/variables/types.ts

@@ -12,7 +12,7 @@ import { NEW_VARIABLE_ID } from './state/types';
 import { VariableQueryProps } from '../../types';
 
 export enum VariableRefresh {
-  never,
+  never, // removed from the UI
   onDashboardLoad,
   onTimeRangeChanged,
 }