diff --git a/changelogs/fragments/6427.yml b/changelogs/fragments/6427.yml new file mode 100644 index 000000000000..74066816aad3 --- /dev/null +++ b/changelogs/fragments/6427.yml @@ -0,0 +1,2 @@ +feat: +- [Workspace] Allow making apps available in workspaces using `workspaceAvailability` ([#6427](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6427)) \ No newline at end of file diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index 691ba64cf00a..0796f476149f 100644 --- a/src/core/public/application/application_service.test.ts +++ b/src/core/public/application/application_service.test.ts @@ -44,8 +44,16 @@ import { httpServiceMock } from '../http/http_service.mock'; import { overlayServiceMock } from '../overlays/overlay_service.mock'; import { MockLifecycle } from './test_types'; import { ApplicationService } from './application_service'; -import { App, PublicAppInfo, AppNavLinkStatus, AppStatus, AppUpdater } from './types'; +import { + App, + PublicAppInfo, + AppNavLinkStatus, + AppStatus, + AppUpdater, + WorkspaceAvailability, +} from './types'; import { act } from 'react-dom/test-utils'; +import { workspacesServiceMock } from '../mocks'; const createApp = (props: Partial): App => { return { @@ -68,7 +76,11 @@ describe('#setup()', () => { context: contextServiceMock.createSetupContract(), redirectTo: jest.fn(), }; - startDeps = { http, overlays: overlayServiceMock.createStartContract() }; + startDeps = { + http, + overlays: overlayServiceMock.createStartContract(), + workspaces: workspacesServiceMock.createStartContract(), + }; service = new ApplicationService(); }); @@ -398,7 +410,11 @@ describe('#start()', () => { context: contextServiceMock.createSetupContract(), redirectTo: jest.fn(), }; - startDeps = { http, overlays: overlayServiceMock.createStartContract() }; + startDeps = { + http, + overlays: overlayServiceMock.createStartContract(), + workspaces: workspacesServiceMock.createStartContract(), + }; service = new ApplicationService(); }); @@ -539,6 +555,32 @@ describe('#start()', () => { 'http://localhost/base-path/app/app2/deep/link' ); }); + + it('creates URL when the app is not accessible in a workspace', async () => { + const httpMock = httpServiceMock.createSetupContract({ + basePath: '/base-path', + clientBasePath: '/client-base-path', + }); + const { register } = service.setup({ + ...setupDeps, + http: httpMock, + }); + // register a app that can only be accessed out of a workspace + register( + Symbol(), + createApp({ + id: 'app1', + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, + }) + ); + const { getUrlForApp } = await service.start({ + ...startDeps, + http: httpMock, + }); + + expect(getUrlForApp('app1')).toBe('/base-path/app/app1'); + expect(getUrlForApp('app2')).toBe('/base-path/client-base-path/app/app2'); + }); }); describe('navigateToApp', () => { @@ -766,6 +808,46 @@ describe('#start()', () => { `); }); + it('refresh the page if navigate to a app not accessible within a workspace', async () => { + // Save the original assign method + const originalLocation = window.location; + delete (window as any).location; + + // Mocking the window object + window.location = { + ...originalLocation, + assign: jest.fn(), + }; + + const { register } = service.setup(setupDeps); + // register a app that can only be accessed out of a workspace + register( + Symbol(), + createApp({ + id: 'app1', + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, + }) + ); + const workspaces = workspacesServiceMock.createStartContract(); + workspaces.currentWorkspaceId$.next('foo'); + const http = httpServiceMock.createStartContract({ + basePath: '/base-path', + clientBasePath: '/client-base-path', + }); + const { navigateToApp } = await service.start({ + ...startDeps, + workspaces, + http, + }); + await navigateToApp('app1'); + + expect(window.location.assign).toBeCalledWith('/base-path/app/app1'); + await navigateToApp('app2'); + // assign should not be called + expect(window.location.assign).toBeCalledTimes(1); + window.location = originalLocation; + }); + describe('when `replace` option is true', () => { it('use `history.replace` instead of `history.push`', async () => { service.setup(setupDeps); @@ -869,7 +951,11 @@ describe('#stop()', () => { http, context: contextServiceMock.createSetupContract(), }; - startDeps = { http, overlays: overlayServiceMock.createStartContract() }; + startDeps = { + http, + overlays: overlayServiceMock.createStartContract(), + workspaces: workspacesServiceMock.createStartContract(), + }; service = new ApplicationService(); }); diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index 62c13694e245..76747490a305 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -54,9 +54,11 @@ import { InternalApplicationStart, Mounter, NavigateToAppOptions, + WorkspaceAvailability, } from './types'; import { getLeaveAction, isConfirmAction } from './application_leave'; import { appendAppPath, parseAppUrl, relativeToAbsolute, getAppInfo } from './utils'; +import { WorkspacesStart } from '../workspace'; interface SetupDeps { context: ContextSetup; @@ -69,6 +71,7 @@ interface SetupDeps { interface StartDeps { http: HttpStart; overlays: OverlayStart; + workspaces: WorkspacesStart; } // Mount functions with two arguments are assumed to expect deprecated `context` object. @@ -213,7 +216,7 @@ export class ApplicationService { }; } - public async start({ http, overlays }: StartDeps): Promise { + public async start({ http, overlays, workspaces }: StartDeps): Promise { if (!this.mountContext) { throw new Error('ApplicationService#setup() must be invoked before start.'); } @@ -259,6 +262,22 @@ export class ApplicationService { const shouldNavigate = navigatingToSameApp ? true : await this.shouldNavigate(overlays); if (shouldNavigate) { + const targetApp = applications$.value.get(appId); + if ( + workspaces.currentWorkspaceId$.value && + targetApp?.workspaceAvailability === WorkspaceAvailability.outsideWorkspace + ) { + // If user is inside a workspace and the target app is not available within a workspace + // refresh the page by doing a hard navigation + window.location.assign( + http.basePath.prepend(getAppUrl(availableMounters, appId, path), { + // Set withoutClientBasePath to true remove the workspace path prefix + withoutClientBasePath: true, + }) + ); + return; + } + if (path === undefined) { path = applications$.value.get(appId)?.defaultPath; } @@ -293,7 +312,11 @@ export class ApplicationService { appId, { path, absolute = false }: { path?: string; absolute?: boolean } = {} ) => { - const relUrl = http.basePath.prepend(getAppUrl(availableMounters, appId, path)); + const targetApp = applications$.value.get(appId); + const relUrl = http.basePath.prepend(getAppUrl(availableMounters, appId, path), { + withoutClientBasePath: + targetApp?.workspaceAvailability === WorkspaceAvailability.outsideWorkspace, + }); return absolute ? relativeToAbsolute(relUrl) : relUrl; }, navigateToApp, diff --git a/src/core/public/application/index.ts b/src/core/public/application/index.ts index b1a9f47b72e0..3f1edab0218c 100644 --- a/src/core/public/application/index.ts +++ b/src/core/public/application/index.ts @@ -54,4 +54,5 @@ export { // Internal types InternalApplicationSetup, InternalApplicationStart, + WorkspaceAvailability, } from './types'; diff --git a/src/core/public/application/integration_tests/application_service.test.tsx b/src/core/public/application/integration_tests/application_service.test.tsx index 9d53d99c9d8c..a463dec892f3 100644 --- a/src/core/public/application/integration_tests/application_service.test.tsx +++ b/src/core/public/application/integration_tests/application_service.test.tsx @@ -41,6 +41,7 @@ import { overlayServiceMock } from '../../overlays/overlay_service.mock'; import { AppMountParameters } from '../types'; import { Observable } from 'rxjs'; import { MountPoint } from 'opensearch-dashboards/public'; +import { workspacesServiceMock } from '../../mocks'; const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); @@ -67,7 +68,11 @@ describe('ApplicationService', () => { context: contextServiceMock.createSetupContract(), history: history as any, }; - startDeps = { http, overlays: overlayServiceMock.createStartContract() }; + startDeps = { + http, + overlays: overlayServiceMock.createStartContract(), + workspaces: workspacesServiceMock.createStartContract(), + }; service = new ApplicationService(); }); diff --git a/src/core/public/application/types.ts b/src/core/public/application/types.ts index 4744ab34cfd3..71c67f2b967d 100644 --- a/src/core/public/application/types.ts +++ b/src/core/public/application/types.ts @@ -103,6 +103,22 @@ export type AppUpdatableFields = Pick Partial | undefined; +/** + * Visibilities of the application based on if user is within a workspace + * + * @public + */ +export enum WorkspaceAvailability { + /** + * The application is not accessible when user is in a workspace. + */ + outsideWorkspace = 2, + /** + * The application is only accessible when user is in a workspace. + */ + insideWorkspace = 1, +} + /** * @public */ @@ -245,6 +261,13 @@ export interface App { * ``` */ exactRoute?: boolean; + + /** + * Declare if page is available when inside a workspace. + * Defaults to WorkspaceAvailability.outsideWorkspace | WorkspaceAvailability.insideWorkspace, + * indicating the application is available within or out of workspace. + */ + workspaceAvailability?: WorkspaceAvailability; } /** diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index a6f1f6ab317e..2660b4768839 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -268,7 +268,7 @@ export class ChromeService { forceAppSwitcherNavigation$={navLinks.getForceAppSwitcherNavigation$()} helpExtension$={helpExtension$.pipe(takeUntil(this.stop$))} helpSupportUrl$={helpSupportUrl$.pipe(takeUntil(this.stop$))} - homeHref={http.basePath.prepend('/app/home')} + homeHref={application.getUrlForApp('home')} isVisible$={this.isVisible$} opensearchDashboardsVersion={injectedMetadata.getOpenSearchDashboardsVersion()} navLinks$={navLinks.getNavLinks$()} diff --git a/src/core/public/chrome/nav_links/to_nav_link.test.ts b/src/core/public/chrome/nav_links/to_nav_link.test.ts index 1fe2532b7d8f..8d88c15c85ae 100644 --- a/src/core/public/chrome/nav_links/to_nav_link.test.ts +++ b/src/core/public/chrome/nav_links/to_nav_link.test.ts @@ -28,7 +28,12 @@ * under the License. */ -import { PublicAppInfo, AppNavLinkStatus, AppStatus } from '../../application'; +import { + PublicAppInfo, + AppNavLinkStatus, + AppStatus, + WorkspaceAvailability, +} from '../../application'; import { toNavLink } from './to_nav_link'; import { httpServiceMock } from '../../mocks'; @@ -174,4 +179,55 @@ describe('toNavLink', () => { }) ); }); + + it('uses the workspaceAvailability of the application to construct the url', () => { + const httpMock = httpServiceMock.createStartContract({ + basePath: '/base_path', + clientBasePath: '/client_base_path', + }); + expect( + toNavLink( + app({ + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, + }), + httpMock.basePath + ).properties + ).toEqual( + expect.objectContaining({ + url: 'http://localhost/base_path/app/some-id', + baseUrl: 'http://localhost/base_path/app/some-id', + }) + ); + + // When app is accessible inside workspace or outside workspace. + expect( + toNavLink( + app({ + workspaceAvailability: WorkspaceAvailability.insideWorkspace, + }), + httpMock.basePath + ).properties + ).toEqual( + expect.objectContaining({ + url: 'http://localhost/base_path/client_base_path/app/some-id', + baseUrl: 'http://localhost/base_path/client_base_path/app/some-id', + }) + ); + + expect( + toNavLink( + app({ + workspaceAvailability: + // eslint-disable-next-line no-bitwise + WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, + }), + httpMock.basePath + ).properties + ).toEqual( + expect.objectContaining({ + url: 'http://localhost/base_path/client_base_path/app/some-id', + baseUrl: 'http://localhost/base_path/client_base_path/app/some-id', + }) + ); + }); }); diff --git a/src/core/public/chrome/nav_links/to_nav_link.ts b/src/core/public/chrome/nav_links/to_nav_link.ts index 734bb114d73d..79871e380ae1 100644 --- a/src/core/public/chrome/nav_links/to_nav_link.ts +++ b/src/core/public/chrome/nav_links/to_nav_link.ts @@ -28,14 +28,22 @@ * under the License. */ -import { PublicAppInfo, AppNavLinkStatus, AppStatus } from '../../application'; +import { + PublicAppInfo, + AppNavLinkStatus, + AppStatus, + WorkspaceAvailability, +} from '../../application'; import { IBasePath } from '../../http'; import { NavLinkWrapper } from './nav_link'; import { appendAppPath } from '../../application/utils'; export function toNavLink(app: PublicAppInfo, basePath: IBasePath): NavLinkWrapper { const useAppStatus = app.navLinkStatus === AppNavLinkStatus.default; - const relativeBaseUrl = basePath.prepend(app.appRoute!); + let relativeBaseUrl = basePath.prepend(app.appRoute!); + if (app.workspaceAvailability === WorkspaceAvailability.outsideWorkspace) { + relativeBaseUrl = basePath.prepend(app.appRoute!, { withoutClientBasePath: true }); + } const url = relativeToAbsolute(appendAppPath(relativeBaseUrl, app.defaultPath)); const baseUrl = relativeToAbsolute(relativeBaseUrl); diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 5fc9b62cd47c..58e92a1fa355 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -226,8 +226,8 @@ export class CoreSystem { overlays, targetDomElement: notificationsTargetDomElement, }); - const application = await this.application.start({ http, overlays }); const workspaces = this.workspaces.start(); + const application = await this.application.start({ http, overlays, workspaces }); const chrome = await this.chrome.start({ application, docLinks, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 849c954c1262..2107ab668f3e 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -134,6 +134,7 @@ export { AppUpdater, ScopedHistory, NavigateToAppOptions, + WorkspaceAvailability, } from './application'; export { diff --git a/src/plugins/home/public/plugin.ts b/src/plugins/home/public/plugin.ts index 75b39dadd8c0..5749b6fa3882 100644 --- a/src/plugins/home/public/plugin.ts +++ b/src/plugins/home/public/plugin.ts @@ -56,7 +56,7 @@ import { DataPublicPluginStart } from '../../data/public'; import { TelemetryPluginStart } from '../../telemetry/public'; import { UsageCollectionSetup } from '../../usage_collection/public'; import { UrlForwardingSetup, UrlForwardingStart } from '../../url_forwarding/public'; -import { AppNavLinkStatus } from '../../../core/public'; +import { AppNavLinkStatus, WorkspaceAvailability } from '../../../core/public'; import { PLUGIN_ID, HOME_APP_BASE_PATH } from '../common/constants'; import { DataSourcePluginStart } from '../../data_source/public'; import { workWithDataSection } from './application/components/homepage/sections/work_with_data'; @@ -135,6 +135,7 @@ export class HomePublicPlugin const { renderApp } = await import('./application'); return await renderApp(params.element, coreStart, params.history); }, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); urlForwarding.forwardApp('home', 'home'); diff --git a/src/plugins/opensearch_dashboards_overview/public/plugin.ts b/src/plugins/opensearch_dashboards_overview/public/plugin.ts index e38282ff06d6..3af37fc8cfbb 100644 --- a/src/plugins/opensearch_dashboards_overview/public/plugin.ts +++ b/src/plugins/opensearch_dashboards_overview/public/plugin.ts @@ -40,6 +40,7 @@ import { AppStatus, AppNavLinkStatus, Branding, + WorkspaceAvailability, } from '../../../core/public'; import { OpenSearchDashboardsOverviewPluginSetup, @@ -106,6 +107,7 @@ export class OpenSearchDashboardsOverviewPlugin // Render the application return renderApp(coreStart, depsStart as AppPluginStartDependencies, params); }, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); if (home) { diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx index c63b232bb232..31682b6f649a 100644 --- a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx +++ b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.test.tsx @@ -93,7 +93,7 @@ describe('', () => { render(); fireEvent.click(screen.getByText(/select a workspace/i)); fireEvent.click(screen.getByText(/create workspace/i)); - expect(window.location.assign).toHaveBeenCalledWith('https://test.com/app/workspace_create'); + expect(coreStartMock.application.navigateToApp).toHaveBeenCalledWith('workspace_create'); Object.defineProperty(window, 'location', { value: originalLocation, @@ -111,7 +111,7 @@ describe('', () => { render(); fireEvent.click(screen.getByText(/select a workspace/i)); fireEvent.click(screen.getByText(/all workspace/i)); - expect(window.location.assign).toHaveBeenCalledWith('https://test.com/app/workspace_list'); + expect(coreStartMock.application.navigateToApp).toHaveBeenCalledWith('workspace_list'); Object.defineProperty(window, 'location', { value: originalLocation, diff --git a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx index 5b16b9766b22..7262bd8bfdfb 100644 --- a/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx +++ b/src/plugins/workspace/public/components/workspace_menu/workspace_menu.tsx @@ -104,13 +104,7 @@ export const WorkspaceMenu = ({ coreStart }: Props) => { }), key: WORKSPACE_CREATE_APP_ID, onClick: () => { - window.location.assign( - cleanWorkspaceId( - coreStart.application.getUrlForApp(WORKSPACE_CREATE_APP_ID, { - absolute: false, - }) - ) - ); + coreStart.application.navigateToApp(WORKSPACE_CREATE_APP_ID); }, }); workspaceListItems.push({ @@ -120,13 +114,7 @@ export const WorkspaceMenu = ({ coreStart }: Props) => { }), key: WORKSPACE_LIST_APP_ID, onClick: () => { - window.location.assign( - cleanWorkspaceId( - coreStart.application.getUrlForApp(WORKSPACE_LIST_APP_ID, { - absolute: false, - }) - ) - ); + coreStart.application.navigateToApp(WORKSPACE_LIST_APP_ID); }, }); return workspaceListItems; diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 90924248606b..04948c5d580c 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -16,6 +16,7 @@ import { AppUpdater, AppStatus, PublicAppInfo, + WorkspaceAvailability, } from '../../../core/public'; import { WORKSPACE_FATAL_ERROR_APP_ID, @@ -199,6 +200,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderCreatorApp } = await import('./application'); return mountWorkspaceApp(params, renderCreatorApp); }, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); // update @@ -244,6 +246,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderListApp } = await import('./application'); return mountWorkspaceApp(params, renderListApp); }, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); /** diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index ae622a170254..5471abace56d 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -9,6 +9,7 @@ import { filterWorkspaceConfigurableApps, isAppAccessibleInWorkspace, } from './utils'; +import { WorkspaceAvailability } from '../../../core/public'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -152,6 +153,48 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ) ).toBe(true); }); + + it('An app is not accessible within a workspace if its workspaceAvailability is outsideWorkspace', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'home', + title: 'Any app', + mount: jest.fn(), + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, + }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(false); + }); + it('An app is accessible within a workspace if its workspaceAvailability is insideWorkspace', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'home', + title: 'Any app', + mount: jest.fn(), + workspaceAvailability: WorkspaceAvailability.insideWorkspace, + }, + { id: 'workspace_id', name: 'workspace name', features: ['home'] } + ) + ).toBe(true); + }); + it('An app is accessible within a workspace if its workspaceAvailability is inside and outsideWorkspace', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'home', + title: 'Any app', + mount: jest.fn(), + workspaceAvailability: + // eslint-disable-next-line no-bitwise + WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, + }, + { id: 'workspace_id', name: 'workspace name', features: ['home'] } + ) + ).toBe(true); + }); }); describe('workspace utils: filterWorkspaceConfigurableApps', () => { diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 54f5fbad5912..c39c1e6d5958 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -10,6 +10,7 @@ import { DEFAULT_APP_CATEGORIES, PublicAppInfo, WorkspaceObject, + WorkspaceAvailability, } from '../../../core/public'; import { DEFAULT_SELECTED_FEATURES_IDS } from '../common/constants'; @@ -73,6 +74,13 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ * Check if an app is accessible in a workspace based on the workspace configured features */ export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) { + /** + * App is not accessible within workspace if it explicitly declare itself as WorkspaceAvailability.outsideWorkspace + */ + if (app.workspaceAvailability === WorkspaceAvailability.outsideWorkspace) { + return false; + } + /** * When workspace has no features configured, all apps are considered to be accessible */