Skip to content

Commit

Permalink
[Workspace] Jump to non-workspace url when clicking home icon (opense…
Browse files Browse the repository at this point in the history
…arch-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]>
  • Loading branch information
SuZhou-Joe committed Apr 17, 2024
1 parent d911fa7 commit bdd64ff
Show file tree
Hide file tree
Showing 17 changed files with 236 additions and 30 deletions.
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,
WorkspaceAccessibility,
} 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',
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', () => {
Expand Down Expand Up @@ -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);
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
26 changes: 24 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,
WorkspaceAccessibility,
} 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?.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;
}
Expand Down Expand Up @@ -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,
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,
WorkspaceAccessibility,
} 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
22 changes: 22 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 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
*/
Expand Down Expand Up @@ -245,6 +261,12 @@ export interface App<HistoryLocationState = unknown> {
* ```
*/
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;
}

/**
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
41 changes: 40 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,
WorkspaceAccessibility,
} from '../../application';
import { toNavLink } from './to_nav_link';

import { httpServiceMock } from '../../mocks';
Expand Down Expand Up @@ -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',
})
);
});
});
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,
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);

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 @@ -128,6 +128,7 @@ export {
AppUpdater,
ScopedHistory,
NavigateToAppOptions,
WorkspaceAccessibility,
} from './application';

export {
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/home/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');

Expand Down
Loading

0 comments on commit bdd64ff

Please sign in to comment.