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 15, 2024
1 parent 476cffc commit 8e5930a
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 17 deletions.
9 changes: 7 additions & 2 deletions src/plugins/workspace/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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';

export const renderCreatorApp = ({ element }: AppMountParameters, services: Services) => {
Expand All @@ -26,10 +27,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 @@ -35,4 +35,5 @@ export interface WorkspaceFormProps {
onSubmit?: (formData: WorkspaceFormSubmitData) => void;
defaultValues?: WorkspaceFormData;
operationType?: WorkspaceOperationType;
restrictedApps?: Set<string>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,43 @@ import { AppNavLinkStatus, DEFAULT_APP_CATEGORIES } from '../../../../../core/pu
import { convertApplicationsToFeaturesOrGroups } from './utils';

describe('convertApplicationsToFeaturesOrGroups', () => {
it('should not filter out restrict Apps', () => {
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,
},
],
new Set(['foo1'])
)
).toEqual([
{
id: 'foo1',
name: 'Foo 1',
},
{
id: 'bar',
name: 'Bar',
},
]);
});

it('should filter out invisible features', () => {
expect(
convertApplicationsToFeaturesOrGroups([
Expand Down
18 changes: 14 additions & 4 deletions src/plugins/workspace/public/components/workspace_form/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,28 @@ export const getNumberOfErrors = (formErrors: WorkspaceFormErrors) => {
export const convertApplicationsToFeaturesOrGroups = (
applications: Array<
Pick<PublicAppInfo, 'id' | 'title' | 'category' | 'navLinkStatus' | 'chromeless'>
>
>,
restrictedApps?: Set<string>
) => {
const UNDEFINED = 'undefined';

// Filter out all hidden applications and management applications and default selected features
const visibleApplications = applications.filter(
({ navLinkStatus, chromeless, category, id }) =>
const visibleApplications = applications.filter(({ navLinkStatus, chromeless, category, id }) => {
/**
* Restrict apps are apps that can be configured into a workspace, but restrict to access
* because the current workspace didn't have the apps configured, such apps should NOT filter out
*/
if (restrictedApps && restrictedApps.has(id)) {
return true;
}

return (
navLinkStatus !== AppNavLinkStatus.hidden &&
!chromeless &&
!DEFAULT_SELECTED_FEATURES_IDS.includes(id) &&
category?.id !== DEFAULT_APP_CATEGORIES.management.id
);
);
});

/**
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@ export interface WorkspaceFeatureSelectorProps {
>;
selectedFeatures: string[];
onChange: (newFeatures: string[]) => void;
restrictedApps?: Set<string>;
}

export const WorkspaceFeatureSelector = ({
applications,
selectedFeatures,
onChange,
restrictedApps,
}: WorkspaceFeatureSelectorProps) => {
const featuresOrGroups = useMemo(() => convertApplicationsToFeaturesOrGroups(applications), [
applications,
]);
const featuresOrGroups = useMemo(
() => convertApplicationsToFeaturesOrGroups(applications, restrictedApps),
[applications, restrictedApps]
);

const handleFeatureChange = useCallback<EuiCheckboxGroupProps['onChange']>(
(featureId) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => {
applications={applications}
selectedFeatures={formData.features}
onChange={handleFeaturesChange}
restrictedApps={props.restrictedApps}
/>
</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 @@ -8,26 +8,31 @@ import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eu
import { i18n } from '@osd/i18n';
import { WorkspaceAttribute } 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';
import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils';
import { WorkspaceClient } from '../../workspace_client';
import { WorkspaceFormData } from '../workspace_form/types';

export interface WorkspaceUpdaterProps {
restrictedApps$?: BehaviorSubject<Set<string>>;
}

function getFormDataFromWorkspace(
currentWorkspace: WorkspaceAttribute | null | undefined
): WorkspaceFormData {
return (currentWorkspace || {}) as WorkspaceFormData;
}

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

const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null));
const restrictedApps = useObservable(props.restrictedApps$ ?? of(undefined));
const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState<WorkspaceFormData>(
getFormDataFromWorkspace(currentWorkspace)
);
Expand Down Expand Up @@ -108,6 +113,7 @@ export const WorkspaceUpdater = () => {
defaultValues={currentWorkspaceFormData}
onSubmit={handleWorkspaceFormSubmit}
operationType={WorkspaceOperationType.Update}
restrictedApps={restrictedApps}
/>
)}
</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>
);
};
24 changes: 22 additions & 2 deletions src/plugins/workspace/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
AppNavLinkStatus,
AppUpdater,
AppStatus,
DEFAULT_APP_CATEGORIES,
} from '../../../core/public';
import {
WORKSPACE_FATAL_ERROR_APP_ID,
Expand All @@ -30,7 +31,11 @@ import { WorkspaceMenu } from './components/workspace_menu/workspace_menu';
import { getWorkspaceColumn } from './components/workspace_column';
import { isAppAccessibleInWorkspace } from './utils';

type WorkspaceAppType = (params: AppMountParameters, services: Services) => () => void;
type WorkspaceAppType = (
params: AppMountParameters,
services: Services,
props: Record<string, any>
) => () => void;

interface WorkspacePluginSetupDeps {
savedObjectsManagement?: SavedObjectsManagementPluginSetup;
Expand All @@ -41,6 +46,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps>
private currentWorkspaceSubscription?: Subscription;
private currentWorkspaceIdSubscription?: Subscription;
private appUpdater$ = new BehaviorSubject<AppUpdater>(() => undefined);
private restrictedApps$ = new BehaviorSubject(new Set<string>());
private _changeSavedObjectCurrentWorkspace() {
if (this.coreStart) {
return this.coreStart.workspaces.currentWorkspaceId$.subscribe((currentWorkspaceId) => {
Expand All @@ -65,6 +71,20 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps>
if (isAppAccessibleInWorkspace(app, currentWorkspace)) {
return;
}

if (app.status === AppStatus.inaccessible) {
return;
}

/**
* Restricted apps can be configured into a workspace, but they are not configured by the
* current workspace. Apps of management category can NOT configured into a workspace, so
* needs to be excluded.
*/
if (app.category?.id !== DEFAULT_APP_CATEGORIES.management.id) {
this.restrictedApps$.next(this.restrictedApps$.value.add(app.id));
}

/**
* Change the app to `inaccessible` if it is not configured in the workspace
* If trying to access such app, an "Application Not Found" page will be displayed
Expand Down Expand Up @@ -129,7 +149,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps>
workspaceClient,
};

return renderApp(params, services);
return renderApp(params, services, { restrictedApps$: this.restrictedApps$ });
};

// create
Expand Down

0 comments on commit 8e5930a

Please sign in to comment.