Skip to content

Commit

Permalink
remove unnecessary workspace enabled flag from core workspace module (o…
Browse files Browse the repository at this point in the history
…pensearch-project#215)

Update test description per comment

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Co-authored-by: Miki <amoo_miki@yahoo.com>
  • Loading branch information
ruanyl and AMoo-Miki authored Oct 10, 2023
1 parent 87c0c8a commit 903e594
Show file tree
Hide file tree
Showing 15 changed files with 53 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const createStartContractMock = (): jest.Mocked<CapabilitiesStart> => ({
catalogue: {},
management: {},
navLinks: {},
workspaces: {},
}),
});

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 1 addition & 6 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,4 @@ export {

export { __osdBootstrap__ } from './osd_bootstrap';

export {
WorkspacesStart,
WorkspacesSetup,
WorkspacesService,
WorkspaceObservables,
} from './workspace';
export { WorkspacesStart, WorkspacesSetup, WorkspacesService } from './workspace';
8 changes: 2 additions & 6 deletions src/core/public/workspace/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,5 @@
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
export {
WorkspacesStart,
WorkspacesService,
WorkspacesSetup,
WorkspaceObservables,
} from './workspaces_service';

export { WorkspacesStart, WorkspacesService, WorkspacesSetup } from './workspaces_service';
3 changes: 0 additions & 3 deletions src/core/public/workspace/workspaces_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,19 @@ const currentWorkspaceId$ = new BehaviorSubject<string>('');
const workspaceList$ = new BehaviorSubject<WorkspaceAttribute[]>([]);
const currentWorkspace$ = new BehaviorSubject<WorkspaceAttribute | null>(null);
const initialized$ = new BehaviorSubject<boolean>(false);
const workspaceEnabled$ = new BehaviorSubject<boolean>(false);

const createWorkspacesSetupContractMock = () => ({
currentWorkspaceId$,
workspaceList$,
currentWorkspace$,
initialized$,
workspaceEnabled$,
});

const createWorkspacesStartContractMock = () => ({
currentWorkspaceId$,
workspaceList$,
currentWorkspace$,
initialized$,
workspaceEnabled$,
});

export type WorkspacesServiceContract = PublicMethodsOf<WorkspacesService>;
Expand Down
7 changes: 1 addition & 6 deletions src/core/public/workspace/workspaces_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ describe('WorkspacesService', () => {
expect(workspacesStart.initialized$.value).toBe(false);
});

it('workspace is not enabled by default', () => {
expect(workspacesStart.workspaceEnabled$.value).toBe(false);
});

it('currentWorkspace is not set by default', () => {
expect(workspacesStart.currentWorkspace$.value).toBe(null);
expect(workspacesStart.currentWorkspaceId$.value).toBe('');
Expand All @@ -35,7 +31,7 @@ describe('WorkspacesService', () => {
expect(workspacesStart.workspaceList$.value.length).toBe(0);
});

it('the current workspace should also updated after changing current workspace id', () => {
it('currentWorkspace is updated when currentWorkspaceId changes', () => {
expect(workspacesStart.currentWorkspace$.value).toBe(null);

workspacesStart.initialized$.next(true);
Expand Down Expand Up @@ -70,7 +66,6 @@ describe('WorkspacesService', () => {
expect(workspacesStart.currentWorkspaceId$.isStopped).toBe(true);
expect(workspacesStart.currentWorkspace$.isStopped).toBe(true);
expect(workspacesStart.workspaceList$.isStopped).toBe(true);
expect(workspacesStart.workspaceEnabled$.isStopped).toBe(true);
expect(workspacesStart.initialized$.isStopped).toBe(true);
});
});
28 changes: 22 additions & 6 deletions src/core/public/workspace/workspaces_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,31 @@ import { CoreService, WorkspaceAttribute } from '../../types';

type WorkspaceObject = WorkspaceAttribute & { readonly?: boolean };

export interface WorkspaceObservables {
interface WorkspaceObservables {
/**
* Indicates the current activated workspace id, the value should be changed every time
* when switching to a different workspace
*/
currentWorkspaceId$: BehaviorSubject<string>;

/**
* The workspace that is derived from `currentWorkspaceId` and `workspaceList`, if
* `currentWorkspaceId` cannot be found from `workspaceList`, it will return an error
*
* This value MUST NOT set manually from outside of WorkspacesService
*/
currentWorkspace$: BehaviorSubject<WorkspaceObject | null>;

/**
* The list of available workspaces. This workspace list should be set by whoever
* the workspace functionalities
*/
workspaceList$: BehaviorSubject<WorkspaceObject[]>;
workspaceEnabled$: BehaviorSubject<boolean>;

/**
* This is a flag which indicates the WorkspacesService module is initialized and ready
* for consuming by others. For example, the `workspaceList` has been set, etc
*/
initialized$: BehaviorSubject<boolean>;
}

Expand All @@ -30,7 +50,6 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
private workspaceList$ = new BehaviorSubject<WorkspaceObject[]>([]);
private currentWorkspace$ = new BehaviorSubject<WorkspaceObject | null>(null);
private initialized$ = new BehaviorSubject<boolean>(false);
private workspaceEnabled$ = new BehaviorSubject<boolean>(false);

constructor() {
combineLatest([this.initialized$, this.workspaceList$, this.currentWorkspaceId$]).subscribe(
Expand Down Expand Up @@ -67,7 +86,6 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
currentWorkspace$: this.currentWorkspace$,
workspaceList$: this.workspaceList$,
initialized$: this.initialized$,
workspaceEnabled$: this.workspaceEnabled$,
};
}

Expand All @@ -77,15 +95,13 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
currentWorkspace$: this.currentWorkspace$,
workspaceList$: this.workspaceList$,
initialized$: this.initialized$,
workspaceEnabled$: this.workspaceEnabled$,
};
}

public async stop() {
this.currentWorkspace$.unsubscribe();
this.currentWorkspaceId$.unsubscribe();
this.workspaceList$.unsubscribe();
this.workspaceEnabled$.unsubscribe();
this.initialized$.unsubscribe();
}
}
1 change: 1 addition & 0 deletions src/core/server/capabilities/capabilities_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const createCapabilitiesMock = (): Capabilities => {
navLinks: {},
management: {},
catalogue: {},
workspaces: {},
};
};

Expand Down
4 changes: 4 additions & 0 deletions src/core/server/capabilities/capabilities_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe('CapabilitiesService', () => {
"navLinks": Object {
"myLink": true,
},
"workspaces": Object {},
}
`);
});
Expand Down Expand Up @@ -107,6 +108,7 @@ describe('CapabilitiesService', () => {
"B": true,
"C": true,
},
"workspaces": Object {},
}
`);
});
Expand Down Expand Up @@ -134,6 +136,7 @@ describe('CapabilitiesService', () => {
},
"management": Object {},
"navLinks": Object {},
"workspaces": Object {},
}
`);
});
Expand Down Expand Up @@ -192,6 +195,7 @@ describe('CapabilitiesService', () => {
"b": true,
"c": false,
},
"workspaces": Object {},
}
`);
});
Expand Down
1 change: 1 addition & 0 deletions src/core/server/capabilities/capabilities_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ const defaultCapabilities: Capabilities = {
navLinks: {},
management: {},
catalogue: {},
workspaces: {},
};

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('CapabilitiesService', () => {
"catalogue": Object {},
"management": Object {},
"navLinks": Object {},
"workspaces": Object {},
}
`);
});
Expand All @@ -101,6 +102,7 @@ describe('CapabilitiesService', () => {
},
"management": Object {},
"navLinks": Object {},
"workspaces": Object {},
}
`);
});
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/capabilities/resolve_capabilities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('resolveCapabilities', () => {
navLinks: {},
catalogue: {},
management: {},
workspaces: {},
};
request = httpServerMock.createOpenSearchDashboardsRequest();
});
Expand Down Expand Up @@ -75,6 +76,7 @@ describe('resolveCapabilities', () => {
},
"management": Object {},
"navLinks": Object {},
"workspaces": Object {},
}
`);
});
Expand Down
3 changes: 3 additions & 0 deletions src/core/types/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ export interface Capabilities {
/** Catalogue capabilities. Catalogue entries drive the visibility of the OpenSearch Dashboards homepage options. */
catalogue: Record<string, boolean>;

/** Workspaces capabilities. */
workspaces: Record<string, boolean>;

/** Custom capabilities, registered by plugins. undefined if the key does not exist */
[key: string]: Record<string, boolean | Record<string, boolean>> | undefined;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 903e594

Please sign in to comment.