From a1f1666ded0181d27de650578db64b0c4e000caa Mon Sep 17 00:00:00 2001 From: yuboluo Date: Fri, 16 Aug 2024 17:23:18 +0800 Subject: [PATCH] [Workspace] Support getting workspace client from coreStart (#7501) * Support to get workspace client from coreStart Signed-off-by: yubonluo * Changeset file for PR #7501 created/updated * Support to get workspace client from coreStart Signed-off-by: yubonluo * fix test error Signed-off-by: yubonluo * delete irrelevant modification Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo * optimize the code Signed-off-by: yubonluo --------- Signed-off-by: yubonluo Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Yulong Ruan --- changelogs/fragments/7501.yml | 2 + src/core/public/index.ts | 8 ++- src/core/public/workspace/index.ts | 1 + .../workspace/workspaces_service.mock.ts | 6 +- .../workspace/workspaces_service.test.ts | 19 ++++- .../public/workspace/workspaces_service.ts | 19 ++++- .../dashboard_listing.test.tsx.snap | 45 ++++++++++++ .../dashboard_top_nav.test.tsx.snap | 54 ++++++++++++++ .../saved_objects_management/public/index.ts | 1 - .../lib/duplicate_saved_objects.test.ts | 71 ------------------- .../public/lib/duplicate_saved_objects.ts | 21 ------ .../public/lib/index.ts | 1 - .../saved_objects_table.test.tsx.snap | 27 +++++++ .../saved_objects_table.test.mocks.ts | 5 -- .../saved_objects_table.test.tsx | 59 +++++++++------ .../objects_table/saved_objects_table.tsx | 30 ++++---- src/plugins/workspace/public/plugin.ts | 1 + .../workspace/public/workspace_client.mock.ts | 1 + .../workspace/public/workspace_client.test.ts | 18 +++++ .../workspace/public/workspace_client.ts | 31 +++++++- 20 files changed, 283 insertions(+), 137 deletions(-) create mode 100644 changelogs/fragments/7501.yml delete mode 100644 src/plugins/saved_objects_management/public/lib/duplicate_saved_objects.test.ts delete mode 100644 src/plugins/saved_objects_management/public/lib/duplicate_saved_objects.ts diff --git a/changelogs/fragments/7501.yml b/changelogs/fragments/7501.yml new file mode 100644 index 000000000000..3987a2e580fb --- /dev/null +++ b/changelogs/fragments/7501.yml @@ -0,0 +1,2 @@ +refactor: +- [Workspace] Support getting workspaces client from coreStart ([#7501](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7501)) \ No newline at end of file diff --git a/src/core/public/index.ts b/src/core/public/index.ts index ffb3bb684170..979e618fb505 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -391,6 +391,12 @@ export { export { __osdBootstrap__ } from './osd_bootstrap'; -export { WorkspacesStart, WorkspacesSetup, WorkspacesService, WorkspaceObject } from './workspace'; +export { + WorkspacesStart, + WorkspacesSetup, + WorkspacesService, + WorkspaceObject, + IWorkspaceClient, +} from './workspace'; export { debounce } from './utils'; diff --git a/src/core/public/workspace/index.ts b/src/core/public/workspace/index.ts index 712ad657fa65..a605aff02fb1 100644 --- a/src/core/public/workspace/index.ts +++ b/src/core/public/workspace/index.ts @@ -8,4 +8,5 @@ export { WorkspacesService, WorkspacesSetup, WorkspaceObject, + IWorkspaceClient, } from './workspaces_service'; diff --git a/src/core/public/workspace/workspaces_service.mock.ts b/src/core/public/workspace/workspaces_service.mock.ts index 9e8cdfce7393..ae0f4deb1d68 100644 --- a/src/core/public/workspace/workspaces_service.mock.ts +++ b/src/core/public/workspace/workspaces_service.mock.ts @@ -6,7 +6,7 @@ import { BehaviorSubject } from 'rxjs'; import type { PublicMethodsOf } from '@osd/utility-types'; -import { WorkspacesService, WorkspaceObject } from './workspaces_service'; +import { WorkspacesService, WorkspaceObject, IWorkspaceClient } from './workspaces_service'; const createWorkspacesSetupContractMock = () => { const currentWorkspaceId$ = new BehaviorSubject(''); @@ -18,6 +18,7 @@ const createWorkspacesSetupContractMock = () => { workspaceList$, currentWorkspace$, initialized$, + setClient: jest.fn(), }; }; @@ -26,11 +27,14 @@ const createWorkspacesStartContractMock = () => { const workspaceList$ = new BehaviorSubject([]); const currentWorkspace$ = new BehaviorSubject(null); const initialized$ = new BehaviorSubject(false); + const client$ = new BehaviorSubject(null); + return { currentWorkspaceId$, workspaceList$, currentWorkspace$, initialized$, + client$, }; }; diff --git a/src/core/public/workspace/workspaces_service.test.ts b/src/core/public/workspace/workspaces_service.test.ts index 4eca97ef2ed2..076100c8f558 100644 --- a/src/core/public/workspace/workspaces_service.test.ts +++ b/src/core/public/workspace/workspaces_service.test.ts @@ -3,14 +3,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { WorkspacesService, WorkspacesStart } from './workspaces_service'; +import { + WorkspacesService, + WorkspacesSetup, + WorkspacesStart, + IWorkspaceClient, +} from './workspaces_service'; describe('WorkspacesService', () => { let workspaces: WorkspacesService; let workspacesStart: WorkspacesStart; + let workspacesSetUp: WorkspacesSetup; beforeEach(() => { workspaces = new WorkspacesService(); + workspacesSetUp = workspaces.setup(); workspacesStart = workspaces.start(); }); @@ -31,6 +38,16 @@ describe('WorkspacesService', () => { expect(workspacesStart.workspaceList$.value.length).toBe(0); }); + it('client$ is not set by default', () => { + expect(workspacesStart.client$.value).toBe(null); + }); + + it('client is updated when set client', () => { + const client: IWorkspaceClient = { copy: jest.fn() }; + workspacesSetUp.setClient(client); + expect(workspacesStart.client$.value).toEqual(client); + }); + it('currentWorkspace is updated when currentWorkspaceId changes', () => { expect(workspacesStart.currentWorkspace$.value).toBe(null); diff --git a/src/core/public/workspace/workspaces_service.ts b/src/core/public/workspace/workspaces_service.ts index e4cf3bc7a826..6f934c294eba 100644 --- a/src/core/public/workspace/workspaces_service.ts +++ b/src/core/public/workspace/workspaces_service.ts @@ -10,6 +10,10 @@ import { CoreService, WorkspaceAttribute } from '../../types'; export type WorkspaceObject = WorkspaceAttribute & { readonly?: boolean }; +export interface IWorkspaceClient { + copy(objects: any[], targetWorkspace: string, includeReferencesDeep?: boolean): Promise; +} + interface WorkspaceObservables { /** * Indicates the current activated workspace id, the value should be changed every time @@ -43,14 +47,20 @@ enum WORKSPACE_ERROR { WORKSPACE_IS_STALE = 'WORKSPACE_IS_STALE', } -export type WorkspacesSetup = WorkspaceObservables; -export type WorkspacesStart = WorkspaceObservables; +export type WorkspacesSetup = WorkspaceObservables & { + setClient: (client: IWorkspaceClient) => void; +}; + +export type WorkspacesStart = WorkspaceObservables & { + client$: BehaviorSubject; +}; export class WorkspacesService implements CoreService { private currentWorkspaceId$ = new BehaviorSubject(''); private workspaceList$ = new BehaviorSubject([]); private currentWorkspace$ = new BehaviorSubject(null); private initialized$ = new BehaviorSubject(false); + private client$ = new BehaviorSubject(null); constructor() { combineLatest([this.initialized$, this.workspaceList$, this.currentWorkspaceId$]).subscribe( @@ -87,6 +97,9 @@ export class WorkspacesService implements CoreService { + this.client$.next(client); + }, }; } @@ -96,6 +109,7 @@ export class WorkspacesService implements CoreService { - const httpClient = httpServiceMock.createStartContract(); - const objects = [ - { type: 'dashboard', id: '1' }, - { type: 'visualization', id: '2' }, - ]; - const targetWorkspace = '1'; - - it('make http call with all parameter provided', async () => { - const includeReferencesDeep = true; - await duplicateSavedObjects(httpClient, objects, targetWorkspace, includeReferencesDeep); - expect(httpClient.post).toMatchInlineSnapshot(` - [MockFunction] { - "calls": Array [ - Array [ - "/api/workspaces/_duplicate_saved_objects", - Object { - "body": "{\\"objects\\":[{\\"type\\":\\"dashboard\\",\\"id\\":\\"1\\"},{\\"type\\":\\"visualization\\",\\"id\\":\\"2\\"}],\\"includeReferencesDeep\\":true,\\"targetWorkspace\\":\\"1\\"}", - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": undefined, - }, - ], - } - `); - }); - - it('make http call without includeReferencesDeep parameter provided', async () => { - await duplicateSavedObjects(httpClient, objects, targetWorkspace); - expect(httpClient.post).toMatchInlineSnapshot(` - [MockFunction] { - "calls": Array [ - Array [ - "/api/workspaces/_duplicate_saved_objects", - Object { - "body": "{\\"objects\\":[{\\"type\\":\\"dashboard\\",\\"id\\":\\"1\\"},{\\"type\\":\\"visualization\\",\\"id\\":\\"2\\"}],\\"includeReferencesDeep\\":true,\\"targetWorkspace\\":\\"1\\"}", - }, - ], - Array [ - "/api/workspaces/_duplicate_saved_objects", - Object { - "body": "{\\"objects\\":[{\\"type\\":\\"dashboard\\",\\"id\\":\\"1\\"},{\\"type\\":\\"visualization\\",\\"id\\":\\"2\\"}],\\"includeReferencesDeep\\":true,\\"targetWorkspace\\":\\"1\\"}", - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": undefined, - }, - Object { - "type": "return", - "value": undefined, - }, - ], - } - `); - }); -}); diff --git a/src/plugins/saved_objects_management/public/lib/duplicate_saved_objects.ts b/src/plugins/saved_objects_management/public/lib/duplicate_saved_objects.ts deleted file mode 100644 index 560e0c1fddb3..000000000000 --- a/src/plugins/saved_objects_management/public/lib/duplicate_saved_objects.ts +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { HttpStart } from 'src/core/public'; - -export async function duplicateSavedObjects( - http: HttpStart, - objects: any[], - targetWorkspace: string, - includeReferencesDeep: boolean = true -) { - return await http.post('/api/workspaces/_duplicate_saved_objects', { - body: JSON.stringify({ - objects, - includeReferencesDeep, - targetWorkspace, - }), - }); -} diff --git a/src/plugins/saved_objects_management/public/lib/index.ts b/src/plugins/saved_objects_management/public/lib/index.ts index 80630b8780e7..fae58cad3eb2 100644 --- a/src/plugins/saved_objects_management/public/lib/index.ts +++ b/src/plugins/saved_objects_management/public/lib/index.ts @@ -57,4 +57,3 @@ export { extractExportDetails, SavedObjectsExportResultDetails } from './extract export { createFieldList } from './create_field_list'; export { getAllowedTypes } from './get_allowed_types'; export { filterQuery } from './filter_query'; -export { duplicateSavedObjects } from './duplicate_saved_objects'; diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap b/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap index 877cc43a6383..765a84bb0803 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap @@ -334,6 +334,15 @@ exports[`SavedObjectsTable duplicate should allow the user to choose on header w } workspaces={ Object { + "client$": BehaviorSubject { + "_isScalar": false, + "_value": null, + "closed": false, + "hasError": false, + "isStopped": false, + "observers": Array [], + "thrownError": null, + }, "currentWorkspace$": BehaviorSubject { "_isScalar": false, "_value": Object { @@ -512,6 +521,15 @@ exports[`SavedObjectsTable duplicate should allow the user to choose on table wh selectedSavedObjects={Array []} workspaces={ Object { + "client$": BehaviorSubject { + "_isScalar": false, + "_value": null, + "closed": false, + "hasError": false, + "isStopped": false, + "observers": Array [], + "thrownError": null, + }, "currentWorkspace$": BehaviorSubject { "_isScalar": false, "_value": Object { @@ -702,6 +720,15 @@ exports[`SavedObjectsTable duplicate should allow the user to choose on table wh } workspaces={ Object { + "client$": BehaviorSubject { + "_isScalar": false, + "_value": null, + "closed": false, + "hasError": false, + "isStopped": false, + "observers": Array [], + "thrownError": null, + }, "currentWorkspace$": BehaviorSubject { "_isScalar": false, "_value": Object { diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.mocks.ts b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.mocks.ts index f91c7103eeac..b856b8662479 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.mocks.ts +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.mocks.ts @@ -80,8 +80,3 @@ export const getRelationshipsMock = jest.fn(); jest.doMock('../../lib/get_relationships', () => ({ getRelationships: getRelationshipsMock, })); - -export const getDuplicateSavedObjectsMock = jest.fn(); -jest.doMock('../../lib/duplicate_saved_objects', () => ({ - duplicateSavedObjects: getDuplicateSavedObjectsMock, -})); diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx index f8d7e7b8b9ef..092b769e81b1 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.test.tsx @@ -33,7 +33,6 @@ import { fetchExportByTypeAndSearchMock, fetchExportObjectsMock, findObjectsMock, - getDuplicateSavedObjectsMock, getRelationshipsMock, getSavedObjectCountsMock, saveAsMock, @@ -915,7 +914,9 @@ describe('SavedObjectsTable', () => { }); it('should duplicate selected objects', async () => { - getDuplicateSavedObjectsMock.mockImplementation(() => ({ success: true })); + const mockCopy = jest.fn().mockResolvedValue({ success: true }); + workspaces.client$.next({ copy: mockCopy }); + const client = workspaces.client$.getValue(); const component = shallowRender({ applications, workspaces }); component.setState({ isShowingDuplicateModal: true }); @@ -930,8 +931,7 @@ describe('SavedObjectsTable', () => { await component.instance().onDuplicate(mockSelectedSavedObjects, false, 'workspace2', 'bar'); - expect(getDuplicateSavedObjectsMock).toHaveBeenCalledWith( - http, + expect(client?.copy).toHaveBeenCalledWith( [ { id: '1', type: 'dashboard' }, { id: '2', type: 'dashboard' }, @@ -946,7 +946,9 @@ describe('SavedObjectsTable', () => { }); it('should duplicate single object', async () => { - getDuplicateSavedObjectsMock.mockImplementation(() => ({ success: true })); + const mockCopy = jest.fn().mockResolvedValue({ success: true }); + workspaces.client$.next({ copy: mockCopy }); + const client = workspaces.client$.getValue(); const component = shallowRender({ applications, workspaces }); component.setState({ isShowingDuplicateModal: true }); @@ -960,8 +962,7 @@ describe('SavedObjectsTable', () => { .instance() .onDuplicate([mockSelectedSavedObjects[0]], true, 'workspace2', 'bar'); - expect(getDuplicateSavedObjectsMock).toHaveBeenCalledWith( - http, + expect(client?.copy).toHaveBeenCalledWith( [{ id: '1', type: 'dashboard' }], 'workspace2', true @@ -973,6 +974,14 @@ describe('SavedObjectsTable', () => { }); it('should show result flyout when duplicating success and failure coexist', async () => { + const mockCopy = jest.fn().mockResolvedValue(() => ({ + success: false, + successCount: 1, + successResults: [{ id: '1' }], + errors: [{ id: '2' }], + })); + workspaces.client$.next({ copy: mockCopy }); + const client = workspaces.client$.getValue(); const component = shallowRender({ applications, workspaces }); component.setState({ isShowingDuplicateModal: true }); @@ -981,17 +990,9 @@ describe('SavedObjectsTable', () => { // Ensure the state changes are reflected component.update(); - getDuplicateSavedObjectsMock.mockImplementationOnce(() => ({ - success: false, - successCount: 1, - successResults: [{ id: '1' }], - errors: [{ id: '2' }], - })); - await component.instance().onDuplicate(mockSelectedSavedObjects, false, 'workspace2', 'bar'); - expect(getDuplicateSavedObjectsMock).toHaveBeenCalledWith( - http, + expect(client?.copy).toHaveBeenCalledWith( [ { id: '1', type: 'dashboard' }, { id: '2', type: 'dashboard' }, @@ -1005,12 +1006,31 @@ describe('SavedObjectsTable', () => { expect(component.find('DuplicateResultFlyout').length).toEqual(1); }); - it('should catch error when duplicating selected object is fail', async () => { - getDuplicateSavedObjectsMock.mockImplementationOnce(() => undefined); + it('should catch error when workspace client is null', async () => { + const component = shallowRender({ applications, workspaces }); + component.setState({ isShowingDuplicateModal: true }); + workspaces.client$.next(null); + + // Ensure all promises resolve + await new Promise((resolve) => process.nextTick(resolve)); + // Ensure the state changes are reflected + component.update(); + await component.instance().onDuplicate(mockSelectedSavedObjects, false, 'workspace2', 'bar'); + component.update(); + expect(notifications.toasts.addDanger).toHaveBeenCalledWith({ + title: 'Unable to copy 2 saved objects.', + }); + }); + + it('should catch error when duplicating selected object is fail', async () => { const component = shallowRender({ applications, workspaces }); component.setState({ isShowingDuplicateModal: true }); + const mockCopy = jest.fn().mockRejectedValue(() => new Error('Copy operation failed')); + workspaces.client$.next({ copy: mockCopy }); + const client = workspaces.client$.getValue(); + // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); // Ensure the state changes are reflected @@ -1018,8 +1038,7 @@ describe('SavedObjectsTable', () => { await component.instance().onDuplicate(mockSelectedSavedObjects, false, 'workspace2', 'bar'); - expect(getDuplicateSavedObjectsMock).toHaveBeenCalledWith( - http, + expect(client?.copy).toHaveBeenCalledWith( [ { id: '1', type: 'dashboard' }, { id: '2', type: 'dashboard' }, diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx index 76d2ce72c51c..35f77e6efa92 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx @@ -89,7 +89,6 @@ import { findObject, extractExportDetails, SavedObjectsExportResultDetails, - duplicateSavedObjects, } from '../../lib'; import { SavedObjectWithMetadata } from '../../types'; import { @@ -735,12 +734,25 @@ export class SavedObjectsTable extends Component { - const { http, notifications } = this.props; + const { notifications, workspaces } = this.props; + const workspaceClient = workspaces.client$.getValue(); + + const showErrorNotification = () => { + notifications.toasts.addDanger({ + title: i18n.translate('savedObjectsManagement.objectsTable.duplicate.dangerNotification', { + defaultMessage: + 'Unable to copy {errorCount, plural, one {# saved object} other {# saved objects}}.', + values: { errorCount: savedObjects.length }, + }), + }); + }; + if (!workspaceClient) { + showErrorNotification(); + return; + } const objectsToDuplicate = savedObjects.map((obj) => ({ id: obj.id, type: obj.type })); - let result; try { - result = await duplicateSavedObjects( - http, + const result = await workspaceClient.copy( objectsToDuplicate, targetWorkspace, includeReferencesDeep @@ -753,13 +765,7 @@ export class SavedObjectsTable extends Component { expect(workspaceMock.workspaceList$.getValue()).toEqual([]); }); + it('#copy', async () => { + const { workspaceClient, httpSetupMock } = getWorkspaceClient(); + httpSetupMock.fetch.mockResolvedValue({ + success: true, + result: {}, + }); + const body = JSON.stringify({ + objects: [{ id: 1, type: 'url' }], + targetWorkspace: 'workspace-1', + includeReferencesDeep: false, + }); + await workspaceClient.copy([{ id: 1, type: 'url' }], 'workspace-1', false); + expect(httpSetupMock.fetch).toBeCalledWith('/api/workspaces/_duplicate_saved_objects', { + body, + method: 'POST', + }); + }); + it('#init with resultWithWritePermission is not success ', async () => { const { workspaceClient, httpSetupMock, workspaceMock } = getWorkspaceClient(); httpSetupMock.fetch diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts index 6482478c9240..05b3578076cd 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -10,6 +10,7 @@ import { HttpSetup, WorkspaceAttribute, WorkspacesSetup, + IWorkspaceClient, } from '../../../core/public'; import { WorkspacePermissionMode } from '../common/constants'; import { SavedObjectPermissions, WorkspaceAttributeWithPermission } from '../../../core/types'; @@ -48,7 +49,7 @@ interface WorkspaceFindOptions { * * @public */ -export class WorkspaceClient { +export class WorkspaceClient implements IWorkspaceClient { private http: HttpSetup; private workspaces: WorkspacesSetup; @@ -317,6 +318,34 @@ export class WorkspaceClient { return result; } + /** + * copy saved objects to target workspace + * + * @param {Array<{ id: string; type: string }>} objects + * @param {string} targetWorkspace + * @param {boolean} includeReferencesDeep + * @returns {Promise>} result for this operation + */ + public async copy( + objects: Array<{ id: string; type: string }>, + targetWorkspace: string, + includeReferencesDeep: boolean = true + ): Promise> { + const path = this.getPath('_duplicate_saved_objects'); + const body = { + objects, + targetWorkspace, + includeReferencesDeep, + }; + + const result = await this.safeFetch(path, { + method: 'POST', + body: JSON.stringify(body), + }); + + return result; + } + public stop() { this.workspaces.workspaceList$.unsubscribe(); this.workspaces.currentWorkspaceId$.unsubscribe();