Skip to content

Commit

Permalink
Backport/backport 6459 to workspace pr integr (#332)
Browse files Browse the repository at this point in the history
* fix(workspace): apps are missing when updating a workspace

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]>

* refactor workspace list page to use workspaceConfigurableApps

Signed-off-by: Yulong Ruan <[email protected]>

* fix tests

Signed-off-by: Yulong Ruan <[email protected]>

---------

Signed-off-by: Yulong Ruan <[email protected]>
  • Loading branch information
ruanyl authored Apr 19, 2024
1 parent a821167 commit 9d6094f
Show file tree
Hide file tree
Showing 17 changed files with 200 additions and 74 deletions.
27 changes: 21 additions & 6 deletions src/plugins/workspace/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,20 @@ 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';
import { WorkspaceListProps } from './components/workspace_list';

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 +34,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 All @@ -55,10 +66,14 @@ export const renderFatalErrorApp = (params: AppMountParameters, services: Servic
};
};

export const renderListApp = ({ element }: AppMountParameters, services: Services) => {
export const renderListApp = (
{ element }: AppMountParameters,
services: Services,
props: WorkspaceListProps
) => {
ReactDOM.render(
<OpenSearchDashboardsContextProvider services={services}>
<WorkspaceListApp />
<WorkspaceListApp {...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 @@ -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: '/',
},
{
id: 'app-2',
title: 'App 2',
category: { id: 'category-1', label: 'Category 1' },
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/',
},
{
id: 'app-3',
title: 'App 3',
category: { id: 'category-2', label: 'Category 2' },
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/',
},
{
id: 'app-4',
title: 'App 4',
navLinkStatus: AppNavLinkStatus.visible,
status: AppStatus.accessible,
appRoute: '/',
},
];
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 @@ -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
19 changes: 11 additions & 8 deletions src/plugins/workspace/public/components/workspace_list/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import {
EuiSearchBarProps,
} from '@elastic/eui';
import useObservable from 'react-use/lib/useObservable';
import { of } from 'rxjs';
import { BehaviorSubject, of } from 'rxjs';
import { i18n } from '@osd/i18n';
import { debounce, WorkspaceObject } from '../../../../../core/public';
import { debounce, PublicAppInfo, WorkspaceObject } from '../../../../../core/public';
import { WorkspaceAttribute } from '../../../../../core/public';
import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public';
import { switchWorkspace, updateWorkspace } from '../utils/workspace';
Expand All @@ -26,17 +26,20 @@ import { WORKSPACE_CREATE_APP_ID } from '../../../common/constants';

import { cleanWorkspaceId } from '../../../../../core/public';
import { DeleteWorkspaceModal } from '../delete_workspace_modal';
import { useApplications } from '././../../hooks';
import { getSelectedFeatureQuantities } from '../../utils';

const WORKSPACE_LIST_PAGE_DESCRIPTIOIN = i18n.translate('workspace.list.description', {
export interface WorkspaceListProps {
workspaceConfigurableApps$?: BehaviorSubject<PublicAppInfo[]>;
}

const WORKSPACE_LIST_PAGE_DESCRIPTION = i18n.translate('workspace.list.description', {
defaultMessage:
'Workspace allow you to save and organize library items, such as index patterns, visualizations, dashboards, saved searches, and share them with other OpenSearch Dashboards users. You can control which features are visible in each workspace, and which users and groups have read and write access to the library items in the workspace.',
});

const emptyWorkspaceList: WorkspaceObject[] = [];

export const WorkspaceList = () => {
export const WorkspaceList = (props: WorkspaceListProps) => {
const {
services: { workspaces, application, http },
} = useOpenSearchDashboards();
Expand All @@ -54,7 +57,7 @@ export const WorkspaceList = () => {
pageSizeOptions: [5, 10, 20],
});
const [deletedWorkspace, setDeletedWorkspace] = useState<WorkspaceAttribute | null>(null);
const applications = useApplications(application);
const applications = useObservable(props.workspaceConfigurableApps$ ?? of(undefined));

const handleSwitchWorkspace = useCallback(
(id: string) => {
Expand Down Expand Up @@ -115,7 +118,7 @@ export const WorkspaceList = () => {
isExpander: true,
hasActions: true,
render: (features: string[]) => {
const { total, selected } = getSelectedFeatureQuantities(features, applications);
const { total, selected } = getSelectedFeatureQuantities(features, applications || []);
return `${selected}/${total}`;
},
},
Expand Down Expand Up @@ -189,7 +192,7 @@ export const WorkspaceList = () => {
<EuiPageHeader
restrictWidth
pageTitle="Workspaces"
description={WORKSPACE_LIST_PAGE_DESCRIPTIOIN}
description={WORKSPACE_LIST_PAGE_DESCRIPTION}
style={{ paddingBottom: 0, borderBottom: 0 }}
/>
<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 { WorkspaceList } from './workspace_list';
import { WorkspaceList, WorkspaceListProps } from './workspace_list';

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

return (
<I18nProvider>
<WorkspaceList />
<WorkspaceList {...props} />
</I18nProvider>
);
};
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 9d6094f

Please sign in to comment.