Skip to content

Commit

Permalink
[Workspace] Add workspaceAvailability field into application (opensea…
Browse files Browse the repository at this point in the history
…rch-project#6427)

* [Workspace] Jump to non-workspace url when clicking home icon (opensearch-project#316)

* temp: save

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: complete the feature

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: remove useless code

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: bootstrap error

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: bootstrap error

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: page not found error

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: anchor href

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: update toNavLink to comply with workspace

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: change to workspaceless

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: change to workspaceless

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: change to workspaceless

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: register list and create page as workspaceless

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: optimize code

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: update to WorkspaceVisibility

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: add unit test

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: optimize the jump logic

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: unit test

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: make app inaccessible if workspaceAccessibility is No

Signed-off-by: SuZhou-Joe <[email protected]>

---------

Signed-off-by: SuZhou-Joe <[email protected]>

* refactor: using WorkspaceAvailability

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: change test name

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: update wording

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: update test

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: add unit test

Signed-off-by: SuZhou-Joe <[email protected]>

* feat: remove CHANGELOG change

Signed-off-by: SuZhou-Joe <[email protected]>

* Changeset file for PR opensearch-project#6427 created/updated

* Apply suggestions from code review

Co-authored-by: Yulong Ruan <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>

* Update src/plugins/workspace/public/utils.test.ts

Co-authored-by: Miki <[email protected]>
Signed-off-by: SuZhou-Joe <[email protected]>

* Lint src/plugins/workspace/public/utils.test.ts

Signed-off-by: Miki <[email protected]>

* Lint src/plugins/workspace/public/utils.test.ts

Signed-off-by: Miki <[email protected]>

* Lint src/plugins/workspace/public/utils.test.ts

Signed-off-by: Miki <[email protected]>

* fix: lint error

Signed-off-by: SuZhou-Joe <[email protected]>

* fix: unit test

Signed-off-by: SuZhou-Joe <[email protected]>

---------

Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: Miki <[email protected]>
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]>
  • Loading branch information
4 people authored Apr 23, 2024
1 parent d4c9e62 commit 104ee42
Show file tree
Hide file tree
Showing 18 changed files with 279 additions and 29 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/6427.yml
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:/opensearch-project/OpenSearch-Dashboards/pull/6427))
94 changes: 90 additions & 4 deletions src/core/public/application/application_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
});

Expand Down Expand Up @@ -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();
});

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
});

Expand Down
27 changes: 25 additions & 2 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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.');
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/core/public/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,5 @@ export {
// Internal types
InternalApplicationSetup,
InternalApplicationStart,
WorkspaceAvailability,
} from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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();
});

Expand Down
23 changes: 23 additions & 0 deletions src/core/public/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/chrome/chrome_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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$()}
Expand Down
58 changes: 57 additions & 1 deletion src/core/public/chrome/nav_links/to_nav_link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
})
);
});
});
12 changes: 10 additions & 2 deletions src/core/public/chrome/nav_links/to_nav_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/core/public/core_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ export {
AppUpdater,
ScopedHistory,
NavigateToAppOptions,
WorkspaceAvailability,
} from './application';

export {
Expand Down
Loading

0 comments on commit 104ee42

Please sign in to comment.