Skip to content

Commit

Permalink
fix(workspace): apps are missing when updating a workspace
Browse files Browse the repository at this point in the history
This is caused by opensearch-project#6234 which marked apps as inaccessible when the apps
are not configured into the current workspace. However, inaccessible
apps can not be configured into a workspace.

Signed-off-by: Yulong Ruan <[email protected]>
  • Loading branch information
ruanyl committed Apr 17, 2024
1 parent 1a353cc commit 55acbe1
Show file tree
Hide file tree
Showing 13 changed files with 166 additions and 42 deletions.
18 changes: 14 additions & 4 deletions src/plugins/workspace/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@ import { WorkspaceListApp } from './components/workspace_list_app';
import { WorkspaceFatalError } from './components/workspace_fatal_error';
import { WorkspaceCreatorApp } from './components/workspace_creator_app';
import { WorkspaceUpdaterApp } from './components/workspace_updater_app';
import { WorkspaceUpdaterProps } from './components/workspace_updater';
import { Services } from './types';
import { WorkspaceOverviewApp } from './components/workspace_overview_app';
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 All @@ -27,10 +33,14 @@ export const renderCreatorApp = ({ element }: AppMountParameters, services: Serv
};
};

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

it('cannot create workspace when name 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('cannot 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 @@ -109,7 +117,11 @@ describe('WorkspaceCreator', () => {
});

it('cannot 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 @@ -122,15 +134,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 @@ -161,7 +181,11 @@ describe('WorkspaceCreator', () => {
});

