[Workspace] Add workspaceAvailability field into application (#6427) (#…

…6628) * [Workspace] Jump to non-workspace url when clicking home icon (#316) * temp: save * feat: complete the feature * feat: remove useless code * fix: bootstrap error * fix: bootstrap error * fix: page not found error * fix: anchor href * feat: update toNavLink to comply with workspace * feat: change to workspaceless * feat: change to workspaceless * feat: change to workspaceless * feat: register list and create page as workspaceless * feat: optimize code * feat: update to WorkspaceVisibility * feat: add unit test * feat: optimize the jump logic * fix: unit test * feat: make app inaccessible if workspaceAccessibility is No --------- * refactor: using WorkspaceAvailability * feat: change test name * feat: update wording * feat: update test * feat: add unit test * feat: remove CHANGELOG change * Changeset file for PR #6427 created/updated * Apply suggestions from code review * Update src/plugins/workspace/public/utils.test.ts * Lint src/plugins/workspace/public/utils.test.ts * Lint src/plugins/workspace/public/utils.test.ts * Lint src/plugins/workspace/public/utils.test.ts * fix: lint error * fix: unit test --------- (cherry picked from commit 104ee42) Signed-off-by: SuZhou-Joe <[email protected]> Signed-off-by: Miki <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Yulong Ruan <[email protected]> Co-authored-by: Miki <[email protected]>
opensearch-project · Apr 25, 2024 · 0f7267f · 0f7267f
1 parent 7702dfd
commit 0f7267f
Show file tree

Hide file tree

Showing 18 changed files with 279 additions and 29 deletions.
diff --git a/changelogs/fragments/6427.yml 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))
diff --git 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>): 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
@@ -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<InternalApplicationStart> {
+  public async start({ http, overlays, workspaces }: StartDeps): Promise<InternalApplicationStart> {
     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
@@ -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
@@ -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
@@ -103,6 +103,22 @@ export type AppUpdatableFields = Pick<App, 'status' | 'navLinkStatus' | 'tooltip
  */
 export type AppUpdater = (app: App) => Partial<AppUpdatableFields> | 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<HistoryLocationState = unknown> {
    * ```
    */
   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
@@ -269,7 +269,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
@@ -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
@@ -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
@@ -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
@@ -134,6 +134,7 @@ export {
   AppUpdater,
   ScopedHistory,
   NavigateToAppOptions,
+  WorkspaceAvailability,
 } from './application';
 
 export {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		feat:
		- [Workspace] Allow making apps available in workspaces using `workspaceAvailability` ([#6427](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6427))