Skip to content

Commit

Permalink
[Workspace] fix apps are missing when updating a workspace (#6459)
Browse files Browse the repository at this point in the history
* fix(workspace): apps are missing when updating a workspace

This is caused by #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>

* refactor(workspace): store workspace configurable apps in a global
variable

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

* fix linter

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com>
(cherry picked from commit 4746f43)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] and ZilongX committed Apr 23, 2024
1 parent dd923b4 commit d027978
Show file tree
Hide file tree
Showing 16 changed files with 206 additions and 93 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,12 +11,18 @@ import { WorkspaceFatalError } from './components/workspace_fatal_error';
import { WorkspaceCreatorApp } from './components/workspace_creator_app';
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 All @@ -26,10 +32,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('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 @@ -109,7 +117,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 @@ -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 @@ -162,7 +182,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 @@ -189,7 +213,11 @@ describe('WorkspaceCreator', () => {

it('should show danger toasts after create workspace failed', async () => {
workspaceClientCreate.mockReturnValueOnce({ 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 @@ -206,7 +234,11 @@ describe('WorkspaceCreator', () => {
workspaceClientCreate.mockImplementationOnce(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,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';

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 @@ -86,6 +97,7 @@ export const WorkspaceCreator = () => {
application={application}
onSubmit={handleWorkspaceFormSubmit}
operationType={WorkspaceOperationType.Create}
workspaceConfigurableApps={workspaceConfigurableApps}
permissionEnabled={isPermissionEnabled}
permissionLastAdminItemDeletable
/>
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 @@ -55,6 +55,7 @@ export interface WorkspaceFormProps {
onSubmit?: (formData: WorkspaceFormSubmitData) => void;
defaultValues?: WorkspaceFormData;
operationType?: WorkspaceOperationType;
workspaceConfigurableApps?: PublicAppInfo[];
permissionEnabled?: boolean;
permissionLastAdminItemDeletable?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,6 @@ import { WorkspacePermissionMode } from '../../../common/constants';
import { WorkspacePermissionItemType } from './constants';

describe('convertApplicationsToFeaturesOrGroups', () => {
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
17 changes: 2 additions & 15 deletions src/plugins/workspace/public/components/workspace_form/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@

import { i18n } from '@osd/i18n';

import {
AppNavLinkStatus,
DEFAULT_APP_CATEGORIES,
PublicAppInfo,
} from '../../../../../core/public';
import { PublicAppInfo } from '../../../../../core/public';
import type { SavedObjectPermissions } from '../../../../../core/types';
import { DEFAULT_SELECTED_FEATURES_IDS, WorkspacePermissionMode } from '../../../common/constants';
import {
Expand Down Expand Up @@ -65,23 +61,14 @@ export const convertApplicationsToFeaturesOrGroups = (
) => {
const UNDEFINED = 'undefined';

// Filter out all hidden applications and management applications and default selected features
const visibleApplications = applications.filter(
({ navLinkStatus, chromeless, category, id }) =>
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,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 @@ -153,9 +152,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';
Loading

0 comments on commit d027978

Please sign in to comment.