Skip to content

Commit 2ca6444

Browse files
ruanylAMoo-Miki
authored andcommitted
remove unnecessary workspace enabled flag from core workspace module (opensearch-project#215)
Update test description per comment Signed-off-by: Yulong Ruan <ruanyl@amazon.com> Co-authored-by: Miki <amoo_miki@yahoo.com>
1 parent a758fa8 commit 2ca6444

File tree

14 files changed

+52
-114
lines changed

14 files changed

+52
-114
lines changed

src/core/public/application/capabilities/capabilities_service.mock.ts

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const createStartContractMock = (): jest.Mocked<CapabilitiesStart> => ({
3737
catalogue: {},
3838
management: {},
3939
navLinks: {},
40+
workspaces: {},
4041
}),
4142
});
4243

src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/public/workspace/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
* Copyright OpenSearch Contributors
33
* SPDX-License-Identifier: Apache-2.0
44
*/
5+
56
export { WorkspacesStart, WorkspacesService, WorkspacesSetup } from './workspaces_service';

src/core/public/workspace/workspaces_service.mock.ts

-3
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,19 @@ const currentWorkspaceId$ = new BehaviorSubject<string>('');
1313
const workspaceList$ = new BehaviorSubject<WorkspaceAttribute[]>([]);
1414
const currentWorkspace$ = new BehaviorSubject<WorkspaceAttribute | null>(null);
1515
const initialized$ = new BehaviorSubject<boolean>(false);
16-
const workspaceEnabled$ = new BehaviorSubject<boolean>(false);
1716

1817
const createWorkspacesSetupContractMock = () => ({
1918
currentWorkspaceId$,
2019
workspaceList$,
2120
currentWorkspace$,
2221
initialized$,
23-
workspaceEnabled$,
2422
});
2523

2624
const createWorkspacesStartContractMock = () => ({
2725
currentWorkspaceId$,
2826
workspaceList$,
2927
currentWorkspace$,
3028
initialized$,
31-
workspaceEnabled$,
3229
});
3330

3431
export type WorkspacesServiceContract = PublicMethodsOf<WorkspacesService>;

src/core/public/workspace/workspaces_service.test.ts

+1-6
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ describe('WorkspacesService', () => {
2222
expect(workspacesStart.initialized$.value).toBe(false);
2323
});
2424

25-
it('workspace is not enabled by default', () => {
26-
expect(workspacesStart.workspaceEnabled$.value).toBe(false);
27-
});
28-
2925
it('currentWorkspace is not set by default', () => {
3026
expect(workspacesStart.currentWorkspace$.value).toBe(null);
3127
expect(workspacesStart.currentWorkspaceId$.value).toBe('');
@@ -35,7 +31,7 @@ describe('WorkspacesService', () => {
3531
expect(workspacesStart.workspaceList$.value.length).toBe(0);
3632
});
3733

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

4137
workspacesStart.initialized$.next(true);
@@ -70,7 +66,6 @@ describe('WorkspacesService', () => {
7066
expect(workspacesStart.currentWorkspaceId$.isStopped).toBe(true);
7167
expect(workspacesStart.currentWorkspace$.isStopped).toBe(true);
7268
expect(workspacesStart.workspaceList$.isStopped).toBe(true);
73-
expect(workspacesStart.workspaceEnabled$.isStopped).toBe(true);
7469
expect(workspacesStart.initialized$.isStopped).toBe(true);
7570
});
7671
});

src/core/public/workspace/workspaces_service.ts

+23-6
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,33 @@ import { isEqual } from 'lodash';
88

99
import { CoreService, WorkspaceAttribute } from '../../types';
1010

11-
type WorkspaceObject = WorkspaceAttribute & { libraryReadonly?: boolean };
11+
type WorkspaceObject = WorkspaceAttribute & { readonly?: boolean };
12+
1213
interface WorkspaceObservables {
14+
/**
15+
* Indicates the current activated workspace id, the value should be changed every time
16+
* when switching to a different workspace
17+
*/
1318
currentWorkspaceId$: BehaviorSubject<string>;
19+
20+
/**
21+
* The workspace that is derived from `currentWorkspaceId` and `workspaceList`, if
22+
* `currentWorkspaceId` cannot be found from `workspaceList`, it will return an error
23+
*
24+
* This value MUST NOT set manually from outside of WorkspacesService
25+
*/
1426
currentWorkspace$: BehaviorSubject<WorkspaceObject | null>;
27+
28+
/**
29+
* The list of available workspaces. This workspace list should be set by whoever
30+
* the workspace functionalities
31+
*/
1532
workspaceList$: BehaviorSubject<WorkspaceObject[]>;
16-
workspaceEnabled$: BehaviorSubject<boolean>;
33+
34+
/**
35+
* This is a flag which indicates the WorkspacesService module is initialized and ready
36+
* for consuming by others. For example, the `workspaceList` has been set, etc
37+
*/
1738
initialized$: BehaviorSubject<boolean>;
1839
}
1940

