From 2e10933cdf313678ac66c6f485db7901637a02ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Tue, 29 Oct 2024 10:50:23 +0000 Subject: [PATCH] Use TS type for solution id instead of string --- .../src/chrome_service.tsx | 6 ++- .../project_navigation_service.test.ts | 48 +++++++++---------- .../project_navigation_service.ts | 13 ++--- .../core-chrome-browser-internal/src/types.ts | 5 +- .../core/chrome/core-chrome-browser/index.ts | 1 + .../chrome/core-chrome-browser/src/index.ts | 1 + .../src/project_navigation.ts | 10 ++-- src/plugins/navigation/public/plugin.test.ts | 2 +- src/plugins/navigation/public/plugin.tsx | 6 +-- x-pack/plugins/serverless/public/types.ts | 3 +- .../serverless_search/public/plugin.ts | 2 +- 11 files changed, 54 insertions(+), 43 deletions(-) diff --git a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx index 5d86209ec8800..434639b07efdf 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx +++ b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx @@ -36,6 +36,7 @@ import type { ChromeSetProjectBreadcrumbsParams, NavigationTreeDefinition, AppDeepLinkId, + SolutionId, } from '@kbn/core-chrome-browser'; import type { CustomBrandingStart } from '@kbn/core-custom-branding-browser'; import type { @@ -343,7 +344,10 @@ export class ChromeService { LinkId extends AppDeepLinkId = AppDeepLinkId, Id extends string = string, ChildrenId extends string = Id - >(id: string, navigationTree$: Observable>) { + >( + id: SolutionId, + navigationTree$: Observable> + ) { validateChromeStyle(); projectNavigation.initNavigation(id, navigationTree$); } diff --git a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts index d1be94aad246a..124b44e80e30f 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.test.ts @@ -110,7 +110,7 @@ describe('initNavigation()', () => { beforeAll(() => { projectNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -185,7 +185,7 @@ describe('initNavigation()', () => { const { projectNavigation: projNavigation, getNavigationTree: getNavTree } = setupInitNavigation(); projNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -210,7 +210,7 @@ describe('initNavigation()', () => { const { projectNavigation: projNavigation } = setupInitNavigation(); projNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -399,7 +399,7 @@ describe('initNavigation()', () => { // 2. initNavigation() is called projectNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -427,7 +427,7 @@ describe('initNavigation()', () => { }); projectNavigation.initNavigation( - 'foo', + 'es', // @ts-expect-error - We pass a non valid cloudLink that is not TS valid of({ body: [ @@ -533,7 +533,7 @@ describe('breadcrumbs', () => { const obs = subj.asObservable(); if (initiateNavigation) { - projectNavigation.initNavigation('foo', obs); + projectNavigation.initNavigation('es', obs); } return { @@ -740,7 +740,7 @@ describe('breadcrumbs', () => { { text: 'custom1', href: '/custom1' }, { text: 'custom2', href: '/custom1/custom2' }, ]); - projectNavigation.initNavigation('foo', of(mockNavigation)); // init navigation + projectNavigation.initNavigation('es', of(mockNavigation)); // init navigation const breadcrumbs = await firstValueFrom(projectNavigation.getProjectBreadcrumbs$()); expect(breadcrumbs).toHaveLength(4); @@ -779,7 +779,7 @@ describe('getActiveNodes$()', () => { expect(activeNodes).toEqual([]); projectNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -835,7 +835,7 @@ describe('getActiveNodes$()', () => { expect(activeNodes).toEqual([]); projectNavigation.initNavigation( - 'foo', + 'es', of({ body: [ { @@ -889,7 +889,7 @@ describe('getActiveNodes$()', () => { describe('solution navigations', () => { const solution1: SolutionNavigationDefinition = { - id: 'solution1', + id: 'es', title: 'Solution 1', icon: 'logoSolution1', homePage: 'discover', @@ -897,7 +897,7 @@ describe('solution navigations', () => { }; const solution2: SolutionNavigationDefinition = { - id: 'solution2', + id: 'oblt', title: 'Solution 2', icon: 'logoSolution2', homePage: 'app2', @@ -906,7 +906,7 @@ describe('solution navigations', () => { }; const solution3: SolutionNavigationDefinition = { - id: 'solution3', + id: 'security', title: 'Solution 3', icon: 'logoSolution3', homePage: 'discover', @@ -943,30 +943,30 @@ describe('solution navigations', () => { } { - projectNavigation.updateSolutionNavigations({ 1: solution1, 2: solution2 }); + projectNavigation.updateSolutionNavigations({ es: solution1, oblt: solution2 }); const solutionNavs = await lastValueFrom( projectNavigation.getSolutionsNavDefinitions$().pipe(take(1)) ); - expect(solutionNavs).toEqual({ 1: solution1, 2: solution2 }); + expect(solutionNavs).toEqual({ es: solution1, oblt: solution2 }); } { // Test partial update - projectNavigation.updateSolutionNavigations({ 3: solution3 }, false); + projectNavigation.updateSolutionNavigations({ security: solution3 }, false); const solutionNavs = await lastValueFrom( projectNavigation.getSolutionsNavDefinitions$().pipe(take(1)) ); - expect(solutionNavs).toEqual({ 1: solution1, 2: solution2, 3: solution3 }); + expect(solutionNavs).toEqual({ es: solution1, oblt: solution2, security: solution3 }); } { // Test full replacement - projectNavigation.updateSolutionNavigations({ 4: solution3 }, true); + projectNavigation.updateSolutionNavigations({ security: solution3 }, true); const solutionNavs = await lastValueFrom( projectNavigation.getSolutionsNavDefinitions$().pipe(take(1)) ); - expect(solutionNavs).toEqual({ 4: solution3 }); + expect(solutionNavs).toEqual({ security: solution3 }); } }); @@ -980,8 +980,8 @@ describe('solution navigations', () => { expect(activeSolution).toBeNull(); } - projectNavigation.changeActiveSolutionNavigation('2'); // Set **before** the navs are registered - projectNavigation.updateSolutionNavigations({ 1: solution1, 2: solution2 }); + projectNavigation.changeActiveSolutionNavigation('oblt'); // Set **before** the navs are registered + projectNavigation.updateSolutionNavigations({ es: solution1, oblt: solution2 }); { const activeSolution = await lastValueFrom( @@ -994,7 +994,7 @@ describe('solution navigations', () => { expect(activeSolution).toEqual(rest); } - projectNavigation.changeActiveSolutionNavigation('1'); // Set **after** the navs are registered + projectNavigation.changeActiveSolutionNavigation('es'); // Set **after** the navs are registered { const activeSolution = await lastValueFrom( @@ -1027,7 +1027,7 @@ describe('solution navigations', () => { { const fooSolution: SolutionNavigationDefinition = { - id: 'fooSolution', + id: 'es', title: 'Foo solution', icon: 'logoSolution', homePage: 'discover', @@ -1053,8 +1053,8 @@ describe('solution navigations', () => { }), }; - projectNavigation.changeActiveSolutionNavigation('foo'); - projectNavigation.updateSolutionNavigations({ foo: fooSolution }); + projectNavigation.changeActiveSolutionNavigation('es'); + projectNavigation.updateSolutionNavigations({ es: fooSolution }); projectNavigation.setPanelSelectedNode('link2'); // Set the selected node using its id diff --git a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.ts b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.ts index 85c3fd1905adb..62b771d6a9d05 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.ts @@ -17,6 +17,7 @@ import type { NavigationTreeDefinition, SolutionNavigationDefinitions, CloudLinks, + SolutionId, } from '@kbn/core-chrome-browser'; import type { InternalHttpStart } from '@kbn/core-http-browser-internal'; import { @@ -86,9 +87,9 @@ export class ProjectNavigationService { private readonly solutionNavDefinitions$ = new BehaviorSubject({}); // As the active definition **id** and the definitions are set independently, one before the other without // any guarantee of order, we need to store the next active definition id in a separate BehaviorSubject - private readonly nextSolutionNavDefinitionId$ = new BehaviorSubject(null); + private readonly nextSolutionNavDefinitionId$ = new BehaviorSubject(null); // The active solution navigation definition id that has been initiated and is currently active - private readonly activeSolutionNavDefinitionId$ = new BehaviorSubject(null); + private readonly activeSolutionNavDefinitionId$ = new BehaviorSubject(null); private readonly location$ = new BehaviorSubject(createLocation('/')); private deepLinksMap$: Observable> = of({}); private cloudLinks$ = new BehaviorSubject({}); @@ -138,7 +139,7 @@ export class ProjectNavigationService { return this.projectName$.asObservable(); }, initNavigation: ( - id: string, + id: SolutionId, navTreeDefinition$: Observable> ) => { this.initNavigation(id, navTreeDefinition$); @@ -202,7 +203,7 @@ export class ProjectNavigationService { * @param id Id for the navigation tree definition * @param navTreeDefinition$ The navigation tree definition */ - private initNavigation(id: string, navTreeDefinition$: Observable) { + private initNavigation(id: SolutionId, navTreeDefinition$: Observable) { if (this.activeSolutionNavDefinitionId$.getValue() === id) return; if (this.navigationChangeSubscription) { @@ -382,7 +383,7 @@ export class ProjectNavigationService { this.projectHome$.next(homeHref); } - private changeActiveSolutionNavigation(id: string | null) { + private changeActiveSolutionNavigation(id: SolutionId | null) { if (this.nextSolutionNavDefinitionId$.getValue() === id) return; this.nextSolutionNavDefinitionId$.next(id); } @@ -400,7 +401,7 @@ export class ProjectNavigationService { if (!definitions[id]) return null; // We strip out the sideNavComponent from the definition as it should only be used internally - const { sideNavComponent, ...definition } = definitions[id]; + const { sideNavComponent, ...definition } = definitions[id]!; return definition; }) ); diff --git a/packages/core/chrome/core-chrome-browser-internal/src/types.ts b/packages/core/chrome/core-chrome-browser-internal/src/types.ts index 0e6bec4d2678c..36a247e22f847 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/types.ts +++ b/packages/core/chrome/core-chrome-browser-internal/src/types.ts @@ -18,6 +18,7 @@ import type { NavigationTreeDefinitionUI, CloudURLs, SolutionNavigationDefinitions, + SolutionId, } from '@kbn/core-chrome-browser'; import type { Observable } from 'rxjs'; @@ -66,7 +67,7 @@ export interface InternalChromeStart extends ChromeStart { Id extends string = string, ChildrenId extends string = Id >( - id: string, + id: SolutionId, navigationTree$: Observable> ): void; @@ -117,6 +118,6 @@ export interface InternalChromeStart extends ChromeStart { * @param id The id of the active solution navigation. If `null` is provided, the solution navigation * will be replaced with the legacy Kibana navigation. */ - changeActiveSolutionNavigation(id: string | null): void; + changeActiveSolutionNavigation(id: SolutionId | null): void; }; } diff --git a/packages/core/chrome/core-chrome-browser/index.ts b/packages/core/chrome/core-chrome-browser/index.ts index afb2050d12e80..7b8658791340f 100644 --- a/packages/core/chrome/core-chrome-browser/index.ts +++ b/packages/core/chrome/core-chrome-browser/index.ts @@ -60,4 +60,5 @@ export type { SolutionNavigationDefinitions, EuiSideNavItemTypeEnhanced, RenderAs, + SolutionId, } from './src'; diff --git a/packages/core/chrome/core-chrome-browser/src/index.ts b/packages/core/chrome/core-chrome-browser/src/index.ts index efc2fb5636d84..efc709ff512da 100644 --- a/packages/core/chrome/core-chrome-browser/src/index.ts +++ b/packages/core/chrome/core-chrome-browser/src/index.ts @@ -38,6 +38,7 @@ export type { PanelSelectedNode, AppDeepLinkId, AppId, + SolutionId, CloudLinkId, CloudLink, CloudLinks, diff --git a/packages/core/chrome/core-chrome-browser/src/project_navigation.ts b/packages/core/chrome/core-chrome-browser/src/project_navigation.ts index 3e6afeb8f6117..7e9a6e5acba56 100644 --- a/packages/core/chrome/core-chrome-browser/src/project_navigation.ts +++ b/packages/core/chrome/core-chrome-browser/src/project_navigation.ts @@ -42,6 +42,8 @@ import type { AppId as SharedApp, DeepLinkId as SharedLink } from '@kbn/deeplink import type { ChromeNavLink } from './nav_links'; import type { ChromeRecentlyAccessedHistoryItem } from './recently_accessed'; +export type SolutionId = 'es' | 'oblt' | 'security'; + /** @public */ export type AppId = | DevToolsApp @@ -429,7 +431,7 @@ export interface NavigationTreeDefinitionUI { export interface SolutionNavigationDefinition { /** Unique id for the solution navigation. */ - id: string; + id: SolutionId; /** Title for the solution navigation. */ title: string; /** The navigation tree definition */ @@ -442,9 +444,9 @@ export interface SolutionNavigationDefinition { await new Promise((resolve) => setTimeout(resolve)); const definition = { - id: 'es', + id: 'es' as const, title: 'Elasticsearch', navigationTree$: of({ body: [] }), }; diff --git a/src/plugins/navigation/public/plugin.tsx b/src/plugins/navigation/public/plugin.tsx index 9f41405a8438b..e7264efdf51d2 100644 --- a/src/plugins/navigation/public/plugin.tsx +++ b/src/plugins/navigation/public/plugin.tsx @@ -18,7 +18,7 @@ import { } from '@kbn/core/public'; import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public'; import type { Space } from '@kbn/spaces-plugin/public'; -import type { SolutionNavigationDefinition } from '@kbn/core-chrome-browser'; +import type { SolutionId, SolutionNavigationDefinition } from '@kbn/core-chrome-browser'; import { InternalChromeStart } from '@kbn/core-chrome-browser-internal'; import type { PanelContentProvider } from '@kbn/shared-ux-chrome-navigation'; import type { @@ -195,7 +195,7 @@ export class NavigationPublicPlugin } } - if (isProjectNav) { + if (isProjectNav && solutionView !== 'classic') { chrome.project.changeActiveSolutionNavigation(solutionView!); } } @@ -210,6 +210,6 @@ function getIsProjectNav(solutionView?: string) { return Boolean(solutionView) && isKnownSolutionView(solutionView); } -function isKnownSolutionView(solution?: string) { +function isKnownSolutionView(solution?: string): solution is SolutionId { return Boolean(solution) && ['oblt', 'es', 'security'].includes(solution!); } diff --git a/x-pack/plugins/serverless/public/types.ts b/x-pack/plugins/serverless/public/types.ts index 4627d24659b8e..3a416a676ee92 100644 --- a/x-pack/plugins/serverless/public/types.ts +++ b/x-pack/plugins/serverless/public/types.ts @@ -10,6 +10,7 @@ import type { ChromeSetProjectBreadcrumbsParams, SideNavComponent, NavigationTreeDefinition, + SolutionId, } from '@kbn/core-chrome-browser'; import type { CloudSetup, CloudStart } from '@kbn/cloud-plugin/public'; import type { Observable } from 'rxjs'; @@ -26,7 +27,7 @@ export interface ServerlessPluginStart { ) => void; setProjectHome(homeHref: string): void; initNavigation( - id: string, + id: SolutionId, navigationTree$: Observable, config?: { dataTestSubj?: string; diff --git a/x-pack/plugins/serverless_search/public/plugin.ts b/x-pack/plugins/serverless_search/public/plugin.ts index 491252a6d9e9f..3d246e4be2929 100644 --- a/x-pack/plugins/serverless_search/public/plugin.ts +++ b/x-pack/plugins/serverless_search/public/plugin.ts @@ -149,7 +149,7 @@ export class ServerlessSearchPlugin serverless.setProjectHome(services.searchIndices.startRoute); const navigationTree$ = of(navigationTree()); - serverless.initNavigation('search', navigationTree$, { dataTestSubj: 'svlSearchSideNav' }); + serverless.initNavigation('es', navigationTree$, { dataTestSubj: 'svlSearchSideNav' }); const extendCardNavDefinitions = serverless.getNavigationCards( security.authz.isRoleManagementEnabled()