Skip to content

Commit

Permalink
[Workspace] Refactor workspace detail page (opensearch-project#8615) (o…
Browse files Browse the repository at this point in the history
…pensearch-project#8630)

* refactor workspace detail page



* optimize the code



* fix test errors



---------


(cherry picked from commit 436e946)

Signed-off-by: yubonluo <yubonluo@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent b268a04 commit cbd4462
Show file tree
Hide file tree
Showing 36 changed files with 723 additions and 2,258 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { DataSourceConnection, DataSourceConnectionType } from '../../../common/
import { HttpStart, NotificationsStart, SavedObjectsStart } from '../../../../../core/public';
import { AssociationDataSourceModalMode } from '../../../common/constants';
import { Logos } from '../../../../../core/common';
import { ConnectionTypeIcon } from '../workspace_form';
import { ConnectionTypeIcon } from '../workspace_form/connection_type_icon';

const ConnectionIcon = ({
connection: { connectionType, type },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
toMountPoint,
} from '../../../../opensearch_dashboards_react/public';
import { DataSourceAssociation } from './data_source_association';
import { AssociationDataSourceModalContent } from '../workspace_detail/association_data_source_modal';
import { AssociationDataSourceModalContent } from './association_data_source_modal';
import { DataSourceConnectionType } from 'src/plugins/workspace/common/types';
import { BehaviorSubject } from 'rxjs';
import { IWorkspaceClient } from 'opensearch-dashboards/public';
Expand All @@ -23,7 +23,7 @@ jest.mock('../../../../opensearch_dashboards_react/public', () => ({
toMountPoint: jest.fn(),
}));

jest.mock('../workspace_detail/association_data_source_modal', () => ({
jest.mock('./association_data_source_modal', () => ({
AssociationDataSourceModalContent: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
toMountPoint,
useOpenSearchDashboards,
} from '../../../../opensearch_dashboards_react/public';
import { AssociationDataSourceModalContent } from '../workspace_detail/association_data_source_modal';
import { AssociationDataSourceModalContent } from './association_data_source_modal';
import { AssociationDataSourceModalMode } from '../../../common/constants';
import { DataSourceConnection, DataSourceConnectionType } from '../../../common/types';

Expand Down
48 changes: 1 addition & 47 deletions src/plugins/workspace/public/components/utils/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,66 +3,20 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { navigateToWorkspaceDetail, navigateToWorkspacePageWithUseCase } from './workspace';
import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
import { navigateToWorkspacePageWithUseCase } from './workspace';
jest.mock('../../../../../core/public/utils');

import { coreMock } from '../../../../../core/public/mocks';

const coreStartMock = coreMock.createStart();
let mockNavigateToUrl = jest.fn();

const defaultUrl = 'localhost://';

describe('workspace utils', () => {
beforeEach(() => {
mockNavigateToUrl = jest.fn();
coreStartMock.application.navigateToUrl = mockNavigateToUrl;
});

describe('navigateToWorkspaceDetail', () => {
it('should redirect if newUrl is returned', () => {
// @ts-ignore
formatUrlWithWorkspaceId.mockImplementation(() => 'localhost:5601/w/id/app/workspace_detail');
navigateToWorkspaceDetail(
{ application: coreStartMock.application, http: coreStartMock.http },
'id'
);
expect(mockNavigateToUrl).toHaveBeenCalledWith(
'localhost:5601/w/id/app/workspace_detail#/?tab=details'
);
});

it('should redirect to collaborators if newUrl is returned and tab id is collaborators', () => {
// @ts-ignore
formatUrlWithWorkspaceId.mockImplementation(() => 'localhost:5601/w/id/app/workspace_detail');
navigateToWorkspaceDetail(
{ application: coreStartMock.application, http: coreStartMock.http },
'id',
'collaborators'
);
expect(mockNavigateToUrl).toHaveBeenCalledWith(
'localhost:5601/w/id/app/workspace_detail#/?tab=collaborators'
);
});

it('should not redirect if newUrl is not returned', () => {
Object.defineProperty(window, 'location', {
value: {
href: defaultUrl,
},
writable: true,
});
// @ts-ignore
formatUrlWithWorkspaceId.mockImplementation(() => '');
navigateToWorkspaceDetail(
{ application: coreStartMock.application, http: coreStartMock.http },
''
);
expect(mockNavigateToUrl).not.toBeCalled();
});
});

describe('navigateToWorkspacePageWithUseCase', () => {
it('should redirect if newUrl is returned', () => {
coreStartMock.application.getUrlForApp.mockImplementation(
Expand Down
19 changes: 0 additions & 19 deletions src/plugins/workspace/public/components/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,11 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {
WORKSPACE_CREATE_APP_ID,
WORKSPACE_DETAIL_APP_ID,
WORKSPACE_LIST_APP_ID,
} from '../../../common/constants';
import { CoreStart } from '../../../../../core/public';
import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
import { DetailTab } from '../workspace_form/constants';

type Core = Pick<CoreStart, 'application' | 'http'>;

export const navigateToWorkspaceDetail = (
{ application, http }: Core,
workspaceId: string,
tabId: string = DetailTab.Details
) => {
navigateToAppWithinWorkspace(
{ application, http },
workspaceId,
WORKSPACE_DETAIL_APP_ID,
`/?tab=${tabId}`
);
};

export const navigateToWorkspacePageWithUseCase = (
application: Core['application'],
useCaseTitle: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,12 +520,12 @@ describe('WorkspaceCreator', () => {
jest.useRealTimers();
});

it('should redirect to workspace setting collaborators tab if permission enabled', async () => {
it('should redirect to workspace setting collaborators page if permission enabled', async () => {
const { getByTestId } = render(<WorkspaceCreator />);
const navigateToWorkspaceDetailMock = jest.fn();
const navigateToCollaboratorsMock = jest.fn();
jest
.spyOn(workspaceUtilsExports, 'navigateToWorkspaceDetail')
.mockImplementation(navigateToWorkspaceDetailMock);
.spyOn(workspaceUtilsExports, 'navigateToAppWithinWorkspace')
.mockImplementation(navigateToCollaboratorsMock);

// Ensure workspace create form rendered
await waitFor(() => {
Expand All @@ -539,10 +539,10 @@ describe('WorkspaceCreator', () => {
jest.useFakeTimers();
jest.runAllTimers();
await waitFor(() => {
expect(navigateToWorkspaceDetailMock).toHaveBeenCalledWith(
expect(navigateToCollaboratorsMock).toHaveBeenCalledWith(
expect.anything(),
'successResult',
'collaborators'
'workspace_collaborators'
);
});
jest.useRealTimers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import { useLocation } from 'react-router-dom';

import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { PermissionModeId } from '../../../../../core/public';
import { CURRENT_USER_PLACEHOLDER, WORKSPACE_DETAIL_APP_ID } from '../../../common/constants';
import {
CURRENT_USER_PLACEHOLDER,
WORKSPACE_COLLABORATORS_APP_ID,
WORKSPACE_DETAIL_APP_ID,
} from '../../../common/constants';
import {
WorkspaceFormSubmitData,
WorkspaceOperationType,
DetailTab,
WorkspacePermissionItemType,
convertPermissionSettingsToPermissions,
WorkspacePermissionSetting,
Expand All @@ -29,7 +32,7 @@ import { getFirstUseCaseOfFeatureConfigs } from '../../utils';
import { useFormAvailableUseCases } from '../workspace_form/use_form_available_use_cases';
import { NavigationPublicPluginStart } from '../../../../../plugins/navigation/public';
import { DataSourceConnectionType } from '../../../common/types';
import { navigateToWorkspaceDetail } from '../utils/workspace';
import { navigateToAppWithinWorkspace } from '../utils/workspace';
import { WorkspaceCreatorForm } from './workspace_creator_form';
import { optionIdToWorkspacePermissionModesMap } from '../workspace_form/constants';

Expand Down Expand Up @@ -143,10 +146,10 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
// Redirect page after one second, leave one second time to show create successful toast.
window.setTimeout(() => {
if (isPermissionEnabled) {
navigateToWorkspaceDetail(
navigateToAppWithinWorkspace(
{ application, http },
newWorkspaceId,
DetailTab.Collaborators
WORKSPACE_COLLABORATORS_APP_ID
);
return;
}
Expand Down
Loading

0 comments on commit cbd4462

Please sign in to comment.