From 972ec9cf85a7367719fb6afc2e725bfa8aeb4caa Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Mon, 20 Sep 2021 03:55:25 -0400 Subject: [PATCH] Variables: Prevents unnecessary duplicate requests (#39394) (#39401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit e696a9ab42474571b65150fd7aa86f2f4efb7f3e) Co-authored-by: Hugo Häggmark --- .../features/variables/state/actions.test.ts | 61 +++++++++++++++++++ .../app/features/variables/state/actions.ts | 12 ++-- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts index badb5ea7af9..18806f37884 100644 --- a/public/app/features/variables/state/actions.test.ts +++ b/public/app/features/variables/state/actions.test.ts @@ -15,6 +15,7 @@ import { fixSelectedInconsistency, initDashboardTemplating, initVariablesTransaction, + isVariableUrlValueDifferentFromCurrent, processVariables, validateVariableSelectionState, } from './actions'; @@ -775,4 +776,64 @@ describe('shared actions', () => { }); }); }); + + describe('isVariableUrlValueDifferentFromCurrent', () => { + describe('when called with a single valued variable', () => { + describe('and values are equal', () => { + it('then it should return false', () => { + const variable = queryBuilder().withMulti(false).withCurrent('A', 'A').build(); + const urlValue = 'A'; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(false); + }); + }); + + describe('and values are different', () => { + it('then it should return true', () => { + const variable = queryBuilder().withMulti(false).withCurrent('A', 'A').build(); + const urlValue = 'B'; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(true); + }); + }); + }); + + describe('when called with a multi valued variable', () => { + describe('and values are equal', () => { + it('then it should return false', () => { + const variable = queryBuilder().withMulti(true).withCurrent(['A'], ['A']).build(); + const urlValue = ['A']; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(false); + }); + + describe('but urlValue is not an array', () => { + it('then it should return false', () => { + const variable = queryBuilder().withMulti(true).withCurrent(['A'], ['A']).build(); + const urlValue = 'A'; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(false); + }); + }); + }); + + describe('and values are different', () => { + it('then it should return true', () => { + const variable = queryBuilder().withMulti(true).withCurrent(['A'], ['A']).build(); + const urlValue = ['C']; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(true); + }); + + describe('but urlValue is not an array', () => { + it('then it should return true', () => { + const variable = queryBuilder().withMulti(true).withCurrent(['A'], ['A']).build(); + const urlValue = 'C'; + + expect(isVariableUrlValueDifferentFromCurrent(variable, urlValue)).toBe(true); + }); + }); + }); + }); + }); }); diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index b251e08f9a6..5d4a2bae90c 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -622,11 +622,15 @@ export const templateVarsChangedInUrl = (vars: ExtendedUrlQueryMap): ThunkResult } }; -const isVariableUrlValueDifferentFromCurrent = (variable: VariableModel, urlValue: any): boolean => { - const stringUrlValue = ensureStringValues(urlValue); +export function isVariableUrlValueDifferentFromCurrent(variable: VariableModel, urlValue: any): boolean { + const variableValue = variableAdapters.get(variable.type).getValueForUrl(variable); + let stringUrlValue = ensureStringValues(urlValue); + if (Array.isArray(variableValue) && !Array.isArray(stringUrlValue)) { + stringUrlValue = [stringUrlValue]; + } // lodash isEqual handles array of value equality checks as well - return !isEqual(variableAdapters.get(variable.type).getValueForUrl(variable), stringUrlValue); -}; + return !isEqual(variableValue, stringUrlValue); +} const getQueryWithVariables = (getState: () => StoreState): UrlQueryMap => { const queryParams = locationService.getSearchObject();