[navigation-next] fix: redirect to standard index pattern application…

…s while nav group is enabled (#7305) * feat: fix the incorrect jumping logic for Index pattern management Signed-off-by: SuZhou-Joe <[email protected]> * Changeset file for PR #7305 created/updated * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update with comment Signed-off-by: SuZhou-Joe <[email protected]> * feat: update order and remove reset logic Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update Signed-off-by: SuZhou-Joe <[email protected]> * feat: update snapshot Signed-off-by: SuZhou-Joe <[email protected]> * feat: some category change Signed-off-by: SuZhou-Joe <[email protected]> * feat: update category Signed-off-by: SuZhou-Joe <[email protected]> --------- Signed-off-by: SuZhou-Joe <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Jul 21, 2024 · 2c708e3 · 2c708e3
1 parent 08c2a00
commit 2c708e3
Show file tree

Hide file tree

Showing 15 changed files with 167 additions and 108 deletions.
diff --git a/changelogs/fragments/7305.yml 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))
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
@@ -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
@@ -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
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
@@ -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
@@ -223,4 +223,24 @@ describe('<CollapsibleNavGroupEnabled />', () => {
     fireEvent.click(getByTestId('back'));
     expect(getAllByTestId('collapsibleNavAppLink-link-in-analytics').length).toEqual(2);
   });
+
+  it('should hide left navigation when in home page', 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');
+    const { container } = render(<CollapsibleNavGroupEnabled {...props} isNavOpen />);
+    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
@@ -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 './';
@@ -164,6 +164,13 @@ export function NavGroups({
   );
 }
 
+// custom category is used for those features not belong to any of use cases in all use case.
+const customCategory: AppCategory = {
+  id: 'custom',
+  label: i18n.translate('core.ui.customNavList.label', { defaultMessage: 'Custom' }),
+  order: Number.MAX_SAFE_INTEGER,
+};
+
 export function CollapsibleNavGroupEnabled({
   basePath,
   id,
@@ -183,29 +190,6 @@ export function CollapsibleNavGroupEnabled({
   const navGroupsMap = useObservable(observables.navGroupsMap$, {});
   const currentNavGroup = useObservable(observables.currentNavGroup$, undefined);
 
-  const onGroupClick = (
-    e: React.MouseEvent<HTMLButtonElement, 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 +218,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 +268,33 @@ export function CollapsibleNavGroupEnabled({
     return 270;
   }, [isNavOpen]);
 
+  if (appId === 'home') {
+    return null;
+  }
+
+  const onGroupClick = (
+    e: React.MouseEvent<HTMLButtonElement, 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 (
     <EuiFlyout
       data-test-subj="collapsibleNav"
@@ -335,7 +349,12 @@ export function CollapsibleNavGroupEnabled({
           </EuiPanel>
         </div>
         <EuiHorizontalRule margin="none" />
-        <div className="bottom-container" style={{ flexDirection: isNavOpen ? 'row' : 'column' }}>
+        <div
+          className={classNames({
+            'bottom-container': true,
+            'bottom-container-collapsed': !isNavOpen,
+          })}
+        >
           <HeaderNavControls
             navControls$={observables.navControlsLeftBottom$}
             className={classNames({ 'nav-controls-padding': isNavOpen })}

diff --git a/src/core/utils/default_app_categories.ts b/src/core/utils/default_app_categories.ts
@@ -122,6 +122,13 @@ export const DEFAULT_APP_CATEGORIES: Record<string, AppCategory> = 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
@@ -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
@@ -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
@@ -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<RegisterManagementAppArgs, 'basePath'>;
+    managementMock.sections.section.opensearchDashboards.registerApp = (
+      app: Omit<RegisterManagementAppArgs, 'basePath'>
+    ) => {
+      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'
+    );
+  });
 });
Original file line number	Diff line number	Diff line change
		@@ -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))