Skip to content

Commit

Permalink
[navigation]fix: optimize the logic to detect current nav group (#8189)
Browse files Browse the repository at this point in the history
* fix: optimize the logic to detect current nav group

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: optimize code

Signed-off-by: SuZhou-Joe <[email protected]>

* Changeset file for PR #8189 created/updated

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 6fd284f commit 9a88bf5
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 7 deletions.
3 changes: 3 additions & 0 deletions changelogs/fragments/8189.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fix:
- Current nav group will be mapped to global system nav group even if user is in a workspace. ([#8189](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8189))
- Current nav group will be mapped to a nav group even when user is out of a workspace. ([#8189](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8189))
88 changes: 87 additions & 1 deletion src/core/public/chrome/nav_group/nav_group_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ describe('ChromeNavGroupService#start()', () => {
expect(currentNavGroup).toBeUndefined();
});

it('should set current nav group automatically if application only belongs 1 nav group', async () => {
it('should set current nav group automatically if application only belongs to 1 visible nav group', async () => {
const uiSettings = uiSettingsServiceMock.createSetupContract();
const navGroupEnabled$ = new Rx.BehaviorSubject(true);
uiSettings.get$.mockImplementation(() => navGroupEnabled$);
Expand Down Expand Up @@ -424,6 +424,39 @@ describe('ChromeNavGroupService#start()', () => {
expect(currentNavGroup?.id).toEqual('bar-group');
});

it('should be able to find the right nav group when visible nav group length is 1 and is not all 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: 'fooGroupTitle',
description: 'foo description',
},
[mockedNavLinkFoo]
);

const chromeNavGroupServiceStart = await chromeNavGroupService.start({
navLinks: mockedNavLinkService,
application: mockedApplicationService,
breadcrumbsEnricher$: new Rx.BehaviorSubject<ChromeBreadcrumbEnricher | undefined>(undefined),
workspaces: workspacesServiceMock.createStartContract(),
});
mockedApplicationService.navigateToApp(mockedNavLinkFoo.id);

const currentNavGroup = await chromeNavGroupServiceStart
.getCurrentNavGroup$()
.pipe(first())
.toPromise();

expect(currentNavGroup?.id).toEqual('foo');
});

it('should erase current nav group if application can not be found in any of the visible nav groups', async () => {
const uiSettings = uiSettingsServiceMock.createSetupContract();
const navGroupEnabled$ = new Rx.BehaviorSubject(true);
Expand Down Expand Up @@ -469,6 +502,59 @@ describe('ChromeNavGroupService#start()', () => {
expect(currentNavGroup).toBeFalsy();
});

it('should erase current nav group if application can only be found in use case but outside workspace', 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-group',
title: 'fooGroupTitle',
description: 'foo description',
},
[mockedNavLinkFoo]
);

chromeNavGroupServiceSetup.addNavLinksToGroup(
{
id: 'bar-group',
title: 'barGroupTitle',
description: 'bar description',
},
[mockedNavLinkFoo, mockedNavLinkBar]
);

const chromeNavGroupServiceStart = await chromeNavGroupService.start({
navLinks: mockedNavLinkService,
application: {
...mockedApplicationService,
capabilities: Object.freeze({
...mockedApplicationService.capabilities,
workspaces: {
...mockedApplicationService.capabilities.workspaces,
enabled: true,
},
}),
},
breadcrumbsEnricher$: new Rx.BehaviorSubject<ChromeBreadcrumbEnricher | undefined>(undefined),
workspaces: workspacesServiceMock.createStartContract(),
});

chromeNavGroupServiceStart.setCurrentNavGroup('foo-group');

mockedApplicationService.navigateToApp(mockedNavLinkBar.id);
const currentNavGroup = await chromeNavGroupServiceStart
.getCurrentNavGroup$()
.pipe(first())
.toPromise();

expect(currentNavGroup).toBeFalsy();
});

it('should set breadcrumbs enricher when nav group is enabled', async () => {
const uiSettings = uiSettingsServiceMock.createSetupContract();
const navGroupEnabled$ = new Rx.BehaviorSubject(true);
Expand Down
36 changes: 30 additions & 6 deletions src/core/public/chrome/nav_group/nav_group_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
} from '../utils';
import { ChromeNavLinks } from '../nav_links';
import { InternalApplicationStart } from '../../application';
import { NavGroupStatus } from '../../../../core/types';
import { NavGroupStatus, NavGroupType } from '../../../../core/types';
import { ChromeBreadcrumb, ChromeBreadcrumbEnricher } from '../chrome_service';
import { ALL_USE_CASE_ID } from '../../../utils';

Expand Down Expand Up @@ -270,10 +270,15 @@ export class ChromeNavGroupService {
appIdNavGroupMap.set(navLinkId, navGroupSet);
});
};
if (visibleUseCases.length === 1 && visibleUseCases[0].id === ALL_USE_CASE_ID) {
// If the only visible use case is all use case
// All the other nav groups will be visible because all use case can visit all of the nav groups.
Object.values(navGroupMap).forEach((navGroup) => mapAppIdToNavGroup(navGroup));
if (visibleUseCases.length === 1) {
if (visibleUseCases[0].id === ALL_USE_CASE_ID) {
// If the only visible use case is all use case
// All the other nav groups will be visible because all use case can visit all of the nav groups.
Object.values(navGroupMap).forEach((navGroup) => mapAppIdToNavGroup(navGroup));
} else {
// It means we are in a workspace, we should only use the visible use cases
visibleUseCases.forEach((navGroup) => mapAppIdToNavGroup(navGroup));
}
} else {
// Nav group of Hidden status should be filtered out when counting navGroups the currentApp belongs to
Object.values(navGroupMap).forEach((navGroup) => {
Expand All @@ -287,7 +292,26 @@ export class ChromeNavGroupService {

const navGroups = appIdNavGroupMap.get(appId);
if (navGroups && navGroups.size === 1) {
setCurrentNavGroup(navGroups.values().next().value);
const navGroupId = navGroups.values().next().value as string;
/**
* If
* 1. workspace enabled
* 2. outside of workspace: visibleUseCases.length > 1
* 3. the matched nav group is a use case nav group
*
* It means a workspace application is incorrectly opened in global place.
* We need to set current nav group to undefined to not show the use case nav.
*/
const navGroupInfo = navGroupMap[navGroupId];
if (
application.capabilities.workspaces.enabled &&
visibleUseCases.length > 1 &&
navGroupInfo.type !== NavGroupType.SYSTEM
) {
setCurrentNavGroup(undefined);
} else {
setCurrentNavGroup(navGroupId);
}
} else if (!navGroups) {
setCurrentNavGroup(undefined);
}
Expand Down

0 comments on commit 9a88bf5

Please sign in to comment.