From 0d66e03fec1030ca89bd69a8c74ca47e2c173d48 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 12 Apr 2024 16:20:32 +0800 Subject: [PATCH 01/15] [Workspace] Jump to non-workspace url when clicking home icon (#316) * temp: save Signed-off-by: SuZhou-Joe * feat: complete the feature Signed-off-by: SuZhou-Joe * feat: remove useless code Signed-off-by: SuZhou-Joe * fix: bootstrap error Signed-off-by: SuZhou-Joe * fix: bootstrap error Signed-off-by: SuZhou-Joe * fix: page not found error Signed-off-by: SuZhou-Joe * fix: anchor href Signed-off-by: SuZhou-Joe * feat: update toNavLink to comply with workspace Signed-off-by: SuZhou-Joe * feat: change to workspaceless Signed-off-by: SuZhou-Joe * feat: change to workspaceless Signed-off-by: SuZhou-Joe * feat: change to workspaceless Signed-off-by: SuZhou-Joe * feat: register list and create page as workspaceless Signed-off-by: SuZhou-Joe * feat: optimize code Signed-off-by: SuZhou-Joe * feat: update to WorkspaceVisibility Signed-off-by: SuZhou-Joe * feat: add unit test Signed-off-by: SuZhou-Joe * feat: optimize the jump logic Signed-off-by: SuZhou-Joe * fix: unit test Signed-off-by: SuZhou-Joe * feat: make app inaccessible if workspaceAccessibility is No Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- .../application/application_service.test.ts | 94 ++++++++++++++++++- .../application/application_service.tsx | 26 ++++- src/core/public/application/index.ts | 1 + .../application_service.test.tsx | 7 +- src/core/public/application/types.ts | 22 +++++ src/core/public/chrome/chrome_service.tsx | 2 +- .../chrome/nav_links/to_nav_link.test.ts | 41 +++++++- .../public/chrome/nav_links/to_nav_link.ts | 12 ++- src/core/public/core_system.ts | 2 +- src/core/public/index.ts | 1 + src/plugins/home/public/plugin.ts | 3 +- .../public/plugin.ts | 2 + .../workspace_menu/workspace_menu.test.tsx | 4 +- .../workspace_menu/workspace_menu.tsx | 16 +--- src/plugins/workspace/public/plugin.ts | 3 + src/plugins/workspace/public/utils.test.ts | 15 +++ src/plugins/workspace/public/utils.ts | 8 ++ 17 files changed, 230 insertions(+), 29 deletions(-) diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index 691ba64cf00a..ca15c76377d4 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, + WorkspaceAccessibility, +} 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', + workspaceAccessibility: WorkspaceAccessibility.NO, + }) + ); + 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('navigate by using window.location.assign 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', + workspaceAccessibility: WorkspaceAccessibility.NO, + }) + ); + 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..3dca2cfd46a1 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, + WorkspaceAccessibility, } 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?.workspaceAccessibility === WorkspaceAccessibility.NO + ) { + // If user is inside a workspace and the target app is not accessible 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,10 @@ 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?.workspaceAccessibility === WorkspaceAccessibility.NO, + }); return absolute ? relativeToAbsolute(relUrl) : relUrl; }, navigateToApp, diff --git a/src/core/public/application/index.ts b/src/core/public/application/index.ts index b1a9f47b72e0..790c6ac9240c 100644 --- a/src/core/public/application/index.ts +++ b/src/core/public/application/index.ts @@ -54,4 +54,5 @@ export { // Internal types InternalApplicationSetup, InternalApplicationStart, + WorkspaceAccessibility, } 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..977489fda37a 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 WorkspaceAccessibility { + /** + * The application is not accessible when user is in a workspace. + */ + NO = 0, + /** + * The application is only accessible when user is in a workspace. + */ + YES = 1, +} + /** * @public */ @@ -245,6 +261,12 @@ export interface App { * ``` */ exactRoute?: boolean; + + /** + * Declare if page is accessible when inside a workspace. + * Defaults to undefined to indicate the application can be accessible within or out of workspace. + */ + workspaceAccessibility?: WorkspaceAccessibility; } /** 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..85ff753a40d9 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, + WorkspaceAccessibility, +} from '../../application'; import { toNavLink } from './to_nav_link'; import { httpServiceMock } from '../../mocks'; @@ -174,4 +179,38 @@ describe('toNavLink', () => { }) ); }); + + it('uses the workspaceVisibility of the application to construct the url', () => { + const httpMock = httpServiceMock.createStartContract({ + basePath: '/base_path', + clientBasePath: '/client_base_path', + }); + expect( + toNavLink( + app({ + workspaceAccessibility: WorkspaceAccessibility.NO, + }), + httpMock.basePath + ).properties + ).toEqual( + expect.objectContaining({ + url: 'http://localhost/base_path/app/some-id', + baseUrl: 'http://localhost/base_path/app/some-id', + }) + ); + + expect( + toNavLink( + app({ + workspaceAccessibility: WorkspaceAccessibility.YES, + }), + 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..3b03b6d9278e 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, + WorkspaceAccessibility, +} 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.workspaceAccessibility === WorkspaceAccessibility.NO) { + 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..f0d3140d2918 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -134,6 +134,7 @@ export { AppUpdater, ScopedHistory, NavigateToAppOptions, + WorkspaceAccessibility, } from './application'; export { diff --git a/src/plugins/home/public/plugin.ts b/src/plugins/home/public/plugin.ts index 75b39dadd8c0..8186ee0f1da0 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, WorkspaceAccessibility } 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); }, + workspaceAccessibility: WorkspaceAccessibility.NO, }); 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..69b0e9616452 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, + WorkspaceAccessibility, } from '../../../core/public'; import { OpenSearchDashboardsOverviewPluginSetup, @@ -106,6 +107,7 @@ export class OpenSearchDashboardsOverviewPlugin // Render the application return renderApp(coreStart, depsStart as AppPluginStartDependencies, params); }, + workspaceAccessibility: WorkspaceAccessibility.NO, }); 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 abd5fdadbabc..088218ddc269 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -16,6 +16,7 @@ import { AppUpdater, AppStatus, PublicAppInfo, + WorkspaceAccessibility, } 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); }, + workspaceAccessibility: WorkspaceAccessibility.NO, }); // update @@ -244,6 +246,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderListApp } = await import('./application'); return mountWorkspaceApp(params, renderListApp); }, + workspaceAccessibility: WorkspaceAccessibility.NO, }); /** diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index ae622a170254..7e8b2e030214 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 { WorkspaceAccessibility } from '../../../core/public'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -152,6 +153,20 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ) ).toBe(true); }); + + it('An app is not accessible if its workspaceAccessibility is no', () => { + expect( + isAppAccessibleInWorkspace( + { + id: 'home', + title: 'Any app', + mount: jest.fn(), + workspaceAccessibility: WorkspaceAccessibility.NO, + }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(false); + }); }); describe('workspace utils: filterWorkspaceConfigurableApps', () => { diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 54f5fbad5912..5b7f7d305855 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, + WorkspaceAccessibility, } 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 workspaceAccessibility.No + */ + if (app.workspaceAccessibility === WorkspaceAccessibility.NO) { + return false; + } + /** * When workspace has no features configured, all apps are considered to be accessible */ From 397fb713ed867f33ada8957a02fff2ba6b72af7f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 17:22:26 +0800 Subject: [PATCH 02/15] refactor: using WorkspaceAvailability Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 1 + .../application/application_service.test.ts | 6 +++--- .../public/application/application_service.tsx | 9 +++++---- src/core/public/application/index.ts | 2 +- src/core/public/application/types.ts | 16 ++++++++++------ .../public/chrome/nav_links/to_nav_link.test.ts | 6 +++--- src/core/public/chrome/nav_links/to_nav_link.ts | 4 ++-- src/core/public/index.ts | 2 +- src/plugins/home/public/plugin.ts | 4 ++-- .../public/plugin.ts | 4 ++-- src/plugins/workspace/public/plugin.ts | 6 +++--- src/plugins/workspace/public/utils.test.ts | 6 +++--- src/plugins/workspace/public/utils.ts | 6 +++--- 13 files changed, 39 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b9a0b11f941..207fc97aa568 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Workspace] Hide datasource and advanced settings menu in dashboard management when in workspace. ([#6455](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6455)) - [Multiple Datasource] Modify selectable picker to remove group label and close popover after selection ([#6515](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6515)) - [Workspace] Add workspaces filter to saved objects page. ([#6458](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6458)) +- [Workspace] Allow making apps available in workspaces using `workspaceAvailability` ([#6427](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6427)) ### 🐛 Bug Fixes diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index ca15c76377d4..58dd5b7e6435 100644 --- a/src/core/public/application/application_service.test.ts +++ b/src/core/public/application/application_service.test.ts @@ -50,7 +50,7 @@ import { AppNavLinkStatus, AppStatus, AppUpdater, - WorkspaceAccessibility, + WorkspaceAvailability, } from './types'; import { act } from 'react-dom/test-utils'; import { workspacesServiceMock } from '../mocks'; @@ -570,7 +570,7 @@ describe('#start()', () => { Symbol(), createApp({ id: 'app1', - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }) ); const { getUrlForApp } = await service.start({ @@ -825,7 +825,7 @@ describe('#start()', () => { Symbol(), createApp({ id: 'app1', - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }) ); const workspaces = workspacesServiceMock.createStartContract(); diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index 3dca2cfd46a1..5b791faaeaf0 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -54,7 +54,7 @@ import { InternalApplicationStart, Mounter, NavigateToAppOptions, - WorkspaceAccessibility, + WorkspaceAvailability, } from './types'; import { getLeaveAction, isConfirmAction } from './application_leave'; import { appendAppPath, parseAppUrl, relativeToAbsolute, getAppInfo } from './utils'; @@ -265,9 +265,9 @@ export class ApplicationService { const targetApp = applications$.value.get(appId); if ( workspaces.currentWorkspaceId$.value && - targetApp?.workspaceAccessibility === WorkspaceAccessibility.NO + targetApp?.workspaceAvailability === WorkspaceAvailability.outOfWorkspace ) { - // If user is inside a workspace and the target app is not accessible within a workspace + // 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), { @@ -314,7 +314,8 @@ export class ApplicationService { ) => { const targetApp = applications$.value.get(appId); const relUrl = http.basePath.prepend(getAppUrl(availableMounters, appId, path), { - withoutClientBasePath: targetApp?.workspaceAccessibility === WorkspaceAccessibility.NO, + withoutClientBasePath: + targetApp?.workspaceAvailability === WorkspaceAvailability.outOfWorkspace, }); return absolute ? relativeToAbsolute(relUrl) : relUrl; }, diff --git a/src/core/public/application/index.ts b/src/core/public/application/index.ts index 790c6ac9240c..3f1edab0218c 100644 --- a/src/core/public/application/index.ts +++ b/src/core/public/application/index.ts @@ -54,5 +54,5 @@ export { // Internal types InternalApplicationSetup, InternalApplicationStart, - WorkspaceAccessibility, + WorkspaceAvailability, } from './types'; diff --git a/src/core/public/application/types.ts b/src/core/public/application/types.ts index 977489fda37a..fc69855e13a5 100644 --- a/src/core/public/application/types.ts +++ b/src/core/public/application/types.ts @@ -108,15 +108,19 @@ export type AppUpdater = (app: App) => Partial | undefined; * * @public */ -export enum WorkspaceAccessibility { +export enum WorkspaceAvailability { /** * The application is not accessible when user is in a workspace. */ - NO = 0, + outOfWorkspace = 2, /** * The application is only accessible when user is in a workspace. */ - YES = 1, + inWorkspace = 1, + /** + * The application is accessible when user is in or not in a workspace. + */ + both = 0, } /** @@ -263,10 +267,10 @@ export interface App { exactRoute?: boolean; /** - * Declare if page is accessible when inside a workspace. - * Defaults to undefined to indicate the application can be accessible within or out of workspace. + * Declare if page is available when inside a workspace. + * Defaults to WorkspaceAvailability.both to indicate the application is available within or out of workspace. */ - workspaceAccessibility?: WorkspaceAccessibility; + workspaceAvailability?: WorkspaceAvailability; } /** 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 85ff753a40d9..9f2ea229cbef 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 @@ -32,7 +32,7 @@ import { PublicAppInfo, AppNavLinkStatus, AppStatus, - WorkspaceAccessibility, + WorkspaceAvailability, } from '../../application'; import { toNavLink } from './to_nav_link'; @@ -188,7 +188,7 @@ describe('toNavLink', () => { expect( toNavLink( app({ - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }), httpMock.basePath ).properties @@ -202,7 +202,7 @@ describe('toNavLink', () => { expect( toNavLink( app({ - workspaceAccessibility: WorkspaceAccessibility.YES, + workspaceAvailability: WorkspaceAvailability.inWorkspace, }), httpMock.basePath ).properties 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 3b03b6d9278e..d24379ab147f 100644 --- a/src/core/public/chrome/nav_links/to_nav_link.ts +++ b/src/core/public/chrome/nav_links/to_nav_link.ts @@ -32,7 +32,7 @@ import { PublicAppInfo, AppNavLinkStatus, AppStatus, - WorkspaceAccessibility, + WorkspaceAvailability, } from '../../application'; import { IBasePath } from '../../http'; import { NavLinkWrapper } from './nav_link'; @@ -41,7 +41,7 @@ import { appendAppPath } from '../../application/utils'; export function toNavLink(app: PublicAppInfo, basePath: IBasePath): NavLinkWrapper { const useAppStatus = app.navLinkStatus === AppNavLinkStatus.default; let relativeBaseUrl = basePath.prepend(app.appRoute!); - if (app.workspaceAccessibility === WorkspaceAccessibility.NO) { + if (app.workspaceAvailability === WorkspaceAvailability.outOfWorkspace) { relativeBaseUrl = basePath.prepend(app.appRoute!, { withoutClientBasePath: true }); } const url = relativeToAbsolute(appendAppPath(relativeBaseUrl, app.defaultPath)); diff --git a/src/core/public/index.ts b/src/core/public/index.ts index f0d3140d2918..2107ab668f3e 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -134,7 +134,7 @@ export { AppUpdater, ScopedHistory, NavigateToAppOptions, - WorkspaceAccessibility, + WorkspaceAvailability, } from './application'; export { diff --git a/src/plugins/home/public/plugin.ts b/src/plugins/home/public/plugin.ts index 8186ee0f1da0..a644a432c13a 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, WorkspaceAccessibility } 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,7 +135,7 @@ export class HomePublicPlugin const { renderApp } = await import('./application'); return await renderApp(params.element, coreStart, params.history); }, - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }); 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 69b0e9616452..a6f23c9a5ade 100644 --- a/src/plugins/opensearch_dashboards_overview/public/plugin.ts +++ b/src/plugins/opensearch_dashboards_overview/public/plugin.ts @@ -40,7 +40,7 @@ import { AppStatus, AppNavLinkStatus, Branding, - WorkspaceAccessibility, + WorkspaceAvailability, } from '../../../core/public'; import { OpenSearchDashboardsOverviewPluginSetup, @@ -107,7 +107,7 @@ export class OpenSearchDashboardsOverviewPlugin // Render the application return renderApp(coreStart, depsStart as AppPluginStartDependencies, params); }, - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }); if (home) { diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 088218ddc269..df2da8cbe8eb 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -16,7 +16,7 @@ import { AppUpdater, AppStatus, PublicAppInfo, - WorkspaceAccessibility, + WorkspaceAvailability, } from '../../../core/public'; import { WORKSPACE_FATAL_ERROR_APP_ID, @@ -200,7 +200,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderCreatorApp } = await import('./application'); return mountWorkspaceApp(params, renderCreatorApp); }, - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }); // update @@ -246,7 +246,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderListApp } = await import('./application'); return mountWorkspaceApp(params, renderListApp); }, - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }); /** diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 7e8b2e030214..5514849ec4ac 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -9,7 +9,7 @@ import { filterWorkspaceConfigurableApps, isAppAccessibleInWorkspace, } from './utils'; -import { WorkspaceAccessibility } from '../../../core/public'; +import { WorkspaceAvailability } from '../../../core/public'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -154,14 +154,14 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ).toBe(true); }); - it('An app is not accessible if its workspaceAccessibility is no', () => { + it('An app is not accessible if its workspaceAvailability is outOfWorkspace', () => { expect( isAppAccessibleInWorkspace( { id: 'home', title: 'Any app', mount: jest.fn(), - workspaceAccessibility: WorkspaceAccessibility.NO, + workspaceAvailability: WorkspaceAvailability.outOfWorkspace, }, { id: 'workspace_id', name: 'workspace name', features: [] } ) diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 5b7f7d305855..7423678eaf12 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -10,7 +10,7 @@ import { DEFAULT_APP_CATEGORIES, PublicAppInfo, WorkspaceObject, - WorkspaceAccessibility, + WorkspaceAvailability, } from '../../../core/public'; import { DEFAULT_SELECTED_FEATURES_IDS } from '../common/constants'; @@ -75,9 +75,9 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ */ export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) { /** - * App is not accessible within workspace if it explicitly declare itself as workspaceAccessibility.No + * App is not accessible within workspace if it explicitly declare itself as WorkspaceAvailability.outOfWorkspace */ - if (app.workspaceAccessibility === WorkspaceAccessibility.NO) { + if (app.workspaceAvailability === WorkspaceAvailability.outOfWorkspace) { return false; } From 857af9f719b1b9cef853477433161273bf66342f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 15 Apr 2024 17:23:15 +0800 Subject: [PATCH 03/15] feat: change test name Signed-off-by: SuZhou-Joe --- src/core/public/chrome/nav_links/to_nav_link.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9f2ea229cbef..6bac7141efc3 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 @@ -180,7 +180,7 @@ describe('toNavLink', () => { ); }); - it('uses the workspaceVisibility of the application to construct the url', () => { + it('uses the workspaceAvailability of the application to construct the url', () => { const httpMock = httpServiceMock.createStartContract({ basePath: '/base_path', clientBasePath: '/client_base_path', From dc07eacb47b5f1325f758572811938f1280eaf0c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 13:22:44 +0800 Subject: [PATCH 04/15] feat: update wording Signed-off-by: SuZhou-Joe --- .../public/application/application_service.test.ts | 4 ++-- src/core/public/application/application_service.tsx | 4 ++-- src/core/public/application/types.ts | 11 ++++------- src/core/public/chrome/nav_links/to_nav_link.test.ts | 4 ++-- src/core/public/chrome/nav_links/to_nav_link.ts | 2 +- src/plugins/home/public/plugin.ts | 2 +- .../opensearch_dashboards_overview/public/plugin.ts | 2 +- src/plugins/workspace/public/plugin.ts | 4 ++-- src/plugins/workspace/public/utils.test.ts | 2 +- src/plugins/workspace/public/utils.ts | 2 +- 10 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index 58dd5b7e6435..6d448ac7995e 100644 --- a/src/core/public/application/application_service.test.ts +++ b/src/core/public/application/application_service.test.ts @@ -570,7 +570,7 @@ describe('#start()', () => { Symbol(), createApp({ id: 'app1', - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }) ); const { getUrlForApp } = await service.start({ @@ -825,7 +825,7 @@ describe('#start()', () => { Symbol(), createApp({ id: 'app1', - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }) ); const workspaces = workspacesServiceMock.createStartContract(); diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index 5b791faaeaf0..76747490a305 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -265,7 +265,7 @@ export class ApplicationService { const targetApp = applications$.value.get(appId); if ( workspaces.currentWorkspaceId$.value && - targetApp?.workspaceAvailability === WorkspaceAvailability.outOfWorkspace + 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 @@ -315,7 +315,7 @@ export class ApplicationService { const targetApp = applications$.value.get(appId); const relUrl = http.basePath.prepend(getAppUrl(availableMounters, appId, path), { withoutClientBasePath: - targetApp?.workspaceAvailability === WorkspaceAvailability.outOfWorkspace, + targetApp?.workspaceAvailability === WorkspaceAvailability.outsideWorkspace, }); return absolute ? relativeToAbsolute(relUrl) : relUrl; }, diff --git a/src/core/public/application/types.ts b/src/core/public/application/types.ts index fc69855e13a5..71c67f2b967d 100644 --- a/src/core/public/application/types.ts +++ b/src/core/public/application/types.ts @@ -112,15 +112,11 @@ export enum WorkspaceAvailability { /** * The application is not accessible when user is in a workspace. */ - outOfWorkspace = 2, + outsideWorkspace = 2, /** * The application is only accessible when user is in a workspace. */ - inWorkspace = 1, - /** - * The application is accessible when user is in or not in a workspace. - */ - both = 0, + insideWorkspace = 1, } /** @@ -268,7 +264,8 @@ export interface App { /** * Declare if page is available when inside a workspace. - * Defaults to WorkspaceAvailability.both to indicate the application is available within or out of 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/nav_links/to_nav_link.test.ts b/src/core/public/chrome/nav_links/to_nav_link.test.ts index 6bac7141efc3..bfac3a5754cd 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 @@ -188,7 +188,7 @@ describe('toNavLink', () => { expect( toNavLink( app({ - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }), httpMock.basePath ).properties @@ -202,7 +202,7 @@ describe('toNavLink', () => { expect( toNavLink( app({ - workspaceAvailability: WorkspaceAvailability.inWorkspace, + workspaceAvailability: WorkspaceAvailability.insideWorkspace, }), httpMock.basePath ).properties 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 d24379ab147f..79871e380ae1 100644 --- a/src/core/public/chrome/nav_links/to_nav_link.ts +++ b/src/core/public/chrome/nav_links/to_nav_link.ts @@ -41,7 +41,7 @@ import { appendAppPath } from '../../application/utils'; export function toNavLink(app: PublicAppInfo, basePath: IBasePath): NavLinkWrapper { const useAppStatus = app.navLinkStatus === AppNavLinkStatus.default; let relativeBaseUrl = basePath.prepend(app.appRoute!); - if (app.workspaceAvailability === WorkspaceAvailability.outOfWorkspace) { + if (app.workspaceAvailability === WorkspaceAvailability.outsideWorkspace) { relativeBaseUrl = basePath.prepend(app.appRoute!, { withoutClientBasePath: true }); } const url = relativeToAbsolute(appendAppPath(relativeBaseUrl, app.defaultPath)); diff --git a/src/plugins/home/public/plugin.ts b/src/plugins/home/public/plugin.ts index a644a432c13a..5749b6fa3882 100644 --- a/src/plugins/home/public/plugin.ts +++ b/src/plugins/home/public/plugin.ts @@ -135,7 +135,7 @@ export class HomePublicPlugin const { renderApp } = await import('./application'); return await renderApp(params.element, coreStart, params.history); }, - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + 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 a6f23c9a5ade..3af37fc8cfbb 100644 --- a/src/plugins/opensearch_dashboards_overview/public/plugin.ts +++ b/src/plugins/opensearch_dashboards_overview/public/plugin.ts @@ -107,7 +107,7 @@ export class OpenSearchDashboardsOverviewPlugin // Render the application return renderApp(coreStart, depsStart as AppPluginStartDependencies, params); }, - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); if (home) { diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index df2da8cbe8eb..959fd4f3d753 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -200,7 +200,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderCreatorApp } = await import('./application'); return mountWorkspaceApp(params, renderCreatorApp); }, - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); // update @@ -246,7 +246,7 @@ export class WorkspacePlugin implements Plugin<{}, {}, WorkspacePluginSetupDeps> const { renderListApp } = await import('./application'); return mountWorkspaceApp(params, renderListApp); }, - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }); /** diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 5514849ec4ac..658bacc70a83 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -161,7 +161,7 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { id: 'home', title: 'Any app', mount: jest.fn(), - workspaceAvailability: WorkspaceAvailability.outOfWorkspace, + workspaceAvailability: WorkspaceAvailability.outsideWorkspace, }, { id: 'workspace_id', name: 'workspace name', features: [] } ) diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 7423678eaf12..c4aceec14b26 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -77,7 +77,7 @@ export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) /** * App is not accessible within workspace if it explicitly declare itself as WorkspaceAvailability.outOfWorkspace */ - if (app.workspaceAvailability === WorkspaceAvailability.outOfWorkspace) { + if (app.workspaceAvailability === WorkspaceAvailability.outsideWorkspace) { return false; } From 4373bd6988092a5935017bdee221174492977b8d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 17 Apr 2024 18:08:56 +0800 Subject: [PATCH 05/15] feat: update test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/utils.test.ts | 2 +- src/plugins/workspace/public/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 658bacc70a83..62a44f50859d 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -154,7 +154,7 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ).toBe(true); }); - it('An app is not accessible if its workspaceAvailability is outOfWorkspace', () => { + it('An app is not accessible if its workspaceAvailability is outsideWorkspace', () => { expect( isAppAccessibleInWorkspace( { diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index c4aceec14b26..c39c1e6d5958 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -75,7 +75,7 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ */ export function isAppAccessibleInWorkspace(app: App, workspace: WorkspaceObject) { /** - * App is not accessible within workspace if it explicitly declare itself as WorkspaceAvailability.outOfWorkspace + * App is not accessible within workspace if it explicitly declare itself as WorkspaceAvailability.outsideWorkspace */ if (app.workspaceAvailability === WorkspaceAvailability.outsideWorkspace) { return false; From 9ca591b7a04a74768af7f4ab108d9b09c9ee06ca Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 18 Apr 2024 01:06:20 +0800 Subject: [PATCH 06/15] feat: add unit test Signed-off-by: SuZhou-Joe --- .../public/chrome/nav_links/to_nav_link.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 bfac3a5754cd..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 @@ -199,6 +199,7 @@ describe('toNavLink', () => { }) ); + // When app is accessible inside workspace or outside workspace. expect( toNavLink( app({ @@ -212,5 +213,21 @@ describe('toNavLink', () => { 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', + }) + ); }); }); From 5a000af3405d29580caae4827e49284527ff6e84 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 18 Apr 2024 09:52:54 +0800 Subject: [PATCH 07/15] feat: remove CHANGELOG change Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 207fc97aa568..6b9a0b11f941 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,7 +105,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Workspace] Hide datasource and advanced settings menu in dashboard management when in workspace. ([#6455](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6455)) - [Multiple Datasource] Modify selectable picker to remove group label and close popover after selection ([#6515](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6515)) - [Workspace] Add workspaces filter to saved objects page. ([#6458](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6458)) -- [Workspace] Allow making apps available in workspaces using `workspaceAvailability` ([#6427](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6427)) ### 🐛 Bug Fixes From 1c5026206b3e4b07c2d015fcae172a5e2842c42a Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Thu, 18 Apr 2024 02:41:34 +0000 Subject: [PATCH 08/15] Changeset file for PR #6427 created/updated --- changelogs/fragments/6427.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/6427.yml 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 From 8eb9fc23dd1b1de56353e362d1155e73e3dc9963 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 19 Apr 2024 13:42:29 +0800 Subject: [PATCH 09/15] Apply suggestions from code review Co-authored-by: Yulong Ruan Signed-off-by: SuZhou-Joe --- src/core/public/application/application_service.test.ts | 2 +- src/plugins/workspace/public/utils.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index 6d448ac7995e..0796f476149f 100644 --- a/src/core/public/application/application_service.test.ts +++ b/src/core/public/application/application_service.test.ts @@ -808,7 +808,7 @@ describe('#start()', () => { `); }); - it('navigate by using window.location.assign if navigate to a app not accessible within a workspace', async () => { + 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; diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 62a44f50859d..f2af4d783877 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -154,7 +154,7 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ).toBe(true); }); - it('An app is not accessible if its workspaceAvailability is outsideWorkspace', () => { + it('An app is not accessible within a workspace if its workspaceAvailability is outsideWorkspace', () => { expect( isAppAccessibleInWorkspace( { From d6ae907d6497744635db9ca42c6dc034f06b0698 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 23 Apr 2024 07:56:28 +0800 Subject: [PATCH 10/15] Update src/plugins/workspace/public/utils.test.ts Co-authored-by: Miki Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/utils.test.ts | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index f2af4d783877..a6faa65a192e 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -167,6 +167,35 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ) ).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: [] } + ) + ).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(), + // eslint-disable-next-line no-bitwise + workspaceAvailability: WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, + }, + { id: 'workspace_id', name: 'workspace name', features: [] } + ) + ).toBe(true); + }); }); describe('workspace utils: filterWorkspaceConfigurableApps', () => { From 3b1a6122bdb4f8f333bb3b4c32ee01f928208adb Mon Sep 17 00:00:00 2001 From: Miki Date: Mon, 22 Apr 2024 17:20:23 -0700 Subject: [PATCH 11/15] Lint src/plugins/workspace/public/utils.test.ts Signed-off-by: Miki --- src/plugins/workspace/public/utils.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index a6faa65a192e..a5bf50821446 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -167,7 +167,6 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ) ).toBe(false); }); - it('An app is accessible within a workspace if its workspaceAvailability is insideWorkspace', () => { expect( isAppAccessibleInWorkspace( From d0619d49432a4a4000a4e8d4ab127f8bbdd6c2fb Mon Sep 17 00:00:00 2001 From: Miki Date: Mon, 22 Apr 2024 17:20:38 -0700 Subject: [PATCH 12/15] Lint src/plugins/workspace/public/utils.test.ts Signed-off-by: Miki --- src/plugins/workspace/public/utils.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index a5bf50821446..9f3aa9aa5474 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -180,7 +180,6 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { ) ).toBe(true); }); - it('An app is accessible within a workspace if its workspaceAvailability is inside and outsideWorkspace', () => { expect( isAppAccessibleInWorkspace( From 7a2eaf632c7c4d8087c4b695be84296be08ae7eb Mon Sep 17 00:00:00 2001 From: Miki Date: Mon, 22 Apr 2024 17:20:51 -0700 Subject: [PATCH 13/15] Lint src/plugins/workspace/public/utils.test.ts Signed-off-by: Miki --- src/plugins/workspace/public/utils.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 9f3aa9aa5474..d678012f84e8 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -188,7 +188,8 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { title: 'Any app', mount: jest.fn(), // eslint-disable-next-line no-bitwise - workspaceAvailability: WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, + workspaceAvailability: + WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, }, { id: 'workspace_id', name: 'workspace name', features: [] } ) From 766143aa10a8aa8af8ec0987a3e6c9e0554d3875 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 23 Apr 2024 09:08:13 +0800 Subject: [PATCH 14/15] fix: lint error Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index d678012f84e8..8a3567cf7e56 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -187,8 +187,8 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { id: 'home', title: 'Any app', mount: jest.fn(), - // eslint-disable-next-line no-bitwise workspaceAvailability: + // eslint-disable-next-line no-bitwise WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, }, { id: 'workspace_id', name: 'workspace name', features: [] } From fffd0ee7db8a8e8b4235365ac30ca685c6524b1e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 23 Apr 2024 09:59:52 +0800 Subject: [PATCH 15/15] fix: unit test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/utils.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 8a3567cf7e56..5471abace56d 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -176,7 +176,7 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { mount: jest.fn(), workspaceAvailability: WorkspaceAvailability.insideWorkspace, }, - { id: 'workspace_id', name: 'workspace name', features: [] } + { id: 'workspace_id', name: 'workspace name', features: ['home'] } ) ).toBe(true); }); @@ -191,7 +191,7 @@ describe('workspace utils: isAppAccessibleInWorkspace', () => { // eslint-disable-next-line no-bitwise WorkspaceAvailability.insideWorkspace | WorkspaceAvailability.outsideWorkspace, }, - { id: 'workspace_id', name: 'workspace name', features: [] } + { id: 'workspace_id', name: 'workspace name', features: ['home'] } ) ).toBe(true); });