From 7a0d6e219a8d5514a8e11cd41893af18831c56dd Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Wed, 4 Jan 2023 14:56:34 +0100 Subject: [PATCH 1/8] Add bulk delete endpoint to the saved objects management API --- .../public/lib/bulk_delete_objects.ts | 31 +++++++ .../public/lib/index.ts | 1 + .../server/routes/bulk_delete.ts | 40 +++++++++ .../server/routes/index.test.ts | 8 +- .../server/routes/index.ts | 2 + .../saved_objects_management/bulk_delete.ts | 82 +++++++++++++++++++ .../apis/saved_objects_management/index.ts | 1 + 7 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts create mode 100644 src/plugins/saved_objects_management/server/routes/bulk_delete.ts create mode 100644 test/api_integration/apis/saved_objects_management/bulk_delete.ts diff --git a/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts new file mode 100644 index 0000000000000..a50cedaaba7b3 --- /dev/null +++ b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { HttpStart } from '@kbn/core/public'; +import { SavedObjectError } from '@kbn/core-saved-objects-common'; + +interface SavedObjectDeleteRequest { + id: string; + type: string; +} + +interface SavedObjectDeleteStatus { + id: string; + success: boolean; + type: string; + error?: SavedObjectError; +} + +export function bulkDeleteObjects( + http: HttpStart, + objects: SavedObjectDeleteRequest[] +): Promise { + return http.post('/api/kibana/management/saved_objects/_bulk_delete', { + body: JSON.stringify(objects), + }); +} diff --git a/src/plugins/saved_objects_management/public/lib/index.ts b/src/plugins/saved_objects_management/public/lib/index.ts index 258387c39ecd9..1833e4e75f327 100644 --- a/src/plugins/saved_objects_management/public/lib/index.ts +++ b/src/plugins/saved_objects_management/public/lib/index.ts @@ -18,6 +18,7 @@ export type { ProcessedImportResponse, FailedImport } from './process_import_res export { processImportResponse } from './process_import_response'; export { getDefaultTitle } from './get_default_title'; export { findObjects } from './find_objects'; +export { bulkDeleteObjects } from './bulk_delete_objects'; export { bulkGetObjects } from './bulk_get_objects'; export type { SavedObjectsExportResultDetails } from './extract_export_details'; export { extractExportDetails } from './extract_export_details'; diff --git a/src/plugins/saved_objects_management/server/routes/bulk_delete.ts b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts new file mode 100644 index 0000000000000..e33aef5ee4113 --- /dev/null +++ b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { schema } from '@kbn/config-schema'; +import { IRouter } from '@kbn/core/server'; + +export const registerBulkDeleteRoute = (router: IRouter) => { + router.post( + { + path: '/api/kibana/management/saved_objects/_bulk_delete', + validate: { + body: schema.arrayOf( + schema.object({ + type: schema.string(), + id: schema.string(), + }) + ), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const { getClient, typeRegistry } = (await context.core).savedObjects; + + const objects = req.body; + const uniqueTypes = objects.reduce((acc, { type }) => acc.add(type), new Set()); + const includedHiddenTypes = Array.from(uniqueTypes).filter( + (type) => typeRegistry.isHidden(type) && typeRegistry.isImportableAndExportable(type) + ); + + const client = getClient({ includedHiddenTypes }); + const response = await client.bulkDelete(objects, { force: true }); + + return res.ok({ body: response.statuses }); + }) + ); +}; diff --git a/src/plugins/saved_objects_management/server/routes/index.test.ts b/src/plugins/saved_objects_management/server/routes/index.test.ts index 8c640d261d212..cf07f942a83fb 100644 --- a/src/plugins/saved_objects_management/server/routes/index.test.ts +++ b/src/plugins/saved_objects_management/server/routes/index.test.ts @@ -24,7 +24,7 @@ describe('registerRoutes', () => { expect(httpSetup.createRouter).toHaveBeenCalledTimes(1); expect(router.get).toHaveBeenCalledTimes(3); - expect(router.post).toHaveBeenCalledTimes(2); + expect(router.post).toHaveBeenCalledTimes(3); expect(router.get).toHaveBeenCalledWith( expect.objectContaining({ @@ -32,6 +32,12 @@ describe('registerRoutes', () => { }), expect.any(Function) ); + expect(router.post).toHaveBeenCalledWith( + expect.objectContaining({ + path: '/api/kibana/management/saved_objects/_bulk_delete', + }), + expect.any(Function) + ); expect(router.post).toHaveBeenCalledWith( expect.objectContaining({ path: '/api/kibana/management/saved_objects/_bulk_get', diff --git a/src/plugins/saved_objects_management/server/routes/index.ts b/src/plugins/saved_objects_management/server/routes/index.ts index a66a3aa28470f..019cc4cc26db0 100644 --- a/src/plugins/saved_objects_management/server/routes/index.ts +++ b/src/plugins/saved_objects_management/server/routes/index.ts @@ -9,6 +9,7 @@ import { HttpServiceSetup } from '@kbn/core/server'; import { ISavedObjectsManagement } from '../services'; import { registerFindRoute } from './find'; +import { registerBulkDeleteRoute } from './bulk_delete'; import { registerBulkGetRoute } from './bulk_get'; import { registerScrollForCountRoute } from './scroll_count'; import { registerRelationshipsRoute } from './relationships'; @@ -22,6 +23,7 @@ interface RegisterRouteOptions { export function registerRoutes({ http, managementServicePromise }: RegisterRouteOptions) { const router = http.createRouter(); registerFindRoute(router, managementServicePromise); + registerBulkDeleteRoute(router); registerBulkGetRoute(router, managementServicePromise); registerScrollForCountRoute(router); registerRelationshipsRoute(router, managementServicePromise); diff --git a/test/api_integration/apis/saved_objects_management/bulk_delete.ts b/test/api_integration/apis/saved_objects_management/bulk_delete.ts new file mode 100644 index 0000000000000..336cb76c6be86 --- /dev/null +++ b/test/api_integration/apis/saved_objects_management/bulk_delete.ts @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import expect from '@kbn/expect'; +import type { Response } from 'supertest'; +import type { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const kibanaServer = getService('kibanaServer'); + + describe('_bulk_delete', () => { + const endpoint = '/api/kibana/management/saved_objects/_bulk_delete'; + const validObject = { type: 'visualization', id: 'dd7caf20-9efd-11e7-acb3-3dab96693fab' }; + const invalidObject = { type: 'wigwags', id: 'foo' }; + + beforeEach(() => + kibanaServer.importExport.load( + 'test/api_integration/fixtures/kbn_archiver/saved_objects/basic.json' + ) + ); + afterEach(() => + kibanaServer.importExport.unload( + 'test/api_integration/fixtures/kbn_archiver/saved_objects/basic.json' + ) + ); + + function expectSuccess(index: number, { body }: Response) { + const { type, id, error } = body[index]; + expect(type).to.eql(validObject.type); + expect(id).to.eql(validObject.id); + expect(error).to.equal(undefined); + } + + function expectBadRequest(index: number, { body }: Response) { + const { type, id, error } = body[index]; + expect(type).to.eql(invalidObject.type); + expect(id).to.eql(invalidObject.id); + expect(error).to.eql({ + message: `Unsupported saved object type: '${invalidObject.type}': Bad Request`, + statusCode: 400, + error: 'Bad Request', + }); + } + + it('should return 200 for an existing object', async () => + await supertest + .post(endpoint) + .send([validObject]) + .expect(200) + .then((response: Response) => { + expect(response.body).to.have.length(1); + expectSuccess(0, response); + })); + + it('should return error for invalid object type', async () => + await supertest + .post(endpoint) + .send([invalidObject]) + .expect(200) + .then((response: Response) => { + expect(response.body).to.have.length(1); + expectBadRequest(0, response); + })); + + it('should return mix of successes and errors', async () => + await supertest + .post(endpoint) + .send([validObject, invalidObject]) + .expect(200) + .then((response: Response) => { + expect(response.body).to.have.length(2); + expectSuccess(0, response); + expectBadRequest(1, response); + })); + }); +} diff --git a/test/api_integration/apis/saved_objects_management/index.ts b/test/api_integration/apis/saved_objects_management/index.ts index 208ded1d50706..2adf4524a37f7 100644 --- a/test/api_integration/apis/saved_objects_management/index.ts +++ b/test/api_integration/apis/saved_objects_management/index.ts @@ -11,6 +11,7 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('saved objects management apis', () => { loadTestFile(require.resolve('./find')); + loadTestFile(require.resolve('./bulk_delete')); loadTestFile(require.resolve('./bulk_get')); loadTestFile(require.resolve('./relationships')); loadTestFile(require.resolve('./scroll_count')); From 8c19806df93ed26cd94ed87ca91780647dbcb76b Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Thu, 5 Jan 2023 12:34:12 +0100 Subject: [PATCH 2/8] Update single saved object view to use deletion endpoint --- .../saved_object_view.test.mocks.ts | 5 + .../object_view/saved_object_view.test.tsx | 141 ++++++++---------- .../object_view/saved_object_view.tsx | 35 ++++- .../saved_objects_edition_page.tsx | 1 - 4 files changed, 92 insertions(+), 90 deletions(-) diff --git a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.mocks.ts b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.mocks.ts index 7243955100690..57cb3bb07fdd3 100644 --- a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.mocks.ts +++ b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.mocks.ts @@ -23,3 +23,8 @@ export const bulkGetObjectsMock = jest.fn(); jest.doMock('../../lib/bulk_get_objects', () => ({ bulkGetObjects: bulkGetObjectsMock, })); + +export const bulkDeleteObjectsMock = jest.fn(); +jest.doMock('../../lib/bulk_delete_objects', () => ({ + bulkDeleteObjects: bulkDeleteObjectsMock, +})); diff --git a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.tsx b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.tsx index fd7d71a39dcd2..9bbbfb98a6298 100644 --- a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.tsx +++ b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.test.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { bulkGetObjectsMock } from './saved_object_view.test.mocks'; +import { bulkDeleteObjectsMock, bulkGetObjectsMock } from './saved_object_view.test.mocks'; import React from 'react'; import { ShallowWrapper } from 'enzyme'; @@ -16,13 +16,13 @@ import { httpServiceMock, overlayServiceMock, notificationServiceMock, - savedObjectsServiceMock, applicationServiceMock, uiSettingsServiceMock, scopedHistoryMock, docLinksServiceMock, } from '@kbn/core/public/mocks'; +import type { SavedObjectWithMetadata } from '../../types'; import { SavedObjectEdition, SavedObjectEditionProps, @@ -36,7 +36,6 @@ describe('SavedObjectEdition', () => { let http: ReturnType; let overlays: ReturnType; let notifications: ReturnType; - let savedObjects: ReturnType; let uiSettings: ReturnType; let history: ReturnType; let applications: ReturnType; @@ -56,7 +55,6 @@ describe('SavedObjectEdition', () => { http = httpServiceMock.createStartContract(); overlays = overlayServiceMock.createStartContract(); notifications = notificationServiceMock.createStartContract(); - savedObjects = savedObjectsServiceMock.createStartContract(); uiSettings = uiSettingsServiceMock.createStartContract(); history = scopedHistoryMock.create(); docLinks = docLinksServiceMock.createStartContract(); @@ -81,35 +79,32 @@ describe('SavedObjectEdition', () => { capabilities: applications.capabilities, overlays, notifications, - savedObjectsClient: savedObjects.client, history, uiSettings, docLinks: docLinks.links, }; - bulkGetObjectsMock.mockImplementation(() => [{}]); + bulkDeleteObjectsMock.mockResolvedValue([{}]); }); it('should render normally', async () => { - bulkGetObjectsMock.mockImplementation(() => - Promise.resolve([ - { - id: '1', - type: 'dashboard', - attributes: { - title: `MyDashboard*`, - }, - meta: { - title: `MyDashboard*`, - icon: 'dashboardApp', - inAppUrl: { - path: '/app/dashboards#/view/1', - uiCapabilitiesPath: 'management.kibana.dashboard', - }, + bulkGetObjectsMock.mockResolvedValue([ + { + id: '1', + type: 'dashboard', + attributes: { + title: `MyDashboard*`, + }, + meta: { + title: `MyDashboard*`, + icon: 'dashboardApp', + inAppUrl: { + path: '/app/dashboards#/view/1', + uiCapabilitiesPath: 'management.kibana.dashboard', }, }, - ]) - ); + } as SavedObjectWithMetadata, + ]); const component = shallowRender(); // Ensure all promises resolve await resolvePromises(); @@ -119,15 +114,15 @@ describe('SavedObjectEdition', () => { }); it('should add danger toast when bulk get fails', async () => { - bulkGetObjectsMock.mockImplementation(() => - Promise.resolve([ - { - error: { - message: 'Not found', - }, + bulkGetObjectsMock.mockResolvedValue([ + { + error: { + error: '', + message: 'Not found', + statusCode: 404, }, - ]) - ); + } as SavedObjectWithMetadata, + ]); const component = shallowRender({ notFoundType: 'does_not_exist' }); await resolvePromises(); @@ -165,8 +160,8 @@ describe('SavedObjectEdition', () => { }, hiddenType: false, }, - }; - bulkGetObjectsMock.mockImplementation(() => Promise.resolve([savedObjectItem])); + } as SavedObjectWithMetadata; + bulkGetObjectsMock.mockResolvedValue([savedObjectItem]); applications.capabilities = { navLinks: {}, management: {}, @@ -232,14 +227,9 @@ describe('SavedObjectEdition', () => { }, hiddenType: false, }, - }; + } as SavedObjectWithMetadata; - it('should display a confirmation message on deleting the saved object', async () => { - bulkGetObjectsMock.mockImplementation(() => Promise.resolve([savedObjectItem])); - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - delete: jest.fn().mockImplementation(() => ({})), - }; + beforeEach(() => { applications.capabilities = { navLinks: {}, management: {}, @@ -250,13 +240,13 @@ describe('SavedObjectEdition', () => { delete: true, }, }; + }); + + it('should display a confirmation message on deleting the saved object', async () => { + bulkGetObjectsMock.mockResolvedValue([savedObjectItem]); overlays.openConfirm.mockResolvedValue(false); - const component = shallowRender({ - capabilities: applications.capabilities, - savedObjectsClient: mockSavedObjectsClient, - overlays, - }); + const component = shallowRender(); await resolvePromises(); component.update(); @@ -272,28 +262,10 @@ describe('SavedObjectEdition', () => { }); it('should route back if action is confirm and user accepted', async () => { - bulkGetObjectsMock.mockImplementation(() => Promise.resolve([savedObjectItem])); - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - delete: jest.fn().mockImplementation(() => ({})), - }; - applications.capabilities = { - navLinks: {}, - management: {}, - catalogue: {}, - savedObjectsManagement: { - read: true, - edit: false, - delete: true, - }, - }; + bulkGetObjectsMock.mockResolvedValue([savedObjectItem]); overlays.openConfirm.mockResolvedValue(true); - const component = shallowRender({ - capabilities: applications.capabilities, - savedObjectsClient: mockSavedObjectsClient, - overlays, - }); + const component = shallowRender(); await resolvePromises(); component.update(); @@ -303,27 +275,34 @@ describe('SavedObjectEdition', () => { }); it('should not enable delete if the saved object is hidden', async () => { - bulkGetObjectsMock.mockImplementation(() => - Promise.resolve([{ ...savedObjectItem, meta: { hiddenType: true } }]) - ); - applications.capabilities = { - navLinks: {}, - management: {}, - catalogue: {}, - savedObjectsManagement: { - read: true, - edit: false, - delete: true, - }, - }; - const component = shallowRender({ - capabilities: applications.capabilities, - }); + bulkGetObjectsMock.mockResolvedValue([{ ...savedObjectItem, meta: { hiddenType: true } }]); + const component = shallowRender(); await resolvePromises(); component.update(); expect(component.find('Header').prop('canDelete')).toBe(false); }); + + it('should show a danger toast when bulk deletion fails', async () => { + bulkGetObjectsMock.mockResolvedValue([savedObjectItem]); + bulkDeleteObjectsMock.mockResolvedValue([ + { + error: { message: 'Something went wrong.' }, + success: false, + }, + ]); + + const component = shallowRender(); + await resolvePromises(); + + component.update(); + await component.instance().delete(); + expect(notifications.toasts.addDanger).toHaveBeenCalledWith( + expect.objectContaining({ + text: 'Something went wrong.', + }) + ); + }); }); }); diff --git a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx index 6a695858fca35..85d7fb7fc8790 100644 --- a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx +++ b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx @@ -13,7 +13,6 @@ import { get } from 'lodash'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { Capabilities, - SavedObjectsClientContract, OverlayStart, NotificationsStart, ScopedHistory, @@ -22,7 +21,7 @@ import { DocLinksStart, } from '@kbn/core/public'; import { Header, Inspect, NotFoundErrors } from './components'; -import { bulkGetObjects } from '../../lib/bulk_get_objects'; +import { bulkDeleteObjects, bulkGetObjects } from '../../lib'; import { SavedObjectWithMetadata } from '../../types'; import './saved_object_view.scss'; export interface SavedObjectEditionProps { @@ -33,7 +32,6 @@ export interface SavedObjectEditionProps { overlays: OverlayStart; notifications: NotificationsStart; notFoundType?: string; - savedObjectsClient: SavedObjectsClientContract; history: ScopedHistory; uiSettings: IUiSettingsClient; docLinks: DocLinksStart['links']; @@ -129,7 +127,7 @@ export class SavedObjectEdition extends Component< } async delete() { - const { id, savedObjectsClient, overlays, notifications } = this.props; + const { http, id, overlays, notifications } = this.props; const { type, object } = this.state; const confirmed = await overlays.openConfirm( @@ -152,11 +150,32 @@ export class SavedObjectEdition extends Component< buttonColor: 'danger', } ); - if (confirmed) { - await savedObjectsClient.delete(type, id); - notifications.toasts.addSuccess(`Deleted '${object!.attributes.title}' ${type} object`); - this.redirectToListing(); + if (!confirmed) { + return; } + + const [{ success, error }] = await bulkDeleteObjects(http, [{ id, type }]); + + if (!success) { + notifications.toasts.addDanger({ + title: i18n.translate( + 'savedObjectsManagement.objectView.unableDeleteSavedObjectNotificationMessage', + { + defaultMessage: `Failed to delete '{title}' {type} object`, + values: { + type, + title: object!.attributes.title, + }, + } + ), + text: error?.message, + }); + + return; + } + + notifications.toasts.addSuccess(`Deleted '${object!.attributes.title}' ${type} object`); + this.redirectToListing(); } redirectToListing() { diff --git a/src/plugins/saved_objects_management/public/management_section/saved_objects_edition_page.tsx b/src/plugins/saved_objects_management/public/management_section/saved_objects_edition_page.tsx index f77c7bfed86d6..a3ed60fe32808 100644 --- a/src/plugins/saved_objects_management/public/management_section/saved_objects_edition_page.tsx +++ b/src/plugins/saved_objects_management/public/management_section/saved_objects_edition_page.tsx @@ -57,7 +57,6 @@ const SavedObjectsEditionPage = ({ id={id} savedObjectType={type} http={coreStart.http} - savedObjectsClient={coreStart.savedObjects.client} overlays={coreStart.overlays} notifications={coreStart.notifications} capabilities={capabilities} From 2ec3f0ee0dac503797f38de1a77d941c688eda9b Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Thu, 5 Jan 2023 18:32:11 +0100 Subject: [PATCH 3/8] Update saved objects table to use deletion endpoint --- .../saved_objects_table.test.mocks.ts | 10 ++ .../saved_objects_table.test.tsx | 100 +++++++----------- .../objects_table/saved_objects_table.tsx | 44 +++++--- .../saved_objects_table_page.tsx | 1 - 4 files changed, 78 insertions(+), 77 deletions(-) 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 8c6b3db0bce94..e5ece8b707cf1 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 @@ -58,3 +58,13 @@ export const getRelationshipsMock = jest.fn(); jest.doMock('../../lib/get_relationships', () => ({ getRelationships: getRelationshipsMock, })); + +export const bulkGetObjectsMock = jest.fn(); +jest.doMock('../../lib/bulk_get_objects', () => ({ + bulkGetObjects: bulkGetObjectsMock, +})); + +export const bulkDeleteObjectsMock = jest.fn(); +jest.doMock('../../lib/bulk_delete_objects', () => ({ + bulkDeleteObjects: bulkDeleteObjectsMock, +})); 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 28a31890664b9..e8b875d943214 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 @@ -7,6 +7,8 @@ */ import { + bulkDeleteObjectsMock, + bulkGetObjectsMock, extractExportDetailsMock, fetchExportByTypeAndSearchMock, fetchExportObjectsMock, @@ -17,6 +19,7 @@ import { } from './saved_objects_table.test.mocks'; import React from 'react'; +import { pick } from 'lodash'; import { Query } from '@elastic/eui'; import { ShallowWrapper } from 'enzyme'; import { shallowWithI18nProvider } from '@kbn/test-jest-helpers'; @@ -24,7 +27,6 @@ import { httpServiceMock, overlayServiceMock, notificationServiceMock, - savedObjectsServiceMock, applicationServiceMock, } from '@kbn/core/public/mocks'; import { dataPluginMock } from '@kbn/data-plugin/public/mocks'; @@ -85,7 +87,6 @@ describe('SavedObjectsTable', () => { let http: ReturnType; let overlays: ReturnType; let notifications: ReturnType; - let savedObjects: ReturnType; let search: ReturnType['search']; const shallowRender = (overrides: Partial = {}) => { @@ -104,7 +105,6 @@ describe('SavedObjectsTable', () => { http = httpServiceMock.createStartContract(); overlays = overlayServiceMock.createStartContract(); notifications = notificationServiceMock.createStartContract(); - savedObjects = savedObjectsServiceMock.createStartContract(); search = dataPluginMock.createStartContract().search; const applications = applicationServiceMock.createStartContract(); @@ -132,7 +132,6 @@ describe('SavedObjectsTable', () => { allowedTypes, actionRegistry: actionServiceMock.createStart(), columnRegistry: columnServiceMock.createStart(), - savedObjectsClient: savedObjects.client, dataViews: dataViewPluginMocks.createStartContract(), http, overlays, @@ -236,15 +235,9 @@ describe('SavedObjectsTable', () => { _id: obj.id, _source: {}, })); + bulkGetObjectsMock.mockResolvedValue(mockSavedObjects); - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - bulkGet: jest.fn().mockImplementation(() => ({ - savedObjects: mockSavedObjects, - })), - }; - - const component = shallowRender({ savedObjectsClient: mockSavedObjectsClient }); + const component = shallowRender(); // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); @@ -272,13 +265,7 @@ describe('SavedObjectsTable', () => { _id: obj.id, _source: {}, })); - - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - bulkGet: jest.fn().mockImplementation(() => ({ - savedObjects: mockSavedObjects, - })), - }; + bulkGetObjectsMock.mockResolvedValue(mockSavedObjects); extractExportDetailsMock.mockImplementation(() => ({ exportedCount: 2, @@ -288,7 +275,7 @@ describe('SavedObjectsTable', () => { excludedObjects: [], })); - const component = shallowRender({ savedObjectsClient: mockSavedObjectsClient }); + const component = shallowRender(); // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); @@ -319,13 +306,7 @@ describe('SavedObjectsTable', () => { _id: obj.id, _source: {}, })); - - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - bulkGet: jest.fn().mockImplementation(() => ({ - savedObjects: mockSavedObjects, - })), - }; + bulkGetObjectsMock.mockResolvedValue(mockSavedObjects); extractExportDetailsMock.mockImplementation(() => ({ exportedCount: 2, @@ -335,7 +316,7 @@ describe('SavedObjectsTable', () => { excludedObjects: [{ id: '7', type: 'visualisation' }], })); - const component = shallowRender({ savedObjectsClient: mockSavedObjectsClient }); + const component = shallowRender(); // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); @@ -560,16 +541,13 @@ describe('SavedObjectsTable', () => { type: obj.type, source: {}, })); + bulkGetObjectsMock.mockResolvedValue(mockSavedObjects); + bulkDeleteObjectsMock.mockResolvedValueOnce([ + { id: '1', type: 'index-pattern', success: true }, + { id: '3', type: 'dashboard', success: true }, + ]); - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - bulkGet: jest.fn().mockImplementation(() => ({ - savedObjects: mockSavedObjects, - })), - delete: jest.fn(), - }; - - const component = shallowRender({ savedObjectsClient: mockSavedObjectsClient }); + const component = shallowRender(); // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); @@ -582,23 +560,20 @@ describe('SavedObjectsTable', () => { await component.instance().delete(); expect(defaultProps.dataViews.clearCache).toHaveBeenCalled(); - expect(mockSavedObjectsClient.delete).toHaveBeenCalledWith( - mockSavedObjects[0].type, - mockSavedObjects[0].id, - { force: true } - ); - expect(mockSavedObjectsClient.delete).toHaveBeenCalledWith( - mockSavedObjects[1].type, - mockSavedObjects[1].id, - { force: true } + expect(bulkDeleteObjectsMock).toHaveBeenCalledWith( + expect.anything(), + expect.arrayContaining([ + expect.objectContaining(pick(mockSavedObjects[0], 'id', 'type')), + expect.objectContaining(pick(mockSavedObjects[1], 'id', 'type')), + ]) ); expect(component.state('selectedSavedObjects').length).toBe(0); }); - it('should not delete hidden selected objects', async () => { + it('should show a notification when deletion failed', async () => { const mockSelectedSavedObjects = [ { id: '1', type: 'index-pattern', meta: {} }, - { id: '3', type: 'hidden-type', meta: { hiddenType: true } }, + { id: '3', type: 'hidden-type', meta: {} }, ] as SavedObjectWithMetadata[]; const mockSavedObjects = mockSelectedSavedObjects.map((obj) => ({ @@ -606,16 +581,18 @@ describe('SavedObjectsTable', () => { type: obj.type, source: {}, })); + bulkGetObjectsMock.mockResolvedValue(mockSavedObjects); + bulkDeleteObjectsMock.mockResolvedValueOnce([ + { id: '1', type: 'index-pattern', success: true }, + { + id: '3', + type: 'hidden-type', + success: false, + error: { message: 'Something went wrong.' }, + }, + ]); - const mockSavedObjectsClient = { - ...defaultProps.savedObjectsClient, - bulkGet: jest.fn().mockImplementation(() => ({ - savedObjects: mockSavedObjects, - })), - delete: jest.fn(), - }; - - const component = shallowRender({ savedObjectsClient: mockSavedObjectsClient }); + const component = shallowRender(); // Ensure all promises resolve await new Promise((resolve) => process.nextTick(resolve)); @@ -628,10 +605,11 @@ describe('SavedObjectsTable', () => { await component.instance().delete(); expect(defaultProps.dataViews.clearCache).toHaveBeenCalled(); - expect(mockSavedObjectsClient.delete).toHaveBeenCalledTimes(1); - expect(mockSavedObjectsClient.delete).toHaveBeenCalledWith('index-pattern', '1', { - force: true, - }); + expect(notifications.toasts.addDanger).toHaveBeenCalledWith( + expect.objectContaining({ + text: 'Something went wrong.', + }) + ); }); }); }); 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 e548391b53b5d..5068af34a5302 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 @@ -7,18 +7,12 @@ */ import React, { Component } from 'react'; -import { debounce } from 'lodash'; +import { debounce, matches } from 'lodash'; // @ts-expect-error import { saveAs } from '@elastic/filesaver'; import { EuiSpacer, Query, CriteriaWithPagination } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { - SavedObjectsClientContract, - HttpStart, - OverlayStart, - NotificationsStart, - ApplicationStart, -} from '@kbn/core/public'; +import { HttpStart, OverlayStart, NotificationsStart, ApplicationStart } from '@kbn/core/public'; import type { SavedObjectsFindOptions } from '@kbn/core-saved-objects-api-server'; import { RedirectAppLinks } from '@kbn/kibana-react-plugin/public'; import { SavedObjectsTaggingApi } from '@kbn/saved-objects-tagging-oss-plugin/public'; @@ -32,6 +26,7 @@ import { fetchExportObjects, fetchExportByTypeAndSearch, findObjects, + bulkDeleteObjects, bulkGetObjects, extractExportDetails, SavedObjectsExportResultDetails, @@ -60,7 +55,6 @@ export interface SavedObjectsTableProps { allowedTypes: SavedObjectManagementTypeInfo[]; actionRegistry: SavedObjectsManagementActionServiceStart; columnRegistry: SavedObjectsManagementColumnServiceStart; - savedObjectsClient: SavedObjectsClientContract; dataViews: DataViewsContract; taggingApi?: SavedObjectsTaggingApi; http: HttpStart; @@ -507,7 +501,7 @@ export class SavedObjectsTable extends Component { - const { savedObjectsClient } = this.props; + const { http, notifications } = this.props; const { selectedSavedObjects, isDeleting } = this.state; if (isDeleting) { @@ -521,14 +515,34 @@ export class SavedObjectsTable extends Component !object.meta.hiddenType) - .map((object) => savedObjectsClient.delete(object.type, object.id, { force: true })); - await Promise.all(deletes); + const deleteStatus = await bulkDeleteObjects( + http, + selectedSavedObjects.map(({ id, type }) => ({ id, type })) + ); + + deleteStatus + .filter(({ success }) => !success) + .forEach(({ id, type, error }) => { + notifications.toasts.addDanger({ + title: i18n.translate( + 'savedObjectsManagement.objectView.unableDeleteSavedObjectNotificationMessage', + { + defaultMessage: `Failed to delete '{title}' {type} object`, + values: { + type, + title: selectedSavedObjects.find(matches({ id, type }))?.meta?.title, + }, + } + ), + text: error?.message, + }); + }); // Unset this this.setState({ - selectedSavedObjects: [], + selectedSavedObjects: selectedSavedObjects.filter(({ id, type }) => + deleteStatus.some(matches({ id, type, success: false })) + ), }); // Fetching all data diff --git a/src/plugins/saved_objects_management/public/management_section/saved_objects_table_page.tsx b/src/plugins/saved_objects_management/public/management_section/saved_objects_table_page.tsx index dac81d87730c0..ec083bf2f5c00 100644 --- a/src/plugins/saved_objects_management/public/management_section/saved_objects_table_page.tsx +++ b/src/plugins/saved_objects_management/public/management_section/saved_objects_table_page.tsx @@ -83,7 +83,6 @@ const SavedObjectsTablePage = ({ actionRegistry={actionRegistry} columnRegistry={columnRegistry} taggingApi={taggingApi} - savedObjectsClient={coreStart.savedObjects.client} dataViews={dataViewsApi} search={dataStart.search} http={coreStart.http} From 8d3f3ca7a724866f3371e171d7f84d2426546d7e Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Mon, 9 Jan 2023 16:42:12 +0100 Subject: [PATCH 4/8] Prevent bulk deletion endpoint from deleting hidden types --- .../server/routes/bulk_delete.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/plugins/saved_objects_management/server/routes/bulk_delete.ts b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts index e33aef5ee4113..5b646d0b4c392 100644 --- a/src/plugins/saved_objects_management/server/routes/bulk_delete.ts +++ b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts @@ -23,15 +23,10 @@ export const registerBulkDeleteRoute = (router: IRouter) => { }, }, router.handleLegacyErrors(async (context, req, res) => { - const { getClient, typeRegistry } = (await context.core).savedObjects; + const { getClient } = (await context.core).savedObjects; const objects = req.body; - const uniqueTypes = objects.reduce((acc, { type }) => acc.add(type), new Set()); - const includedHiddenTypes = Array.from(uniqueTypes).filter( - (type) => typeRegistry.isHidden(type) && typeRegistry.isImportableAndExportable(type) - ); - - const client = getClient({ includedHiddenTypes }); + const client = getClient(); const response = await client.bulkDelete(objects, { force: true }); return res.ok({ body: response.statuses }); From 81e2700b4c1bd74b3a9b752af0d70a1624d16fd5 Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Mon, 17 Jan 2022 21:22:34 +0100 Subject: [PATCH 5/8] Fix deletion in the saved objects table not to attempt to delete hidden objects --- .../objects_table/saved_objects_table.test.tsx | 1 + .../management_section/objects_table/saved_objects_table.tsx | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) 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 e8b875d943214..6df065d3ae737 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 @@ -534,6 +534,7 @@ describe('SavedObjectsTable', () => { const mockSelectedSavedObjects = [ { id: '1', type: 'index-pattern', meta: {} }, { id: '3', type: 'dashboard', meta: {} }, + { id: '4', type: 'dashboard', meta: { hiddenType: false } }, ] as SavedObjectWithMetadata[]; const mockSavedObjects = mockSelectedSavedObjects.map((obj) => ({ 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 5068af34a5302..5eb98c2f37e9d 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 @@ -517,7 +517,9 @@ export class SavedObjectsTable extends Component ({ id, type })) + selectedSavedObjects + .filter((object) => !object.meta.hiddenType) + .map(({ id, type }) => ({ id, type })) ); deleteStatus From eeeca57e43b56bca132469874ca1942a0011b0fe Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Wed, 18 Jan 2023 16:51:27 +0100 Subject: [PATCH 6/8] Update toast to show a number of deleted objects instead of error messages per each object --- .../saved_objects_table.test.tsx | 4 +-- .../objects_table/saved_objects_table.tsx | 25 ++++++------------- 2 files changed, 10 insertions(+), 19 deletions(-) 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 6df065d3ae737..e08d0cde6c7cd 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 @@ -606,9 +606,9 @@ describe('SavedObjectsTable', () => { await component.instance().delete(); expect(defaultProps.dataViews.clearCache).toHaveBeenCalled(); - expect(notifications.toasts.addDanger).toHaveBeenCalledWith( + expect(notifications.toasts.addInfo).toHaveBeenCalledWith( expect.objectContaining({ - text: 'Something went wrong.', + title: expect.stringContaining('1 object.'), }) ); }); 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 5eb98c2f37e9d..0ecd3e0d78a06 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 @@ -522,23 +522,14 @@ export class SavedObjectsTable extends Component ({ id, type })) ); - deleteStatus - .filter(({ success }) => !success) - .forEach(({ id, type, error }) => { - notifications.toasts.addDanger({ - title: i18n.translate( - 'savedObjectsManagement.objectView.unableDeleteSavedObjectNotificationMessage', - { - defaultMessage: `Failed to delete '{title}' {type} object`, - values: { - type, - title: selectedSavedObjects.find(matches({ id, type }))?.meta?.title, - }, - } - ), - text: error?.message, - }); - }); + notifications.toasts.addInfo({ + title: i18n.translate('savedObjectsManagement.objectsTable.delete.successNotification', { + defaultMessage: `Successfully deleted {count, plural, one {# object} other {# objects}}.`, + values: { + count: deleteStatus.filter(({ success }) => !!success).length, + }, + }), + }); // Unset this this.setState({ From 2e4e4b6a81b09b4bac266a2231597882cd9a4580 Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Wed, 18 Jan 2023 16:57:46 +0100 Subject: [PATCH 7/8] Rename bulk delete API endpoint to internal --- .../public/lib/bulk_delete_objects.ts | 9 ++++++--- .../server/routes/bulk_delete.ts | 2 +- .../saved_objects_management/server/routes/index.test.ts | 2 +- .../apis/saved_objects_management/bulk_delete.ts | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts index a50cedaaba7b3..19a572b5d7a12 100644 --- a/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts +++ b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts @@ -25,7 +25,10 @@ export function bulkDeleteObjects( http: HttpStart, objects: SavedObjectDeleteRequest[] ): Promise { - return http.post('/api/kibana/management/saved_objects/_bulk_delete', { - body: JSON.stringify(objects), - }); + return http.post( + '/internal/kibana/management/saved_objects/_bulk_delete', + { + body: JSON.stringify(objects), + } + ); } diff --git a/src/plugins/saved_objects_management/server/routes/bulk_delete.ts b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts index 5b646d0b4c392..e3f4db044ef2f 100644 --- a/src/plugins/saved_objects_management/server/routes/bulk_delete.ts +++ b/src/plugins/saved_objects_management/server/routes/bulk_delete.ts @@ -12,7 +12,7 @@ import { IRouter } from '@kbn/core/server'; export const registerBulkDeleteRoute = (router: IRouter) => { router.post( { - path: '/api/kibana/management/saved_objects/_bulk_delete', + path: '/internal/kibana/management/saved_objects/_bulk_delete', validate: { body: schema.arrayOf( schema.object({ diff --git a/src/plugins/saved_objects_management/server/routes/index.test.ts b/src/plugins/saved_objects_management/server/routes/index.test.ts index cf07f942a83fb..d9c261cabce0f 100644 --- a/src/plugins/saved_objects_management/server/routes/index.test.ts +++ b/src/plugins/saved_objects_management/server/routes/index.test.ts @@ -34,7 +34,7 @@ describe('registerRoutes', () => { ); expect(router.post).toHaveBeenCalledWith( expect.objectContaining({ - path: '/api/kibana/management/saved_objects/_bulk_delete', + path: '/internal/kibana/management/saved_objects/_bulk_delete', }), expect.any(Function) ); diff --git a/test/api_integration/apis/saved_objects_management/bulk_delete.ts b/test/api_integration/apis/saved_objects_management/bulk_delete.ts index 336cb76c6be86..8a9f582823de8 100644 --- a/test/api_integration/apis/saved_objects_management/bulk_delete.ts +++ b/test/api_integration/apis/saved_objects_management/bulk_delete.ts @@ -15,7 +15,7 @@ export default function ({ getService }: FtrProviderContext) { const kibanaServer = getService('kibanaServer'); describe('_bulk_delete', () => { - const endpoint = '/api/kibana/management/saved_objects/_bulk_delete'; + const endpoint = '/internal/kibana/management/saved_objects/_bulk_delete'; const validObject = { type: 'visualization', id: 'dd7caf20-9efd-11e7-acb3-3dab96693fab' }; const invalidObject = { type: 'wigwags', id: 'foo' }; From e8eb77298bc53afaa4bbc6c010e193eff177eac6 Mon Sep 17 00:00:00 2001 From: Michael Dokolin Date: Wed, 18 Jan 2023 21:45:54 +0100 Subject: [PATCH 8/8] Fix used saved objects types --- .../public/lib/bulk_delete_objects.ts | 9 ++------- .../management_section/object_view/saved_object_view.tsx | 7 +++---- src/plugins/saved_objects_management/tsconfig.json | 1 + 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts index 19a572b5d7a12..30a02f8fa42aa 100644 --- a/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts +++ b/src/plugins/saved_objects_management/public/lib/bulk_delete_objects.ts @@ -7,12 +7,7 @@ */ import { HttpStart } from '@kbn/core/public'; -import { SavedObjectError } from '@kbn/core-saved-objects-common'; - -interface SavedObjectDeleteRequest { - id: string; - type: string; -} +import { SavedObjectError, SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common'; interface SavedObjectDeleteStatus { id: string; @@ -23,7 +18,7 @@ interface SavedObjectDeleteStatus { export function bulkDeleteObjects( http: HttpStart, - objects: SavedObjectDeleteRequest[] + objects: SavedObjectTypeIdTuple[] ): Promise { return http.post( '/internal/kibana/management/saved_objects/_bulk_delete', diff --git a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx index 85d7fb7fc8790..e5e62fbf95c92 100644 --- a/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx +++ b/src/plugins/saved_objects_management/public/management_section/object_view/saved_object_view.tsx @@ -144,7 +144,7 @@ export class SavedObjectEdition extends Component< title: i18n.translate('savedObjectsManagement.deleteConfirm.modalTitle', { defaultMessage: `Delete '{title}'?`, values: { - title: object?.attributes?.title || 'saved Kibana object', + title: object?.meta?.title || 'saved Kibana object', }, }), buttonColor: 'danger', @@ -155,7 +155,6 @@ export class SavedObjectEdition extends Component< } const [{ success, error }] = await bulkDeleteObjects(http, [{ id, type }]); - if (!success) { notifications.toasts.addDanger({ title: i18n.translate( @@ -164,7 +163,7 @@ export class SavedObjectEdition extends Component< defaultMessage: `Failed to delete '{title}' {type} object`, values: { type, - title: object!.attributes.title, + title: object?.meta?.title, }, } ), @@ -174,7 +173,7 @@ export class SavedObjectEdition extends Component< return; } - notifications.toasts.addSuccess(`Deleted '${object!.attributes.title}' ${type} object`); + notifications.toasts.addSuccess(`Deleted '${object?.meta?.title}' ${type} object`); this.redirectToListing(); } diff --git a/src/plugins/saved_objects_management/tsconfig.json b/src/plugins/saved_objects_management/tsconfig.json index cad061f1a27e0..aa1449d4a9c2e 100644 --- a/src/plugins/saved_objects_management/tsconfig.json +++ b/src/plugins/saved_objects_management/tsconfig.json @@ -22,6 +22,7 @@ "@kbn/i18n-react", "@kbn/test-jest-helpers", "@kbn/core-saved-objects-api-server", + "@kbn/core-saved-objects-common", "@kbn/monaco", "@kbn/config-schema", ],