From 90e7d45ebc8ffd5e963ecede4018eb676136ddf3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 19 Jun 2024 17:57:16 +0800 Subject: [PATCH 01/35] feat: introduce new interface for use case Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 10 +++ src/core/public/chrome/chrome_service.test.ts | 85 ++++++++++++++++++- src/core/public/chrome/chrome_service.tsx | 56 +++++++++++- src/core/public/chrome/index.ts | 2 + src/core/public/index.ts | 4 + 5 files changed, 154 insertions(+), 3 deletions(-) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 6ec765a3bb0b..1f6bd78e75ca 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -112,6 +112,16 @@ Gets an Observable of the array of recently accessed history :- chrome.docTitle.change('My application title') chrome.docTitle.change(['My application', 'My section']) ``` +### UseCaseService: +- Interface : ChromeUseCase +- Methods : +Register a use case :- + +`registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => void;` + +Gets an Observable of the array of registered use cases :- + +`getUseCases$: Observable>` ### UI : ###### consists of tsx/scss files && renders UI components from css Library e.g `````` diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 154f07caf46c..44e87f23b06d 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -31,7 +31,7 @@ import { shallow } from 'enzyme'; import React from 'react'; import * as Rx from 'rxjs'; -import { take, toArray } from 'rxjs/operators'; +import { first, take, toArray } from 'rxjs/operators'; import { App, PublicAppInfo } from '../application'; import { applicationServiceMock } from '../application/application_service.mock'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; @@ -39,7 +39,7 @@ import { httpServiceMock } from '../http/http_service.mock'; import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; -import { ChromeService } from './chrome_service'; +import { ChromeRegistrationNavLink, ChromeService, ChromeUseCase } from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -110,6 +110,27 @@ afterAll(() => { (window as any).localStorage = originalLocalStorage; }); +const mockedUseCaseFoo: ChromeUseCase = { + id: 'foo', + title: 'foo', + description: 'foo', +}; + +const mockedUseCaseBar: ChromeUseCase = { + id: 'bar', + title: 'bar', + description: 'bar', +}; + +const mockedNavLinkFoo: ChromeRegistrationNavLink = { + id: 'foo', +}; + +const mockedNavLinkBar: ChromeRegistrationNavLink = { + id: 'bar', + title: 'bar', +}; + describe('setup', () => { afterEach(() => { jest.restoreAllMocks(); @@ -146,6 +167,45 @@ describe('setup', () => { '[ChromeService] An existing custom collapsible navigation bar header render has been overridden.' ); }); + + it('should be able registerNavLink', async () => { + const warnMock = jest.fn(); + jest.spyOn(console, 'warn').mockImplementation(warnMock); + const chrome = new ChromeService({ browserSupportsCsp: true }); + + const chromeSetup = chrome.setup(); + + chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); + chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); + chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkBar); + const useCaseList = await chromeSetup.getUseCases$().pipe(first()).toPromise(); + expect(useCaseList.length).toEqual(2); + expect(useCaseList[0].navLinks.length).toEqual(2); + expect(useCaseList[1].navLinks.length).toEqual(1); + expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + expect(warnMock).toBeCalledTimes(0); + }); + + it('should output warning message if registerNavLink with same use case id and navLink id', async () => { + const warnMock = jest.fn(); + jest.spyOn(console, 'warn').mockImplementation(warnMock); + const chrome = new ChromeService({ browserSupportsCsp: true }); + + const chromeSetup = chrome.setup(); + + chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); + chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); + chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); + const useCaseList = await chromeSetup.getUseCases$().pipe(first()).toPromise(); + expect(useCaseList.length).toEqual(2); + expect(useCaseList[0].navLinks.length).toEqual(1); + expect(useCaseList[1].navLinks.length).toEqual(1); + expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + expect(warnMock).toBeCalledTimes(1); + expect(warnMock).toBeCalledWith( + `[ChromeService] Navlink of ${mockedUseCaseFoo.id} has already been registered in use case ${mockedUseCaseFoo.id}` + ); + }); }); describe('start', () => { @@ -485,6 +545,27 @@ describe('start', () => { `); }); }); + + describe('use case', () => { + it('should be able to get the use cases registered through registerNavLinks', async () => { + const startDeps = defaultStartDeps([]); + const chrome = new ChromeService({ browserSupportsCsp: true }); + + const chromeSetup = chrome.setup(); + + chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); + chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); + + const chromeStart = await chrome.start(startDeps); + + const useCaseList = await chromeStart.getUseCases$().pipe(first()).toPromise(); + + expect(useCaseList.length).toEqual(2); + expect(useCaseList[0].navLinks.length).toEqual(1); + expect(useCaseList[1].navLinks.length).toEqual(1); + expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + }); + }); }); describe('stop', () => { diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 2660b4768839..9e339987d060 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -111,6 +111,9 @@ export class ChromeService { private readonly navLinks = new NavLinksService(); private readonly recentlyAccessed = new RecentlyAccessedService(); private readonly docTitle = new DocTitleService(); + private readonly useCaseList$ = new BehaviorSubject< + Array + >([]); private collapsibleNavHeaderRender?: CollapsibleNavHeaderRender; constructor(private readonly params: ConstructorParams) {} @@ -147,7 +150,7 @@ export class ChromeService { ); } - public setup() { + public setup(): ChromeSetup { return { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => { if (this.collapsibleNavHeaderRender) { @@ -158,6 +161,29 @@ export class ChromeService { } this.collapsibleNavHeaderRender = render; }, + registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => { + const currentUseCaseList = [...this.useCaseList$.getValue()]; + const findItem = currentUseCaseList.find((item) => item.id === useCase.id); + if (findItem) { + const findNavLinks = findItem.navLinks.find((link) => link.id === navLink.id); + if (findNavLinks) { + // eslint-disable-next-line no-console + console.warn( + `[ChromeService] Navlink of ${navLink.id} has already been registered in use case ${useCase.id}` + ); + return; + } + findItem.navLinks.push(navLink); + } else { + currentUseCaseList.push({ + ...useCase, + navLinks: [navLink], + }); + } + + this.useCaseList$.next(currentUseCaseList); + }, + getUseCases$: () => this.useCaseList$.pipe(takeUntil(this.stop$)), }; } @@ -339,6 +365,8 @@ export class ChromeService { setCustomNavLink: (customNavLink?: ChromeNavLink) => { customNavLink$.next(customNavLink); }, + + getUseCases$: () => this.useCaseList$.pipe(takeUntil(this.stop$)), }; } @@ -360,6 +388,14 @@ export class ChromeService { */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; + registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => void; + getUseCases$: () => Observable< + Array< + ChromeUseCase & { + navLinks: ChromeRegistrationNavLink[]; + } + > + >; } /** @@ -486,6 +522,8 @@ export interface ChromeStart { * Get an observable of the current locked state of the nav drawer. */ getIsNavDrawerLocked$(): Observable; + + getUseCases$: ChromeSetup['getUseCases$']; } /** @internal */ @@ -496,3 +534,19 @@ export interface InternalChromeStart extends ChromeStart { */ getHeaderComponent(): JSX.Element; } + +/** @public */ +export interface ChromeUseCase { + id: string; + title: string; + description: string; + order?: number; + icon?: string; + reserved?: boolean; // use cases like data administration / settings and setup is not configurable through workspace +} + +/** @public */ +export interface ChromeRegistrationNavLink + extends Omit { + title?: ChromeNavLink['title']; +} diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index eb92ccdc6ba3..9e08c0434ac7 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -36,6 +36,8 @@ export { ChromeStart, InternalChromeStart, ChromeHelpExtension, + ChromeUseCase, + ChromeRegistrationNavLink, } from './chrome_service'; export { ChromeHelpExtensionMenuLink, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 10d1928d6cf2..bb76efa08da7 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -71,6 +71,8 @@ import { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, + ChromeUseCase, + ChromeRegistrationNavLink, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; import { HttpSetup, HttpStart } from './http'; @@ -366,6 +368,8 @@ export { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, + ChromeUseCase, + ChromeRegistrationNavLink, }; export { __osdBootstrap__ } from './osd_bootstrap'; From 2fdc8b2a7a26811e4f2e22d0fa7f803d3b9b4be5 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Wed, 19 Jun 2024 10:01:13 +0000 Subject: [PATCH 02/35] Changeset file for PR #7060 created/updated --- changelogs/fragments/7060.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7060.yml diff --git a/changelogs/fragments/7060.yml b/changelogs/fragments/7060.yml new file mode 100644 index 000000000000..025da8b6d14a --- /dev/null +++ b/changelogs/fragments/7060.yml @@ -0,0 +1,2 @@ +feat: +- Introduce new interface for use case ([#7060](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7060)) \ No newline at end of file From ac685babf08caacc8f6dd456079094b49881f37f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 19 Jun 2024 18:27:57 +0800 Subject: [PATCH 03/35] feat: introduce new interface for use case Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index b6ce429528a7..5cd096e9ffa1 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -36,6 +36,7 @@ import { getLogosMock } from '../../common/mocks'; const createSetupContractMock = () => { return { registerCollapsibleNavHeader: jest.fn(), + registerNavLink: jest.fn(), }; }; @@ -86,6 +87,7 @@ const createStartContractMock = () => { getIsNavDrawerLocked$: jest.fn(), getCustomNavLink$: jest.fn(), setCustomNavLink: jest.fn(), + getUseCases$: jest.fn(), }; startContract.navLinks.getAll.mockReturnValue([]); startContract.getIsVisible$.mockReturnValue(new BehaviorSubject(false)); @@ -95,6 +97,7 @@ const createStartContractMock = () => { startContract.getCustomNavLink$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getHelpExtension$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getIsNavDrawerLocked$.mockReturnValue(new BehaviorSubject(false)); + startContract.getUseCases$.mockReturnValue(new BehaviorSubject([])); return startContract; }; From 9e2fdac0b250461242a77ef180eea01d40b795b9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 19 Jun 2024 18:36:31 +0800 Subject: [PATCH 04/35] feat: introduce new interface for use case Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index 5cd096e9ffa1..31aef7d60ad0 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -37,6 +37,7 @@ const createSetupContractMock = () => { return { registerCollapsibleNavHeader: jest.fn(), registerNavLink: jest.fn(), + getUseCases$: () => new BehaviorSubject([]), }; }; From 385ac49933aebc00f46425c437ecdf7ea9ae8be7 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 10:12:43 +0800 Subject: [PATCH 05/35] feat: introduce new interface for use case Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index 31aef7d60ad0..8ec35b0ae6ed 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -34,10 +34,12 @@ import { ChromeBadge, ChromeBreadcrumb, ChromeService, InternalChromeStart } fro import { getLogosMock } from '../../common/mocks'; const createSetupContractMock = () => { + const getUserCaseMock = jest.fn(); + getUserCaseMock.mockReturnValue(new BehaviorSubject([])); return { registerCollapsibleNavHeader: jest.fn(), registerNavLink: jest.fn(), - getUseCases$: () => new BehaviorSubject([]), + getUseCases$: getUserCaseMock, }; }; From b672b60a032c835e8cff697ab1519125132d50dd Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 10:40:23 +0800 Subject: [PATCH 06/35] fix: update test snapshot Signed-off-by: SuZhou-Joe --- .../__snapshots__/dashboard_listing.test.tsx.snap | 5 +++++ .../__snapshots__/dashboard_top_nav.test.tsx.snap | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap index f4c5ca0b0a0a..9e4896623342 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -148,6 +148,7 @@ exports[`dashboard listing hideWriteControls 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1322,6 +1323,7 @@ exports[`dashboard listing render table listing with initial filters from URL 1` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2557,6 +2559,7 @@ exports[`dashboard listing renders call to action when no dashboards exist 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3792,6 +3795,7 @@ exports[`dashboard listing renders table rows 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5027,6 +5031,7 @@ exports[`dashboard listing renders warning when listingLimit is exceeded 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 831eb5bd61c0..4c9933489483 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -136,6 +136,7 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1136,6 +1137,7 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2136,6 +2138,7 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3136,6 +3139,7 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -4136,6 +4140,7 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5136,6 +5141,7 @@ exports[`Dashboard top nav render with all components 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], + "getUseCases$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { From 3584dcd530af2ffb58c1c60472dd64795ce11f6e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 13:31:57 +0800 Subject: [PATCH 07/35] fix: update based on comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 31 +++++--- src/core/public/chrome/chrome_service.mock.ts | 12 ++-- src/core/public/chrome/chrome_service.test.ts | 59 +++++++-------- src/core/public/chrome/chrome_service.tsx | 71 ++++++++++--------- src/core/public/chrome/index.ts | 2 +- src/core/public/index.ts | 4 +- .../dashboard_listing.test.tsx.snap | 10 +-- .../dashboard_top_nav.test.tsx.snap | 12 ++-- 8 files changed, 108 insertions(+), 93 deletions(-) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 1f6bd78e75ca..03393bd41c27 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -112,16 +112,31 @@ Gets an Observable of the array of recently accessed history :- chrome.docTitle.change('My application title') chrome.docTitle.change(['My application', 'My section']) ``` -### UseCaseService: -- Interface : ChromeUseCase +### GroupService: +- Interface : ChromeNavGroup - Methods : -Register a use case :- - -`registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => void;` - -Gets an Observable of the array of registered use cases :- +Register a group :- + +`addNavToGroup: (group: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => void;` + +Gets an Observable of the array of registered groups :- + +`getGroupsMap$: Observable>` +##### Register a new group with a navLink + + ```ts + chrome.addNavToGroup( + { + id: 'my-group', + title: 'A demo group', + description: 'description for demo group' + }, + { + id: 'nav' + } + ) + ``` -`getUseCases$: Observable>` ### UI : ###### consists of tsx/scss files && renders UI components from css Library e.g `````` diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index 8ec35b0ae6ed..d67a2b8bceba 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -34,12 +34,12 @@ import { ChromeBadge, ChromeBreadcrumb, ChromeService, InternalChromeStart } fro import { getLogosMock } from '../../common/mocks'; const createSetupContractMock = () => { - const getUserCaseMock = jest.fn(); - getUserCaseMock.mockReturnValue(new BehaviorSubject([])); + const getGroupsMapMock = jest.fn(); + getGroupsMapMock.mockReturnValue(new BehaviorSubject({})); return { registerCollapsibleNavHeader: jest.fn(), - registerNavLink: jest.fn(), - getUseCases$: getUserCaseMock, + addNavToGroup: jest.fn(), + getGroupsMap$: getGroupsMapMock, }; }; @@ -90,7 +90,7 @@ const createStartContractMock = () => { getIsNavDrawerLocked$: jest.fn(), getCustomNavLink$: jest.fn(), setCustomNavLink: jest.fn(), - getUseCases$: jest.fn(), + getGroupsMap$: jest.fn(), }; startContract.navLinks.getAll.mockReturnValue([]); startContract.getIsVisible$.mockReturnValue(new BehaviorSubject(false)); @@ -100,7 +100,7 @@ const createStartContractMock = () => { startContract.getCustomNavLink$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getHelpExtension$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getIsNavDrawerLocked$.mockReturnValue(new BehaviorSubject(false)); - startContract.getUseCases$.mockReturnValue(new BehaviorSubject([])); + startContract.getGroupsMap$.mockReturnValue(new BehaviorSubject({})); return startContract; }; diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 44e87f23b06d..e609b349a675 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -39,7 +39,7 @@ import { httpServiceMock } from '../http/http_service.mock'; import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; -import { ChromeRegistrationNavLink, ChromeService, ChromeUseCase } from './chrome_service'; +import { ChromeRegistrationNavLink, ChromeService, ChromeNavGroup } from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -110,13 +110,13 @@ afterAll(() => { (window as any).localStorage = originalLocalStorage; }); -const mockedUseCaseFoo: ChromeUseCase = { +const mockedGroupFoo: ChromeNavGroup = { id: 'foo', title: 'foo', description: 'foo', }; -const mockedUseCaseBar: ChromeUseCase = { +const mockedGroupBar: ChromeNavGroup = { id: 'bar', title: 'bar', description: 'bar', @@ -128,7 +128,6 @@ const mockedNavLinkFoo: ChromeRegistrationNavLink = { const mockedNavLinkBar: ChromeRegistrationNavLink = { id: 'bar', - title: 'bar', }; describe('setup', () => { @@ -168,42 +167,39 @@ describe('setup', () => { ); }); - it('should be able registerNavLink', async () => { + it('should be able addNavToGroup', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); - chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); - chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkBar); - const useCaseList = await chromeSetup.getUseCases$().pipe(first()).toPromise(); - expect(useCaseList.length).toEqual(2); - expect(useCaseList[0].navLinks.length).toEqual(2); - expect(useCaseList[1].navLinks.length).toEqual(1); - expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); + chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); + chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkBar); + const groupsMap = await chromeSetup.getGroupsMap$().pipe(first()).toPromise(); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(2); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupFoo.id].id).toEqual(mockedGroupFoo.id); expect(warnMock).toBeCalledTimes(0); }); - it('should output warning message if registerNavLink with same use case id and navLink id', async () => { + it('should output warning message if addNavToGroup with same group id and navLink id', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); - chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); - chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); - const useCaseList = await chromeSetup.getUseCases$().pipe(first()).toPromise(); - expect(useCaseList.length).toEqual(2); - expect(useCaseList[0].navLinks.length).toEqual(1); - expect(useCaseList[1].navLinks.length).toEqual(1); - expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); + chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); + chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); + const groupsMap = await chromeSetup.getGroupsMap$().pipe(first()).toPromise(); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); expect(warnMock).toBeCalledTimes(1); expect(warnMock).toBeCalledWith( - `[ChromeService] Navlink of ${mockedUseCaseFoo.id} has already been registered in use case ${mockedUseCaseFoo.id}` + `[ChromeService] Navlink of ${mockedGroupFoo.id} has already been registered in group ${mockedGroupFoo.id}` ); }); }); @@ -546,24 +542,23 @@ describe('start', () => { }); }); - describe('use case', () => { - it('should be able to get the use cases registered through registerNavLinks', async () => { + describe('group', () => { + it('should be able to get the groups registered through addNavToGroups', async () => { const startDeps = defaultStartDeps([]); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.registerNavLink(mockedUseCaseFoo, mockedNavLinkFoo); - chromeSetup.registerNavLink(mockedUseCaseBar, mockedNavLinkBar); + chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); + chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); const chromeStart = await chrome.start(startDeps); - const useCaseList = await chromeStart.getUseCases$().pipe(first()).toPromise(); + const groupsMap = await chromeStart.getGroupsMap$().pipe(first()).toPromise(); - expect(useCaseList.length).toEqual(2); - expect(useCaseList[0].navLinks.length).toEqual(1); - expect(useCaseList[1].navLinks.length).toEqual(1); - expect(useCaseList[0].id).toEqual(mockedUseCaseFoo.id); + expect(Object.keys(groupsMap).length).toEqual(2); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); }); }); }); diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 9e339987d060..bbab3d2f5cf3 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -29,6 +29,7 @@ */ import { EuiBreadcrumb, IconType } from '@elastic/eui'; +import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; import React from 'react'; import { FormattedMessage } from '@osd/i18n/react'; import { BehaviorSubject, combineLatest, merge, Observable, of, ReplaySubject } from 'rxjs'; @@ -48,7 +49,7 @@ import { ChromeNavLinks, NavLinksService, ChromeNavLink } from './nav_links'; import { ChromeRecentlyAccessed, RecentlyAccessedService } from './recently_accessed'; import { Header } from './ui'; import { ChromeHelpExtensionMenuLink } from './ui/header/header_help_menu'; -import { Branding } from '../'; +import { AppCategory, Branding } from '../'; import { getLogos } from '../../common'; import type { Logos } from '../../common/types'; import { OverlayStart } from '../overlays'; @@ -102,6 +103,10 @@ export interface StartDeps { type CollapsibleNavHeaderRender = () => JSX.Element | null; +type NavGroupItemInMap = ChromeNavGroup & { + navLinks: ChromeRegistrationNavLink[]; +}; + /** @internal */ export class ChromeService { private isVisible$!: Observable; @@ -111,9 +116,7 @@ export class ChromeService { private readonly navLinks = new NavLinksService(); private readonly recentlyAccessed = new RecentlyAccessedService(); private readonly docTitle = new DocTitleService(); - private readonly useCaseList$ = new BehaviorSubject< - Array - >([]); + private readonly groupsMap$ = new BehaviorSubject>({}); private collapsibleNavHeaderRender?: CollapsibleNavHeaderRender; constructor(private readonly params: ConstructorParams) {} @@ -161,29 +164,31 @@ export class ChromeService { } this.collapsibleNavHeaderRender = render; }, - registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => { - const currentUseCaseList = [...this.useCaseList$.getValue()]; - const findItem = currentUseCaseList.find((item) => item.id === useCase.id); - if (findItem) { - const findNavLinks = findItem.navLinks.find((link) => link.id === navLink.id); - if (findNavLinks) { + addNavToGroup: (navGroup: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => { + // Construct a new groups map pointer. + const currentGroupsMap = { ...this.groupsMap$.getValue() }; + const matchedGroup = currentGroupsMap[navGroup.id]; + if (matchedGroup) { + const links = matchedGroup.navLinks; + const IfNavExist = !!links.find((link) => link.id === navLink.id); + if (IfNavExist) { // eslint-disable-next-line no-console console.warn( - `[ChromeService] Navlink of ${navLink.id} has already been registered in use case ${useCase.id}` + `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` ); return; } - findItem.navLinks.push(navLink); + matchedGroup.navLinks.push(navLink); } else { - currentUseCaseList.push({ - ...useCase, + currentGroupsMap[navGroup.id] = { + ...navGroup, navLinks: [navLink], - }); + }; } - this.useCaseList$.next(currentUseCaseList); + this.groupsMap$.next(currentGroupsMap); }, - getUseCases$: () => this.useCaseList$.pipe(takeUntil(this.stop$)), + getGroupsMap$: () => this.groupsMap$.pipe(takeUntil(this.stop$)), }; } @@ -366,7 +371,7 @@ export class ChromeService { customNavLink$.next(customNavLink); }, - getUseCases$: () => this.useCaseList$.pipe(takeUntil(this.stop$)), + getGroupsMap$: () => this.groupsMap$.pipe(takeUntil(this.stop$)), }; } @@ -388,14 +393,8 @@ export class ChromeService { */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; - registerNavLink: (useCase: ChromeUseCase, navLink: ChromeRegistrationNavLink) => void; - getUseCases$: () => Observable< - Array< - ChromeUseCase & { - navLinks: ChromeRegistrationNavLink[]; - } - > - >; + addNavToGroup: (group: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => void; + getGroupsMap$: () => Observable>; } /** @@ -523,7 +522,7 @@ export interface ChromeStart { */ getIsNavDrawerLocked$(): Observable; - getUseCases$: ChromeSetup['getUseCases$']; + getGroupsMap$: ChromeSetup['getGroupsMap$']; } /** @internal */ @@ -535,18 +534,24 @@ export interface InternalChromeStart extends ChromeStart { getHeaderComponent(): JSX.Element; } +export enum GROUP_TYPE { + SYSTEM_GROUP = 'SYSTEM_GROUP', + USE_CASE_GROUP = 'USE_CASE_GROUP', +} + /** @public */ -export interface ChromeUseCase { +export interface ChromeNavGroup { id: string; title: string; description: string; order?: number; - icon?: string; - reserved?: boolean; // use cases like data administration / settings and setup is not configurable through workspace + icon?: EuiIconType; + type: GROUP_TYPE; // group with type of GROUP_TYPE.SYSTEM_GROUP will always be displayed before USE_CASE_GROUP } /** @public */ -export interface ChromeRegistrationNavLink - extends Omit { - title?: ChromeNavLink['title']; +export interface ChromeRegistrationNavLink { + id: string; + category?: AppCategory; + order?: number; } diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index 9e08c0434ac7..04166e60ae81 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -36,7 +36,7 @@ export { ChromeStart, InternalChromeStart, ChromeHelpExtension, - ChromeUseCase, + ChromeNavGroup, ChromeRegistrationNavLink, } from './chrome_service'; export { diff --git a/src/core/public/index.ts b/src/core/public/index.ts index bb76efa08da7..90226cf3f35e 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -71,7 +71,7 @@ import { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, - ChromeUseCase, + ChromeNavGroup, ChromeRegistrationNavLink, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; @@ -368,7 +368,7 @@ export { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, - ChromeUseCase, + ChromeNavGroup, ChromeRegistrationNavLink, }; diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap index 9e4896623342..e3bc7ba03864 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -148,7 +148,7 @@ exports[`dashboard listing hideWriteControls 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1323,7 +1323,7 @@ exports[`dashboard listing render table listing with initial filters from URL 1` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2559,7 +2559,7 @@ exports[`dashboard listing renders call to action when no dashboards exist 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3795,7 +3795,7 @@ exports[`dashboard listing renders table rows 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5031,7 +5031,7 @@ exports[`dashboard listing renders warning when listingLimit is exceeded 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 4c9933489483..f2bf9dfdc3db 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -136,7 +136,7 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1137,7 +1137,7 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2138,7 +2138,7 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3139,7 +3139,7 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -4140,7 +4140,7 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5141,7 +5141,7 @@ exports[`Dashboard top nav render with all components 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getUseCases$": [MockFunction], + "getGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { From f879c83f529016499a4050b3a8fb761947a9ccc7 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Thu, 20 Jun 2024 05:36:04 +0000 Subject: [PATCH 08/35] Changeset file for PR #7060 created/updated --- changelogs/fragments/7060.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/7060.yml b/changelogs/fragments/7060.yml index 025da8b6d14a..80c8c85ee3c7 100644 --- a/changelogs/fragments/7060.yml +++ b/changelogs/fragments/7060.yml @@ -1,2 +1,2 @@ feat: -- Introduce new interface for use case ([#7060](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7060)) \ No newline at end of file +- Introduce new interface for group ([#7060](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7060)) \ No newline at end of file From 7306f05bfddb5984c0da465444f944787dca238d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 13:43:25 +0800 Subject: [PATCH 09/35] fix: update based on comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.test.ts | 9 ++++++++- src/core/public/chrome/index.ts | 1 + src/core/public/index.ts | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index e609b349a675..6c560fe5a5b0 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -39,7 +39,12 @@ import { httpServiceMock } from '../http/http_service.mock'; import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; -import { ChromeRegistrationNavLink, ChromeService, ChromeNavGroup } from './chrome_service'; +import { + ChromeRegistrationNavLink, + ChromeService, + ChromeNavGroup, + GROUP_TYPE, +} from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -114,12 +119,14 @@ const mockedGroupFoo: ChromeNavGroup = { id: 'foo', title: 'foo', description: 'foo', + type: GROUP_TYPE.SYSTEM_GROUP, }; const mockedGroupBar: ChromeNavGroup = { id: 'bar', title: 'bar', description: 'bar', + type: GROUP_TYPE.USE_CASE_GROUP, }; const mockedNavLinkFoo: ChromeRegistrationNavLink = { diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index 04166e60ae81..ad2e4aa9cb4c 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -38,6 +38,7 @@ export { ChromeHelpExtension, ChromeNavGroup, ChromeRegistrationNavLink, + GROUP_TYPE, } from './chrome_service'; export { ChromeHelpExtensionMenuLink, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 90226cf3f35e..2fa59a3cb323 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -73,6 +73,7 @@ import { RightNavigationButtonProps, ChromeNavGroup, ChromeRegistrationNavLink, + GROUP_TYPE, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; import { HttpSetup, HttpStart } from './http'; @@ -370,6 +371,7 @@ export { RightNavigationButtonProps, ChromeNavGroup, ChromeRegistrationNavLink, + GROUP_TYPE, }; export { __osdBootstrap__ } from './osd_bootstrap'; From 308d63d5c06ba80e63e480d74a147ae5c20159b5 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 13:48:57 +0800 Subject: [PATCH 10/35] fix: update based on comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index bbab3d2f5cf3..5caa695bbeb3 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -546,7 +546,7 @@ export interface ChromeNavGroup { description: string; order?: number; icon?: EuiIconType; - type: GROUP_TYPE; // group with type of GROUP_TYPE.SYSTEM_GROUP will always be displayed before USE_CASE_GROUP + type: GROUP_TYPE; // group with type of GROUP_TYPE.SYSTEM_GROUP will always display before USE_CASE_GROUP } /** @public */ From c23b04d001bd3ae57c56b96fac5c66d717f81ff7 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 20 Jun 2024 17:41:44 +0800 Subject: [PATCH 11/35] fix: update based on comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 5caa695bbeb3..0e45a04cd000 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -170,8 +170,8 @@ export class ChromeService { const matchedGroup = currentGroupsMap[navGroup.id]; if (matchedGroup) { const links = matchedGroup.navLinks; - const IfNavExist = !!links.find((link) => link.id === navLink.id); - if (IfNavExist) { + const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); + if (isLinkExistInGroup) { // eslint-disable-next-line no-console console.warn( `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` From e894656d657f9624113dc5b951761eaed0e27955 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 21 Jun 2024 13:41:40 +0800 Subject: [PATCH 12/35] feat: update based on comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 10 +-- src/core/public/chrome/chrome_service.mock.ts | 8 +- src/core/public/chrome/chrome_service.test.ts | 31 ++++--- src/core/public/chrome/chrome_service.tsx | 80 ++++++++++++------- src/core/public/chrome/index.ts | 2 +- src/core/public/index.ts | 4 +- .../dashboard_listing.test.tsx.snap | 10 +-- .../dashboard_top_nav.test.tsx.snap | 12 +-- 8 files changed, 86 insertions(+), 71 deletions(-) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 03393bd41c27..337334a49f4b 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -117,23 +117,23 @@ Gets an Observable of the array of recently accessed history :- - Methods : Register a group :- -`addNavToGroup: (group: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => void;` +`addNavLinksToGroup: (group: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void;` Gets an Observable of the array of registered groups :- -`getGroupsMap$: Observable>` +`getNavGroupsMap$: Observable>` ##### Register a new group with a navLink ```ts - chrome.addNavToGroup( + chrome.addNavLinksToGroup( { id: 'my-group', title: 'A demo group', description: 'description for demo group' }, - { + [{ id: 'nav' - } + }] ) ``` diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index d67a2b8bceba..727ebff1fe7a 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -38,8 +38,8 @@ const createSetupContractMock = () => { getGroupsMapMock.mockReturnValue(new BehaviorSubject({})); return { registerCollapsibleNavHeader: jest.fn(), - addNavToGroup: jest.fn(), - getGroupsMap$: getGroupsMapMock, + addNavLinksToGroup: jest.fn(), + getNavGroupsMap$: getGroupsMapMock, }; }; @@ -90,7 +90,7 @@ const createStartContractMock = () => { getIsNavDrawerLocked$: jest.fn(), getCustomNavLink$: jest.fn(), setCustomNavLink: jest.fn(), - getGroupsMap$: jest.fn(), + getNavGroupsMap$: jest.fn(), }; startContract.navLinks.getAll.mockReturnValue([]); startContract.getIsVisible$.mockReturnValue(new BehaviorSubject(false)); @@ -100,7 +100,7 @@ const createStartContractMock = () => { startContract.getCustomNavLink$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getHelpExtension$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getIsNavDrawerLocked$.mockReturnValue(new BehaviorSubject(false)); - startContract.getGroupsMap$.mockReturnValue(new BehaviorSubject({})); + startContract.getNavGroupsMap$.mockReturnValue(new BehaviorSubject({})); return startContract; }; diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 6c560fe5a5b0..d69cf69f16e7 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -43,7 +43,7 @@ import { ChromeRegistrationNavLink, ChromeService, ChromeNavGroup, - GROUP_TYPE, + NavGroupType, } from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -119,14 +119,13 @@ const mockedGroupFoo: ChromeNavGroup = { id: 'foo', title: 'foo', description: 'foo', - type: GROUP_TYPE.SYSTEM_GROUP, + type: NavGroupType.SYSTEM, }; const mockedGroupBar: ChromeNavGroup = { id: 'bar', title: 'bar', description: 'bar', - type: GROUP_TYPE.USE_CASE_GROUP, }; const mockedNavLinkFoo: ChromeRegistrationNavLink = { @@ -174,34 +173,32 @@ describe('setup', () => { ); }); - it('should be able addNavToGroup', async () => { + it('should be able addNavLinksToGroup', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); - chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); - chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkBar); - const groupsMap = await chromeSetup.getGroupsMap$().pipe(first()).toPromise(); + chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedGroupFoo, mockedGroupBar]); + chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); + const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(2); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); expect(groupsMap[mockedGroupFoo.id].id).toEqual(mockedGroupFoo.id); expect(warnMock).toBeCalledTimes(0); }); - it('should output warning message if addNavToGroup with same group id and navLink id', async () => { + it('should output warning message if addNavLinksToGroup with same group id and navLink id', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); - chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); - chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); - const groupsMap = await chromeSetup.getGroupsMap$().pipe(first()).toPromise(); + chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo, mockedGroupFoo]); + chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); + const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); expect(warnMock).toBeCalledTimes(1); @@ -550,18 +547,18 @@ describe('start', () => { }); describe('group', () => { - it('should be able to get the groups registered through addNavToGroups', async () => { + it('should be able to get the groups registered through addNavLinksToGroups', async () => { const startDeps = defaultStartDeps([]); const chrome = new ChromeService({ browserSupportsCsp: true }); const chromeSetup = chrome.setup(); - chromeSetup.addNavToGroup(mockedGroupFoo, mockedNavLinkFoo); - chromeSetup.addNavToGroup(mockedGroupBar, mockedNavLinkBar); + chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo]); + chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); const chromeStart = await chrome.start(startDeps); - const groupsMap = await chromeStart.getGroupsMap$().pipe(first()).toPromise(); + const groupsMap = await chromeStart.getNavGroupsMap$().pipe(first()).toPromise(); expect(Object.keys(groupsMap).length).toEqual(2); expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 0e45a04cd000..e440992ddd29 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -116,7 +116,7 @@ export class ChromeService { private readonly navLinks = new NavLinksService(); private readonly recentlyAccessed = new RecentlyAccessedService(); private readonly docTitle = new DocTitleService(); - private readonly groupsMap$ = new BehaviorSubject>({}); + private readonly navGroupsMap$ = new BehaviorSubject>({}); private collapsibleNavHeaderRender?: CollapsibleNavHeaderRender; constructor(private readonly params: ConstructorParams) {} @@ -153,6 +153,33 @@ export class ChromeService { ); } + private addNavLinkToGroup( + currentGroupsMap: Record, + navGroup: ChromeNavGroup, + navLink: ChromeRegistrationNavLink + ) { + const matchedGroup = currentGroupsMap[navGroup.id]; + if (matchedGroup) { + const links = matchedGroup.navLinks; + const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); + if (isLinkExistInGroup) { + // eslint-disable-next-line no-console + console.warn( + `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` + ); + return currentGroupsMap; + } + matchedGroup.navLinks.push(navLink); + } else { + currentGroupsMap[navGroup.id] = { + ...navGroup, + navLinks: [navLink], + }; + } + + return currentGroupsMap; + } + public setup(): ChromeSetup { return { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => { @@ -164,31 +191,18 @@ export class ChromeService { } this.collapsibleNavHeaderRender = render; }, - addNavToGroup: (navGroup: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => { + addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => { // Construct a new groups map pointer. - const currentGroupsMap = { ...this.groupsMap$.getValue() }; - const matchedGroup = currentGroupsMap[navGroup.id]; - if (matchedGroup) { - const links = matchedGroup.navLinks; - const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); - if (isLinkExistInGroup) { - // eslint-disable-next-line no-console - console.warn( - `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` - ); - return; - } - matchedGroup.navLinks.push(navLink); - } else { - currentGroupsMap[navGroup.id] = { - ...navGroup, - navLinks: [navLink], - }; - } + const currentGroupsMap = { ...this.navGroupsMap$.getValue() }; + + const navGroupsMapAfterAdd = navLinks.reduce( + (groupsMap, navLink) => this.addNavLinkToGroup(groupsMap, navGroup, navLink), + currentGroupsMap + ); - this.groupsMap$.next(currentGroupsMap); + this.navGroupsMap$.next(navGroupsMapAfterAdd); }, - getGroupsMap$: () => this.groupsMap$.pipe(takeUntil(this.stop$)), + getNavGroupsMap$: () => this.navGroupsMap$.pipe(takeUntil(this.stop$)), }; } @@ -371,7 +385,7 @@ export class ChromeService { customNavLink$.next(customNavLink); }, - getGroupsMap$: () => this.groupsMap$.pipe(takeUntil(this.stop$)), + getNavGroupsMap$: () => this.navGroupsMap$.pipe(takeUntil(this.stop$)), }; } @@ -393,8 +407,8 @@ export class ChromeService { */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; - addNavToGroup: (group: ChromeNavGroup, navLink: ChromeRegistrationNavLink) => void; - getGroupsMap$: () => Observable>; + addNavLinksToGroup: (group: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void; + getNavGroupsMap$: () => Observable>; } /** @@ -522,7 +536,7 @@ export interface ChromeStart { */ getIsNavDrawerLocked$(): Observable; - getGroupsMap$: ChromeSetup['getGroupsMap$']; + getNavGroupsMap$: ChromeSetup['getNavGroupsMap$']; } /** @internal */ @@ -534,9 +548,8 @@ export interface InternalChromeStart extends ChromeStart { getHeaderComponent(): JSX.Element; } -export enum GROUP_TYPE { - SYSTEM_GROUP = 'SYSTEM_GROUP', - USE_CASE_GROUP = 'USE_CASE_GROUP', +export enum NavGroupType { + SYSTEM = 'system', } /** @public */ @@ -546,7 +559,12 @@ export interface ChromeNavGroup { description: string; order?: number; icon?: EuiIconType; - type: GROUP_TYPE; // group with type of GROUP_TYPE.SYSTEM_GROUP will always display before USE_CASE_GROUP + + // group with type of NavGroupType.SYSTEM: + // 1. Always display before USE_CASE_GROUP. + // 2. Not pickable within workspace creation page. + // Default: NavGroupType.useCase + type?: NavGroupType; } /** @public */ diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index ad2e4aa9cb4c..258a17a73b80 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -38,7 +38,7 @@ export { ChromeHelpExtension, ChromeNavGroup, ChromeRegistrationNavLink, - GROUP_TYPE, + NavGroupType, } from './chrome_service'; export { ChromeHelpExtensionMenuLink, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 2fa59a3cb323..534abb675e97 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -73,7 +73,7 @@ import { RightNavigationButtonProps, ChromeNavGroup, ChromeRegistrationNavLink, - GROUP_TYPE, + NavGroupType, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; import { HttpSetup, HttpStart } from './http'; @@ -371,7 +371,7 @@ export { RightNavigationButtonProps, ChromeNavGroup, ChromeRegistrationNavLink, - GROUP_TYPE, + NavGroupType, }; export { __osdBootstrap__ } from './osd_bootstrap'; diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap index e3bc7ba03864..0e911b8fb7f3 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -148,7 +148,7 @@ exports[`dashboard listing hideWriteControls 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1323,7 +1323,7 @@ exports[`dashboard listing render table listing with initial filters from URL 1` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2559,7 +2559,7 @@ exports[`dashboard listing renders call to action when no dashboards exist 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3795,7 +3795,7 @@ exports[`dashboard listing renders table rows 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5031,7 +5031,7 @@ exports[`dashboard listing renders warning when listingLimit is exceeded 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index f2bf9dfdc3db..0719d615fe03 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -136,7 +136,7 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1137,7 +1137,7 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2138,7 +2138,7 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3139,7 +3139,7 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -4140,7 +4140,7 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5141,7 +5141,7 @@ exports[`Dashboard top nav render with all components 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getGroupsMap$": [MockFunction], + "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { From 4dceb952fb5d306b94505b0dcf4ba7dfda6fa6ec Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 21 Jun 2024 17:58:27 +0800 Subject: [PATCH 13/35] feat: update comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index e440992ddd29..a6b3226947a5 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -560,10 +560,13 @@ export interface ChromeNavGroup { order?: number; icon?: EuiIconType; - // group with type of NavGroupType.SYSTEM: - // 1. Always display before USE_CASE_GROUP. - // 2. Not pickable within workspace creation page. - // Default: NavGroupType.useCase + /** + * Groups with type of NavGroupType.SYSTEM will: + * 1. Always display before USE_CASE_GROUP. + * 2. Not be pickable within the workspace creation page. + * + * @default undefined indicates it is of type useCase + */ type?: NavGroupType; } From e3ba6cc588d772f451ec557d70cde83a7507dada Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 24 Jun 2024 11:54:42 +0800 Subject: [PATCH 14/35] feat: add default nav group Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 25 +------ src/core/public/chrome/index.ts | 2 - src/core/public/index.ts | 6 +- src/core/types/index.ts | 1 + src/core/types/nav_group.ts | 28 ++++++++ src/core/utils/default_app_categories.ts | 21 ++++++ src/core/utils/default_nav_groups.ts | 82 +++++++++++++++++++++++ src/core/utils/index.ts | 1 + 8 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 src/core/types/nav_group.ts create mode 100644 src/core/utils/default_nav_groups.ts diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index a6b3226947a5..7fa41dbc4a55 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -29,7 +29,6 @@ */ import { EuiBreadcrumb, IconType } from '@elastic/eui'; -import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; import React from 'react'; import { FormattedMessage } from '@osd/i18n/react'; import { BehaviorSubject, combineLatest, merge, Observable, of, ReplaySubject } from 'rxjs'; @@ -49,7 +48,7 @@ import { ChromeNavLinks, NavLinksService, ChromeNavLink } from './nav_links'; import { ChromeRecentlyAccessed, RecentlyAccessedService } from './recently_accessed'; import { Header } from './ui'; import { ChromeHelpExtensionMenuLink } from './ui/header/header_help_menu'; -import { AppCategory, Branding } from '../'; +import { AppCategory, Branding, ChromeNavGroup } from '../'; import { getLogos } from '../../common'; import type { Logos } from '../../common/types'; import { OverlayStart } from '../overlays'; @@ -548,28 +547,6 @@ export interface InternalChromeStart extends ChromeStart { getHeaderComponent(): JSX.Element; } -export enum NavGroupType { - SYSTEM = 'system', -} - -/** @public */ -export interface ChromeNavGroup { - id: string; - title: string; - description: string; - order?: number; - icon?: EuiIconType; - - /** - * Groups with type of NavGroupType.SYSTEM will: - * 1. Always display before USE_CASE_GROUP. - * 2. Not be pickable within the workspace creation page. - * - * @default undefined indicates it is of type useCase - */ - type?: NavGroupType; -} - /** @public */ export interface ChromeRegistrationNavLink { id: string; diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index 258a17a73b80..e5bcf3072bd1 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -36,9 +36,7 @@ export { ChromeStart, InternalChromeStart, ChromeHelpExtension, - ChromeNavGroup, ChromeRegistrationNavLink, - NavGroupType, } from './chrome_service'; export { ChromeHelpExtensionMenuLink, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 534abb675e97..02a350f2a429 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -71,9 +71,7 @@ import { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, - ChromeNavGroup, ChromeRegistrationNavLink, - NavGroupType, } from './chrome'; import { FatalErrorsSetup, FatalErrorsStart, FatalErrorInfo } from './fatal_errors'; import { HttpSetup, HttpStart } from './http'; @@ -117,6 +115,8 @@ export { StringValidationRegex, StringValidationRegexString, WorkspaceAttribute, + ChromeNavGroup, + NavGroupType, } from '../types'; export { @@ -369,9 +369,7 @@ export { RightNavigationOrder, RightNavigationButton, RightNavigationButtonProps, - ChromeNavGroup, ChromeRegistrationNavLink, - NavGroupType, }; export { __osdBootstrap__ } from './osd_bootstrap'; diff --git a/src/core/types/index.ts b/src/core/types/index.ts index f65d683f6bdc..3e4a15436faf 100644 --- a/src/core/types/index.ts +++ b/src/core/types/index.ts @@ -41,3 +41,4 @@ export * from './serializable'; export * from './custom_branding'; export * from './workspace'; export * from './cross_compatibility'; +export * from './nav_group'; diff --git a/src/core/types/nav_group.ts b/src/core/types/nav_group.ts new file mode 100644 index 000000000000..8909bcbde082 --- /dev/null +++ b/src/core/types/nav_group.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; + +export enum NavGroupType { + SYSTEM = 'system', +} + +/** @public */ +export interface ChromeNavGroup { + id: string; + title: string; + description: string; + order?: number; + icon?: EuiIconType; + + /** + * Groups with type of NavGroupType.SYSTEM will: + * 1. Always display before USE_CASE_GROUP. + * 2. Not be pickable within the workspace creation page. + * + * @default undefined indicates it is of type useCase + */ + type?: NavGroupType; +} diff --git a/src/core/utils/default_app_categories.ts b/src/core/utils/default_app_categories.ts index 3c0920624e1b..e0b748021199 100644 --- a/src/core/utils/default_app_categories.ts +++ b/src/core/utils/default_app_categories.ts @@ -73,4 +73,25 @@ export const DEFAULT_APP_CATEGORIES: Record = Object.freeze order: 5000, euiIconType: 'managementApp', }, + investigate: { + id: 'investigate', + label: i18n.translate('core.ui.investigate.label', { + defaultMessage: 'Investigate', + }), + order: 1000, + }, + dashboardAndReport: { + id: 'dashboardAndReport', + label: i18n.translate('core.ui.dashboardAndReport.label', { + defaultMessage: 'Dashboard and report', + }), + order: 2000, + }, + analyzeSearch: { + id: 'analyzeSearch', + label: i18n.translate('core.ui.analyzeSearch.label', { + defaultMessage: 'Analyze search', + }), + order: 4000, + }, }); diff --git a/src/core/utils/default_nav_groups.ts b/src/core/utils/default_nav_groups.ts new file mode 100644 index 000000000000..eda0a6c4f1a3 --- /dev/null +++ b/src/core/utils/default_nav_groups.ts @@ -0,0 +1,82 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { i18n } from '@osd/i18n'; +import { ChromeNavGroup, NavGroupType } from '../types'; + +const defaultNavGroups = { + dataAdministration: { + id: 'dataAdministration', + title: i18n.translate('core.ui.group.dataAdministration.title', { + defaultMessage: 'data administration', + }), + description: i18n.translate('core.ui.group.dataAdministration.description', { + defaultMessage: 'Apply policies or security on your data.', + }), + order: 1000, + type: NavGroupType.SYSTEM, + }, + settingsAndSetup: { + id: 'settingsAndSetup', + title: i18n.translate('core.ui.group.settingsAndSetup.title', { + defaultMessage: 'settings and setup', + }), + description: i18n.translate('core.ui.group.settingsAndSetup.description', { + defaultMessage: 'Set up your cluster with index patterns.', + }), + order: 2000, + type: NavGroupType.SYSTEM, + }, + observability: { + id: 'observability', + title: i18n.translate('core.ui.group.observability.title', { + defaultMessage: 'Observability', + }), + description: i18n.translate('core.ui.group.observability.description', { + defaultMessage: + 'Gain visibility into system health, performance, and reliability through monitoring and analysis of logs, metrics, and traces.', + }), + order: 3000, + }, + 'security-analytics': { + id: 'security-analytics', + title: i18n.translate('core.ui.group.security.analytics.title', { + defaultMessage: 'Security Analytics', + }), + description: i18n.translate('core.ui.group.security.analytics.description', { + defaultMessage: + 'Detect and investigate potential security threats and vulnerabilities across your systems and data.', + }), + order: 4000, + }, + analytics: { + id: 'analytics', + title: i18n.translate('core.ui.group.analytics.title', { + defaultMessage: 'Analytics', + }), + description: i18n.translate('core.ui.group.analytics.description', { + defaultMessage: + 'Analyze data to derive insights, identify patterns and trends, and make data-driven decisions.', + }), + order: 5000, + }, + search: { + id: 'search', + title: i18n.translate('core.ui.group.search.title', { + defaultMessage: 'Search', + }), + description: i18n.translate('core.ui.group.search.description', { + defaultMessage: + "Quickly find and explore relevant information across your organization's data sources.", + }), + order: 6000, + }, +} as const; + +/** @internal */ +export const DEFAULT_NAV_GROUPS: Record< + keyof typeof defaultNavGroups, + ChromeNavGroup +> = Object.freeze(defaultNavGroups); diff --git a/src/core/utils/index.ts b/src/core/utils/index.ts index 9b58b7ef6d0d..a79b5d0bf2f7 100644 --- a/src/core/utils/index.ts +++ b/src/core/utils/index.ts @@ -44,3 +44,4 @@ export { PUBLIC_WORKSPACE_NAME, } from './constants'; export { getWorkspaceIdFromUrl, formatUrlWithWorkspaceId, cleanWorkspaceId } from './workspace'; +export { DEFAULT_NAV_GROUPS } from './default_nav_groups'; From c4d5a01d88dbe235837a969c89accf6a5b5d2855 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 24 Jun 2024 11:56:32 +0800 Subject: [PATCH 15/35] fix: type error Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.test.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index d69cf69f16e7..9d7865da4b75 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -39,12 +39,7 @@ import { httpServiceMock } from '../http/http_service.mock'; import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; -import { - ChromeRegistrationNavLink, - ChromeService, - ChromeNavGroup, - NavGroupType, -} from './chrome_service'; +import { ChromeRegistrationNavLink, ChromeService } from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -115,14 +110,13 @@ afterAll(() => { (window as any).localStorage = originalLocalStorage; }); -const mockedGroupFoo: ChromeNavGroup = { +const mockedGroupFoo = { id: 'foo', title: 'foo', description: 'foo', - type: NavGroupType.SYSTEM, }; -const mockedGroupBar: ChromeNavGroup = { +const mockedGroupBar = { id: 'bar', title: 'bar', description: 'bar', From 5a543d5d53c29d9b5ad9ee7efd8e511af0d6c163 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 24 Jun 2024 11:58:47 +0800 Subject: [PATCH 16/35] fix: type error Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 9d7865da4b75..820715b2ff9c 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -167,7 +167,7 @@ describe('setup', () => { ); }); - it('should be able addNavLinksToGroup', async () => { + it('should be able to `addNavLinksToGroup`', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); @@ -183,7 +183,7 @@ describe('setup', () => { expect(warnMock).toBeCalledTimes(0); }); - it('should output warning message if addNavLinksToGroup with same group id and navLink id', async () => { + it('should output warning message if `addNavLinksToGroup` with same group id and navLink id', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); const chrome = new ChromeService({ browserSupportsCsp: true }); From 65725953fb390d3cf3ee71e2fe5d1daec856a869 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 24 Jun 2024 18:17:13 +0800 Subject: [PATCH 17/35] feat: expose DEFAULT_NAV_GROUPS in src/core/public Signed-off-by: SuZhou-Joe --- src/core/public/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 02a350f2a429..a60c81e97ec8 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -104,6 +104,7 @@ export { cleanWorkspaceId, PUBLIC_WORKSPACE_ID, PUBLIC_WORKSPACE_NAME, + DEFAULT_NAV_GROUPS, } from '../utils'; export { AppCategory, From 2bb4910527dceb823f6b51c95a43e40cbd5e9453 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 25 Jun 2024 17:39:09 +0800 Subject: [PATCH 18/35] feat: export NavGroupItemInMap type Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 7fa41dbc4a55..964e7fcfc3b0 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -102,7 +102,7 @@ export interface StartDeps { type CollapsibleNavHeaderRender = () => JSX.Element | null; -type NavGroupItemInMap = ChromeNavGroup & { +export type NavGroupItemInMap = ChromeNavGroup & { navLinks: ChromeRegistrationNavLink[]; }; From b1dde2854799fddb9de8df31fe6bad3aa9b7d72c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 26 Jun 2024 10:28:54 +0800 Subject: [PATCH 19/35] feat: update README Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 24 ----------------------- src/core/public/chrome/chrome_service.tsx | 15 ++++++++++++++ 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 337334a49f4b..836b373bb66c 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -112,30 +112,6 @@ Gets an Observable of the array of recently accessed history :- chrome.docTitle.change('My application title') chrome.docTitle.change(['My application', 'My section']) ``` -### GroupService: -- Interface : ChromeNavGroup -- Methods : -Register a group :- - -`addNavLinksToGroup: (group: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void;` - -Gets an Observable of the array of registered groups :- - -`getNavGroupsMap$: Observable>` -##### Register a new group with a navLink - - ```ts - chrome.addNavLinksToGroup( - { - id: 'my-group', - title: 'A demo group', - description: 'description for demo group' - }, - [{ - id: 'nav' - }] - ) - ``` ### UI : ###### consists of tsx/scss files && renders UI components from css Library e.g `````` diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 964e7fcfc3b0..7264b66ebdf3 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -403,6 +403,18 @@ export class ChromeService { * ```ts * core.chrome.registerCollapsibleNavHeader(() => ) * ``` + * + * @example + * Add navLinks to a nav group: + * ```ts + * core.chrome.addNavLinksToGroup(DEFAULT_NAV_GROUPS, [{ id: 'id-for-your-application' }]) + * ``` + * + * @example + * Get the observable of registered navGroups map: + * ```ts + * core.chrome.getNavGroupsMap$(); + * ``` */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; @@ -535,6 +547,9 @@ export interface ChromeStart { */ getIsNavDrawerLocked$(): Observable; + /** + * Get an observable of the nav groups + */ getNavGroupsMap$: ChromeSetup['getNavGroupsMap$']; } From eb430c03f8f00947eba5d0a626b008f1a98b2762 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 26 Jun 2024 10:29:38 +0800 Subject: [PATCH 20/35] feat: update README Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 836b373bb66c..6ec765a3bb0b 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -112,7 +112,6 @@ Gets an Observable of the array of recently accessed history :- chrome.docTitle.change('My application title') chrome.docTitle.change(['My application', 'My section']) ``` - ### UI : ###### consists of tsx/scss files && renders UI components from css Library e.g `````` From 0f0e4cca3ed9b6d4238dcc18bba44042a8603c2e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 26 Jun 2024 10:31:29 +0800 Subject: [PATCH 21/35] feat: update interface Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 7264b66ebdf3..8585ec02d952 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -565,6 +565,7 @@ export interface InternalChromeStart extends ChromeStart { /** @public */ export interface ChromeRegistrationNavLink { id: string; + title?: string; category?: AppCategory; order?: number; } From 7a61f47d01a0670a36d27573231d79cb7b78edaf Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:06:23 +0800 Subject: [PATCH 22/35] refactor: move navGroup related interface into a service Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.test.ts | 78 +---------------- src/core/public/chrome/chrome_service.tsx | 87 +++---------------- src/core/public/chrome/index.ts | 2 +- src/core/public/chrome/nav_group/index.ts | 12 +++ .../nav_group/nav_group_service.test.ts | 80 +++++++++++++++++ .../chrome/nav_group/nav_group_service.ts | 80 +++++++++++++++++ 6 files changed, 189 insertions(+), 150 deletions(-) create mode 100644 src/core/public/chrome/nav_group/index.ts create mode 100644 src/core/public/chrome/nav_group/nav_group_service.test.ts create mode 100644 src/core/public/chrome/nav_group/nav_group_service.ts diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 820715b2ff9c..154f07caf46c 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -31,7 +31,7 @@ import { shallow } from 'enzyme'; import React from 'react'; import * as Rx from 'rxjs'; -import { first, take, toArray } from 'rxjs/operators'; +import { take, toArray } from 'rxjs/operators'; import { App, PublicAppInfo } from '../application'; import { applicationServiceMock } from '../application/application_service.mock'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; @@ -39,7 +39,7 @@ import { httpServiceMock } from '../http/http_service.mock'; import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { notificationServiceMock } from '../notifications/notifications_service.mock'; import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; -import { ChromeRegistrationNavLink, ChromeService } from './chrome_service'; +import { ChromeService } from './chrome_service'; import { getAppInfo } from '../application/utils'; import { overlayServiceMock } from '../mocks'; @@ -110,26 +110,6 @@ afterAll(() => { (window as any).localStorage = originalLocalStorage; }); -const mockedGroupFoo = { - id: 'foo', - title: 'foo', - description: 'foo', -}; - -const mockedGroupBar = { - id: 'bar', - title: 'bar', - description: 'bar', -}; - -const mockedNavLinkFoo: ChromeRegistrationNavLink = { - id: 'foo', -}; - -const mockedNavLinkBar: ChromeRegistrationNavLink = { - id: 'bar', -}; - describe('setup', () => { afterEach(() => { jest.restoreAllMocks(); @@ -166,40 +146,6 @@ describe('setup', () => { '[ChromeService] An existing custom collapsible navigation bar header render has been overridden.' ); }); - - it('should be able to `addNavLinksToGroup`', async () => { - const warnMock = jest.fn(); - jest.spyOn(console, 'warn').mockImplementation(warnMock); - const chrome = new ChromeService({ browserSupportsCsp: true }); - - const chromeSetup = chrome.setup(); - - chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedGroupFoo, mockedGroupBar]); - chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); - const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); - expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(2); - expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); - expect(groupsMap[mockedGroupFoo.id].id).toEqual(mockedGroupFoo.id); - expect(warnMock).toBeCalledTimes(0); - }); - - it('should output warning message if `addNavLinksToGroup` with same group id and navLink id', async () => { - const warnMock = jest.fn(); - jest.spyOn(console, 'warn').mockImplementation(warnMock); - const chrome = new ChromeService({ browserSupportsCsp: true }); - - const chromeSetup = chrome.setup(); - - chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo, mockedGroupFoo]); - chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); - const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); - expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); - expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); - expect(warnMock).toBeCalledTimes(1); - expect(warnMock).toBeCalledWith( - `[ChromeService] Navlink of ${mockedGroupFoo.id} has already been registered in group ${mockedGroupFoo.id}` - ); - }); }); describe('start', () => { @@ -539,26 +485,6 @@ describe('start', () => { `); }); }); - - describe('group', () => { - it('should be able to get the groups registered through addNavLinksToGroups', async () => { - const startDeps = defaultStartDeps([]); - const chrome = new ChromeService({ browserSupportsCsp: true }); - - const chromeSetup = chrome.setup(); - - chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo]); - chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); - - const chromeStart = await chrome.start(startDeps); - - const groupsMap = await chromeStart.getNavGroupsMap$().pipe(first()).toPromise(); - - expect(Object.keys(groupsMap).length).toEqual(2); - expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); - expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); - }); - }); }); describe('stop', () => { diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 8585ec02d952..5ad25a50098c 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -48,10 +48,15 @@ import { ChromeNavLinks, NavLinksService, ChromeNavLink } from './nav_links'; import { ChromeRecentlyAccessed, RecentlyAccessedService } from './recently_accessed'; import { Header } from './ui'; import { ChromeHelpExtensionMenuLink } from './ui/header/header_help_menu'; -import { AppCategory, Branding, ChromeNavGroup } from '../'; +import { Branding } from '../'; import { getLogos } from '../../common'; import type { Logos } from '../../common/types'; import { OverlayStart } from '../overlays'; +import { + ChromeNavGroupService, + ChromeNavGroupServiceSetupContract, + ChromeNavGroupServiceStartContract, +} from './nav_group'; export { ChromeNavControls, ChromeRecentlyAccessed, ChromeDocTitle }; @@ -102,10 +107,6 @@ export interface StartDeps { type CollapsibleNavHeaderRender = () => JSX.Element | null; -export type NavGroupItemInMap = ChromeNavGroup & { - navLinks: ChromeRegistrationNavLink[]; -}; - /** @internal */ export class ChromeService { private isVisible$!: Observable; @@ -115,7 +116,7 @@ export class ChromeService { private readonly navLinks = new NavLinksService(); private readonly recentlyAccessed = new RecentlyAccessedService(); private readonly docTitle = new DocTitleService(); - private readonly navGroupsMap$ = new BehaviorSubject>({}); + private readonly navGroup = new ChromeNavGroupService(); private collapsibleNavHeaderRender?: CollapsibleNavHeaderRender; constructor(private readonly params: ConstructorParams) {} @@ -152,34 +153,8 @@ export class ChromeService { ); } - private addNavLinkToGroup( - currentGroupsMap: Record, - navGroup: ChromeNavGroup, - navLink: ChromeRegistrationNavLink - ) { - const matchedGroup = currentGroupsMap[navGroup.id]; - if (matchedGroup) { - const links = matchedGroup.navLinks; - const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); - if (isLinkExistInGroup) { - // eslint-disable-next-line no-console - console.warn( - `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` - ); - return currentGroupsMap; - } - matchedGroup.navLinks.push(navLink); - } else { - currentGroupsMap[navGroup.id] = { - ...navGroup, - navLinks: [navLink], - }; - } - - return currentGroupsMap; - } - public setup(): ChromeSetup { + const navGroup = this.navGroup.setup(); return { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => { if (this.collapsibleNavHeaderRender) { @@ -190,18 +165,7 @@ export class ChromeService { } this.collapsibleNavHeaderRender = render; }, - addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => { - // Construct a new groups map pointer. - const currentGroupsMap = { ...this.navGroupsMap$.getValue() }; - - const navGroupsMapAfterAdd = navLinks.reduce( - (groupsMap, navLink) => this.addNavLinkToGroup(groupsMap, navGroup, navLink), - currentGroupsMap - ); - - this.navGroupsMap$.next(navGroupsMapAfterAdd); - }, - getNavGroupsMap$: () => this.navGroupsMap$.pipe(takeUntil(this.stop$)), + navGroup, }; } @@ -230,6 +194,7 @@ export class ChromeService { const navLinks = this.navLinks.start({ application, http }); const recentlyAccessed = await this.recentlyAccessed.start({ http }); const docTitle = this.docTitle.start({ document: window.document }); + const navGroup = await this.navGroup.start(); // erase chrome fields from a previous app while switching to a next app application.currentAppId$.subscribe(() => { @@ -298,6 +263,7 @@ export class ChromeService { recentlyAccessed, docTitle, logos, + navGroup, getHeaderComponent: () => (
{ customNavLink$.next(customNavLink); }, - - getNavGroupsMap$: () => this.navGroupsMap$.pipe(takeUntil(this.stop$)), }; } @@ -404,22 +368,10 @@ export class ChromeService { * core.chrome.registerCollapsibleNavHeader(() => ) * ``` * - * @example - * Add navLinks to a nav group: - * ```ts - * core.chrome.addNavLinksToGroup(DEFAULT_NAV_GROUPS, [{ id: 'id-for-your-application' }]) - * ``` - * - * @example - * Get the observable of registered navGroups map: - * ```ts - * core.chrome.getNavGroupsMap$(); - * ``` */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; - addNavLinksToGroup: (group: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void; - getNavGroupsMap$: () => Observable>; + navGroup: ChromeNavGroupServiceSetupContract; } /** @@ -457,6 +409,8 @@ export interface ChromeStart { recentlyAccessed: ChromeRecentlyAccessed; /** {@inheritdoc ChromeDocTitle} */ docTitle: ChromeDocTitle; + /** {@inheritdoc NavGroupService} */ + navGroup: ChromeNavGroupServiceStartContract; /** {@inheritdoc Logos} */ readonly logos: Logos; @@ -546,11 +500,6 @@ export interface ChromeStart { * Get an observable of the current locked state of the nav drawer. */ getIsNavDrawerLocked$(): Observable; - - /** - * Get an observable of the nav groups - */ - getNavGroupsMap$: ChromeSetup['getNavGroupsMap$']; } /** @internal */ @@ -561,11 +510,3 @@ export interface InternalChromeStart extends ChromeStart { */ getHeaderComponent(): JSX.Element; } - -/** @public */ -export interface ChromeRegistrationNavLink { - id: string; - title?: string; - category?: AppCategory; - order?: number; -} diff --git a/src/core/public/chrome/index.ts b/src/core/public/chrome/index.ts index e5bcf3072bd1..ec64291a36d2 100644 --- a/src/core/public/chrome/index.ts +++ b/src/core/public/chrome/index.ts @@ -36,7 +36,6 @@ export { ChromeStart, InternalChromeStart, ChromeHelpExtension, - ChromeRegistrationNavLink, } from './chrome_service'; export { ChromeHelpExtensionMenuLink, @@ -51,3 +50,4 @@ export { ChromeRecentlyAccessed, ChromeRecentlyAccessedHistoryItem } from './rec export { ChromeNavControl, ChromeNavControls } from './nav_controls'; export { ChromeDocTitle } from './doc_title'; export { RightNavigationOrder } from './constants'; +export { ChromeRegistrationNavLink } from './nav_group'; diff --git a/src/core/public/chrome/nav_group/index.ts b/src/core/public/chrome/nav_group/index.ts new file mode 100644 index 000000000000..fae7bd3d0451 --- /dev/null +++ b/src/core/public/chrome/nav_group/index.ts @@ -0,0 +1,12 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export { + ChromeNavGroupService, + ChromeNavGroupServiceSetupContract, + ChromeNavGroupServiceStartContract, + ChromeRegistrationNavLink, + NavGroupItemInMap, +} from './nav_group_service'; diff --git a/src/core/public/chrome/nav_group/nav_group_service.test.ts b/src/core/public/chrome/nav_group/nav_group_service.test.ts new file mode 100644 index 000000000000..4b7fbdfb9b82 --- /dev/null +++ b/src/core/public/chrome/nav_group/nav_group_service.test.ts @@ -0,0 +1,80 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { first } from 'rxjs/operators'; +import { ChromeNavGroupService, ChromeRegistrationNavLink } from './nav_group_service'; + +const mockedGroupFoo = { + id: 'foo', + title: 'foo', + description: 'foo', +}; + +const mockedGroupBar = { + id: 'bar', + title: 'bar', + description: 'bar', +}; + +const mockedNavLinkFoo: ChromeRegistrationNavLink = { + id: 'foo', +}; + +const mockedNavLinkBar: ChromeRegistrationNavLink = { + id: 'bar', +}; + +const getSetup = () => new ChromeNavGroupService().setup(); + +describe('ChromeNavGroupService#setup()', () => { + it('should be able to `addNavLinksToGroup`', async () => { + const warnMock = jest.fn(); + jest.spyOn(console, 'warn').mockImplementation(warnMock); + const chromeNavGroupServiceSetup = getSetup(); + + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupFoo, [mockedGroupFoo, mockedGroupBar]); + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); + const groupsMap = await chromeNavGroupServiceSetup.getNavGroupsMap$().pipe(first()).toPromise(); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(2); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupFoo.id].id).toEqual(mockedGroupFoo.id); + expect(warnMock).toBeCalledTimes(0); + }); + + it('should output warning message if `addNavLinksToGroup` with same group id and navLink id', async () => { + const warnMock = jest.fn(); + jest.spyOn(console, 'warn').mockImplementation(warnMock); + + const chromeSetup = getSetup(); + + chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo, mockedGroupFoo]); + chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); + const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); + expect(warnMock).toBeCalledTimes(1); + expect(warnMock).toBeCalledWith( + `[ChromeService] Navlink of ${mockedGroupFoo.id} has already been registered in group ${mockedGroupFoo.id}` + ); + }); +}); + +describe('ChromeNavGroupService#start()', () => { + it('should be able to get the groups registered through addNavLinksToGroups', async () => { + const chromeNavGroupService = new ChromeNavGroupService(); + const chromeSetup = chromeNavGroupService.setup(); + + chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo]); + chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); + + const chromeStart = await chromeNavGroupService.start(); + + const groupsMap = await chromeStart.getNavGroupsMap$().pipe(first()).toPromise(); + + expect(Object.keys(groupsMap).length).toEqual(2); + expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); + }); +}); diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts new file mode 100644 index 000000000000..7bbd87a71444 --- /dev/null +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -0,0 +1,80 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { BehaviorSubject, Observable } from 'rxjs'; +import { AppCategory, ChromeNavGroup } from 'opensearch-dashboards/public'; + +/** @public */ +export interface ChromeRegistrationNavLink { + id: string; + title?: string; + category?: AppCategory; + order?: number; +} + +export type NavGroupItemInMap = ChromeNavGroup & { + navLinks: ChromeRegistrationNavLink[]; +}; + +export interface ChromeNavGroupServiceSetupContract { + addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void; + getNavGroupsMap$: () => Observable>; +} + +export interface ChromeNavGroupServiceStartContract { + getNavGroupsMap$: () => Observable>; +} + +/** @internal */ +export class ChromeNavGroupService { + private readonly navGroupsMap$ = new BehaviorSubject>({}); + private addNavLinkToGroup( + currentGroupsMap: Record, + navGroup: ChromeNavGroup, + navLink: ChromeRegistrationNavLink + ) { + const matchedGroup = currentGroupsMap[navGroup.id]; + if (matchedGroup) { + const links = matchedGroup.navLinks; + const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); + if (isLinkExistInGroup) { + // eslint-disable-next-line no-console + console.warn( + `[ChromeService] Navlink of ${navLink.id} has already been registered in group ${navGroup.id}` + ); + return currentGroupsMap; + } + matchedGroup.navLinks.push(navLink); + } else { + currentGroupsMap[navGroup.id] = { + ...navGroup, + navLinks: [navLink], + }; + } + + return currentGroupsMap; + } + setup(): ChromeNavGroupServiceSetupContract { + return { + addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => { + // Construct a new groups map pointer. + const currentGroupsMap = { ...this.navGroupsMap$.getValue() }; + + const navGroupsMapAfterAdd = navLinks.reduce( + (groupsMap, navLink) => this.addNavLinkToGroup(groupsMap, navGroup, navLink), + currentGroupsMap + ); + + this.navGroupsMap$.next(navGroupsMapAfterAdd); + }, + getNavGroupsMap$: () => this.navGroupsMap$.pipe(), + }; + } + async start(): Promise { + return { + getNavGroupsMap$: () => this.navGroupsMap$.pipe(), + }; + } +} From b3167e39fe4864f99d7d4de63d651c2315f3be76 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:22:35 +0800 Subject: [PATCH 23/35] feat: remove useless code Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 13 +++++++------ src/core/public/chrome/chrome_service.tsx | 1 - 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index 727ebff1fe7a..e25d31c820cd 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -34,12 +34,12 @@ import { ChromeBadge, ChromeBreadcrumb, ChromeService, InternalChromeStart } fro import { getLogosMock } from '../../common/mocks'; const createSetupContractMock = () => { - const getGroupsMapMock = jest.fn(); - getGroupsMapMock.mockReturnValue(new BehaviorSubject({})); return { registerCollapsibleNavHeader: jest.fn(), - addNavLinksToGroup: jest.fn(), - getNavGroupsMap$: getGroupsMapMock, + navGroup: { + getNavGroupsMap$: jest.fn(() => new BehaviorSubject({})), + addNavLinksToGroup: jest.fn(), + }, }; }; @@ -74,6 +74,9 @@ const createStartContractMock = () => { getCenter$: jest.fn(), getRight$: jest.fn(), }, + navGroup: { + getNavGroupsMap$: jest.fn(() => new BehaviorSubject({})), + }, setAppTitle: jest.fn(), setIsVisible: jest.fn(), getIsVisible$: jest.fn(), @@ -90,7 +93,6 @@ const createStartContractMock = () => { getIsNavDrawerLocked$: jest.fn(), getCustomNavLink$: jest.fn(), setCustomNavLink: jest.fn(), - getNavGroupsMap$: jest.fn(), }; startContract.navLinks.getAll.mockReturnValue([]); startContract.getIsVisible$.mockReturnValue(new BehaviorSubject(false)); @@ -100,7 +102,6 @@ const createStartContractMock = () => { startContract.getCustomNavLink$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getHelpExtension$.mockReturnValue(new BehaviorSubject(undefined)); startContract.getIsNavDrawerLocked$.mockReturnValue(new BehaviorSubject(false)); - startContract.getNavGroupsMap$.mockReturnValue(new BehaviorSubject({})); return startContract; }; diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 5ad25a50098c..2e30038476f3 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -367,7 +367,6 @@ export class ChromeService { * ```ts * core.chrome.registerCollapsibleNavHeader(() => ) * ``` - * */ export interface ChromeSetup { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => void; From 2b9605a99b36398a21f560be2b3b790ceb965450 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:38:26 +0800 Subject: [PATCH 24/35] [Navigation-Next] Add nav group enabled in chrome service(7072) Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 2 + src/core/public/chrome/chrome_service.test.ts | 41 ++++++++++++++++++- src/core/public/chrome/chrome_service.tsx | 9 +++- .../chrome/nav_group/nav_group_service.ts | 23 ++++++++++- src/core/public/core_system.ts | 2 +- .../ui_settings/settings/navigation.test.ts | 20 +++++++-- .../server/ui_settings/settings/navigation.ts | 12 ++++++ .../dashboard_listing.test.tsx.snap | 5 --- .../dashboard_top_nav.test.tsx.snap | 6 --- 9 files changed, 98 insertions(+), 22 deletions(-) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index e25d31c820cd..a825412f9f78 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -39,6 +39,7 @@ const createSetupContractMock = () => { navGroup: { getNavGroupsMap$: jest.fn(() => new BehaviorSubject({})), addNavLinksToGroup: jest.fn(), + getNavGroupEnabled: jest.fn(), }, }; }; @@ -76,6 +77,7 @@ const createStartContractMock = () => { }, navGroup: { getNavGroupsMap$: jest.fn(() => new BehaviorSubject({})), + getNavGroupEnabled: jest.fn(), }, setAppTitle: jest.fn(), setIsVisible: jest.fn(), diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 154f07caf46c..59ecb48343cf 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -90,6 +90,8 @@ async function start({ }: { options?: any; cspConfigMock?: any; startDeps?: ReturnType } = {}) { const service = new ChromeService(options); + service.setup({ uiSettings: startDeps.uiSettings }); + if (cspConfigMock) { startDeps.injectedMetadata.getCspConfig.mockReturnValue(cspConfigMock); } @@ -119,8 +121,9 @@ describe('setup', () => { const customHeaderMock = React.createElement('TestCustomNavHeader'); const renderMock = jest.fn().mockReturnValue(customHeaderMock); const chrome = new ChromeService({ browserSupportsCsp: true }); + const uiSettings = uiSettingsServiceMock.createSetupContract(); - const chromeSetup = chrome.setup(); + const chromeSetup = chrome.setup({ uiSettings }); chromeSetup.registerCollapsibleNavHeader(renderMock); const chromeStart = await chrome.start(defaultStartDeps()); @@ -135,8 +138,9 @@ describe('setup', () => { const customHeaderMock = React.createElement('TestCustomNavHeader'); const renderMock = jest.fn().mockReturnValue(customHeaderMock); const chrome = new ChromeService({ browserSupportsCsp: true }); + const uiSettings = uiSettingsServiceMock.createSetupContract(); - const chromeSetup = chrome.setup(); + const chromeSetup = chrome.setup({ uiSettings }); // call 1st time chromeSetup.registerCollapsibleNavHeader(renderMock); // call 2nd time @@ -146,6 +150,15 @@ describe('setup', () => { '[ChromeService] An existing custom collapsible navigation bar header render has been overridden.' ); }); + + it('should return navGroupEnabled from ui settings', () => { + const chrome = new ChromeService({ browserSupportsCsp: true }); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); + + const chromeSetup = chrome.setup({ uiSettings }); + expect(chromeSetup.getNavGroupEnabled()).toBe(true); + }); }); describe('start', () => { @@ -485,6 +498,15 @@ describe('start', () => { `); }); }); + + it('should return navGroupEnabled from ui settings', async () => { + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); + const startDeps = defaultStartDeps(); + const { chrome } = await start({ startDeps: { ...startDeps, uiSettings } }); + + expect(chrome.getNavGroupEnabled()).toBe(true); + }); }); describe('stop', () => { @@ -516,4 +538,19 @@ describe('stop', () => { ).toPromise() ).resolves.toBe(undefined); }); + + it('should not update navGroupEnabled after stopped', async () => { + const uiSettings = uiSettingsServiceMock.createSetupContract(); + const navGroupEnabled$ = new Rx.BehaviorSubject(true); + uiSettings.get$.mockImplementation(() => navGroupEnabled$); + const startDeps = defaultStartDeps(); + const { chrome, service } = await start({ startDeps: { ...startDeps, uiSettings } }); + + navGroupEnabled$.next(false); + expect(chrome.getNavGroupEnabled()).toBe(false); + + service.stop(); + navGroupEnabled$.next(true); + expect(chrome.getNavGroupEnabled()).toBe(false); + }); }); diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 2e30038476f3..6ca811cf948e 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -95,6 +95,10 @@ interface ConstructorParams { browserSupportsCsp: boolean; } +export interface SetupDeps { + uiSettings: IUiSettingsClient; +} + export interface StartDeps { application: InternalApplicationStart; docLinks: DocLinksStart; @@ -153,8 +157,8 @@ export class ChromeService { ); } - public setup(): ChromeSetup { - const navGroup = this.navGroup.setup(); + public setup({ uiSettings }: SetupDeps): ChromeSetup { + const navGroup = this.navGroup.setup({ uiSettings }); return { registerCollapsibleNavHeader: (render: CollapsibleNavHeaderRender) => { if (this.collapsibleNavHeaderRender) { @@ -355,6 +359,7 @@ export class ChromeService { public stop() { this.navLinks.stop(); this.stop$.next(); + this.navGroup.stop(); } } diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 7bbd87a71444..3459537e4173 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -3,8 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, Observable, Subscription } from 'rxjs'; import { AppCategory, ChromeNavGroup } from 'opensearch-dashboards/public'; +import { IUiSettingsClient } from '../../ui_settings'; /** @public */ export interface ChromeRegistrationNavLink { @@ -21,15 +22,22 @@ export type NavGroupItemInMap = ChromeNavGroup & { export interface ChromeNavGroupServiceSetupContract { addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void; getNavGroupsMap$: () => Observable>; + /** + * Get a boolean value to indicates whether use case is enabled + */ + getNavGroupEnabled: () => boolean; } export interface ChromeNavGroupServiceStartContract { getNavGroupsMap$: () => Observable>; + getNavGroupEnabled: ChromeNavGroupServiceSetupContract['getNavGroupEnabled']; } /** @internal */ export class ChromeNavGroupService { private readonly navGroupsMap$ = new BehaviorSubject>({}); + private navGroupEnabled: boolean = false; + private navGroupEnabledUiSettingsSubscription: Subscription | undefined; private addNavLinkToGroup( currentGroupsMap: Record, navGroup: ChromeNavGroup, @@ -56,7 +64,13 @@ export class ChromeNavGroupService { return currentGroupsMap; } - setup(): ChromeNavGroupServiceSetupContract { + setup({ uiSettings }: { uiSettings: IUiSettingsClient }): ChromeNavGroupServiceSetupContract { + this.navGroupEnabledUiSettingsSubscription = uiSettings + .get$('navGroupEnabled', false) + .subscribe((value) => { + this.navGroupEnabled = value; + }); + return { addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => { // Construct a new groups map pointer. @@ -70,11 +84,16 @@ export class ChromeNavGroupService { this.navGroupsMap$.next(navGroupsMapAfterAdd); }, getNavGroupsMap$: () => this.navGroupsMap$.pipe(), + getNavGroupEnabled: () => this.navGroupEnabled, }; } async start(): Promise { return { getNavGroupsMap$: () => this.navGroupsMap$.pipe(), + getNavGroupEnabled: () => this.navGroupEnabled, }; } + async stop() { + this.navGroupEnabledUiSettingsSubscription?.unsubscribe(); + } } diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 58e92a1fa355..b01833d3e08b 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -171,7 +171,7 @@ export class CoreSystem { }); const application = this.application.setup({ context, http }); this.coreApp.setup({ application, http, injectedMetadata, notifications }); - const chrome = this.chrome.setup(); + const chrome = this.chrome.setup({ uiSettings }); const core: InternalCoreSetup = { application, diff --git a/src/core/server/ui_settings/settings/navigation.test.ts b/src/core/server/ui_settings/settings/navigation.test.ts index 06081e451dd7..c8a9b545cab7 100644 --- a/src/core/server/ui_settings/settings/navigation.test.ts +++ b/src/core/server/ui_settings/settings/navigation.test.ts @@ -58,10 +58,22 @@ describe('navigation settings', () => { expect(() => validate('modern')).not.toThrow(); expect(() => validate('legacy')).not.toThrow(); expect(() => validate('invalid')).toThrowErrorMatchingInlineSnapshot(` -"types that failed validation: -- [0]: expected value to equal [modern] -- [1]: expected value to equal [legacy]" -`); + "types that failed validation: + - [0]: expected value to equal [modern] + - [1]: expected value to equal [legacy]" + `); + }); + }); + + describe('navGroupEnabled', () => { + const validate = getValidationFn(navigationSettings.navGroupEnabled); + + it('should only accept valid values', () => { + expect(() => validate(false)).not.toThrow(); + expect(() => validate(true)).not.toThrow(); + expect(() => validate('invalid')).toThrowErrorMatchingInlineSnapshot( + `"expected value of type [boolean] but got [string]"` + ); }); }); }); diff --git a/src/core/server/ui_settings/settings/navigation.ts b/src/core/server/ui_settings/settings/navigation.ts index 2ea04e180f03..a270797133ef 100644 --- a/src/core/server/ui_settings/settings/navigation.ts +++ b/src/core/server/ui_settings/settings/navigation.ts @@ -80,5 +80,17 @@ export const getNavigationSettings = (): Record => { category: ['appearance'], schema: schema.oneOf([schema.literal('modern'), schema.literal('legacy')]), }, + navGroupEnabled: { + name: i18n.translate('core.ui_settings.params.navGroupEnabledName', { + defaultMessage: 'Enable nav group', + }), + value: false, + description: i18n.translate('core.ui_settings.params.navGroupEnabledDesc', { + defaultMessage: 'Used to control whether navigation items are grouped into use cases.', + }), + category: ['appearance'], + requiresPageReload: true, + schema: schema.boolean(), + }, }; }; diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap index 0e911b8fb7f3..f4c5ca0b0a0a 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -148,7 +148,6 @@ exports[`dashboard listing hideWriteControls 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1323,7 +1322,6 @@ exports[`dashboard listing render table listing with initial filters from URL 1` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2559,7 +2557,6 @@ exports[`dashboard listing renders call to action when no dashboards exist 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3795,7 +3792,6 @@ exports[`dashboard listing renders table rows 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5031,7 +5027,6 @@ exports[`dashboard listing renders warning when listingLimit is exceeded 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 0719d615fe03..831eb5bd61c0 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -136,7 +136,6 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -1137,7 +1136,6 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -2138,7 +2136,6 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -3139,7 +3136,6 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -4140,7 +4136,6 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { @@ -5141,7 +5136,6 @@ exports[`Dashboard top nav render with all components 1`] = ` "getHelpExtension$": [MockFunction], "getIsNavDrawerLocked$": [MockFunction], "getIsVisible$": [MockFunction], - "getNavGroupsMap$": [MockFunction], "logos": Object { "AnimatedMark": Object { "dark": Object { From 336e5f6307b00b6689667af7e417e123a6b5dcdf Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:41:01 +0800 Subject: [PATCH 25/35] Remove useless code Signed-off-by: SuZhou-Joe --- .../ui_settings/settings/navigation.test.ts | 20 ++++--------------- .../server/ui_settings/settings/navigation.ts | 12 ----------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/core/server/ui_settings/settings/navigation.test.ts b/src/core/server/ui_settings/settings/navigation.test.ts index c8a9b545cab7..06081e451dd7 100644 --- a/src/core/server/ui_settings/settings/navigation.test.ts +++ b/src/core/server/ui_settings/settings/navigation.test.ts @@ -58,22 +58,10 @@ describe('navigation settings', () => { expect(() => validate('modern')).not.toThrow(); expect(() => validate('legacy')).not.toThrow(); expect(() => validate('invalid')).toThrowErrorMatchingInlineSnapshot(` - "types that failed validation: - - [0]: expected value to equal [modern] - - [1]: expected value to equal [legacy]" - `); - }); - }); - - describe('navGroupEnabled', () => { - const validate = getValidationFn(navigationSettings.navGroupEnabled); - - it('should only accept valid values', () => { - expect(() => validate(false)).not.toThrow(); - expect(() => validate(true)).not.toThrow(); - expect(() => validate('invalid')).toThrowErrorMatchingInlineSnapshot( - `"expected value of type [boolean] but got [string]"` - ); +"types that failed validation: +- [0]: expected value to equal [modern] +- [1]: expected value to equal [legacy]" +`); }); }); }); diff --git a/src/core/server/ui_settings/settings/navigation.ts b/src/core/server/ui_settings/settings/navigation.ts index a270797133ef..2ea04e180f03 100644 --- a/src/core/server/ui_settings/settings/navigation.ts +++ b/src/core/server/ui_settings/settings/navigation.ts @@ -80,17 +80,5 @@ export const getNavigationSettings = (): Record => { category: ['appearance'], schema: schema.oneOf([schema.literal('modern'), schema.literal('legacy')]), }, - navGroupEnabled: { - name: i18n.translate('core.ui_settings.params.navGroupEnabledName', { - defaultMessage: 'Enable nav group', - }), - value: false, - description: i18n.translate('core.ui_settings.params.navGroupEnabledDesc', { - defaultMessage: 'Used to control whether navigation items are grouped into use cases.', - }), - category: ['appearance'], - requiresPageReload: true, - schema: schema.boolean(), - }, }; }; From 409e332ce64af521ccedcf3e4fbcbb09282c3825 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:52:01 +0800 Subject: [PATCH 26/35] use homepage flag Signed-off-by: SuZhou-Joe --- src/core/public/chrome/nav_group/nav_group_service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 3459537e4173..bd5078916630 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -66,7 +66,7 @@ export class ChromeNavGroupService { } setup({ uiSettings }: { uiSettings: IUiSettingsClient }): ChromeNavGroupServiceSetupContract { this.navGroupEnabledUiSettingsSubscription = uiSettings - .get$('navGroupEnabled', false) + .get$('home:useNewHomePage', false) .subscribe((value) => { this.navGroupEnabled = value; }); From d2ecfd9d2c6149a9fece309b135adeb253b93079 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 16:58:00 +0800 Subject: [PATCH 27/35] feat: update README Signed-off-by: SuZhou-Joe --- src/core/public/chrome/README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/core/public/chrome/README.md b/src/core/public/chrome/README.md index 6ec765a3bb0b..7d86aa7f2c1c 100644 --- a/src/core/public/chrome/README.md +++ b/src/core/public/chrome/README.md @@ -6,6 +6,7 @@ - [Nav Links Service](#navlinksservice-) - [Recently Accessed Service](#recentlyaccessedservice-) - [Doc Title Service](#doctitleservice-) +- [Nav Group Service](#chromenavgroupservice-) - [UI](#ui-) ## About : @@ -112,6 +113,31 @@ Gets an Observable of the array of recently accessed history :- chrome.docTitle.change('My application title') chrome.docTitle.change(['My application', 'My section']) ``` +### ChromeNavGroupService: +- Interface : ChromeNavGroup +- Signature : ```navGroup: ChromeNavGroupService``` +- Methods : +Add nav links to group :- + +`addNavLinksToGroup: (group: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void;` + +Gets an Observable of the array of registered groups :- + +`getNavGroupsMap$: Observable>` +##### Register a new group with a navLink + + ```ts + coreSetup.chrome.navGroup.addNavLinksToGroup( + { + id: 'my-group', + title: 'A demo group', + description: 'description for demo group' + }, + [{ + id: 'nav' + }] + ) + ``` ### UI : ###### consists of tsx/scss files && renders UI components from css Library e.g `````` From 7713b2201d383511df82cb3dcaa52632d1b471f8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 27 Jun 2024 17:38:45 +0800 Subject: [PATCH 28/35] feat: expose sorted navLinks Signed-off-by: SuZhou-Joe --- .../chrome/nav_group/nav_group_service.ts | 37 +++++++-- src/core/public/chrome/utils.ts | 81 +++++++++++++++++++ 2 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 src/core/public/chrome/utils.ts diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index bd5078916630..e43f4eafff6f 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -3,9 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject, Observable, Subscription } from 'rxjs'; -import { AppCategory, ChromeNavGroup } from 'opensearch-dashboards/public'; +import { BehaviorSubject, combineLatest, Observable, Subscription } from 'rxjs'; +import { AppCategory, ChromeNavGroup, ChromeNavLink } from 'opensearch-dashboards/public'; +import { map } from 'rxjs/operators'; import { IUiSettingsClient } from '../../ui_settings'; +import { + flattenLinksOrCategories, + fullfillRegistrationLinksToChromeNavLinks, + getOrderedLinksOrCategories, +} from '../utils'; /** @public */ export interface ChromeRegistrationNavLink { @@ -13,6 +19,11 @@ export interface ChromeRegistrationNavLink { title?: string; category?: AppCategory; order?: number; + + /** + * link with parentNavLinkId field will be displayed as nested items in navigation. + */ + parentNavLinkId?: string; } export type NavGroupItemInMap = ChromeNavGroup & { @@ -21,7 +32,6 @@ export type NavGroupItemInMap = ChromeNavGroup & { export interface ChromeNavGroupServiceSetupContract { addNavLinksToGroup: (navGroup: ChromeNavGroup, navLinks: ChromeRegistrationNavLink[]) => void; - getNavGroupsMap$: () => Observable>; /** * Get a boolean value to indicates whether use case is enabled */ @@ -36,6 +46,7 @@ export interface ChromeNavGroupServiceStartContract { /** @internal */ export class ChromeNavGroupService { private readonly navGroupsMap$ = new BehaviorSubject>({}); + private navLinks$: Observable>> = new BehaviorSubject([]); private navGroupEnabled: boolean = false; private navGroupEnabledUiSettingsSubscription: Subscription | undefined; private addNavLinkToGroup( @@ -64,6 +75,23 @@ export class ChromeNavGroupService { return currentGroupsMap; } + private getLinksSortedNavGroupsMap() { + return combineLatest([this.navGroupsMap$, this.navLinks$]).pipe( + map(([navGroupsMap, navLinks]) => { + return Object.keys(navGroupsMap).reduce((sortedNavGroupsMap, navGroupId) => { + const navGroup = navGroupsMap[navGroupId]; + const sortedNavLinks = getOrderedLinksOrCategories( + fullfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks) + ); + sortedNavGroupsMap[navGroupId] = { + ...navGroup, + navLinks: flattenLinksOrCategories(sortedNavLinks), + }; + return sortedNavGroupsMap; + }, {} as Record); + }) + ); + } setup({ uiSettings }: { uiSettings: IUiSettingsClient }): ChromeNavGroupServiceSetupContract { this.navGroupEnabledUiSettingsSubscription = uiSettings .get$('home:useNewHomePage', false) @@ -83,13 +111,12 @@ export class ChromeNavGroupService { this.navGroupsMap$.next(navGroupsMapAfterAdd); }, - getNavGroupsMap$: () => this.navGroupsMap$.pipe(), getNavGroupEnabled: () => this.navGroupEnabled, }; } async start(): Promise { return { - getNavGroupsMap$: () => this.navGroupsMap$.pipe(), + getNavGroupsMap$: () => this.getLinksSortedNavGroupsMap(), getNavGroupEnabled: () => this.navGroupEnabled, }; } diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts new file mode 100644 index 000000000000..0570113bc43e --- /dev/null +++ b/src/core/public/chrome/utils.ts @@ -0,0 +1,81 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AppCategory } from 'opensearch-dashboards/public'; +import { groupBy, sortBy } from 'lodash'; +import { ChromeNavLink } from './nav_links'; +import { ChromeRegistrationNavLink } from './nav_group'; + +const LinkItemType = { + LINK: 'link', + CATEGORY: 'category', +} as const; + +export type LinkItem = { order?: number } & ( + | { itemType: 'link'; link: ChromeNavLink } + | { itemType: 'category'; category?: AppCategory; links?: ChromeNavLink[] } +); + +function getAllCategories(allCategorizedLinks: Record) { + const allCategories = {} as Record; + + for (const [key, value] of Object.entries(allCategorizedLinks)) { + allCategories[key] = value[0].category; + } + + return allCategories; +} + +export function fullfillRegistrationLinksToChromeNavLinks( + registerNavLinks: ChromeRegistrationNavLink[], + navLinks: ChromeNavLink[] +): Array { + const allExistingNavLinkId = navLinks.map((link) => link.id); + return ( + registerNavLinks + ?.filter((navLink) => allExistingNavLinkId.includes(navLink.id)) + .map((navLink) => ({ + ...navLinks[allExistingNavLinkId.indexOf(navLink.id)], + ...navLink, + })) || [] + ); +} + +export const getOrderedLinks = (navLinks: ChromeNavLink[]): ChromeNavLink[] => + sortBy(navLinks, (link) => link.order); + +export function getOrderedLinksOrCategories(navLinks: ChromeNavLink[]): LinkItem[] { + const groupedNavLinks = groupBy(navLinks, (link) => link?.category?.id); + const { undefined: unknowns = [], ...allCategorizedLinks } = groupedNavLinks; + const categoryDictionary = getAllCategories(allCategorizedLinks); + return sortBy( + [ + ...unknowns.map((linkWithoutCategory) => ({ + itemType: LinkItemType.LINK, + link: linkWithoutCategory, + order: linkWithoutCategory.order, + })), + ...Object.keys(allCategorizedLinks).map((categoryKey) => ({ + itemType: LinkItemType.CATEGORY, + category: categoryDictionary[categoryKey], + order: categoryDictionary[categoryKey]?.order, + links: getOrderedLinks(allCategorizedLinks[categoryKey]), + })), + ], + (item) => item.order + ); +} + +export function flattenLinksOrCategories(linkItems: LinkItem[]): ChromeNavLink[] { + return linkItems.reduce((acc, item) => { + if (item.itemType === LinkItemType.LINK) { + acc.push(item.link); + } else if (item.itemType === LinkItemType.CATEGORY) { + acc.push(...(item.links || [])); + } + + return acc; + }, [] as ChromeNavLink[]); +} From 6886aaa664f36def3e9758c4975537728adc62e6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 28 Jun 2024 10:07:05 +0800 Subject: [PATCH 29/35] fix: unit test Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.test.ts | 33 ---- src/core/public/chrome/chrome_service.tsx | 2 +- .../nav_group/nav_group_service.test.ts | 164 ++++++++++++++++-- .../chrome/nav_group/nav_group_service.ts | 46 +++-- 4 files changed, 177 insertions(+), 68 deletions(-) diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 59ecb48343cf..1ef3fd34e5d9 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -150,15 +150,6 @@ describe('setup', () => { '[ChromeService] An existing custom collapsible navigation bar header render has been overridden.' ); }); - - it('should return navGroupEnabled from ui settings', () => { - const chrome = new ChromeService({ browserSupportsCsp: true }); - const uiSettings = uiSettingsServiceMock.createSetupContract(); - uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); - - const chromeSetup = chrome.setup({ uiSettings }); - expect(chromeSetup.getNavGroupEnabled()).toBe(true); - }); }); describe('start', () => { @@ -498,15 +489,6 @@ describe('start', () => { `); }); }); - - it('should return navGroupEnabled from ui settings', async () => { - const uiSettings = uiSettingsServiceMock.createSetupContract(); - uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); - const startDeps = defaultStartDeps(); - const { chrome } = await start({ startDeps: { ...startDeps, uiSettings } }); - - expect(chrome.getNavGroupEnabled()).toBe(true); - }); }); describe('stop', () => { @@ -538,19 +520,4 @@ describe('stop', () => { ).toPromise() ).resolves.toBe(undefined); }); - - it('should not update navGroupEnabled after stopped', async () => { - const uiSettings = uiSettingsServiceMock.createSetupContract(); - const navGroupEnabled$ = new Rx.BehaviorSubject(true); - uiSettings.get$.mockImplementation(() => navGroupEnabled$); - const startDeps = defaultStartDeps(); - const { chrome, service } = await start({ startDeps: { ...startDeps, uiSettings } }); - - navGroupEnabled$.next(false); - expect(chrome.getNavGroupEnabled()).toBe(false); - - service.stop(); - navGroupEnabled$.next(true); - expect(chrome.getNavGroupEnabled()).toBe(false); - }); }); diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 6ca811cf948e..9dafa412c30c 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -198,7 +198,7 @@ export class ChromeService { const navLinks = this.navLinks.start({ application, http }); const recentlyAccessed = await this.recentlyAccessed.start({ http }); const docTitle = this.docTitle.start({ document: window.document }); - const navGroup = await this.navGroup.start(); + const navGroup = await this.navGroup.start({ navLinks }); // erase chrome fields from a previous app while switching to a next app application.currentAppId$.subscribe(() => { diff --git a/src/core/public/chrome/nav_group/nav_group_service.test.ts b/src/core/public/chrome/nav_group/nav_group_service.test.ts index 4b7fbdfb9b82..969d7fffdca4 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.test.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.test.ts @@ -3,8 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as Rx from 'rxjs'; import { first } from 'rxjs/operators'; import { ChromeNavGroupService, ChromeRegistrationNavLink } from './nav_group_service'; +import { uiSettingsServiceMock } from '../../ui_settings/ui_settings_service.mock'; +import { NavLinksService } from '../nav_links'; +import { applicationServiceMock, httpServiceMock } from '../../mocks'; +import { AppCategory } from 'opensearch-dashboards/public'; const mockedGroupFoo = { id: 'foo', @@ -20,23 +25,72 @@ const mockedGroupBar = { const mockedNavLinkFoo: ChromeRegistrationNavLink = { id: 'foo', + order: 10, }; const mockedNavLinkBar: ChromeRegistrationNavLink = { id: 'bar', + order: 20, }; -const getSetup = () => new ChromeNavGroupService().setup(); +const mockedCategoryFoo: AppCategory = { + id: 'foo', + order: 15, + label: 'foo', +}; + +const mockedCategoryBar: AppCategory = { + id: 'bar', + order: 25, + label: 'bar', +}; + +const mockedHttpService = httpServiceMock.createStartContract(); +const mockedApplicationService = applicationServiceMock.createInternalStartContract(); +const mockedNavLink = new NavLinksService(); +const mockedNavLinkService = mockedNavLink.start({ + http: mockedHttpService, + application: mockedApplicationService, +}); describe('ChromeNavGroupService#setup()', () => { + const mockedGetNavLinks = jest.fn(); + jest.spyOn(mockedNavLinkService, 'getNavLinks$').mockImplementation(mockedGetNavLinks); + mockedGetNavLinks.mockReturnValue( + new Rx.BehaviorSubject([ + { + id: 'foo', + }, + { + id: 'bar', + }, + { + id: 'foo-in-category-foo', + }, + { + id: 'foo-in-category-bar', + }, + { + id: 'bar-in-category-foo', + }, + { + id: 'bar-in-category-bar', + }, + ]) + ); it('should be able to `addNavLinksToGroup`', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); - const chromeNavGroupServiceSetup = getSetup(); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + const chromeNavGroupService = new ChromeNavGroupService(); + const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings }); chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupFoo, [mockedGroupFoo, mockedGroupBar]); chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); - const groupsMap = await chromeNavGroupServiceSetup.getNavGroupsMap$().pipe(first()).toPromise(); + const chromeNavGroupServiceStart = await chromeNavGroupService.start({ + navLinks: mockedNavLinkService, + }); + const groupsMap = await chromeNavGroupServiceStart.getNavGroupsMap$().pipe(first()).toPromise(); expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(2); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); expect(groupsMap[mockedGroupFoo.id].id).toEqual(mockedGroupFoo.id); @@ -46,12 +100,19 @@ describe('ChromeNavGroupService#setup()', () => { it('should output warning message if `addNavLinksToGroup` with same group id and navLink id', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + const chromeNavGroupService = new ChromeNavGroupService(); + const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings }); - const chromeSetup = getSetup(); - - chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo, mockedGroupFoo]); - chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); - const groupsMap = await chromeSetup.getNavGroupsMap$().pipe(first()).toPromise(); + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupFoo, [ + mockedNavLinkFoo, + mockedGroupFoo, + ]); + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupBar, [mockedGroupBar]); + const chromeNavGroupServiceStart = await chromeNavGroupService.start({ + navLinks: mockedNavLinkService, + }); + const groupsMap = await chromeNavGroupServiceStart.getNavGroupsMap$().pipe(first()).toPromise(); expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); expect(warnMock).toBeCalledTimes(1); @@ -59,22 +120,93 @@ describe('ChromeNavGroupService#setup()', () => { `[ChromeService] Navlink of ${mockedGroupFoo.id} has already been registered in group ${mockedGroupFoo.id}` ); }); + + it('should return navGroupEnabled from ui settings', () => { + const chrome = new ChromeNavGroupService(); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); + + const chromeSetup = chrome.setup({ uiSettings }); + expect(chromeSetup.getNavGroupEnabled()).toBe(true); + }); }); describe('ChromeNavGroupService#start()', () => { - it('should be able to get the groups registered through addNavLinksToGroups', async () => { + it('should be able to get the groups registered through addNavLinksToGroups with sorted order', async () => { const chromeNavGroupService = new ChromeNavGroupService(); - const chromeSetup = chromeNavGroupService.setup(); - - chromeSetup.addNavLinksToGroup(mockedGroupFoo, [mockedNavLinkFoo]); - chromeSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); - - const chromeStart = await chromeNavGroupService.start(); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings }); + + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupFoo, [ + mockedNavLinkFoo, + { + ...mockedNavLinkFoo, + id: 'foo-in-category-foo', + category: mockedCategoryFoo, + }, + { + ...mockedNavLinkBar, + id: 'bar-in-category-foo', + category: mockedCategoryFoo, + }, + { + ...mockedNavLinkFoo, + id: 'foo-in-category-bar', + category: mockedCategoryBar, + }, + { + ...mockedNavLinkBar, + id: 'bar-in-category-bar', + category: mockedCategoryBar, + }, + mockedNavLinkBar, + ]); + chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); + + const chromeStart = await chromeNavGroupService.start({ navLinks: mockedNavLinkService }); const groupsMap = await chromeStart.getNavGroupsMap$().pipe(first()).toPromise(); expect(Object.keys(groupsMap).length).toEqual(2); - expect(groupsMap[mockedGroupFoo.id].navLinks.length).toEqual(1); + expect(groupsMap[mockedGroupFoo.id].navLinks.map((item) => item.id)).toEqual([ + 'foo', + 'foo-in-category-foo', + 'bar-in-category-foo', + 'bar', + 'foo-in-category-bar', + 'bar-in-category-bar', + ]); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); }); + + it('should return navGroupEnabled from ui settings', async () => { + const chromeNavGroupService = new ChromeNavGroupService(); + const uiSettings = uiSettingsServiceMock.createSetupContract(); + uiSettings.get$.mockImplementation(() => new Rx.BehaviorSubject(true)); + chromeNavGroupService.setup({ uiSettings }); + const chromeNavGroupServiceStart = await chromeNavGroupService.start({ + navLinks: mockedNavLinkService, + }); + + expect(chromeNavGroupServiceStart.getNavGroupEnabled()).toBe(true); + }); + + it('should not update navGroupEnabled after stopped', async () => { + const uiSettings = uiSettingsServiceMock.createSetupContract(); + const navGroupEnabled$ = new Rx.BehaviorSubject(true); + uiSettings.get$.mockImplementation(() => navGroupEnabled$); + + const chromeNavGroupService = new ChromeNavGroupService(); + chromeNavGroupService.setup({ uiSettings }); + const chromeNavGroupServiceStart = await chromeNavGroupService.start({ + navLinks: mockedNavLinkService, + }); + + navGroupEnabled$.next(false); + expect(chromeNavGroupServiceStart.getNavGroupEnabled()).toBe(false); + + chromeNavGroupService.stop(); + navGroupEnabled$.next(true); + expect(chromeNavGroupServiceStart.getNavGroupEnabled()).toBe(false); + }); }); diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index e43f4eafff6f..341ae3cdd0ce 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -3,15 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject, combineLatest, Observable, Subscription } from 'rxjs'; +import { BehaviorSubject, combineLatest, Observable, ReplaySubject, Subscription } from 'rxjs'; import { AppCategory, ChromeNavGroup, ChromeNavLink } from 'opensearch-dashboards/public'; -import { map } from 'rxjs/operators'; +import { map, takeUntil } from 'rxjs/operators'; import { IUiSettingsClient } from '../../ui_settings'; import { flattenLinksOrCategories, fullfillRegistrationLinksToChromeNavLinks, getOrderedLinksOrCategories, } from '../utils'; +import { ChromeNavLinks, NavLinksService } from '../nav_links'; /** @public */ export interface ChromeRegistrationNavLink { @@ -46,6 +47,7 @@ export interface ChromeNavGroupServiceStartContract { /** @internal */ export class ChromeNavGroupService { private readonly navGroupsMap$ = new BehaviorSubject>({}); + private readonly stop$ = new ReplaySubject(1); private navLinks$: Observable>> = new BehaviorSubject([]); private navGroupEnabled: boolean = false; private navGroupEnabledUiSettingsSubscription: Subscription | undefined; @@ -76,21 +78,23 @@ export class ChromeNavGroupService { return currentGroupsMap; } private getLinksSortedNavGroupsMap() { - return combineLatest([this.navGroupsMap$, this.navLinks$]).pipe( - map(([navGroupsMap, navLinks]) => { - return Object.keys(navGroupsMap).reduce((sortedNavGroupsMap, navGroupId) => { - const navGroup = navGroupsMap[navGroupId]; - const sortedNavLinks = getOrderedLinksOrCategories( - fullfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks) - ); - sortedNavGroupsMap[navGroupId] = { - ...navGroup, - navLinks: flattenLinksOrCategories(sortedNavLinks), - }; - return sortedNavGroupsMap; - }, {} as Record); - }) - ); + return combineLatest([this.navGroupsMap$, this.navLinks$]) + .pipe(takeUntil(this.stop$)) + .pipe( + map(([navGroupsMap, navLinks]) => { + return Object.keys(navGroupsMap).reduce((sortedNavGroupsMap, navGroupId) => { + const navGroup = navGroupsMap[navGroupId]; + const sortedNavLinks = getOrderedLinksOrCategories( + fullfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks) + ); + sortedNavGroupsMap[navGroupId] = { + ...navGroup, + navLinks: flattenLinksOrCategories(sortedNavLinks), + }; + return sortedNavGroupsMap; + }, {} as Record); + }) + ); } setup({ uiSettings }: { uiSettings: IUiSettingsClient }): ChromeNavGroupServiceSetupContract { this.navGroupEnabledUiSettingsSubscription = uiSettings @@ -114,13 +118,19 @@ export class ChromeNavGroupService { getNavGroupEnabled: () => this.navGroupEnabled, }; } - async start(): Promise { + async start({ + navLinks, + }: { + navLinks: ChromeNavLinks; + }): Promise { + this.navLinks$ = navLinks.getNavLinks$(); return { getNavGroupsMap$: () => this.getLinksSortedNavGroupsMap(), getNavGroupEnabled: () => this.navGroupEnabled, }; } async stop() { + this.stop$.next(); this.navGroupEnabledUiSettingsSubscription?.unsubscribe(); } } From 26a5ed657c2c5b65da44e2df8a73506103c19d29 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 28 Jun 2024 10:11:47 +0800 Subject: [PATCH 30/35] fix: bootstrap error Signed-off-by: SuZhou-Joe --- src/core/public/chrome/chrome_service.mock.ts | 1 - src/core/public/chrome/chrome_service.tsx | 2 +- src/core/public/chrome/nav_group/nav_group_service.ts | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts index a825412f9f78..62f35ca693a6 100644 --- a/src/core/public/chrome/chrome_service.mock.ts +++ b/src/core/public/chrome/chrome_service.mock.ts @@ -37,7 +37,6 @@ const createSetupContractMock = () => { return { registerCollapsibleNavHeader: jest.fn(), navGroup: { - getNavGroupsMap$: jest.fn(() => new BehaviorSubject({})), addNavLinksToGroup: jest.fn(), getNavGroupEnabled: jest.fn(), }, diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 9dafa412c30c..822b4c23822e 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -358,8 +358,8 @@ export class ChromeService { public stop() { this.navLinks.stop(); - this.stop$.next(); this.navGroup.stop(); + this.stop$.next(); } } diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 341ae3cdd0ce..7792deb8a288 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -12,7 +12,7 @@ import { fullfillRegistrationLinksToChromeNavLinks, getOrderedLinksOrCategories, } from '../utils'; -import { ChromeNavLinks, NavLinksService } from '../nav_links'; +import { ChromeNavLinks } from '../nav_links'; /** @public */ export interface ChromeRegistrationNavLink { @@ -77,7 +77,7 @@ export class ChromeNavGroupService { return currentGroupsMap; } - private getLinksSortedNavGroupsMap() { + private getSortedNavGroupsMap() { return combineLatest([this.navGroupsMap$, this.navLinks$]) .pipe(takeUntil(this.stop$)) .pipe( @@ -125,7 +125,7 @@ export class ChromeNavGroupService { }): Promise { this.navLinks$ = navLinks.getNavLinks$(); return { - getNavGroupsMap$: () => this.getLinksSortedNavGroupsMap(), + getNavGroupsMap$: () => this.getSortedNavGroupsMap(), getNavGroupEnabled: () => this.navGroupEnabled, }; } From f3e1049117a6ad68088efba9f527afaa6c592011 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 28 Jun 2024 11:04:10 +0800 Subject: [PATCH 31/35] fix: snapshot error Signed-off-by: SuZhou-Joe --- src/core/public/chrome/utils.test.ts | 138 ++++++++++++++++++ src/core/public/chrome/utils.ts | 4 +- .../dashboard_listing.test.tsx.snap | 20 +++ .../dashboard_top_nav.test.tsx.snap | 24 +++ 4 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 src/core/public/chrome/utils.test.ts diff --git a/src/core/public/chrome/utils.test.ts b/src/core/public/chrome/utils.test.ts new file mode 100644 index 000000000000..7d5b82228855 --- /dev/null +++ b/src/core/public/chrome/utils.test.ts @@ -0,0 +1,138 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ChromeRegistrationNavLink } from './nav_group'; +import { ChromeNavLink } from './nav_links'; +import { + getAllCategories, + fullfillRegistrationLinksToChromeNavLinks, + getOrderedLinks, + getOrderedLinksOrCategories, + flattenLinksOrCategories, +} from './utils'; + +const mockedNonCategoryLink = { + id: 'no-category', + title: 'no-category', + baseUrl: '', + href: '', + order: 6, +}; + +const mockedNavLinkA = { + id: 'a', + title: 'a', + baseUrl: '', + href: '', + category: { + id: 'a', + label: 'a', + order: 10, + }, + order: 10, +}; + +const mockedNavLinkB = { + id: 'b', + title: 'b', + baseUrl: '', + href: '', + category: { + id: 'b', + label: 'b', + order: 5, + }, + order: 5, +}; + +describe('getAllCategories', () => { + it('should return all categories', () => { + const links = { + a: [mockedNavLinkA], + b: [mockedNavLinkB], + }; + const categories = getAllCategories(links); + expect(categories).toEqual({ + a: { + id: 'a', + label: 'a', + order: 10, + }, + b: { + id: 'b', + label: 'b', + order: 5, + }, + }); + }); +}); + +describe('fullfillRegistrationLinksToChromeNavLinks', () => { + it('should return fullfilled ChromeNavLink', () => { + const registrationNavLinks: ChromeRegistrationNavLink[] = [ + { + id: 'a', + title: 'a', + category: { + id: 'a', + label: 'a', + order: 10, + }, + }, + { + id: 'b', + }, + ]; + const navLinks: ChromeNavLink[] = [mockedNavLinkA, mockedNavLinkB]; + const fullfilledResult = fullfillRegistrationLinksToChromeNavLinks( + registrationNavLinks, + navLinks + ); + expect(fullfilledResult).toEqual([mockedNavLinkA, mockedNavLinkB]); + }); +}); + +describe('getOrderedLinks', () => { + it('should return ordered links', () => { + const navLinks = [mockedNavLinkA, mockedNavLinkB]; + const orderedLinks = getOrderedLinks(navLinks); + expect(orderedLinks).toEqual([mockedNavLinkB, mockedNavLinkA]); + }); +}); + +describe('getOrderedLinksOrCategories', () => { + it('should return ordered links', () => { + const navLinks = [mockedNonCategoryLink, mockedNavLinkA, mockedNavLinkB]; + const orderedLinks = getOrderedLinksOrCategories(navLinks); + expect(orderedLinks[0]).toEqual( + expect.objectContaining({ + category: mockedNavLinkB.category, + }) + ); + expect(orderedLinks[1]).toEqual( + expect.objectContaining({ + link: mockedNonCategoryLink, + }) + ); + expect(orderedLinks[2]).toEqual( + expect.objectContaining({ + category: mockedNavLinkA.category, + }) + ); + }); +}); + +describe('flattenLinksOrCategories', () => { + it('should return flattened links', () => { + const navLinks = [mockedNonCategoryLink, mockedNavLinkA, mockedNavLinkB]; + const orderedLinks = getOrderedLinksOrCategories(navLinks); + const flattenedLinks = flattenLinksOrCategories(orderedLinks); + expect(flattenedLinks.map((item) => item.id)).toEqual([ + mockedNavLinkB.id, + mockedNonCategoryLink.id, + mockedNavLinkA.id, + ]); + }); +}); diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts index 0570113bc43e..6f301d507e0c 100644 --- a/src/core/public/chrome/utils.ts +++ b/src/core/public/chrome/utils.ts @@ -8,7 +8,7 @@ import { groupBy, sortBy } from 'lodash'; import { ChromeNavLink } from './nav_links'; import { ChromeRegistrationNavLink } from './nav_group'; -const LinkItemType = { +export const LinkItemType = { LINK: 'link', CATEGORY: 'category', } as const; @@ -18,7 +18,7 @@ export type LinkItem = { order?: number } & ( | { itemType: 'category'; category?: AppCategory; links?: ChromeNavLink[] } ); -function getAllCategories(allCategorizedLinks: Record) { +export function getAllCategories(allCategorizedLinks: Record) { const allCategories = {} as Record; for (const [key, value] of Object.entries(allCategorizedLinks)) { diff --git a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap index f4c5ca0b0a0a..6e60bd8bead2 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap @@ -219,6 +219,10 @@ exports[`dashboard listing hideWriteControls 1`] = ` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -1393,6 +1397,10 @@ exports[`dashboard listing render table listing with initial filters from URL 1` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -2628,6 +2636,10 @@ exports[`dashboard listing renders call to action when no dashboards exist 1`] = "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -3863,6 +3875,10 @@ exports[`dashboard listing renders table rows 1`] = ` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -5098,6 +5114,10 @@ exports[`dashboard listing renders warning when listingLimit is exceeded 1`] = ` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 831eb5bd61c0..da537e452c4e 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -207,6 +207,10 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -1207,6 +1211,10 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -2207,6 +2215,10 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -3207,6 +3219,10 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -4207,6 +4223,10 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], @@ -5207,6 +5227,10 @@ exports[`Dashboard top nav render with all components 1`] = ` "registerLeft": [MockFunction], "registerRight": [MockFunction], }, + "navGroup": Object { + "getNavGroupEnabled": [MockFunction], + "getNavGroupsMap$": [MockFunction], + }, "navLinks": Object { "enableForcedAppSwitcherNavigation": [MockFunction], "get": [MockFunction], From b81657429f748effc27764d7f2581cd0d18cdc4e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 28 Jun 2024 15:18:27 +0800 Subject: [PATCH 32/35] feat: update according to comment Signed-off-by: SuZhou-Joe --- src/core/public/chrome/nav_group/nav_group_service.ts | 4 ++-- src/core/public/chrome/utils.test.ts | 8 ++++---- src/core/public/chrome/utils.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 7792deb8a288..6ee34e2b1301 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -9,7 +9,7 @@ import { map, takeUntil } from 'rxjs/operators'; import { IUiSettingsClient } from '../../ui_settings'; import { flattenLinksOrCategories, - fullfillRegistrationLinksToChromeNavLinks, + fulfillRegistrationLinksToChromeNavLinks, getOrderedLinksOrCategories, } from '../utils'; import { ChromeNavLinks } from '../nav_links'; @@ -85,7 +85,7 @@ export class ChromeNavGroupService { return Object.keys(navGroupsMap).reduce((sortedNavGroupsMap, navGroupId) => { const navGroup = navGroupsMap[navGroupId]; const sortedNavLinks = getOrderedLinksOrCategories( - fullfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks) + fulfillRegistrationLinksToChromeNavLinks(navGroup.navLinks, navLinks) ); sortedNavGroupsMap[navGroupId] = { ...navGroup, diff --git a/src/core/public/chrome/utils.test.ts b/src/core/public/chrome/utils.test.ts index 7d5b82228855..ed163b753eba 100644 --- a/src/core/public/chrome/utils.test.ts +++ b/src/core/public/chrome/utils.test.ts @@ -7,7 +7,7 @@ import { ChromeRegistrationNavLink } from './nav_group'; import { ChromeNavLink } from './nav_links'; import { getAllCategories, - fullfillRegistrationLinksToChromeNavLinks, + fulfillRegistrationLinksToChromeNavLinks, getOrderedLinks, getOrderedLinksOrCategories, flattenLinksOrCategories, @@ -69,7 +69,7 @@ describe('getAllCategories', () => { }); }); -describe('fullfillRegistrationLinksToChromeNavLinks', () => { +describe('fulfillRegistrationLinksToChromeNavLinks', () => { it('should return fullfilled ChromeNavLink', () => { const registrationNavLinks: ChromeRegistrationNavLink[] = [ { @@ -86,11 +86,11 @@ describe('fullfillRegistrationLinksToChromeNavLinks', () => { }, ]; const navLinks: ChromeNavLink[] = [mockedNavLinkA, mockedNavLinkB]; - const fullfilledResult = fullfillRegistrationLinksToChromeNavLinks( + const fulfilledResult = fulfillRegistrationLinksToChromeNavLinks( registrationNavLinks, navLinks ); - expect(fullfilledResult).toEqual([mockedNavLinkA, mockedNavLinkB]); + expect(fulfilledResult).toEqual([mockedNavLinkA, mockedNavLinkB]); }); }); diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts index 6f301d507e0c..9d5080245de3 100644 --- a/src/core/public/chrome/utils.ts +++ b/src/core/public/chrome/utils.ts @@ -28,7 +28,7 @@ export function getAllCategories(allCategorizedLinks: Record { From 7c2a5161f2176b6b19236e14696b723d6946c894 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 28 Jun 2024 18:47:11 +0800 Subject: [PATCH 33/35] feat: support parent link Signed-off-by: SuZhou-Joe --- .../nav_group/nav_group_service.test.ts | 57 +++++---- src/core/public/chrome/utils.ts | 121 ++++++++++++++---- 2 files changed, 128 insertions(+), 50 deletions(-) diff --git a/src/core/public/chrome/nav_group/nav_group_service.test.ts b/src/core/public/chrome/nav_group/nav_group_service.test.ts index 969d7fffdca4..8398b64fcb65 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.test.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.test.ts @@ -53,31 +53,35 @@ const mockedNavLinkService = mockedNavLink.start({ application: mockedApplicationService, }); +const mockedGetNavLinks = jest.fn(); +jest.spyOn(mockedNavLinkService, 'getNavLinks$').mockImplementation(mockedGetNavLinks); +mockedGetNavLinks.mockReturnValue( + new Rx.BehaviorSubject([ + { + id: 'foo', + }, + { + id: 'bar', + }, + { + id: 'foo-in-category-foo', + }, + { + id: 'foo-in-category-bar', + }, + { + id: 'bar-in-category-foo', + }, + { + id: 'bar-in-category-bar', + }, + { + id: 'link-with-parent-nav-link-id', + }, + ]) +); + describe('ChromeNavGroupService#setup()', () => { - const mockedGetNavLinks = jest.fn(); - jest.spyOn(mockedNavLinkService, 'getNavLinks$').mockImplementation(mockedGetNavLinks); - mockedGetNavLinks.mockReturnValue( - new Rx.BehaviorSubject([ - { - id: 'foo', - }, - { - id: 'bar', - }, - { - id: 'foo-in-category-foo', - }, - { - id: 'foo-in-category-bar', - }, - { - id: 'bar-in-category-foo', - }, - { - id: 'bar-in-category-bar', - }, - ]) - ); it('should be able to `addNavLinksToGroup`', async () => { const warnMock = jest.fn(); jest.spyOn(console, 'warn').mockImplementation(warnMock); @@ -160,6 +164,10 @@ describe('ChromeNavGroupService#start()', () => { category: mockedCategoryBar, }, mockedNavLinkBar, + { + id: 'link-with-parent-nav-link-id', + parentNavLinkId: 'not-exist-id', + }, ]); chromeNavGroupServiceSetup.addNavLinksToGroup(mockedGroupBar, [mockedNavLinkBar]); @@ -175,6 +183,7 @@ describe('ChromeNavGroupService#start()', () => { 'bar', 'foo-in-category-bar', 'bar-in-category-bar', + 'link-with-parent-nav-link-id', ]); expect(groupsMap[mockedGroupBar.id].navLinks.length).toEqual(1); }); diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts index 9d5080245de3..f80ab50a580f 100644 --- a/src/core/public/chrome/utils.ts +++ b/src/core/public/chrome/utils.ts @@ -11,11 +11,13 @@ import { ChromeRegistrationNavLink } from './nav_group'; export const LinkItemType = { LINK: 'link', CATEGORY: 'category', + PARENT_LINK: 'parentLink', } as const; export type LinkItem = { order?: number } & ( - | { itemType: 'link'; link: ChromeNavLink } - | { itemType: 'category'; category?: AppCategory; links?: ChromeNavLink[] } + | { itemType: 'link'; link: ChromeNavLink & ChromeRegistrationNavLink } + | { itemType: 'parentLink'; link?: ChromeNavLink & ChromeRegistrationNavLink; links: LinkItem[] } + | { itemType: 'category'; category?: AppCategory; links?: LinkItem[] } ); export function getAllCategories(allCategorizedLinks: Record) { @@ -31,7 +33,7 @@ export function getAllCategories(allCategorizedLinks: Record { +): Array { const allExistingNavLinkId = navLinks.map((link) => link.id); return ( registerNavLinks @@ -46,36 +48,103 @@ export function fulfillRegistrationLinksToChromeNavLinks( export const getOrderedLinks = (navLinks: ChromeNavLink[]): ChromeNavLink[] => sortBy(navLinks, (link) => link.order); -export function getOrderedLinksOrCategories(navLinks: ChromeNavLink[]): LinkItem[] { - const groupedNavLinks = groupBy(navLinks, (link) => link?.category?.id); - const { undefined: unknowns = [], ...allCategorizedLinks } = groupedNavLinks; - const categoryDictionary = getAllCategories(allCategorizedLinks); - return sortBy( - [ - ...unknowns.map((linkWithoutCategory) => ({ - itemType: LinkItemType.LINK, - link: linkWithoutCategory, - order: linkWithoutCategory.order, - })), - ...Object.keys(allCategorizedLinks).map((categoryKey) => ({ - itemType: LinkItemType.CATEGORY, - category: categoryDictionary[categoryKey], - order: categoryDictionary[categoryKey]?.order, - links: getOrderedLinks(allCategorizedLinks[categoryKey]), - })), - ], - (item) => item.order - ); -} - export function flattenLinksOrCategories(linkItems: LinkItem[]): ChromeNavLink[] { return linkItems.reduce((acc, item) => { if (item.itemType === LinkItemType.LINK) { acc.push(item.link); + } else if (item.itemType === LinkItemType.PARENT_LINK) { + if (item.link) { + acc.push(item.link); + } + acc.push(...flattenLinksOrCategories(item.links)); } else if (item.itemType === LinkItemType.CATEGORY) { - acc.push(...(item.links || [])); + acc.push(...flattenLinksOrCategories(item.links || [])); } return acc; }, [] as ChromeNavLink[]); } + +export const generateItemTypeByLink = ( + navLink: ChromeNavLink & ChromeRegistrationNavLink, + navLinksGoupedByParentNavLink: Record +): LinkItem => { + const navLinksUnderParentId = navLinksGoupedByParentNavLink[navLink.id]; + + if (navLinksUnderParentId) { + return { + itemType: LinkItemType.PARENT_LINK, + link: navLink, + links: getOrderedLinks(navLinksUnderParentId || []).map((navLinkUnderParentId) => + generateItemTypeByLink(navLinkUnderParentId, navLinksGoupedByParentNavLink) + ), + order: navLink?.order, + }; + } else { + return { + itemType: LinkItemType.LINK, + link: navLink, + order: navLink.order, + }; + } +}; + +/** + * This function accept navLinks and gives a grouped result for category / parent nav link + * @param navLinks + * @returns LinkItem[] + */ +export function getOrderedLinksOrCategories( + navLinks: Array +): LinkItem[] { + // Get the nav links group by parent nav link + const allNavLinksWithParentNavLink = navLinks.filter((navLink) => navLink.parentNavLinkId); + const navLinksGoupedByParentNavLink = groupBy( + allNavLinksWithParentNavLink, + (navLink) => navLink.parentNavLinkId + ); + + // Group all the nav links without parentNavLinkId + const groupedNavLinks = groupBy( + navLinks.filter((item) => !item.parentNavLinkId), + (link) => link?.category?.id + ); + const { undefined: unknowns = [], ...allCategorizedLinks } = groupedNavLinks; + const categoryDictionary = getAllCategories(allCategorizedLinks); + + // Get all the parent nav ids that defined by nested items but can not find matched parent nav in navLinks + const unusedParentNavLinks = Object.keys(navLinksGoupedByParentNavLink).filter( + (navLinkId) => !navLinks.find((navLink) => navLink.id === navLinkId) + ); + + const result = [ + // Nav links without category, the order is determined by link itself + ...unknowns.map((linkWithoutCategory) => + generateItemTypeByLink(linkWithoutCategory, navLinksGoupedByParentNavLink) + ), + // Nav links with category, the order is determined by category order + ...Object.keys(allCategorizedLinks).map((categoryKey) => { + return { + itemType: LinkItemType.CATEGORY, + category: categoryDictionary[categoryKey], + order: categoryDictionary[categoryKey]?.order, + links: getOrderedLinks(allCategorizedLinks[categoryKey]).map((navLink) => + generateItemTypeByLink(navLink, navLinksGoupedByParentNavLink) + ), + }; + }), + // Nav links that should have belong to a parent nav id + // but not find matched parent nav in navLinks + // should be treated as normal link + ...unusedParentNavLinks.reduce((total, groupId) => { + return [ + ...total, + ...navLinksGoupedByParentNavLink[groupId].map((navLink) => + generateItemTypeByLink(navLink, navLinksGoupedByParentNavLink) + ), + ]; + }, [] as LinkItem[]), + ]; + + return sortBy(result, (item) => item.order); +} From 8374812132c289cfba823a3ea40e672e81cb49c6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Jul 2024 15:38:14 +0800 Subject: [PATCH 34/35] feat: update according to comment Signed-off-by: SuZhou-Joe --- .../chrome/nav_group/nav_group_service.ts | 6 +- src/core/public/chrome/utils.ts | 55 ++++++++++++++----- src/core/types/nav_group.ts | 10 ++-- src/plugins/home/server/ui_settings.ts | 1 + 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/core/public/chrome/nav_group/nav_group_service.ts b/src/core/public/chrome/nav_group/nav_group_service.ts index 6ee34e2b1301..072b9f67f2e7 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -59,7 +59,7 @@ export class ChromeNavGroupService { const matchedGroup = currentGroupsMap[navGroup.id]; if (matchedGroup) { const links = matchedGroup.navLinks; - const isLinkExistInGroup = !!links.find((link) => link.id === navLink.id); + const isLinkExistInGroup = links.some((link) => link.id === navLink.id); if (isLinkExistInGroup) { // eslint-disable-next-line no-console console.warn( @@ -77,7 +77,7 @@ export class ChromeNavGroupService { return currentGroupsMap; } - private getSortedNavGroupsMap() { + private getSortedNavGroupsMap$() { return combineLatest([this.navGroupsMap$, this.navLinks$]) .pipe(takeUntil(this.stop$)) .pipe( @@ -125,7 +125,7 @@ export class ChromeNavGroupService { }): Promise { this.navLinks$ = navLinks.getNavLinks$(); return { - getNavGroupsMap$: () => this.getSortedNavGroupsMap(), + getNavGroupsMap$: () => this.getSortedNavGroupsMap$(), getNavGroupEnabled: () => this.navGroupEnabled, }; } diff --git a/src/core/public/chrome/utils.ts b/src/core/public/chrome/utils.ts index f80ab50a580f..8a9dec8b2145 100644 --- a/src/core/public/chrome/utils.ts +++ b/src/core/public/chrome/utils.ts @@ -4,10 +4,26 @@ */ import { AppCategory } from 'opensearch-dashboards/public'; -import { groupBy, sortBy } from 'lodash'; import { ChromeNavLink } from './nav_links'; import { ChromeRegistrationNavLink } from './nav_group'; +type KeyOf = keyof T; + +const sortBy = (key: KeyOf) => { + return (a: T, b: T): number => (a[key] > b[key] ? 1 : b[key] > a[key] ? -1 : 0); +}; + +const groupBy = (array: T[], getKey: (item: T) => string | undefined): Record => { + return array.reduce((result, currentValue) => { + const groupKey = String(getKey(currentValue)); + if (!result[groupKey]) { + result[groupKey] = []; + } + result[groupKey].push(currentValue); + return result; + }, {} as Record); +}; + export const LinkItemType = { LINK: 'link', CATEGORY: 'category', @@ -20,7 +36,9 @@ export type LinkItem = { order?: number } & ( | { itemType: 'category'; category?: AppCategory; links?: LinkItem[] } ); -export function getAllCategories(allCategorizedLinks: Record) { +export function getAllCategories( + allCategorizedLinks: Record> +) { const allCategories = {} as Record; for (const [key, value] of Object.entries(allCategorizedLinks)) { @@ -30,6 +48,13 @@ export function getAllCategories(allCategorizedLinks: Record + */ export function fulfillRegistrationLinksToChromeNavLinks( registerNavLinks: ChromeRegistrationNavLink[], navLinks: ChromeNavLink[] @@ -37,7 +62,7 @@ export function fulfillRegistrationLinksToChromeNavLinks( const allExistingNavLinkId = navLinks.map((link) => link.id); return ( registerNavLinks - ?.filter((navLink) => allExistingNavLinkId.includes(navLink.id)) + .filter((navLink) => allExistingNavLinkId.includes(navLink.id)) .map((navLink) => ({ ...navLinks[allExistingNavLinkId.indexOf(navLink.id)], ...navLink, @@ -46,7 +71,7 @@ export function fulfillRegistrationLinksToChromeNavLinks( } export const getOrderedLinks = (navLinks: ChromeNavLink[]): ChromeNavLink[] => - sortBy(navLinks, (link) => link.order); + navLinks.sort(sortBy('order')); export function flattenLinksOrCategories(linkItems: LinkItem[]): ChromeNavLink[] { return linkItems.reduce((acc, item) => { @@ -67,18 +92,18 @@ export function flattenLinksOrCategories(linkItems: LinkItem[]): ChromeNavLink[] export const generateItemTypeByLink = ( navLink: ChromeNavLink & ChromeRegistrationNavLink, - navLinksGoupedByParentNavLink: Record + navLinksGroupedByParentNavLink: Record ): LinkItem => { - const navLinksUnderParentId = navLinksGoupedByParentNavLink[navLink.id]; + const navLinksUnderParentId = navLinksGroupedByParentNavLink[navLink.id]; if (navLinksUnderParentId) { return { itemType: LinkItemType.PARENT_LINK, link: navLink, links: getOrderedLinks(navLinksUnderParentId || []).map((navLinkUnderParentId) => - generateItemTypeByLink(navLinkUnderParentId, navLinksGoupedByParentNavLink) + generateItemTypeByLink(navLinkUnderParentId, navLinksGroupedByParentNavLink) ), - order: navLink?.order, + order: navLink.order, }; } else { return { @@ -99,7 +124,7 @@ export function getOrderedLinksOrCategories( ): LinkItem[] { // Get the nav links group by parent nav link const allNavLinksWithParentNavLink = navLinks.filter((navLink) => navLink.parentNavLinkId); - const navLinksGoupedByParentNavLink = groupBy( + const navLinksGroupedByParentNavLink = groupBy( allNavLinksWithParentNavLink, (navLink) => navLink.parentNavLinkId ); @@ -113,14 +138,14 @@ export function getOrderedLinksOrCategories( const categoryDictionary = getAllCategories(allCategorizedLinks); // Get all the parent nav ids that defined by nested items but can not find matched parent nav in navLinks - const unusedParentNavLinks = Object.keys(navLinksGoupedByParentNavLink).filter( + const unusedParentNavLinks = Object.keys(navLinksGroupedByParentNavLink).filter( (navLinkId) => !navLinks.find((navLink) => navLink.id === navLinkId) ); const result = [ // Nav links without category, the order is determined by link itself ...unknowns.map((linkWithoutCategory) => - generateItemTypeByLink(linkWithoutCategory, navLinksGoupedByParentNavLink) + generateItemTypeByLink(linkWithoutCategory, navLinksGroupedByParentNavLink) ), // Nav links with category, the order is determined by category order ...Object.keys(allCategorizedLinks).map((categoryKey) => { @@ -129,7 +154,7 @@ export function getOrderedLinksOrCategories( category: categoryDictionary[categoryKey], order: categoryDictionary[categoryKey]?.order, links: getOrderedLinks(allCategorizedLinks[categoryKey]).map((navLink) => - generateItemTypeByLink(navLink, navLinksGoupedByParentNavLink) + generateItemTypeByLink(navLink, navLinksGroupedByParentNavLink) ), }; }), @@ -139,12 +164,12 @@ export function getOrderedLinksOrCategories( ...unusedParentNavLinks.reduce((total, groupId) => { return [ ...total, - ...navLinksGoupedByParentNavLink[groupId].map((navLink) => - generateItemTypeByLink(navLink, navLinksGoupedByParentNavLink) + ...navLinksGroupedByParentNavLink[groupId].map((navLink) => + generateItemTypeByLink(navLink, navLinksGroupedByParentNavLink) ), ]; }, [] as LinkItem[]), ]; - return sortBy(result, (item) => item.order); + return result.sort(sortBy('order')); } diff --git a/src/core/types/nav_group.ts b/src/core/types/nav_group.ts index 8909bcbde082..e1dda3d122c2 100644 --- a/src/core/types/nav_group.ts +++ b/src/core/types/nav_group.ts @@ -5,6 +5,12 @@ import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; +/** + * Groups with type of NavGroupType.SYSTEM will: + * 1. Always display before USE_CASE_GROUP. + * 2. Not be pickable within the workspace creation page. + * + */ export enum NavGroupType { SYSTEM = 'system', } @@ -16,11 +22,7 @@ export interface ChromeNavGroup { description: string; order?: number; icon?: EuiIconType; - /** - * Groups with type of NavGroupType.SYSTEM will: - * 1. Always display before USE_CASE_GROUP. - * 2. Not be pickable within the workspace creation page. * * @default undefined indicates it is of type useCase */ diff --git a/src/plugins/home/server/ui_settings.ts b/src/plugins/home/server/ui_settings.ts index eb94aeb39227..7df590ba5480 100644 --- a/src/plugins/home/server/ui_settings.ts +++ b/src/plugins/home/server/ui_settings.ts @@ -25,5 +25,6 @@ export const uiSettings: Record = { defaultMessage: 'Try the new home page', }), schema: schema.boolean(), + requiresPageReload: true, }, }; From 35c1db7cf96a3df7c4391345b367b2c9200df0b5 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Jul 2024 15:42:56 +0800 Subject: [PATCH 35/35] feat: update according to comment Signed-off-by: SuZhou-Joe --- src/core/types/nav_group.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/core/types/nav_group.ts b/src/core/types/nav_group.ts index e1dda3d122c2..0359e14b8a07 100644 --- a/src/core/types/nav_group.ts +++ b/src/core/types/nav_group.ts @@ -6,10 +6,11 @@ import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; /** - * Groups with type of NavGroupType.SYSTEM will: - * 1. Always display before USE_CASE_GROUP. - * 2. Not be pickable within the workspace creation page. + * There are two types of navGroup: + * 1: system nav group, like data administration / settings and setup + * 2: use case group, like observability. * + * by default the nav group will be regarded as use case group. */ export enum NavGroupType { SYSTEM = 'system', @@ -22,9 +23,5 @@ export interface ChromeNavGroup { description: string; order?: number; icon?: EuiIconType; - /** - * - * @default undefined indicates it is of type useCase - */ type?: NavGroupType; }