From d30677d72b58ad0c52fc553630ef1553de7313f9 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 22 Jul 2024 12:59:17 +0800 Subject: [PATCH] [navigation-next] fix: redirect to standard index pattern applications while nav group is enabled (#7346) * [navigation-next] fix: redirect to standard index pattern applications while nav group is enabled (#7305) * feat: fix the incorrect jumping logic for Index pattern management Signed-off-by: SuZhou-Joe * Changeset file for PR #7305 created/updated * feat: update Signed-off-by: SuZhou-Joe * feat: update with comment Signed-off-by: SuZhou-Joe * feat: update order and remove reset logic Signed-off-by: SuZhou-Joe * feat: update Signed-off-by: SuZhou-Joe * feat: update Signed-off-by: SuZhou-Joe * feat: update snapshot Signed-off-by: SuZhou-Joe * feat: some category change Signed-off-by: SuZhou-Joe * feat: update category Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 2c708e320222f7eee43e96247a71529e3f0207b4) Signed-off-by: github-actions[bot] * feat: change the order Signed-off-by: SuZhou-Joe * feat: hide left navigation when workspace enabled Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: SuZhou-Joe --- changelogs/fragments/7305.yml | 2 + .../nav_group/nav_group_service.test.ts | 47 ----------- .../chrome/nav_group/nav_group_service.ts | 12 --- ...ollapsible_nav_group_enabled.test.tsx.snap | 7 +- .../header/collapsible_nav_group_enabled.scss | 17 +++- .../collapsible_nav_group_enabled.test.tsx | 26 ++++++ .../header/collapsible_nav_group_enabled.tsx | 80 ++++++++++++------- src/core/public/chrome/ui/header/header.tsx | 1 + src/core/utils/default_app_categories.ts | 9 ++- src/plugins/dashboard/public/plugin.tsx | 4 +- .../data_source_management/public/plugin.ts | 15 ++-- .../public/plugin.test.ts | 36 +++++++++ .../index_pattern_management/public/plugin.ts | 20 +++-- .../management_app/management_app.tsx | 5 +- src/plugins/management/public/plugin.ts | 2 + .../saved_objects_management/public/plugin.ts | 8 ++ 16 files changed, 182 insertions(+), 109 deletions(-) create mode 100644 changelogs/fragments/7305.yml diff --git a/changelogs/fragments/7305.yml b/changelogs/fragments/7305.yml new file mode 100644 index 000000000000..ebb9060e2121 --- /dev/null +++ b/changelogs/fragments/7305.yml @@ -0,0 +1,2 @@ +feat: +- [navigation-next] fix: redirect to standard index pattern applications while nav group is enabled ([#7305](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7305)) \ No newline at end of file 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 bc18483178fd..90911309ff9a 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 @@ -311,53 +311,6 @@ describe('ChromeNavGroupService#start()', () => { expect(sessionStorageMock.getItem(CURRENT_NAV_GROUP_ID)).toBeFalsy(); expect(currentNavGroup).toBeUndefined(); }); - - it('should reset current nav group if app not belongs to any nav group', async () => { - const uiSettings = uiSettingsServiceMock.createSetupContract(); - const navGroupEnabled$ = new Rx.BehaviorSubject(true); - uiSettings.get$.mockImplementation(() => navGroupEnabled$); - - const chromeNavGroupService = new ChromeNavGroupService(); - const chromeNavGroupServiceSetup = chromeNavGroupService.setup({ uiSettings }); - - chromeNavGroupServiceSetup.addNavLinksToGroup( - { - id: 'foo', - title: 'foo title', - description: 'foo description', - }, - [{ id: 'foo-app1' }] - ); - - const chromeNavGroupServiceStart = await chromeNavGroupService.start({ - navLinks: mockedNavLinkService, - application: mockedApplicationService, - }); - - // set an existing nav group id - chromeNavGroupServiceStart.setCurrentNavGroup('foo'); - - expect(sessionStorageMock.getItem(CURRENT_NAV_GROUP_ID)).toEqual('foo'); - - let currentNavGroup = await chromeNavGroupServiceStart - .getCurrentNavGroup$() - .pipe(first()) - .toPromise(); - - expect(currentNavGroup?.id).toEqual('foo'); - - // navigate to app don't belongs to any nav group - mockedApplicationService.navigateToApp('bar-app'); - - currentNavGroup = await chromeNavGroupServiceStart - .getCurrentNavGroup$() - .pipe(first()) - .toPromise(); - - // verify current nav group been reset - expect(currentNavGroup).toBeFalsy(); - expect(sessionStorageMock.getItem(CURRENT_NAV_GROUP_ID)).toBeFalsy(); - }); }); describe('nav group updater', () => { 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 bde7d0d9111a..bdf69b151da9 100644 --- a/src/core/public/chrome/nav_group/nav_group_service.ts +++ b/src/core/public/chrome/nav_group/nav_group_service.ts @@ -212,18 +212,6 @@ export class ChromeNavGroupService { } }; - // erase current nav group when switch app don't belongs to any nav group - application.currentAppId$.subscribe((appId) => { - const navGroupMap = this.navGroupsMap$.getValue(); - const appIdsWithNavGroup = Object.values(navGroupMap).flatMap(({ navLinks: links }) => - links.map(({ id }) => id) - ); - - if (appId && !appIdsWithNavGroup.includes(appId)) { - setCurrentNavGroup(undefined); - } - }); - const currentNavGroupSorted$ = combineLatest([ this.getSortedNavGroupsMap$(), this.currentNavGroup$, diff --git a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap index 61fb739ad6c2..56600b067583 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav_group_enabled.test.tsx.snap @@ -1,5 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` should hide left navigation when in home page when workspace is enabled 1`] = `
`; + exports[` should render correctly 1`] = `
should render correctly 1`] = ` />
@@ -266,8 +267,7 @@ exports[` should render correctly 2`] = ` class="euiHorizontalRule euiHorizontalRule--full" />
@@ -341,7 +341,6 @@ exports[` should show all use case by default and />
diff --git a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss index 50e822bae295..77626cab7eb7 100644 --- a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss +++ b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.scss @@ -2,8 +2,8 @@ border: none !important; .nav-link-item { - padding: $ouiSize / 4 $ouiSize; - border-radius: $ouiSize; + padding: calc($euiSize / 4) $euiSize; + border-radius: $euiSize; box-shadow: none; margin-bottom: 0; margin-top: 0; @@ -39,11 +39,20 @@ } .bottom-container { - padding: 0 $ouiSize; + padding: 0 $euiSize; display: flex; + + &.bottom-container-collapsed { + flex-direction: column; + align-items: center; + + > * { + margin: $euiSizeS 0; + } + } } .nav-controls-padding { - padding: $ouiSize; + padding: $euiSize; } } diff --git a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx index b08029553b50..fa4abffac36c 100644 --- a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx +++ b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.test.tsx @@ -18,6 +18,7 @@ import { httpServiceMock } from '../../../mocks'; import { getLogos } from '../../../../common'; import { ALL_USE_CASE_ID, DEFAULT_NAV_GROUPS } from '../../../../public'; import { CollapsibleNavTopProps } from './collapsible_nav_group_enabled_top'; +import { capabilitiesServiceMock } from '../../../application/capabilities/capabilities_service.mock'; jest.mock('./collapsible_nav_group_enabled_top', () => ({ CollapsibleNavTop: (props: CollapsibleNavTopProps) => ( @@ -166,6 +167,7 @@ describe('', () => { currentNavGroup$.next(undefined); } }, + capabilities: { ...capabilitiesServiceMock.createStartContract().capabilities }, ...props, }; } @@ -223,4 +225,28 @@ describe('', () => { fireEvent.click(getByTestId('back')); expect(getAllByTestId('collapsibleNavAppLink-link-in-analytics').length).toEqual(2); }); + + it('should hide left navigation when in home page when workspace is enabled', async () => { + const props = mockProps({ + navGroupsMap: { + [DEFAULT_NAV_GROUPS.analytics.id]: { + ...DEFAULT_NAV_GROUPS.analytics, + navLinks: [ + { + id: 'link-in-analytics', + title: 'link-in-analytics', + showInAllNavGroup: true, + }, + ], + }, + }, + }); + props.appId$ = new BehaviorSubject('home'); + if (props.capabilities.workspaces) { + (props.capabilities.workspaces as Record) = {}; + (props.capabilities.workspaces as Record).enabled = true; + } + const { container } = render(); + expect(container).toMatchSnapshot(); + }); }); diff --git a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx index 0575dc997fc7..68b031232370 100644 --- a/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx +++ b/src/core/public/chrome/ui/header/collapsible_nav_group_enabled.tsx @@ -19,7 +19,7 @@ import useObservable from 'react-use/lib/useObservable'; import * as Rx from 'rxjs'; import classNames from 'classnames'; import { ChromeNavControl, ChromeNavLink } from '../..'; -import { NavGroupStatus } from '../../../../types'; +import { AppCategory, NavGroupStatus } from '../../../../types'; import { InternalApplicationStart } from '../../../application/types'; import { HttpStart } from '../../../http'; import { OnIsLockedUpdate } from './'; @@ -36,7 +36,7 @@ import { LinkItem, LinkItemType, } from '../../utils'; -import { ALL_USE_CASE_ID } from '../../../../../core/utils'; +import { ALL_USE_CASE_ID, DEFAULT_APP_CATEGORIES } from '../../../../../core/utils'; import { CollapsibleNavTop } from './collapsible_nav_group_enabled_top'; import { HeaderNavControls } from './header_nav_controls'; @@ -58,6 +58,7 @@ export interface CollapsibleNavGroupEnabledProps { navControlsLeftBottom$: Rx.Observable; currentNavGroup$: Rx.Observable; setCurrentNavGroup: ChromeNavGroupServiceStartContract['setCurrentNavGroup']; + capabilities: InternalApplicationStart['capabilities']; } interface NavGroupsProps { @@ -164,6 +165,14 @@ export function NavGroups({ ); } +// Custom category is used for those features not belong to any of use cases in all use case. +// and the custom category should always sit before manage category +const customCategory: AppCategory = { + id: 'custom', + label: i18n.translate('core.ui.customNavList.label', { defaultMessage: 'Custom' }), + order: (DEFAULT_APP_CATEGORIES.manage.order || 0) - 500, +}; + export function CollapsibleNavGroupEnabled({ basePath, id, @@ -176,6 +185,7 @@ export function CollapsibleNavGroupEnabled({ navigateToUrl, logos, setCurrentNavGroup, + capabilities, ...observables }: CollapsibleNavGroupEnabledProps) { const navLinks = useObservable(observables.navLinks$, []).filter((link) => !link.hidden); @@ -183,29 +193,6 @@ export function CollapsibleNavGroupEnabled({ const navGroupsMap = useObservable(observables.navGroupsMap$, {}); const currentNavGroup = useObservable(observables.currentNavGroup$, undefined); - const onGroupClick = ( - e: React.MouseEvent, - group: NavGroupItemInMap - ) => { - const fulfilledLinks = fulfillRegistrationLinksToChromeNavLinks( - navGroupsMap[group.id]?.navLinks, - navLinks - ); - setCurrentNavGroup(group.id); - - // the `navGroupsMap[group.id]?.navLinks` has already been sorted - const firstLink = fulfilledLinks[0]; - if (firstLink) { - const propsForEui = createEuiListItem({ - link: firstLink, - appId, - dataTestSubj: 'collapsibleNavAppLink', - navigateToApp, - }); - propsForEui.onClick(e); - } - }; - const navLinksForRender: ChromeNavLink[] = useMemo(() => { if (currentNavGroup) { return fulfillRegistrationLinksToChromeNavLinks( @@ -234,7 +221,10 @@ export function CollapsibleNavGroupEnabled({ navLinks .filter((link) => !linkIdsWithUseGroupInfo.includes(link.id)) .forEach((navLink) => { - navLinksForAll.push(navLink); + navLinksForAll.push({ + ...navLink, + category: customCategory, + }); }); // Append all the links registered to all use case @@ -281,6 +271,37 @@ export function CollapsibleNavGroupEnabled({ return 270; }, [isNavOpen]); + // For now, only home page need to hide left navigation + // when workspace is enabled. + // If there are more pages need to hide left navigation in the future + // need to come up with a mechanism to register. + if (capabilities.workspaces.enabled && appId === 'home') { + return null; + } + + const onGroupClick = ( + e: React.MouseEvent, + group: NavGroupItemInMap + ) => { + const fulfilledLinks = fulfillRegistrationLinksToChromeNavLinks( + navGroupsMap[group.id]?.navLinks, + navLinks + ); + setCurrentNavGroup(group.id); + + // the `navGroupsMap[group.id]?.navLinks` has already been sorted + const firstLink = fulfilledLinks[0]; + if (firstLink) { + const propsForEui = createEuiListItem({ + link: firstLink, + appId, + dataTestSubj: 'collapsibleNavAppLink', + navigateToApp, + }); + propsForEui.onClick(e); + } + }; + return ( -
+
) : ( = Object.freeze label: i18n.translate('core.ui.manageNav.label', { defaultMessage: 'Manage', }), - order: 7000, + order: 8000, + }, + manageData: { + id: 'manageData', + label: i18n.translate('core.ui.manageDataNav.label', { + defaultMessage: 'Manage data', + }), + order: 1000, }, }); diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index afa3b6daf281..bbe000f12b79 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -456,14 +456,14 @@ export class DashboardPlugin core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.observability, [ { id: app.id, - order: 300, + order: 400, category: undefined, }, ]); core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS['security-analytics'], [ { id: app.id, - order: 300, + order: 400, category: undefined, }, ]); diff --git a/src/plugins/data_source_management/public/plugin.ts b/src/plugins/data_source_management/public/plugin.ts index 59f9cc91d613..df4edf40ddfd 100644 --- a/src/plugins/data_source_management/public/plugin.ts +++ b/src/plugins/data_source_management/public/plugin.ts @@ -146,11 +146,8 @@ export class DataSourceManagementPlugin core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.dataAdministration, [ { id: DSM_APP_ID_FOR_STANDARD_APPLICATION, - category: { - id: DSM_APP_ID_FOR_STANDARD_APPLICATION, - label: PLUGIN_NAME, - order: 200, - }, + category: DEFAULT_APP_CATEGORIES.manageData, + order: 100, }, ]); @@ -186,6 +183,14 @@ export class DataSourceManagementPlugin }, ]); + core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [ + { + id: DSM_APP_ID_FOR_STANDARD_APPLICATION, + category: DEFAULT_APP_CATEGORIES.manage, + order: 100, + }, + ]); + const registerAuthenticationMethod = (authMethod: AuthenticationMethod) => { if (this.started) { throw new Error( diff --git a/src/plugins/index_pattern_management/public/plugin.test.ts b/src/plugins/index_pattern_management/public/plugin.test.ts index e207af770af3..a0525acae2dc 100644 --- a/src/plugins/index_pattern_management/public/plugin.test.ts +++ b/src/plugins/index_pattern_management/public/plugin.test.ts @@ -7,6 +7,11 @@ import { coreMock } from '../../../core/public/mocks'; import { IndexPatternManagementPlugin } from './plugin'; import { urlForwardingPluginMock } from '../../url_forwarding/public/mocks'; import { managementPluginMock } from '../../management/public/mocks'; +import { + ManagementApp, + ManagementAppMountParams, + RegisterManagementAppArgs, +} from 'src/plugins/management/public'; describe('DiscoverPlugin', () => { it('setup successfully', () => { @@ -22,4 +27,35 @@ describe('DiscoverPlugin', () => { expect(setupMock.application.register).toBeCalledTimes(1); expect(setupMock.chrome.navGroup.addNavLinksToGroup).toBeCalledTimes(5); }); + + it('when new navigation is enabled, should navigate to standard IPM app', async () => { + const setupMock = coreMock.createSetup(); + const startMock = coreMock.createStart(); + setupMock.getStartServices.mockResolvedValue([startMock, {}, {}]); + const initializerContext = coreMock.createPluginInitializerContext(); + const pluginInstance = new IndexPatternManagementPlugin(initializerContext); + const managementMock = managementPluginMock.createSetupContract(); + let applicationRegistration = {} as Omit; + managementMock.sections.section.opensearchDashboards.registerApp = ( + app: Omit + ) => { + applicationRegistration = app; + return {} as ManagementApp; + }; + + setupMock.chrome.navGroup.getNavGroupEnabled.mockReturnValue(true); + startMock.application.getUrlForApp.mockReturnValue('/app/indexPatterns'); + + pluginInstance.setup(setupMock, { + urlForwarding: urlForwardingPluginMock.createSetupContract(), + management: managementMock, + }); + + await applicationRegistration.mount({} as ManagementAppMountParams); + + expect(startMock.application.getUrlForApp).toBeCalledWith('indexPatterns'); + expect(startMock.application.navigateToUrl).toBeCalledWith( + 'http://localhost/app/indexPatterns' + ); + }); }); diff --git a/src/plugins/index_pattern_management/public/plugin.ts b/src/plugins/index_pattern_management/public/plugin.ts index 7ee82dbcc3b0..ef462374129e 100644 --- a/src/plugins/index_pattern_management/public/plugin.ts +++ b/src/plugins/index_pattern_management/public/plugin.ts @@ -111,6 +111,17 @@ export class IndexPatternManagementPlugin title: sectionsHeader, order: 0, mount: async (params) => { + if (core.chrome.navGroup.getNavGroupEnabled()) { + const [coreStart] = await core.getStartServices(); + const urlForStandardIPMApp = new URL( + coreStart.application.getUrlForApp(IPM_APP_ID), + window.location.href + ); + const targetUrl = new URL(window.location.href); + targetUrl.pathname = urlForStandardIPMApp.pathname; + coreStart.application.navigateToUrl(targetUrl.toString()); + return () => {}; + } const { mountManagementSection } = await import('./management_app'); return mountManagementSection( @@ -178,14 +189,11 @@ export class IndexPatternManagementPlugin }, ]); - core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.dataAdministration, [ + core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [ { id: IPM_APP_ID, - category: { - id: IPM_APP_ID, - label: sectionsHeader, - order: 100, - }, + category: DEFAULT_APP_CATEGORIES.manage, + order: 200, }, ]); diff --git a/src/plugins/management/public/components/management_app/management_app.tsx b/src/plugins/management/public/components/management_app/management_app.tsx index b2109ceb08ca..c30243563b01 100644 --- a/src/plugins/management/public/components/management_app/management_app.tsx +++ b/src/plugins/management/public/components/management_app/management_app.tsx @@ -51,6 +51,7 @@ export interface ManagementAppDependencies { sections: SectionsServiceStart; opensearchDashboardsVersion: string; setBreadcrumbs: (newBreadcrumbs: ChromeBreadcrumb[]) => void; + hideInAppNavigation?: boolean; } export const ManagementApp = ({ dependencies, history }: ManagementAppProps) => { @@ -89,7 +90,9 @@ export const ManagementApp = ({ dependencies, history }: ManagementAppProps) => return ( - + {dependencies.hideInAppNavigation ? null : ( + + )}