From dc8dc7fd9c9cc1b4b2b868d3096725edf947b705 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 3 Apr 2020 10:33:09 +0200 Subject: [PATCH 1/8] fix createHref to prepend with scoped history basePath + add option to exclude it. --- ...na-plugin-core-public.scopedhistory.createhref.md | 4 ++-- .../kibana-plugin-core-public.scopedhistory.md | 2 +- .../data/server/kibana-plugin-plugins-data-server.md | 1 - src/core/public/application/scoped_history.test.ts | 12 +++++++++++- src/core/public/application/scoped_history.ts | 11 +++++++++-- src/core/public/public.api.md | 2 +- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md index 7058656d099477..c2db3567af10fa 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md @@ -4,10 +4,10 @@ ## ScopedHistory.createHref property -Creates an href (string) to the location. +Creates an href (string) to the location. If `prependBasePath` is true (default), it will prepend the location's path with the scoped history basePath. Signature: ```typescript -createHref: (location: LocationDescriptorObject) => string; +createHref: (location: LocationDescriptorObject, prependBasePath?: boolean) => string; ``` diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md index 5ea47d2090d714..1d772011b6436d 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md @@ -28,7 +28,7 @@ export declare class ScopedHistory implements Hi | --- | --- | --- | --- | | [action](./kibana-plugin-core-public.scopedhistory.action.md) | | Action | The last action dispatched on the history stack. | | [block](./kibana-plugin-core-public.scopedhistory.block.md) | | (prompt?: string | boolean | History.TransitionPromptHook<HistoryLocationState> | undefined) => UnregisterCallback | Not supported. Use [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md). | -| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | (location: LocationDescriptorObject<HistoryLocationState>) => string | Creates an href (string) to the location. | +| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | (location: LocationDescriptorObject<HistoryLocationState>, prependBasePath?: boolean) => string | Creates an href (string) to the location. If prependBasePath is true (default), it will prepend the location's path with the scoped history basePath. | | [createSubHistory](./kibana-plugin-core-public.scopedhistory.createsubhistory.md) | | <SubHistoryLocationState = unknown>(basePath: string) => ScopedHistory<SubHistoryLocationState> | Creates a ScopedHistory for a subpath of this ScopedHistory. Useful for applications that may have sub-apps that do not need access to the containing application's history. | | [go](./kibana-plugin-core-public.scopedhistory.go.md) | | (n: number) => void | Send the user forward or backwards in the history stack. | | [goBack](./kibana-plugin-core-public.scopedhistory.goback.md) | | () => void | Send the user one location back in the history stack. Equivalent to calling [ScopedHistory.go(-1)](./kibana-plugin-core-public.scopedhistory.go.md). If no more entries are available backwards, this is a no-op. | diff --git a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md index 259d725b3bf0d9..e756eb9b729050 100644 --- a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md +++ b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md @@ -23,7 +23,6 @@ | Function | Description | | --- | --- | | [getDefaultSearchParams(config)](./kibana-plugin-plugins-data-server.getdefaultsearchparams.md) | | -| [getTotalLoaded({ total, failed, successful })](./kibana-plugin-plugins-data-server.gettotalloaded.md) | | | [parseInterval(interval)](./kibana-plugin-plugins-data-server.parseinterval.md) | | | [plugin(initializerContext)](./kibana-plugin-plugins-data-server.plugin.md) | Static code to be shared externally | | [shouldReadFieldFromDocValues(aggregatable, esType)](./kibana-plugin-plugins-data-server.shouldreadfieldfromdocvalues.md) | | diff --git a/src/core/public/application/scoped_history.test.ts b/src/core/public/application/scoped_history.test.ts index c01eb50830516e..d5c330bc93e0df 100644 --- a/src/core/public/application/scoped_history.test.ts +++ b/src/core/public/application/scoped_history.test.ts @@ -268,8 +268,18 @@ describe('ScopedHistory', () => { const gh = createMemoryHistory(); gh.push('/app/wow'); const h = new ScopedHistory(gh, '/app/wow'); - expect(h.createHref({ pathname: '' })).toEqual(`/`); + expect(h.createHref({ pathname: '' })).toEqual(`/app/wow`); expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' })).toEqual( + `/app/wow/new-page?alpha=true` + ); + }); + + it('skips the scoped history path when `prependBasePath` is false', () => { + const gh = createMemoryHistory(); + gh.push('/app/wow'); + const h = new ScopedHistory(gh, '/app/wow'); + expect(h.createHref({ pathname: '' }, false)).toEqual(`/`); + expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' }, false)).toEqual( `/new-page?alpha=true` ); }); diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index c5febc7604feb0..93107307d8237c 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -219,12 +219,19 @@ export class ScopedHistory /** * Creates an href (string) to the location. + * If `prependBasePath` is true (default), it will prepend the location's path with the scoped history basePath. * * @param location + * @param prependBasePath */ - public createHref = (location: LocationDescriptorObject): Href => { + public createHref = ( + location: LocationDescriptorObject, + prependBasePath: boolean = true + ): Href => { this.verifyActive(); - return this.parentHistory.createHref(location); + return this.parentHistory.createHref( + prependBasePath ? this.prependBasePath(location) : location + ); }; private prependBasePath(path: Path): Path; diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 9f7f649f1e2a5b..53626a85de16a0 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -1195,7 +1195,7 @@ export class ScopedHistory implements History | undefined) => UnregisterCallback; - createHref: (location: LocationDescriptorObject) => string; + createHref: (location: LocationDescriptorObject, prependBasePath?: boolean) => string; createSubHistory: (basePath: string) => ScopedHistory; go: (n: number) => void; goBack: () => void; From 460abe1b321826e09bdf08eaa181b4cf0d0b3909 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 7 Apr 2020 10:33:18 +0200 Subject: [PATCH 2/8] fix prependBasePath behavior --- src/core/public/application/scoped_history.test.ts | 11 +++++++++++ src/core/public/application/scoped_history.ts | 5 ++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/core/public/application/scoped_history.test.ts b/src/core/public/application/scoped_history.test.ts index d5c330bc93e0df..d734a667844b86 100644 --- a/src/core/public/application/scoped_history.test.ts +++ b/src/core/public/application/scoped_history.test.ts @@ -269,6 +269,17 @@ describe('ScopedHistory', () => { gh.push('/app/wow'); const h = new ScopedHistory(gh, '/app/wow'); expect(h.createHref({ pathname: '' })).toEqual(`/app/wow`); + expect(h.createHref({})).toEqual(`/app/wow`); + expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' })).toEqual( + `/app/wow/new-page?alpha=true` + ); + }); + + it('behave correctly with slash-ending basePath', () => { + const gh = createMemoryHistory(); + gh.push('/app/wow/'); + const h = new ScopedHistory(gh, '/app/wow/'); + expect(h.createHref({ pathname: '' })).toEqual(`/app/wow/`); expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' })).toEqual( `/app/wow/new-page?alpha=true` ); diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 93107307d8237c..67e98ce8582529 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -252,7 +252,7 @@ export class ScopedHistory pathname: pathOrLocation.pathname !== undefined ? this.prependBasePathToString(pathOrLocation.pathname) - : undefined, + : this.basePath, }; } } @@ -261,8 +261,7 @@ export class ScopedHistory * Prepends the base path to string. */ private prependBasePathToString(path: string): string { - path = path.startsWith('/') ? path.slice(1) : path; - return path.length ? `${this.basePath}/${path}` : this.basePath; + return path.length ? `${this.basePath}/${path}`.replace(/\/{2,}/g, '/') : this.basePath; } /** From abd75e1b0298e93db51072dadb79e072f56f5715 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 7 Apr 2020 11:23:24 +0200 Subject: [PATCH 3/8] fix test plugins urls --- .../plugins/core_plugin_a/public/application.tsx | 2 +- .../plugins/core_plugin_b/public/application.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugin_functional/plugins/core_plugin_a/public/application.tsx b/test/plugin_functional/plugins/core_plugin_a/public/application.tsx index abea970749cbc0..159bb54f50903a 100644 --- a/test/plugin_functional/plugins/core_plugin_a/public/application.tsx +++ b/test/plugin_functional/plugins/core_plugin_a/public/application.tsx @@ -95,7 +95,7 @@ const Nav = withRouter(({ history, navigateToApp }: NavProps) => ( { id: 'home', name: 'Home', - onClick: () => history.push('/'), + onClick: () => history.push(''), 'data-test-subj': 'fooNavHome', }, { diff --git a/test/plugin_functional/plugins/core_plugin_b/public/application.tsx b/test/plugin_functional/plugins/core_plugin_b/public/application.tsx index 447307920c04c7..01a63f9782563e 100644 --- a/test/plugin_functional/plugins/core_plugin_b/public/application.tsx +++ b/test/plugin_functional/plugins/core_plugin_b/public/application.tsx @@ -102,7 +102,7 @@ const Nav = withRouter(({ history, navigateToApp }: NavProps) => ( { id: 'home', name: 'Home', - onClick: () => navigateToApp('bar', { path: '/' }), + onClick: () => navigateToApp('bar', { path: '' }), 'data-test-subj': 'barNavHome', }, { From 7604932b80c5537be7ec32fe1e9047af371d066b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 7 Apr 2020 23:01:56 +0200 Subject: [PATCH 4/8] add pathname to endpoint url builder methods --- .../applications/endpoint/view/alerts/url_from_query_params.ts | 1 + .../applications/endpoint/view/hosts/url_from_query_params.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts index e037d000e6e8f6..bc287eb2444eba 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts @@ -26,6 +26,7 @@ export function urlFromQueryParams( ): Partial { const search = querystring.stringify(queryParams); return { + pathname: '/alerts', search, }; } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts index 225aad8cab0206..0f6795d01f2e6f 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts @@ -12,6 +12,7 @@ export function urlFromQueryParams( ): Partial { const search = querystring.stringify(queryParams); return { + pathname: '/hosts', search, }; } From eff1faf76930c49f184a4febd0676de6853bafcf Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 7 Apr 2020 23:10:44 +0200 Subject: [PATCH 5/8] Revert "add pathname to endpoint url builder methods" This reverts commit 7604932b --- .../applications/endpoint/view/alerts/url_from_query_params.ts | 1 - .../applications/endpoint/view/hosts/url_from_query_params.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts index bc287eb2444eba..e037d000e6e8f6 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts @@ -26,7 +26,6 @@ export function urlFromQueryParams( ): Partial { const search = querystring.stringify(queryParams); return { - pathname: '/alerts', search, }; } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts index 0f6795d01f2e6f..225aad8cab0206 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/url_from_query_params.ts @@ -12,7 +12,6 @@ export function urlFromQueryParams( ): Partial { const search = querystring.stringify(queryParams); return { - pathname: '/hosts', search, }; } From e4b8eebd7050f8542af29611d4a2e891f730497f Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 7 Apr 2020 23:31:11 +0200 Subject: [PATCH 6/8] adapt createHref instead of prependBasePath --- src/core/public/application/scoped_history.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 67e98ce8582529..6c145dfff842de 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -229,9 +229,17 @@ export class ScopedHistory prependBasePath: boolean = true ): Href => { this.verifyActive(); - return this.parentHistory.createHref( - prependBasePath ? this.prependBasePath(location) : location - ); + if (prependBasePath) { + location = this.prependBasePath(location); + if (location.pathname === undefined) { + // we always want to create an url relative to the basePath + // so if pathname is not present, we use the history's basePath as default + // we are doing that here because `prependBasePath` should not + // alter pathname for other method calls + location.pathname = this.basePath; + } + } + return this.parentHistory.createHref(location); }; private prependBasePath(path: Path): Path; @@ -252,7 +260,7 @@ export class ScopedHistory pathname: pathOrLocation.pathname !== undefined ? this.prependBasePathToString(pathOrLocation.pathname) - : this.basePath, + : undefined, }; } } From 353277d5e831a19a83e8cfcddcf82605f7cb48d9 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 10 Apr 2020 10:29:09 +0200 Subject: [PATCH 7/8] use object options for createHref --- src/core/public/application/scoped_history.test.ts | 8 ++++---- src/core/public/application/scoped_history.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/public/application/scoped_history.test.ts b/src/core/public/application/scoped_history.test.ts index d734a667844b86..a56cffef1e2f27 100644 --- a/src/core/public/application/scoped_history.test.ts +++ b/src/core/public/application/scoped_history.test.ts @@ -289,10 +289,10 @@ describe('ScopedHistory', () => { const gh = createMemoryHistory(); gh.push('/app/wow'); const h = new ScopedHistory(gh, '/app/wow'); - expect(h.createHref({ pathname: '' }, false)).toEqual(`/`); - expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' }, false)).toEqual( - `/new-page?alpha=true` - ); + expect(h.createHref({ pathname: '' }, { prependBasePath: false })).toEqual(`/`); + expect( + h.createHref({ pathname: '/new-page', search: '?alpha=true' }, { prependBasePath: false }) + ).toEqual(`/new-page?alpha=true`); }); }); diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index 6c145dfff842de..9fa8f0b7f81481 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -226,7 +226,7 @@ export class ScopedHistory */ public createHref = ( location: LocationDescriptorObject, - prependBasePath: boolean = true + { prependBasePath = true }: { prependBasePath?: boolean } = {} ): Href => { this.verifyActive(); if (prependBasePath) { From 1fb7fc63b0ce1b0a5a478a147f1500e5cb2e7ac4 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 10 Apr 2020 10:33:15 +0200 Subject: [PATCH 8/8] update generated doc --- .../kibana-plugin-core-public.scopedhistory.createhref.md | 4 +++- .../core/public/kibana-plugin-core-public.scopedhistory.md | 2 +- src/core/public/public.api.md | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md index c2db3567af10fa..6bbab43ff6ffcb 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.createhref.md @@ -9,5 +9,7 @@ Creates an href (string) to the location. If `prependBasePath` is true (default) Signature: ```typescript -createHref: (location: LocationDescriptorObject, prependBasePath?: boolean) => string; +createHref: (location: LocationDescriptorObject, { prependBasePath }?: { + prependBasePath?: boolean | undefined; + }) => string; ``` diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md index 1d772011b6436d..fa29b32c0bafc1 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md @@ -28,7 +28,7 @@ export declare class ScopedHistory implements Hi | --- | --- | --- | --- | | [action](./kibana-plugin-core-public.scopedhistory.action.md) | | Action | The last action dispatched on the history stack. | | [block](./kibana-plugin-core-public.scopedhistory.block.md) | | (prompt?: string | boolean | History.TransitionPromptHook<HistoryLocationState> | undefined) => UnregisterCallback | Not supported. Use [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md). | -| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | (location: LocationDescriptorObject<HistoryLocationState>, prependBasePath?: boolean) => string | Creates an href (string) to the location. If prependBasePath is true (default), it will prepend the location's path with the scoped history basePath. | +| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | (location: LocationDescriptorObject<HistoryLocationState>, { prependBasePath }?: {
prependBasePath?: boolean | undefined;
}) => string | Creates an href (string) to the location. If prependBasePath is true (default), it will prepend the location's path with the scoped history basePath. | | [createSubHistory](./kibana-plugin-core-public.scopedhistory.createsubhistory.md) | | <SubHistoryLocationState = unknown>(basePath: string) => ScopedHistory<SubHistoryLocationState> | Creates a ScopedHistory for a subpath of this ScopedHistory. Useful for applications that may have sub-apps that do not need access to the containing application's history. | | [go](./kibana-plugin-core-public.scopedhistory.go.md) | | (n: number) => void | Send the user forward or backwards in the history stack. | | [goBack](./kibana-plugin-core-public.scopedhistory.goback.md) | | () => void | Send the user one location back in the history stack. Equivalent to calling [ScopedHistory.go(-1)](./kibana-plugin-core-public.scopedhistory.go.md). If no more entries are available backwards, this is a no-op. | diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index b8a3720b6a5e2a..6d95d1bc7069c5 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -1196,7 +1196,9 @@ export class ScopedHistory implements History | undefined) => UnregisterCallback; - createHref: (location: LocationDescriptorObject, prependBasePath?: boolean) => string; + createHref: (location: LocationDescriptorObject, { prependBasePath }?: { + prependBasePath?: boolean | undefined; + }) => string; createSubHistory: (basePath: string) => ScopedHistory; go: (n: number) => void; goBack: () => void;