@@ -29,7 +50,6 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
2950
private workspaceList$ = new BehaviorSubject<WorkspaceAttribute[]>([]);
3051
private currentWorkspace$ = new BehaviorSubject<WorkspaceAttribute | null>(null);
3152
private initialized$ = new BehaviorSubject<boolean>(false);
32-
private workspaceEnabled$ = new BehaviorSubject<boolean>(false);
3353

3454
constructor() {
3555
combineLatest([this.initialized$, this.workspaceList$, this.currentWorkspaceId$]).subscribe(
@@ -66,7 +86,6 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
6686
currentWorkspace$: this.currentWorkspace$,
6787
workspaceList$: this.workspaceList$,
6888
initialized$: this.initialized$,
69-
workspaceEnabled$: this.workspaceEnabled$,
7089
};
7190
}
7291

@@ -76,15 +95,13 @@ export class WorkspacesService implements CoreService<WorkspacesSetup, Workspace
7695
currentWorkspace$: this.currentWorkspace$,
7796
workspaceList$: this.workspaceList$,
7897
initialized$: this.initialized$,
79-
workspaceEnabled$: this.workspaceEnabled$,
8098
};
8199
}
82100

83101
public async stop() {
84102
this.currentWorkspace$.unsubscribe();
85103
this.currentWorkspaceId$.unsubscribe();
86104
this.workspaceList$.unsubscribe();
87-
this.workspaceEnabled$.unsubscribe();
88105
this.initialized$.unsubscribe();
89106
}
90107
}

src/core/server/capabilities/capabilities_service.mock.ts

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const createCapabilitiesMock = (): Capabilities => {
5252
navLinks: {},
5353
management: {},
5454
catalogue: {},
55+
workspaces: {},
5556
};
5657
};
5758

src/core/server/capabilities/capabilities_service.test.ts

+4
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ describe('CapabilitiesService', () => {
7272
"navLinks": Object {
7373
"myLink": true,
7474
},
75+
"workspaces": Object {},
7576
}
7677
`);
7778
});
@@ -107,6 +108,7 @@ describe('CapabilitiesService', () => {
107108
"B": true,
108109
"C": true,
109110
},
111+
"workspaces": Object {},
110112
}
111113
`);
112114
});
@@ -134,6 +136,7 @@ describe('CapabilitiesService', () => {
134136
},
135137
"management": Object {},
136138
"navLinks": Object {},
139+
"workspaces": Object {},
137140
}
138141
`);
139142
});
@@ -192,6 +195,7 @@ describe('CapabilitiesService', () => {
192195
"b": true,
193196
"c": false,
194197
},
198+
"workspaces": Object {},
195199
}
196200
`);
197201
});

src/core/server/capabilities/capabilities_service.ts

+1
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ const defaultCapabilities: Capabilities = {
123123
navLinks: {},
124124
management: {},
125125
catalogue: {},
126+
workspaces: {},
126127
};
127128

128129
/** @internal */

src/core/server/capabilities/integration_tests/capabilities_service.test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ describe('CapabilitiesService', () => {
7979
"catalogue": Object {},
8080
"management": Object {},
8181
"navLinks": Object {},
82+
"workspaces": Object {},
8283
}
8384
`);
8485
});
@@ -101,6 +102,7 @@ describe('CapabilitiesService', () => {
101102
},
102103
"management": Object {},
103104
"navLinks": Object {},
105+
"workspaces": Object {},
104106
}
105107
`);
106108
});

src/core/server/capabilities/resolve_capabilities.test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe('resolveCapabilities', () => {
4242
navLinks: {},
4343
catalogue: {},
4444
management: {},
45+
workspaces: {},
4546
};
4647
request = httpServerMock.createOpenSearchDashboardsRequest();
4748
});
@@ -75,6 +76,7 @@ describe('resolveCapabilities', () => {
7576
},
7677
"management": Object {},
7778
"navLinks": Object {},
79+
"workspaces": Object {},
7880
}
7981
`);
8082
});

src/core/types/capabilities.ts

+3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ export interface Capabilities {
4747
/** Catalogue capabilities. Catalogue entries drive the visibility of the OpenSearch Dashboards homepage options. */
4848
catalogue: Record<string, boolean>;
4949

50+
/** Workspaces capabilities. */
51+
workspaces: Record<string, boolean>;
52+
5053
/** Custom capabilities, registered by plugins. undefined if the key does not exist */
5154
[key: string]: Record<string, boolean | Record<string, boolean>> | undefined;
5255
}

src/plugins/dashboard/public/application/components/dashboard_listing/__snapshots__/dashboard_listing.test.tsx.snap

+5-45
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)