Dashboard: Variable without a current value in json model causes cras…

…h on load (grafana#24261) * Variables: Fixes variable initilization and default values * fixed failing tests. Co-authored-by: Marcus Andersson <marcus.andersson@grafana.com>
digitalocean · May 5, 2020 · dd848da · dd848da
1 parent 79007cc
commit dd848da
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 7 deletions.
diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts
@@ -196,7 +196,7 @@ describe('shared actions', () => {
         ${['A', 'B', 'C']} | ${'B'}       | ${'C'}       | ${'B'}
         ${['A', 'B', 'C']} | ${'X'}       | ${undefined} | ${'A'}
         ${['A', 'B', 'C']} | ${'X'}       | ${'C'}       | ${'C'}
-        ${undefined}       | ${'B'}       | ${undefined} | ${'A'}
+        ${undefined}       | ${'B'}       | ${undefined} | ${'should not dispatch setCurrentVariableValue'}
       `('then correct actions are dispatched', async ({ withOptions, withCurrent, defaultValue, expected }) => {
         let custom;
 

diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts
@@ -308,7 +308,9 @@ export const validateVariableSelectionState = (
     // 3. use the first value
     if (variableInState.options) {
       const option = variableInState.options[0];
-      return setValue(variableInState, option);
+      if (option) {
+        return setValue(variableInState, option);
+      }
     }
 
     // 4... give up

diff --git a/public/app/features/variables/state/sharedReducer.test.ts b/public/app/features/variables/state/sharedReducer.test.ts
@@ -1,4 +1,5 @@
 import cloneDeep from 'lodash/cloneDeep';
+import { default as lodashDefaults } from 'lodash/defaults';
 
 import { reducerTester } from '../../../../test/core/redux/reducerTester';
 import {
@@ -29,15 +30,20 @@ variableAdapters.setInit(() => [createQueryVariableAdapter()]);
 describe('sharedReducer', () => {
   describe('when addVariable is dispatched', () => {
     it('then state should be correct', () => {
-      const model = ({ name: 'name from model', type: 'type from model' } as unknown) as QueryVariableModel;
+      const model = ({
+        name: 'name from model',
+        type: 'type from model',
+        current: undefined,
+      } as unknown) as QueryVariableModel;
+
       const payload = toVariablePayload({ id: '0', type: 'query' }, { global: true, index: 0, model });
+
       reducerTester<VariablesState>()
         .givenReducer(sharedReducer, { ...initialVariablesState })
         .whenActionIsDispatched(addVariable(payload))
         .thenStateShouldEqual({
           [0]: {
-            ...initialQueryVariableModelState,
-            ...model,
+            ...lodashDefaults({}, model, initialQueryVariableModelState),
             id: '0',
             global: true,
             index: 0,

diff --git a/public/app/features/variables/state/sharedReducer.ts b/public/app/features/variables/state/sharedReducer.ts
@@ -1,5 +1,6 @@
 import { createSlice, PayloadAction } from '@reduxjs/toolkit';
 import cloneDeep from 'lodash/cloneDeep';
+import { default as lodashDefaults } from 'lodash/defaults';
 
 import { VariableType } from '@grafana/data';
 import { VariableModel, VariableOption, VariableWithOptions } from '../../templating/types';
@@ -16,13 +17,16 @@ const sharedReducerSlice = createSlice({
   reducers: {
     addVariable: (state: VariablesState, action: PayloadAction<VariablePayload<AddVariable>>) => {
       const id = action.payload.id ?? action.payload.data.model.name; // for testing purposes we can call this with an id
+      const initialState = cloneDeep(variableAdapters.get(action.payload.type).initialState);
+      const model = cloneDeep(action.payload.data.model);
+
       const variable = {
-        ...cloneDeep(variableAdapters.get(action.payload.type).initialState),
-        ...action.payload.data.model,
+        ...lodashDefaults({}, model, initialState),
         id: id,
         index: action.payload.data.index,
         global: action.payload.data.global,
       };
+
       state[id] = variable;
     },
     addInitLock: (state: VariablesState, action: PayloadAction<VariablePayload>) => {