From c2410c1f30037ee06672ec8ef8730895ae8b4ed2 Mon Sep 17 00:00:00 2001 From: Oscar Kilhed Date: Mon, 6 Oct 2025 12:58:54 +0200 Subject: [PATCH] [release-11.6.7] URLParams: Stringify true values as key=true always (fixes issues with variables with true value) (#112045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * URLParams: Stringify true values as key=true always (fixes issues with variables with true value) (#106440) * Restore prev fix * added one more test * Fix linkUrl issue (cherry picked from commit 1564e1bac9b1b7a0c07a9ee7d83988fe0e70998b) * add betterer results * fix old angular test to reflect new behavior of link_srv --------- Co-authored-by: Torkel Ödegaard --- .betterer.results | 3 --- packages/grafana-data/src/utils/url.test.ts | 10 ++++++---- packages/grafana-data/src/utils/url.ts | 7 +------ .../src/services/LocationService.test.tsx | 7 +++++++ public/app/angular/AngularLocationWrapper.test.ts | 6 +++--- .../dashboard-scene/sharing/ShareLinkTab.test.tsx | 2 +- public/app/features/dashboard/services/TimeSrv.test.ts | 2 +- public/app/features/panel/panellinks/link_srv.ts | 8 ++++---- 8 files changed, 23 insertions(+), 22 deletions(-) diff --git a/.betterer.results b/.betterer.results index 12373b43fad..6aa6201b2e3 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5240,9 +5240,6 @@ exports[`better eslint`] = { "public/app/features/panel/panellinks/linkSuppliers.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], - "public/app/features/panel/panellinks/link_srv.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"] - ], "public/app/features/playlist/PlaylistCard.tsx:5381": [ [0, 0, 0, "\'@grafana/ui/src/unstable\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] ], diff --git a/packages/grafana-data/src/utils/url.test.ts b/packages/grafana-data/src/utils/url.test.ts index 58e20733593..d0be86cb209 100644 --- a/packages/grafana-data/src/utils/url.test.ts +++ b/packages/grafana-data/src/utils/url.test.ts @@ -11,7 +11,9 @@ describe('toUrlParams', () => { isNull: null, isUndefined: undefined, }); - expect(url).toBe('server=backend-01&hasSpace=has%20space&many=1&many=2&many=3&true&number=20&isNull=&isUndefined='); + expect(url).toBe( + 'server=backend-01&hasSpace=has%20space&many=1&many=2&many=3&true=true&number=20&isNull=&isUndefined=' + ); }); it('should encode the same way as angularjs', () => { const url = urlUtil.toUrlParams({ @@ -25,7 +27,7 @@ describe('toUrlParams', () => { bool1: true, bool2: false, }); - expect(url).toBe('bool1&bool2=false'); + expect(url).toBe('bool1=true&bool2=false'); }); it("should encode the following special characters [!'()*]", () => { const url = urlUtil.toUrlParams({ @@ -45,7 +47,7 @@ describe('toUrlParams', () => { oneMore: false, }); expect(params).toBe( - 'server=backend-01&hasSpace=has%20space&many=1&many=2&many=3&true&number=20&isNull=&isUndefined=&oneMore=false' + 'server=backend-01&hasSpace=has%20space&many=1&many=2&many=3&true=true&number=20&isNull=&isUndefined=&oneMore=false' ); }); @@ -61,7 +63,7 @@ describe('toUrlParams', () => { bool1: true, bool2: false, }); - expect(url).toBe('bool1&bool2=false'); + expect(url).toBe('bool1=true&bool2=false'); }); }); diff --git a/packages/grafana-data/src/utils/url.ts b/packages/grafana-data/src/utils/url.ts index b98ee5f5b64..d1557ff905b 100644 --- a/packages/grafana-data/src/utils/url.ts +++ b/packages/grafana-data/src/utils/url.ts @@ -60,12 +60,7 @@ function toUrlParams(a: any, encodeAsAngularJS = true) { const add = (k: string, v: any) => { v = typeof v === 'function' ? v() : v === null ? '' : v === undefined ? '' : v; - if (typeof v !== 'boolean') { - s[s.length] = encodingFunction(k, true) + '=' + encodingFunction(v, true); - } else { - const valueQueryPart = v ? '' : '=' + encodingFunction('false', true); - s[s.length] = encodingFunction(k, true) + valueQueryPart; - } + s[s.length] = encodingFunction(k, true) + '=' + encodingFunction(v, true); }; const buildParams = (prefix: string, obj: any) => { diff --git a/packages/grafana-runtime/src/services/LocationService.test.tsx b/packages/grafana-runtime/src/services/LocationService.test.tsx index b3c6b7af441..6378b8fdde2 100644 --- a/packages/grafana-runtime/src/services/LocationService.test.tsx +++ b/packages/grafana-runtime/src/services/LocationService.test.tsx @@ -38,6 +38,13 @@ describe('LocationService', () => { expect(locationService.getLocation().search).toBe('?servers=A&servers=B&servers=C'); }); + it('should handle boolean string values', () => { + locationService.push('/?query1=false&query2=true&query3'); + locationService.partial({ newProp: 'a' }); + + expect(locationService.getLocation().search).toBe('?query1=false&query2=true&query3=true&newProp=a'); + }); + it('persist state', () => { locationService.push({ pathname: '/d/123', diff --git a/public/app/angular/AngularLocationWrapper.test.ts b/public/app/angular/AngularLocationWrapper.test.ts index 7bd20c89f8c..483942e05e0 100644 --- a/public/app/angular/AngularLocationWrapper.test.ts +++ b/public/app/angular/AngularLocationWrapper.test.ts @@ -88,14 +88,14 @@ describe('AngularLocationWrapper', () => { locationService.push('/path/b'); wrapper.search('x=y&c'); expect(wrapper.search()).toEqual({ x: 'y', c: true }); - expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?x=y&c'); + expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?x=y&c=true'); }); it('search() should accept object', function () { locationService.push('/path/b'); wrapper.search({ one: '1', two: true }); expect(wrapper.search()).toEqual({ one: '1', two: true }); - expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?one=1&two'); + expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?one=1&two=true'); }); it('should copy object', function () { @@ -106,7 +106,7 @@ describe('AngularLocationWrapper', () => { obj.one = 'changed'; expect(wrapper.search()).toEqual({ one: '1', two: true }); - expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?one=1&two'); + expect(wrapper.absUrl()).toBe('http://www.domain.com:9877/path/b?one=1&two=true'); }); it('should change single parameter', function () { diff --git a/public/app/features/dashboard-scene/sharing/ShareLinkTab.test.tsx b/public/app/features/dashboard-scene/sharing/ShareLinkTab.test.tsx index cded7ef3abe..4e80dd3c660 100644 --- a/public/app/features/dashboard-scene/sharing/ShareLinkTab.test.tsx +++ b/public/app/features/dashboard-scene/sharing/ShareLinkTab.test.tsx @@ -88,7 +88,7 @@ describe('ShareLinkTab', () => { await screen.findByRole('link', { name: selectors.pages.SharePanelModal.linkToRenderedImage }) ).toHaveAttribute( 'href', - 'http://dashboards.grafana.com/grafana/render/d-solo/dash-1?from=2019-02-11T13:00:00.000Z&to=2019-02-11T19:00:00.000Z&panelId=panel-12&__feature.dashboardSceneSolo&width=1000&height=500&tz=Pacific%2FEaster' + 'http://dashboards.grafana.com/grafana/render/d-solo/dash-1?from=2019-02-11T13:00:00.000Z&to=2019-02-11T19:00:00.000Z&panelId=panel-12&__feature.dashboardSceneSolo=true&width=1000&height=500&tz=Pacific%2FEaster' ); }); }); diff --git a/public/app/features/dashboard/services/TimeSrv.test.ts b/public/app/features/dashboard/services/TimeSrv.test.ts index 5418d88994f..981911116ea 100644 --- a/public/app/features/dashboard/services/TimeSrv.test.ts +++ b/public/app/features/dashboard/services/TimeSrv.test.ts @@ -297,7 +297,7 @@ describe('timeSrv', () => { timeSrv.setTime({ from: 'now-1h', to: 'now-10s' }); timeSrv.setTime({ from: 'now-1h', to: 'now-10s' }); - expect(locationUpdates[1].search).toEqual('?kiosk&from=now-1h&to=now-10s'); + expect(locationUpdates[1].search).toEqual('?kiosk=true&from=now-1h&to=now-10s'); }); it('should not change the URL if the updateUrl param is false', () => { diff --git a/public/app/features/panel/panellinks/link_srv.ts b/public/app/features/panel/panellinks/link_srv.ts index 19a7920fc7d..6559b7eed94 100644 --- a/public/app/features/panel/panellinks/link_srv.ts +++ b/public/app/features/panel/panellinks/link_srv.ts @@ -252,18 +252,18 @@ export interface LinkService { export class LinkSrv implements LinkService { getLinkUrl(link: DashboardLink) { - let params: { [key: string]: any } = {}; + let url = link.url ?? ''; if (link.keepTime) { - params[`\$${DataLinkBuiltInVars.keepTime}`] = true; + url = urlUtil.appendQueryToUrl(url, `\$${DataLinkBuiltInVars.keepTime}`); } if (link.includeVars) { - params[`\$${DataLinkBuiltInVars.includeVars}`] = true; + url = urlUtil.appendQueryToUrl(url, `\$${DataLinkBuiltInVars.includeVars}`); } - let url = locationUtil.assureBaseUrl(urlUtil.appendQueryToUrl(link.url || '', urlUtil.toUrlParams(params))); url = getTemplateSrv().replace(url); + url = locationUtil.assureBaseUrl(url); return getConfig().disableSanitizeHtml ? url : textUtil.sanitizeUrl(url); }