From fd2641a542e6ae563acf0de7be14e7bf6e805784 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Tue, 31 Jan 2023 09:27:40 +0000 Subject: [PATCH] Navigation: Sign in button now works correctly when served under a sub path (#62504) make sure getUrlForPartial always includes the basePath + unit tests --- .betterer.results | 11 -- .../grafana-data/src/utils/location.test.ts | 112 ++++++++++++++++-- packages/grafana-data/src/utils/location.ts | 2 +- 3 files changed, 104 insertions(+), 21 deletions(-) diff --git a/.betterer.results b/.betterer.results index f20d99d74fb..d061bee340f 100644 --- a/.betterer.results +++ b/.betterer.results @@ -719,17 +719,6 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "1"] ], - "packages/grafana-data/src/utils/location.test.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"], - [0, 0, 0, "Unexpected any. Specify a different type.", "8"] - ], "packages/grafana-data/src/utils/location.ts:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/packages/grafana-data/src/utils/location.test.ts b/packages/grafana-data/src/utils/location.test.ts index b56660bbd01..33717820fb1 100644 --- a/packages/grafana-data/src/utils/location.test.ts +++ b/packages/grafana-data/src/utils/location.test.ts @@ -1,3 +1,7 @@ +import { Location } from 'history'; + +import { GrafanaConfig } from '../types'; + import { locationUtil } from './location'; describe('locationUtil', () => { @@ -29,9 +33,9 @@ describe('locationUtil', () => { describe('when appSubUrl configured', () => { beforeEach(() => { locationUtil.initialize({ - config: { appSubUrl: '/subUrl' } as any, - getVariablesUrlParams: (() => {}) as any, - getTimeRangeForUrl: (() => {}) as any, + config: { appSubUrl: '/subUrl' } as GrafanaConfig, + getVariablesUrlParams: jest.fn(), + getTimeRangeForUrl: jest.fn(), }); }); test('relative url', () => { @@ -65,9 +69,9 @@ describe('locationUtil', () => { describe('when appSubUrl not configured', () => { beforeEach(() => { locationUtil.initialize({ - config: {} as any, - getVariablesUrlParams: (() => {}) as any, - getTimeRangeForUrl: (() => {}) as any, + config: {} as GrafanaConfig, + getVariablesUrlParams: jest.fn(), + getTimeRangeForUrl: jest.fn(), }); }); @@ -115,12 +119,102 @@ describe('locationUtil', () => { }); }); + describe('getUrlForPartial', () => { + const mockLocation: Location = { + hash: '', + pathname: '/', + search: '', + state: {}, + }; + describe('when appSubUrl is not configured', () => { + beforeEach(() => { + locationUtil.initialize({ + config: { + appSubUrl: '', + } as GrafanaConfig, + getVariablesUrlParams: jest.fn(), + getTimeRangeForUrl: jest.fn(), + }); + }); + + it('can add params', () => { + expect(locationUtil.getUrlForPartial(mockLocation, { forceLogin: 'true' })).toEqual('/?forceLogin=true'); + }); + + it('can remove params using undefined', () => { + expect( + locationUtil.getUrlForPartial( + { + ...mockLocation, + search: '?a=1', + }, + { a: undefined } + ) + ).toEqual('/'); + }); + + it('can remove params using null', () => { + expect( + locationUtil.getUrlForPartial( + { + ...mockLocation, + search: '?a=1', + }, + { a: null } + ) + ).toEqual('/'); + }); + }); + + describe('when appSubUrl is configured', () => { + beforeEach(() => { + locationUtil.initialize({ + config: { + appSubUrl: '/subpath', + } as GrafanaConfig, + getVariablesUrlParams: jest.fn(), + getTimeRangeForUrl: jest.fn(), + }); + }); + + it('can add params', () => { + expect(locationUtil.getUrlForPartial(mockLocation, { forceLogin: 'true' })).toEqual( + '/subpath/?forceLogin=true' + ); + }); + + it('can remove params using undefined', () => { + expect( + locationUtil.getUrlForPartial( + { + ...mockLocation, + search: '?a=1', + }, + { a: undefined } + ) + ).toEqual('/subpath/'); + }); + + it('can remove params using null', () => { + expect( + locationUtil.getUrlForPartial( + { + ...mockLocation, + search: '?a=1', + }, + { a: null } + ) + ).toEqual('/subpath/'); + }); + }); + }); + describe('updateSearchParams', () => { beforeEach(() => { locationUtil.initialize({ - config: {} as any, - getVariablesUrlParams: (() => {}) as any, - getTimeRangeForUrl: (() => {}) as any, + config: {} as GrafanaConfig, + getVariablesUrlParams: jest.fn(), + getTimeRangeForUrl: jest.fn(), }); }); diff --git a/packages/grafana-data/src/utils/location.ts b/packages/grafana-data/src/utils/location.ts index ee538687394..a0d70396f19 100644 --- a/packages/grafana-data/src/utils/location.ts +++ b/packages/grafana-data/src/utils/location.ts @@ -78,7 +78,7 @@ const getUrlForPartial = (location: Location, searchParamsToUpdate: Record< searchParams[key] = searchParamsToUpdate[key]; } } - return urlUtil.renderUrl(location.pathname, searchParams); + return assureBaseUrl(urlUtil.renderUrl(location.pathname, searchParams)); }; /**