From 47e162d6b240025e6ee03c7b85dbfe65810fa8fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Wed, 8 Jul 2020 13:22:09 +0200
Subject: [PATCH] Templating: Fixed recursive queries triggered when switching
 dashboard settings view (#26137)

* Templating: Fixed recursive queries triggered when going into dashboard settings

* Fixed unused import

* use locationUtil

(cherry picked from commit cace879c96dd5bdc75132de090618da9db565ffb)
---
 public/app/core/services/bridge_srv.test.ts   | 10 +++++
 public/app/core/services/bridge_srv.ts        | 39 ++++++++++++++++++-
 .../DashboardSettings/SettingsCtrl.ts         |  7 +---
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/public/app/core/services/bridge_srv.test.ts b/public/app/core/services/bridge_srv.test.ts
index f668e0f988e..da0b2a8b2e5 100644
--- a/public/app/core/services/bridge_srv.test.ts
+++ b/public/app/core/services/bridge_srv.test.ts
@@ -40,4 +40,14 @@ describe('when checking template variables', () => {
     expect(findTemplateVarChanges(b, a)).toBeUndefined();
     expect(findTemplateVarChanges(a, b)).toBeUndefined();
   });
+
+  it('then should ignore empty array values', () => {
+    const a: UrlQueryMap = {
+      'var-adhoc': [],
+    };
+    const b: UrlQueryMap = {};
+
+    expect(findTemplateVarChanges(b, a)).toBeUndefined();
+    expect(findTemplateVarChanges(a, b)).toBeUndefined();
+  });
 });
diff --git a/public/app/core/services/bridge_srv.ts b/public/app/core/services/bridge_srv.ts
index 440385d887d..7ecfe00c51b 100644
--- a/public/app/core/services/bridge_srv.ts
+++ b/public/app/core/services/bridge_srv.ts
@@ -8,6 +8,7 @@ import { GrafanaRootScope } from 'app/routes/GrafanaCtrl';
 import { locationUtil, UrlQueryMap } from '@grafana/data';
 import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
 import { templateVarsChangedInUrl } from 'app/features/variables/state/actions';
+import { isArray, isEqual } from 'lodash';
 
 // Services that handles angular -> redux store sync & other react <-> angular sync
 export class BridgeSrv {
@@ -15,6 +16,7 @@ export class BridgeSrv {
   private lastQuery: UrlQueryMap = {};
   private lastPath = '';
   private angularUrl: string;
+  private lastUrl: string | null = null;
 
   /** @ngInject */
   constructor(
@@ -62,6 +64,11 @@ export class BridgeSrv {
       const state = store.getState();
       const url = state.location.url;
 
+      // No url change ignore redux store change
+      if (url === this.lastUrl) {
+        return;
+      }
+
       if (this.angularUrl !== url) {
         // store angular url right away as otherwise we end up syncing multiple times
         this.angularUrl = url;
@@ -90,7 +97,9 @@ export class BridgeSrv {
       } else {
         this.lastQuery = {};
       }
+
       this.lastPath = state.location.path;
+      this.lastUrl = state.location.url;
     });
 
     appEvents.on(CoreEvents.locationChange, payload => {
@@ -111,19 +120,45 @@ export class BridgeSrv {
 export function findTemplateVarChanges(query: UrlQueryMap, old: UrlQueryMap): UrlQueryMap | undefined {
   let count = 0;
   const changes: UrlQueryMap = {};
+
   for (const key in query) {
     if (!key.startsWith('var-')) {
       continue;
     }
-    if (query[key] !== old[key]) {
-      changes[key] = query[key];
+
+    let oldValue = getUrlValueForComparison(old[key]);
+    let newValue = getUrlValueForComparison(query[key]);
+
+    if (!isEqual(newValue, oldValue)) {
+      changes[key] = newValue;
       count++;
     }
   }
+
+  function getUrlValueForComparison(value: any): any {
+    if (isArray(value)) {
+      if (value.length === 0) {
+        value = undefined;
+      } else if (value.length === 1) {
+        value = value[0];
+      }
+    }
+
+    return value;
+  }
+
   for (const key in old) {
     if (!key.startsWith('var-')) {
       continue;
     }
+
+    const value = old[key];
+
+    // ignore empty array values
+    if (isArray(value) && value.length === 0) {
+      continue;
+    }
+
     if (!query.hasOwnProperty(key)) {
       changes[key] = ''; // removed
       count++;
diff --git a/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts b/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts
index 42a01ecc0a6..cb583fcde23 100644
--- a/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts
+++ b/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts
@@ -1,16 +1,14 @@
-import $ from 'jquery';
 import _ from 'lodash';
 import angular, { ILocationService, IScope } from 'angular';
 import { selectors } from '@grafana/e2e-selectors';
 
 import { appEvents, contextSrv, coreModule } from 'app/core/core';
 import { DashboardModel } from '../../state/DashboardModel';
-import { getConfig } from 'app/core/config';
 import { backendSrv } from 'app/core/services/backend_srv';
 import { DashboardSrv } from '../../services/DashboardSrv';
 import { CoreEvents } from 'app/types';
 import { GrafanaRootScope } from 'app/routes/GrafanaCtrl';
-import { AppEvents, locationUtil, TimeZone } from '@grafana/data';
+import { AppEvents, locationUtil, TimeZone, urlUtil } from '@grafana/data';
 import { promiseToDigest } from '../../../../core/utils/promiseToDigest';
 
 export class SettingsCtrl {
@@ -121,8 +119,7 @@ export class SettingsCtrl {
     const url = this.$location.path();
 
     for (const section of this.sections) {
-      const sectionParams = _.defaults({ editview: section.id }, params);
-      section.url = getConfig().appSubUrl + url + '?' + $.param(sectionParams);
+      section.url = locationUtil.assureBaseUrl(urlUtil.renderUrl(url, { ...params, editview: section.id }));
     }
   }