From 55acbe1aa304eaf5170278c55eeedd3814ee5cc4 Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Mon, 15 Apr 2024 23:35:47 +0800 Subject: [PATCH] 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 --- src/plugins/workspace/public/application.tsx | 18 ++++++-- .../workspace_creator.test.tsx | 42 +++++++++++++++--- .../workspace_creator/workspace_creator.tsx | 14 +++++- .../components/workspace_creator_app.tsx | 5 ++- .../public/components/workspace_form/types.ts | 3 +- .../workspace_feature_selector.tsx | 13 +++--- .../workspace_form/workspace_form.tsx | 3 +- .../components/workspace_updater/index.tsx | 2 +- .../workspace_updater.test.tsx | 31 ++++++++++--- .../workspace_updater/workspace_updater.tsx | 13 +++++- .../components/workspace_updater_app.tsx | 6 +-- src/plugins/workspace/public/plugin.ts | 44 ++++++++++++++++--- src/plugins/workspace/public/utils.ts | 14 ++++++ 13 files changed, 166 insertions(+), 42 deletions(-) diff --git a/src/plugins/workspace/public/application.tsx b/src/plugins/workspace/public/application.tsx index 87fcbc555264..ef15f775115c 100644 --- a/src/plugins/workspace/public/application.tsx +++ b/src/plugins/workspace/public/application.tsx @@ -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( - + , element ); @@ -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( - + , element ); diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index b67870b55294..d988b5de20f2 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -94,13 +94,21 @@ describe('WorkspaceCreator', () => { }); it('cannot create workspace when name empty', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); fireEvent.click(getByTestId('workspaceForm-bottomBar-createButton')); expect(workspaceClientCreate).not.toHaveBeenCalled(); }); it('cannot create workspace with invalid name', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: '~' }, @@ -109,7 +117,11 @@ describe('WorkspaceCreator', () => { }); it('cannot create workspace with invalid description', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, @@ -122,7 +134,11 @@ describe('WorkspaceCreator', () => { }); it('cancel create workspace', async () => { - const { findByText, getByTestId } = render(); + const { findByText, getByTestId } = render( + + ); fireEvent.click(getByTestId('workspaceForm-bottomBar-cancelButton')); await findByText('Discard changes?'); fireEvent.click(getByTestId('confirmModalConfirmButton')); @@ -130,7 +146,11 @@ describe('WorkspaceCreator', () => { }); it('create workspace with detailed information', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, @@ -161,7 +181,11 @@ describe('WorkspaceCreator', () => { }); it('create workspace with customized features', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, @@ -218,7 +242,11 @@ describe('WorkspaceCreator', () => { it('should show danger toasts after create workspace failed', async () => { workspaceClientCreate.mockReturnValue({ result: { id: 'failResult' }, success: false }); - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 4b3e6e57c486..148e8ceeb0f1 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -6,6 +6,10 @@ 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'; @@ -13,10 +17,17 @@ import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; import { convertPermissionSettingsToPermissions } from '../workspace_form/utils'; -export const WorkspaceCreator = () => { +export interface WorkspaceCreatorProps { + workspaceConfigurableApps$?: BehaviorSubject; +} + +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( @@ -92,6 +103,7 @@ export const WorkspaceCreator = () => { operationType={WorkspaceOperationType.Create} permissionEnabled={isPermissionEnabled} permissionLastAdminItemDeletable + workspaceConfigurableApps={workspaceConfigurableApps} /> )} diff --git a/src/plugins/workspace/public/components/workspace_creator_app.tsx b/src/plugins/workspace/public/components/workspace_creator_app.tsx index b74359929352..e384f5d5bfed 100644 --- a/src/plugins/workspace/public/components/workspace_creator_app.tsx +++ b/src/plugins/workspace/public/components/workspace_creator_app.tsx @@ -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(); @@ -29,7 +30,7 @@ export const WorkspaceCreatorApp = () => { return ( - + ); }; diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 5f81ba3e6daa..e0debd53977f 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -5,7 +5,7 @@ import type { WorkspacePermissionItemType, WorkspaceOperationType } from './constants'; import type { WorkspacePermissionMode } from '../../../common/constants'; -import type { ApplicationStart } from '../../../../../core/public'; +import type { ApplicationStart, PublicAppInfo } from '../../../../../core/public'; export type WorkspacePermissionSetting = | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } @@ -50,4 +50,5 @@ export interface WorkspaceFormProps { operationType?: WorkspaceOperationType; permissionEnabled?: boolean; permissionLastAdminItemDeletable?: boolean; + workspaceConfigurableApps?: PublicAppInfo[]; } diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx index e172b851f1f3..34afa3327f4c 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx @@ -20,25 +20,24 @@ import { DEFAULT_APP_CATEGORIES, PublicAppInfo } from '../../../../../core/publi import { WorkspaceFeature, WorkspaceFeatureGroup } from './types'; import { isDefaultCheckedFeatureId, isWorkspaceFeatureGroup } from './utils'; -import { getAllExcludingManagementApps } from '../../utils'; const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', }); -interface WorkspaceFeatureSelectorProps { - applications: PublicAppInfo[]; +export interface WorkspaceFeatureSelectorProps { selectedFeatures: string[]; onChange: (newFeatures: string[]) => void; + workspaceConfigurableApps?: PublicAppInfo[]; } export const WorkspaceFeatureSelector = ({ - applications, selectedFeatures, onChange, + workspaceConfigurableApps, }: WorkspaceFeatureSelectorProps) => { const featureOrGroups = useMemo(() => { - const transformedApplications = applications.map((app) => { + const transformedApplications = (workspaceConfigurableApps || []).map((app) => { if (app.category?.id === DEFAULT_APP_CATEGORIES.opensearchDashboards.id) { return { ...app, @@ -55,7 +54,7 @@ export const WorkspaceFeatureSelector = ({ Array >((previousValue, currentKey) => { const apps = category2Applications[currentKey]; - const features = getAllExcludingManagementApps(apps).map(({ id, title }) => ({ + const features = apps.map(({ id, title }) => ({ id, name: title, })); @@ -73,7 +72,7 @@ export const WorkspaceFeatureSelector = ({ }, ]; }, []); - }, [applications]); + }, [workspaceConfigurableApps]); const handleFeatureChange = useCallback( (featureId) => { diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index b340a71588c9..554a7191ca43 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -39,7 +39,6 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { formData, formErrors, selectedTab, - applications, numberOfErrors, handleFormSubmit, handleColorChange, @@ -154,9 +153,9 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { )} diff --git a/src/plugins/workspace/public/components/workspace_updater/index.tsx b/src/plugins/workspace/public/components/workspace_updater/index.tsx index 711f19fd25f6..b00065a00f64 100644 --- a/src/plugins/workspace/public/components/workspace_updater/index.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/index.tsx @@ -3,4 +3,4 @@ * SPDX-License-Identifier: Apache-2.0 */ -export { WorkspaceUpdater } from './workspace_updater'; +export { WorkspaceUpdater, WorkspaceUpdaterProps } from './workspace_updater'; diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index ff07076d99d1..1c4e426d1701 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -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(); + const { container } = render( + + ); expect(container).toMatchInlineSnapshot(`
`); }); it('cannot create workspace with invalid name', async () => { - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: '~' }, @@ -130,7 +139,11 @@ describe('WorkspaceUpdater', () => { }); it('update workspace successfully', async () => { - const { getByTestId, getByText } = render(); + const { getByTestId, getByText } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, @@ -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(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, @@ -203,7 +220,11 @@ describe('WorkspaceUpdater', () => { workspaceClientUpdate.mockImplementation(() => { throw new Error('update workspace failed'); }); - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const nameInput = getByTestId('workspaceForm-workspaceDetails-nameInputText'); fireEvent.input(nameInput, { target: { value: 'test workspace name' }, diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx index 1f67f2063d9b..554bee7dcd8d 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx @@ -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 { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form'; import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; @@ -19,6 +20,10 @@ import { convertPermissionsToPermissionSettings, } from '../workspace_form/utils'; +export interface WorkspaceUpdaterProps { + workspaceConfigurableApps$?: BehaviorSubject; +} + function getFormDataFromWorkspace( currentWorkspace: WorkspaceAttributeWithPermission | null | undefined ) { @@ -33,7 +38,7 @@ function getFormDataFromWorkspace( }; } -export const WorkspaceUpdater = () => { +export const WorkspaceUpdater = (props: WorkspaceUpdaterProps) => { const { services: { application, workspaces, notifications, http, workspaceClient }, } = useOpenSearchDashboards<{ workspaceClient: WorkspaceClient }>(); @@ -41,6 +46,9 @@ export const WorkspaceUpdater = () => { 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) ); @@ -131,6 +139,7 @@ export const WorkspaceUpdater = () => { operationType={WorkspaceOperationType.Update} permissionEnabled={isPermissionEnabled} permissionLastAdminItemDeletable + workspaceConfigurableApps={workspaceConfigurableApps} /> )} diff --git a/src/plugins/workspace/public/components/workspace_updater_app.tsx b/src/plugins/workspace/public/components/workspace_updater_app.tsx index e16c9ad72e0f..ab106b5c4b7a 100644 --- a/src/plugins/workspace/public/components/workspace_updater_app.tsx +++ b/src/plugins/workspace/public/components/workspace_updater_app.tsx @@ -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(); @@ -29,7 +29,7 @@ export const WorkspaceUpdaterApp = () => { return ( - + ); }; diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 4b43c3438179..576817313c89 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -7,6 +7,7 @@ import { BehaviorSubject, Subscription } from 'rxjs'; import { i18n } from '@osd/i18n'; import { SavedObjectsManagementPluginSetup } from 'src/plugins/saved_objects_management/public'; import { ManagementSetup } from 'src/plugins/management/public'; +import { first } from 'rxjs/operators'; import { AppMountParameters, AppNavLinkStatus, @@ -16,6 +17,7 @@ import { AppUpdater, AppStatus, WorkspaceAccessibility, + PublicAppInfo, } from '../../../core/public'; import { WORKSPACE_FATAL_ERROR_APP_ID, @@ -29,9 +31,13 @@ import { renderWorkspaceMenu } from './render_workspace_menu'; import { Services } from './types'; import { WorkspaceClient } from './workspace_client'; import { getWorkspaceColumn } from './components/workspace_column'; -import { isAppAccessibleInWorkspace } from './utils'; +import { filterWorkspaceConfigurableApps, isAppAccessibleInWorkspace } from './utils'; -type WorkspaceAppType = (params: AppMountParameters, services: Services) => () => void; +type WorkspaceAppType = ( + params: AppMountParameters, + services: Services, + props: Record +) => () => void; interface WorkspacePluginSetupDeps { savedObjectsManagement?: SavedObjectsManagementPluginSetup; @@ -44,6 +50,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { private currentWorkspaceSubscription?: Subscription; private managementCurrentWorkspaceIdSubscription?: Subscription; private appUpdater$ = new BehaviorSubject(() => undefined); + private workspaceConfigurableApps$ = new BehaviorSubject([]); private _changeSavedObjectCurrentWorkspace() { if (this.coreStart) { return this.coreStart.workspaces.currentWorkspaceId$.subscribe((currentWorkspaceId) => { @@ -58,7 +65,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { * Filter nav links by the current workspace, once the current workspace change, the nav links(left nav bar) * should also be updated according to the configured features of the current workspace */ - private filterNavLinks(core: CoreStart) { + private filterNavLinks = (core: CoreStart) => { const currentWorkspace$ = core.workspaces.currentWorkspace$; this.currentWorkspaceSubscription?.unsubscribe(); @@ -68,6 +75,11 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (isAppAccessibleInWorkspace(app, currentWorkspace)) { return; } + + if (app.status === AppStatus.inaccessible) { + return; + } + /** * 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 @@ -76,7 +88,20 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }); } }); - } + }; + + /** + * Initiate an observable with the value of all applications which can be configured by workspace + */ + private setWorkspaceConfigurableApps = async (core: CoreStart) => { + const allApps = await new Promise((resolve) => { + core.application.applications$.pipe(first()).subscribe((apps) => { + resolve([...apps.values()]); + }); + }); + const availableApps = filterWorkspaceConfigurableApps(allApps); + this.workspaceConfigurableApps$.next(availableApps); + }; /** * If workspace is enabled and user has entered workspace, hide advance settings and dataSource menu and disable @@ -157,7 +182,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> { workspaceClient, }; - return renderApp(params, services); + return renderApp(params, services, { + workspaceConfigurableApps$: this.workspaceConfigurableApps$, + }); }; // list @@ -246,8 +273,11 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.currentWorkspaceIdSubscription = this._changeSavedObjectCurrentWorkspace(); - // When starts, filter the nav links based on the current workspace - this.filterNavLinks(core); + this.setWorkspaceConfigurableApps(core).then(() => { + // filter the nav links based on the current workspace + this.filterNavLinks(core); + }); + return {}; } diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index e65d62a58396..a5e8a3f3c8be 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -12,6 +12,7 @@ import { WorkspaceObject, WorkspaceAccessibility, } from '../../../core/public'; +import { DEFAULT_CHECKED_FEATURES_IDS } from '../common/constants'; /** * Checks if a given feature matches the provided feature configuration. @@ -136,3 +137,16 @@ export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) } return false; } + +export const filterWorkspaceConfigurableApps = (applications: PublicAppInfo[]) => { + const visibleApplications = applications.filter(({ navLinkStatus, chromeless, category, id }) => { + return ( + navLinkStatus !== AppNavLinkStatus.hidden && + !chromeless && + !DEFAULT_CHECKED_FEATURES_IDS.includes(id) && + category?.id !== DEFAULT_APP_CATEGORIES.management.id + ); + }); + + return visibleApplications; +};