From 9e25f6ae25c1efb7691a63e741402478a3cd080f Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Thu, 25 Jul 2019 18:26:16 -0400 Subject: [PATCH] Change alert and action HTTP DELETE to return 204 (#41933) * Change alert and action HTTP DELETE to return 204 Prior to this, a 200 was being returned, with some kind of body. That doesn't seem to be the usual response, eg the [Delete space][] API returns a 204 with no body. [Delete space]: https://www.elastic.co/guide/en/kibana/master/spaces-api-delete.html * resolve PR comments --- x-pack/legacy/plugins/actions/server/routes/delete.test.ts | 5 ++--- x-pack/legacy/plugins/actions/server/routes/delete.ts | 5 +++-- x-pack/legacy/plugins/alerting/server/routes/delete.test.ts | 5 ++--- x-pack/legacy/plugins/alerting/server/routes/delete.ts | 5 +++-- x-pack/test/api_integration/apis/actions/delete.ts | 4 ++-- x-pack/test/api_integration/apis/alerting/alerts.ts | 2 +- x-pack/test/api_integration/apis/alerting/create.ts | 2 +- x-pack/test/api_integration/apis/alerting/delete.ts | 4 ++-- x-pack/test/api_integration/apis/alerting/disable.ts | 2 +- x-pack/test/api_integration/apis/alerting/enable.ts | 2 +- x-pack/test/api_integration/apis/alerting/find.ts | 2 +- x-pack/test/api_integration/apis/alerting/get.ts | 2 +- x-pack/test/api_integration/apis/alerting/update.ts | 2 +- 13 files changed, 21 insertions(+), 21 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/routes/delete.test.ts b/x-pack/legacy/plugins/actions/server/routes/delete.test.ts index 37c7f3b2c89a9..a655b804f397f 100644 --- a/x-pack/legacy/plugins/actions/server/routes/delete.test.ts +++ b/x-pack/legacy/plugins/actions/server/routes/delete.test.ts @@ -22,9 +22,8 @@ it('deletes an action with proper parameters', async () => { actionsClient.delete.mockResolvedValueOnce({ success: true }); const { payload, statusCode } = await server.inject(request); - expect(statusCode).toBe(200); - const response = JSON.parse(payload); - expect(response).toEqual({ success: true }); + expect(statusCode).toBe(204); + expect(payload).toEqual(''); expect(actionsClient.delete).toHaveBeenCalledTimes(1); expect(actionsClient.delete.mock.calls[0]).toMatchInlineSnapshot(` Array [ diff --git a/x-pack/legacy/plugins/actions/server/routes/delete.ts b/x-pack/legacy/plugins/actions/server/routes/delete.ts index eed8b7a10cded..6a47b4395d9cd 100644 --- a/x-pack/legacy/plugins/actions/server/routes/delete.ts +++ b/x-pack/legacy/plugins/actions/server/routes/delete.ts @@ -26,10 +26,11 @@ export function deleteRoute(server: Hapi.Server) { .required(), }, }, - async handler(request: DeleteRequest) { + async handler(request: DeleteRequest, h: Hapi.ResponseToolkit) { const { id } = request.params; const actionsClient = request.getActionsClient!(); - return await actionsClient.delete({ id }); + await actionsClient.delete({ id }); + return h.response().code(204); }, }); } diff --git a/x-pack/legacy/plugins/alerting/server/routes/delete.test.ts b/x-pack/legacy/plugins/alerting/server/routes/delete.test.ts index 80cddf4b56ff5..7e3948640034e 100644 --- a/x-pack/legacy/plugins/alerting/server/routes/delete.test.ts +++ b/x-pack/legacy/plugins/alerting/server/routes/delete.test.ts @@ -20,9 +20,8 @@ test('deletes an alert with proper parameters', async () => { alertsClient.delete.mockResolvedValueOnce({}); const { payload, statusCode } = await server.inject(request); - expect(statusCode).toBe(200); - const response = JSON.parse(payload); - expect(response).toEqual({}); + expect(statusCode).toBe(204); + expect(payload).toEqual(''); expect(alertsClient.delete).toHaveBeenCalledTimes(1); expect(alertsClient.delete.mock.calls[0]).toMatchInlineSnapshot(` Array [ diff --git a/x-pack/legacy/plugins/alerting/server/routes/delete.ts b/x-pack/legacy/plugins/alerting/server/routes/delete.ts index 9352bd3726832..7dbb336b3cc1d 100644 --- a/x-pack/legacy/plugins/alerting/server/routes/delete.ts +++ b/x-pack/legacy/plugins/alerting/server/routes/delete.ts @@ -26,10 +26,11 @@ export function deleteAlertRoute(server: Hapi.Server) { .required(), }, }, - async handler(request: DeleteRequest) { + async handler(request: DeleteRequest, h: Hapi.ResponseToolkit) { const { id } = request.params; const alertsClient = request.getAlertsClient!(); - return await alertsClient.delete({ id }); + await alertsClient.delete({ id }); + return h.response().code(204); }, }); } diff --git a/x-pack/test/api_integration/apis/actions/delete.ts b/x-pack/test/api_integration/apis/actions/delete.ts index 2c4a56f3e0fae..92e513ea6f2b6 100644 --- a/x-pack/test/api_integration/apis/actions/delete.ts +++ b/x-pack/test/api_integration/apis/actions/delete.ts @@ -18,11 +18,11 @@ export default function deleteActionTests({ getService }: KibanaFunctionalTestDe beforeEach(() => esArchiver.load('actions/basic')); afterEach(() => esArchiver.unload('actions/basic')); - it('should return 200 when deleting an action', async () => { + it('should return 204 when deleting an action', async () => { await supertest .delete(`/api/action/${ES_ARCHIVER_ACTION_ID}`) .set('kbn-xsrf', 'foo') - .expect(200, {}); + .expect(204, ''); }); it(`should return 404 when action doesn't exist`, async () => { diff --git a/x-pack/test/api_integration/apis/alerting/alerts.ts b/x-pack/test/api_integration/apis/alerting/alerts.ts index 36ef0d8cd70bb..27fabdc3208cc 100644 --- a/x-pack/test/api_integration/apis/alerting/alerts.ts +++ b/x-pack/test/api_integration/apis/alerting/alerts.ts @@ -31,7 +31,7 @@ export default function alertTests({ getService }: KibanaFunctionalTestDefaultPr return supertest .delete(`/api/alert/${id}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); }) ); await esArchiver.unload('actions/basic'); diff --git a/x-pack/test/api_integration/apis/alerting/create.ts b/x-pack/test/api_integration/apis/alerting/create.ts index 9d2b925b8999f..20c1e33f258cc 100644 --- a/x-pack/test/api_integration/apis/alerting/create.ts +++ b/x-pack/test/api_integration/apis/alerting/create.ts @@ -25,7 +25,7 @@ export default function createAlertTests({ getService }: KibanaFunctionalTestDef return supertest .delete(`/api/alert/${id}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); }) ); await esArchiver.unload('actions/basic'); diff --git a/x-pack/test/api_integration/apis/alerting/delete.ts b/x-pack/test/api_integration/apis/alerting/delete.ts index 380d4e6bef635..a287d75c85d4c 100644 --- a/x-pack/test/api_integration/apis/alerting/delete.ts +++ b/x-pack/test/api_integration/apis/alerting/delete.ts @@ -39,11 +39,11 @@ export default function createDeleteTests({ getService }: KibanaFunctionalTestDe }); } - it('should return 200 when deleting an alert and removing scheduled task', async () => { + it('should return 204 when deleting an alert and removing scheduled task', async () => { await supertest .delete(`/api/alert/${alertId}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); let hasThrownError = false; try { await getScheduledTask(scheduledTaskId); diff --git a/x-pack/test/api_integration/apis/alerting/disable.ts b/x-pack/test/api_integration/apis/alerting/disable.ts index 6d8930a887319..44e49f3c2d1b6 100644 --- a/x-pack/test/api_integration/apis/alerting/disable.ts +++ b/x-pack/test/api_integration/apis/alerting/disable.ts @@ -33,7 +33,7 @@ export default function createDisableAlertTests({ await supertest .delete(`/api/alert/${createdAlert.id}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); await esArchiver.unload('actions/basic'); }); diff --git a/x-pack/test/api_integration/apis/alerting/enable.ts b/x-pack/test/api_integration/apis/alerting/enable.ts index 209b184395620..5222ea14b28e7 100644 --- a/x-pack/test/api_integration/apis/alerting/enable.ts +++ b/x-pack/test/api_integration/apis/alerting/enable.ts @@ -33,7 +33,7 @@ export default function createEnableAlertTests({ await supertest .delete(`/api/alert/${createdAlert.id}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); await esArchiver.unload('actions/basic'); }); diff --git a/x-pack/test/api_integration/apis/alerting/find.ts b/x-pack/test/api_integration/apis/alerting/find.ts index 58072aafc4c37..f01b9900e2e13 100644 --- a/x-pack/test/api_integration/apis/alerting/find.ts +++ b/x-pack/test/api_integration/apis/alerting/find.ts @@ -32,7 +32,7 @@ export default function createFindTests({ getService }: KibanaFunctionalTestDefa await supertest .delete(`/api/alert/${alertId}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); await esArchiver.unload('actions/basic'); }); diff --git a/x-pack/test/api_integration/apis/alerting/get.ts b/x-pack/test/api_integration/apis/alerting/get.ts index 288ee29b2b821..d201307949f57 100644 --- a/x-pack/test/api_integration/apis/alerting/get.ts +++ b/x-pack/test/api_integration/apis/alerting/get.ts @@ -32,7 +32,7 @@ export default function createGetTests({ getService }: KibanaFunctionalTestDefau await supertest .delete(`/api/alert/${alertId}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); await esArchiver.unload('actions/basic'); }); diff --git a/x-pack/test/api_integration/apis/alerting/update.ts b/x-pack/test/api_integration/apis/alerting/update.ts index 2d1a443b2d4cf..4c50f48244133 100644 --- a/x-pack/test/api_integration/apis/alerting/update.ts +++ b/x-pack/test/api_integration/apis/alerting/update.ts @@ -32,7 +32,7 @@ export default function createUpdateTests({ getService }: KibanaFunctionalTestDe await supertest .delete(`/api/alert/${createdAlert.id}`) .set('kbn-xsrf', 'foo') - .expect(200); + .expect(204, ''); await esArchiver.unload('actions/basic'); });