Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] [Workspace] fix apps are missing when updating a workspace #6606

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading