Skip to content

Commit

Permalink
refactor(workspace): store workspace configurable apps in a global
Browse files Browse the repository at this point in the history
variable

Signed-off-by: Yulong Ruan <[email protected]>
  • Loading branch information
ruanyl committed Apr 16, 2024
1 parent 8e5930a commit 79da6af
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 150 deletions.
9 changes: 7 additions & 2 deletions src/plugins/workspace/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ import { WorkspaceUpdaterApp } from './components/workspace_updater_app';
import { WorkspaceListApp } from './components/workspace_list_app';
import { WorkspaceUpdaterProps } from './components/workspace_updater';
import { Services } from './types';
import { WorkspaceCreatorProps } from './components/workspace_creator/workspace_creator';

export const renderCreatorApp = ({ element }: AppMountParameters, services: Services) => {
export const renderCreatorApp = (
{ element }: AppMountParameters,
services: Services,
props: WorkspaceCreatorProps
) => {
ReactDOM.render(
<OpenSearchDashboardsContextProvider services={services}>
<WorkspaceCreatorApp />
<WorkspaceCreatorApp {...props} />
</OpenSearchDashboardsContextProvider>,
element
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,21 @@ describe('WorkspaceCreator', () => {
});

it('should not create workspace when name is empty', async () => {
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton'));
expect(workspaceClientCreate).not.toHaveBeenCalled();
});

it('should not create workspace with invalid name', async () => {
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: '~' },
Expand All @@ -103,7 +111,11 @@ describe('WorkspaceCreator', () => {
});

it('should not create workspace with invalid description', async () => {
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand All @@ -116,15 +128,23 @@ describe('WorkspaceCreator', () => {
});

it('cancel create workspace', async () => {
const { findByText, getByTestId } = render(<WorkspaceCreator />);
const { findByText, getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
fireEvent.click(getByTestId('workspaceForm-bottomBar-cancelButton'));
await findByText('Discard changes?');
fireEvent.click(getByTestId('confirmModalConfirmButton'));
expect(navigateToApp).toHaveBeenCalled();
});

it('create workspace with detailed information', async () => {
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand Down Expand Up @@ -155,7 +175,11 @@ describe('WorkspaceCreator', () => {

it('create workspace with customized features', async () => {
setHrefSpy.mockReset();
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand All @@ -181,7 +205,11 @@ describe('WorkspaceCreator', () => {

it('should show danger toasts after create workspace failed', async () => {
workspaceClientCreate.mockReturnValue({ result: { id: 'failResult' }, success: false });
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand All @@ -198,7 +226,11 @@ describe('WorkspaceCreator', () => {
workspaceClientCreate.mockImplementation(async () => {
throw new Error();
});
const { getByTestId } = render(<WorkspaceCreator />);
const { getByTestId } = render(
<WorkspaceCreator
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,27 @@
import React, { useCallback } from 'react';
import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent, EuiSpacer } from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { useObservable } from 'react-use';

import { PublicAppInfo } from 'opensearch-dashboards/public';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form';
import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants';
import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
import { WorkspaceClient } from '../../workspace_client';
import { BehaviorSubject, of } from 'rxjs';

Check failure on line 17 in src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx

View workflow job for this annotation

GitHub Actions / Build and Verify on Linux (ciGroup1)

`rxjs` import should occur before import of `../../../../opensearch_dashboards_react/public`

export interface WorkspaceCreatorProps {
workspaceConfigurableApps$?: BehaviorSubject<PublicAppInfo[]>;
}

export const WorkspaceCreator = () => {
export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
const {
services: { application, notifications, http, workspaceClient },
} = useOpenSearchDashboards<{ workspaceClient: WorkspaceClient }>();
const workspaceConfigurableApps = useObservable(
props.workspaceConfigurableApps$ ?? of(undefined)
);

const handleWorkspaceFormSubmit = useCallback(
async (data: WorkspaceFormSubmitData) => {
Expand Down Expand Up @@ -80,6 +91,7 @@ export const WorkspaceCreator = () => {
application={application}
onSubmit={handleWorkspaceFormSubmit}
operationType={WorkspaceOperationType.Create}
workspaceConfigurableApps={workspaceConfigurableApps}
/>
)}
</EuiPageContent>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import { I18nProvider } from '@osd/i18n/react';
import { i18n } from '@osd/i18n';
import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public';
import { WorkspaceCreator } from './workspace_creator';
import { WorkspaceCreatorProps } from './workspace_creator/workspace_creator';

export const WorkspaceCreatorApp = () => {
export const WorkspaceCreatorApp = (props: WorkspaceCreatorProps) => {
const {
services: { chrome },
} = useOpenSearchDashboards();
Expand All @@ -29,7 +30,7 @@ export const WorkspaceCreatorApp = () => {

return (
<I18nProvider>
<WorkspaceCreator />
<WorkspaceCreator {...props} />
</I18nProvider>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import type { WorkspaceOperationType } from './constants';
import type { ApplicationStart } from '../../../../../core/public';
import type { ApplicationStart, PublicAppInfo } from '../../../../../core/public';

export interface WorkspaceFormSubmitData {
name: string;
Expand Down Expand Up @@ -35,5 +35,5 @@ export interface WorkspaceFormProps {
onSubmit?: (formData: WorkspaceFormSubmitData) => void;
defaultValues?: WorkspaceFormData;
operationType?: WorkspaceOperationType;
restrictedApps?: Set<string>;
workspaceConfigurableApps?: PublicAppInfo[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,72 +7,6 @@ import { AppNavLinkStatus, DEFAULT_APP_CATEGORIES } from '../../../../../core/pu
import { convertApplicationsToFeaturesOrGroups } from './utils';

describe('convertApplicationsToFeaturesOrGroups', () => {
it('should not filter out restrict Apps', () => {
expect(
convertApplicationsToFeaturesOrGroups(
[
{ id: 'foo1', title: 'Foo 1', navLinkStatus: AppNavLinkStatus.hidden },
{ id: 'foo2', title: 'Foo 2', navLinkStatus: AppNavLinkStatus.visible, chromeless: true },
{
id: 'foo3',
title: 'Foo 3',
navLinkStatus: AppNavLinkStatus.visible,
category: DEFAULT_APP_CATEGORIES.management,
},
{
id: 'workspace_overview',
title: 'Workspace Overview',
navLinkStatus: AppNavLinkStatus.visible,
},
{
id: 'bar',
title: 'Bar',
navLinkStatus: AppNavLinkStatus.visible,
},
],
new Set(['foo1'])
)
).toEqual([
{
id: 'foo1',
name: 'Foo 1',
},
{
id: 'bar',
name: 'Bar',
},
]);
});

it('should filter out invisible features', () => {
expect(
convertApplicationsToFeaturesOrGroups([
{ id: 'foo1', title: 'Foo 1', navLinkStatus: AppNavLinkStatus.hidden },
{ id: 'foo2', title: 'Foo 2', navLinkStatus: AppNavLinkStatus.visible, chromeless: true },
{
id: 'foo3',
title: 'Foo 3',
navLinkStatus: AppNavLinkStatus.visible,
category: DEFAULT_APP_CATEGORIES.management,
},
{
id: 'workspace_overview',
title: 'Workspace Overview',
navLinkStatus: AppNavLinkStatus.visible,
},
{
id: 'bar',
title: 'Bar',
navLinkStatus: AppNavLinkStatus.visible,
},
])
).toEqual([
{
id: 'bar',
name: 'Bar',
},
]);
});
it('should group same category applications in same feature group', () => {
expect(
convertApplicationsToFeaturesOrGroups([
Expand Down
29 changes: 3 additions & 26 deletions src/plugins/workspace/public/components/workspace_form/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {
AppNavLinkStatus,
DEFAULT_APP_CATEGORIES,
PublicAppInfo,
} from '../../../../../core/public';
import { PublicAppInfo } from '../../../../../core/public';
import { DEFAULT_SELECTED_FEATURES_IDS } from '../../../common/constants';

import { WorkspaceFeature, WorkspaceFeatureGroup, WorkspaceFormErrors } from './types';
Expand Down Expand Up @@ -44,37 +40,18 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => {
export const convertApplicationsToFeaturesOrGroups = (
applications: Array<
Pick<PublicAppInfo, 'id' | 'title' | 'category' | 'navLinkStatus' | 'chromeless'>
>,
restrictedApps?: Set<string>
>
) => {
const UNDEFINED = 'undefined';

// Filter out all hidden applications and management applications and default selected features
const visibleApplications = applications.filter(({ navLinkStatus, chromeless, category, id }) => {
/**
* Restrict apps are apps that can be configured into a workspace, but restrict to access
* because the current workspace didn't have the apps configured, such apps should NOT filter out
*/
if (restrictedApps && restrictedApps.has(id)) {
return true;
}

return (
navLinkStatus !== AppNavLinkStatus.hidden &&
!chromeless &&
!DEFAULT_SELECTED_FEATURES_IDS.includes(id) &&
category?.id !== DEFAULT_APP_CATEGORIES.management.id
);
});

/**
*
* Convert applications to features map, the map use category label as
* map key and group all same category applications in one array after
* transfer application to feature.
*
**/
const categoryLabel2Features = visibleApplications.reduce<{
const categoryLabel2Features = applications.reduce<{
[key: string]: WorkspaceFeature[];
}>((previousValue, application) => {
const label = application.category?.label || UNDEFINED;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
WorkspaceFeatureSelector,
WorkspaceFeatureSelectorProps,
} from './workspace_feature_selector';
import { AppNavLinkStatus } from '../../../../../core/public';
import { AppNavLinkStatus, AppStatus } from '../../../../../core/public';

const setup = (options?: Partial<WorkspaceFeatureSelectorProps>) => {
const onChangeMock = jest.fn();
Expand All @@ -19,28 +19,36 @@ const setup = (options?: Partial<WorkspaceFeatureSelectorProps>) => {
title: 'App 1',
category: { id: 'category-1', label: 'Category 1' },
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/app-1',
},
{
id: 'app-2',
title: 'App 2',
category: { id: 'category-1', label: 'Category 1' },
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/app-2',
},
{
id: 'app-3',
title: 'App 3',
category: { id: 'category-2', label: 'Category 2' },
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/app-3',
},
{
id: 'app-4',
title: 'App 4',
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/app-4',
},
];
const renderResult = render(
<WorkspaceFeatureSelector
applications={applications}
workspaceConfigurableApps={applications}
selectedFeatures={[]}
onChange={onChangeMock}
{...options}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,19 @@ import { PublicAppInfo } from '../../../../../core/public';
import { isWorkspaceFeatureGroup, convertApplicationsToFeaturesOrGroups } from './utils';

export interface WorkspaceFeatureSelectorProps {
applications: Array<
Pick<PublicAppInfo, 'id' | 'title' | 'category' | 'chromeless' | 'navLinkStatus'>
>;
selectedFeatures: string[];
onChange: (newFeatures: string[]) => void;
restrictedApps?: Set<string>;
workspaceConfigurableApps?: PublicAppInfo[];
}

export const WorkspaceFeatureSelector = ({
applications,
selectedFeatures,
onChange,
restrictedApps,
workspaceConfigurableApps,
}: WorkspaceFeatureSelectorProps) => {
const featuresOrGroups = useMemo(
() => convertApplicationsToFeaturesOrGroups(applications, restrictedApps),
[applications, restrictedApps]
() => convertApplicationsToFeaturesOrGroups(workspaceConfigurableApps ?? []),
[workspaceConfigurableApps]
);

const handleFeatureChange = useCallback<EuiCheckboxGroupProps['onChange']>(
Expand Down
Loading

0 comments on commit 79da6af

Please sign in to comment.