it('create workspace with customized features', 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 @@ -218,7 +242,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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,28 @@
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 { BehaviorSubject, of } from 'rxjs';

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 { convertPermissionSettingsToPermissions } from '../workspace_form/utils';

export const WorkspaceCreator = () => {
export interface WorkspaceCreatorProps {
workspaceConfigurableApps$?: BehaviorSubject<PublicAppInfo[]>;
}

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

const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled;
const handleWorkspaceFormSubmit = useCallback(
Expand Down Expand Up @@ -92,6 +103,7 @@ export const WorkspaceCreator = () => {
operationType={WorkspaceOperationType.Create}
permissionEnabled={isPermissionEnabled}
permissionLastAdminItemDeletable
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 @@ -5,7 +5,7 @@

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

export type WorkspacePermissionSetting =
| { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] }
Expand Down Expand Up @@ -50,4 +50,5 @@ export interface WorkspaceFormProps {
operationType?: WorkspaceOperationType;
permissionEnabled?: boolean;
permissionLastAdminItemDeletable?: boolean;
workspaceConfigurableApps?: PublicAppInfo[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,24 @@ import { DEFAULT_APP_CATEGORIES, PublicAppInfo } from '../../../../../core/publi

import { WorkspaceFeature, WorkspaceFeatureGroup } from './types';
import { isDefaultCheckedFeatureId, isWorkspaceFeatureGroup } from './utils';
import { getAllExcludingManagementApps } from '../../utils';

const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', {
defaultMessage: 'Library',
});

interface WorkspaceFeatureSelectorProps {
applications: PublicAppInfo[];
export interface WorkspaceFeatureSelectorProps {
selectedFeatures: string[];
onChange: (newFeatures: string[]) => void;
workspaceConfigurableApps?: PublicAppInfo[];
}

export const WorkspaceFeatureSelector = ({
applications,
selectedFeatures,
onChange,
workspaceConfigurableApps,
}: WorkspaceFeatureSelectorProps) => {
const featureOrGroups = useMemo(() => {
const transformedApplications = applications.map((app) => {
const transformedApplications = (workspaceConfigurableApps || []).map((app) => {
if (app.category?.id === DEFAULT_APP_CATEGORIES.opensearchDashboards.id) {
return {
...app,
Expand All @@ -55,7 +54,7 @@ export const WorkspaceFeatureSelector = ({
Array<WorkspaceFeature | WorkspaceFeatureGroup>
>((previousValue, currentKey) => {
const apps = category2Applications[currentKey];
const features = getAllExcludingManagementApps(apps).map(({ id, title }) => ({
const features = apps.map(({ id, title }) => ({
id,
name: title,
}));
Expand All @@ -73,7 +72,7 @@ export const WorkspaceFeatureSelector = ({
},
];
}, []);
}, [applications]);
}, [workspaceConfigurableApps]);

const handleFeatureChange = useCallback<EuiCheckboxGroupProps['onChange']>(
(featureId) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
formData,
formErrors,
selectedTab,
applications,
numberOfErrors,
handleFormSubmit,
handleColorChange,
Expand Down Expand Up @@ -154,9 +153,9 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
<EuiHorizontalRule margin="xs" />
<EuiSpacer size="s" />
<WorkspaceFeatureSelector
applications={applications}
selectedFeatures={formData.features}
onChange={handleFeaturesChange}
workspaceConfigurableApps={props.workspaceConfigurableApps}
/>
</EuiPanel>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
* SPDX-License-Identifier: Apache-2.0
*/

export { WorkspaceUpdater } from './workspace_updater';
export { WorkspaceUpdater, WorkspaceUpdaterProps } from './workspace_updater';
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,21 @@ describe('WorkspaceUpdater', () => {

it('cannot render when the name of the current workspace is empty', async () => {
const mockedWorkspacesService = workspacesServiceMock.createSetupContract();
const { container } = render(<WorkspaceUpdater workspacesService={mockedWorkspacesService} />);
const { container } = render(
<WorkspaceUpdater
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
workspacesService={mockedWorkspacesService}
/>
);
expect(container).toMatchInlineSnapshot(`<div />`);
});

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

it('update workspace successfully', async () => {
const { getByTestId, getByText } = render(<WorkspaceUpdater />);
const { getByTestId, getByText } = render(
<WorkspaceUpdater
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand Down Expand Up @@ -186,7 +199,11 @@ describe('WorkspaceUpdater', () => {

it('should show danger toasts after update workspace failed', async () => {
workspaceClientUpdate.mockReturnValue({ result: false, success: false });
const { getByTestId } = render(<WorkspaceUpdater />);
const { getByTestId } = render(
<WorkspaceUpdater
workspaceConfigurableApps$={new BehaviorSubject([...PublicAPPInfoMap.values()])}
/>
);
const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText');
fireEvent.input(nameInput, {
target: { value: 'test workspace name' },
Expand All @@ -203,7 +220,11 @@ describe('WorkspaceUpdater', () => {
workspaceClientUpdate.mockImplementation(() => {
throw new Error('update workspace failed');
});
const { getByTestId } = render(<WorkspaceUpdater />);
const { getByTestId } = render(
<WorkspaceUpdater
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,8 +6,9 @@
import React, { useCallback, useEffect, useState } from 'react';
import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { PublicAppInfo } from 'opensearch-dashboards/public';
import { useObservable } from 'react-use';
import { of } from 'rxjs';
import { BehaviorSubject, of } from 'rxjs';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form';
import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants';
Expand All @@ -19,6 +20,10 @@ import {
convertPermissionsToPermissionSettings,
} from '../workspace_form/utils';

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

function getFormDataFromWorkspace(
currentWorkspace: WorkspaceAttributeWithPermission | null | undefined
) {
Expand All @@ -33,14 +38,17 @@ function getFormDataFromWorkspace(
};
}

export const WorkspaceUpdater = () => {
export const WorkspaceUpdater = (props: WorkspaceUpdaterProps) => {
const {
services: { application, workspaces, notifications, http, workspaceClient },
} = useOpenSearchDashboards<{ workspaceClient: WorkspaceClient }>();

const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled;

const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null));
const workspaceConfigurableApps = useObservable(
props.workspaceConfigurableApps$ ?? of(undefined)
);
const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState(
getFormDataFromWorkspace(currentWorkspace)
);
Expand Down Expand Up @@ -131,6 +139,7 @@ export const WorkspaceUpdater = () => {
operationType={WorkspaceOperationType.Update}
permissionEnabled={isPermissionEnabled}
permissionLastAdminItemDeletable
workspaceConfigurableApps={workspaceConfigurableApps}
/>
)}
</EuiPageContent>
Expand Down
Loading

0 comments on commit 55acbe1

Please sign in to comment.