From fe15d90e98fe0c9423aa771b063d2313118286fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= <hugo.haggmark@grafana.com>
Date: Wed, 14 Oct 2020 17:07:42 +0200
Subject: [PATCH] Variables: Fixes so constants set from url get completed
 state (#28257)

* Variables: Fixes so constant set from url get completed state

* Tests: fixes broken test
---
 .../app/features/variables/state/actions.ts   | 19 ++++++++++++-------
 .../variables/state/processVariable.test.ts   | 11 ++++++-----
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts
index 8866c489d55..c717cc6144f 100644
--- a/public/app/features/variables/state/actions.ts
+++ b/public/app/features/variables/state/actions.ts
@@ -250,7 +250,7 @@ export const processVariable = (
     }
 
     // for variables that aren't updated via url or refresh let's simulate the same state changes
-    dispatch(variableStateCompleted(toVariablePayload(variable)));
+    dispatch(completeVariableLoading(identifier));
   };
 };
 
@@ -276,11 +276,6 @@ export const setOptionFromUrl = (
       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
     const variableFromState = getVariable<VariableWithOptions>(variable.id, getState());
     if (!variableFromState) {
@@ -452,6 +447,8 @@ export const variableUpdated = (
 
     // if we're initializing variables ignore cascading update because we are in a boot up scenario
     if (getState().templating.transaction.status === TransactionStatus.Fetching) {
+      // for all variable types with updates that go the setValueFromUrl path in the update let's make sure their state is set to Done.
+      dispatch(completeVariableLoading(identifier));
       return Promise.resolve();
     }
 
@@ -624,7 +621,7 @@ export const updateOptions = (identifier: VariableIdentifier, rethrow = false):
   try {
     dispatch(variableStateFetching(toVariablePayload(variableInState)));
     await variableAdapters.get(variableInState.type).updateOptions(variableInState);
-    dispatch(variableStateCompleted(toVariablePayload(variableInState)));
+    dispatch(completeVariableLoading(identifier));
   } catch (error) {
     dispatch(variableStateFailed(toVariablePayload(variableInState, { error })));
 
@@ -648,3 +645,11 @@ export const createVariableErrorNotification = (
     `${identifier ? `Templating [${identifier.id}]` : 'Templating'}`,
     `${message} ${error.message}`
   );
+
+export const completeVariableLoading = (identifier: VariableIdentifier): ThunkResult<void> => (dispatch, getState) => {
+  const variableInState = getVariable(identifier.id, getState());
+
+  if (variableInState.state !== LoadingState.Done) {
+    dispatch(variableStateCompleted(toVariablePayload(variableInState)));
+  }
+};
diff --git a/public/app/features/variables/state/processVariable.test.ts b/public/app/features/variables/state/processVariable.test.ts
index 9f611a64da8..572ab71afa9 100644
--- a/public/app/features/variables/state/processVariable.test.ts
+++ b/public/app/features/variables/state/processVariable.test.ts
@@ -132,7 +132,8 @@ describe('processVariable', () => {
         await tester.thenDispatchedActionsShouldEqual(
           setCurrentVariableValue(
             toVariablePayload({ type: 'custom', id: 'custom' }, { option: { text: 'B', value: 'B', selected: false } })
-          )
+          ),
+          variableStateCompleted(toVariablePayload(custom))
         );
       });
     });
@@ -212,13 +213,13 @@ describe('processVariable', () => {
             .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 } }
               )
-            )
+            ),
+            variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryNoDepends' }))
           );
         });
       });
@@ -360,13 +361,13 @@ describe('processVariable', () => {
           );
 
           await tester.thenDispatchedActionsShouldEqual(
-            variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' })),
             setCurrentVariableValue(
               toVariablePayload(
                 { type: 'query', id: 'queryDependsOnCustom' },
                 { option: { text: 'AB', value: 'AB', selected: false } }
               )
-            )
+            ),
+            variableStateCompleted(toVariablePayload({ type: 'query', id: 'queryDependsOnCustom' }))
           );
         });
       });