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 <ruanyl@amazon.com>
  • Loading branch information
ruanyl committed Apr 18, 2024
1 parent a821167 commit 611b43d
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 41 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,29 @@
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';

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 @@ -88,6 +100,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 @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

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

Expand Down Expand Up @@ -57,4 +57,5 @@ export interface WorkspaceFormProps {
operationType?: WorkspaceOperationType;
permissionEnabled?: boolean;
permissionLastAdminItemDeletable?: boolean;
workspaceConfigurableApps?: PublicAppInfo[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,20 @@ 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;
workspaceConfigurableApps?: PublicAppInfo[];
}

export const WorkspaceFeatureSelector = ({
applications,
selectedFeatures,
onChange,
workspaceConfigurableApps,
}: WorkspaceFeatureSelectorProps) => {
const featuresOrGroups = useMemo(() => convertApplicationsToFeaturesOrGroups(applications), [
applications,
]);
const featuresOrGroups = useMemo(
() => convertApplicationsToFeaturesOrGroups(workspaceConfigurableApps ?? []),
[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, getAllByText } = render(<WorkspaceUpdater />);
const { getByTestId, getByText, getAllByText } = 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 { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants';
import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
Expand All @@ -21,6 +22,10 @@ import {
convertPermissionSettingsToPermissions,
} from '../workspace_form';

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

function getFormDataFromWorkspace(
currentWorkspace: WorkspaceAttributeWithPermission | null | undefined
) {
Expand All @@ -35,13 +40,16 @@ 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 @@ -128,6 +136,7 @@ export const WorkspaceUpdater = () => {
operationType={WorkspaceOperationType.Update}
permissionEnabled={isPermissionEnabled}
permissionLastAdminItemDeletable={false}
workspaceConfigurableApps={workspaceConfigurableApps}
/>
)}
</EuiPageContent>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import React, { useEffect } from 'react';
import { I18nProvider } from '@osd/i18n/react';
import { i18n } from '@osd/i18n';
import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public';
import { WorkspaceUpdater } from './workspace_updater';
import { WorkspaceUpdater, WorkspaceUpdaterProps } from './workspace_updater';

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

return (
<I18nProvider>
<WorkspaceUpdater />
<WorkspaceUpdater {...props} />
</I18nProvider>
);
};
Loading

0 comments on commit 611b43d

Please sign in to comment.