From 6acb716b2c7bd89c395517664eebd79e78a6391b Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Mon, 5 Aug 2019 14:42:58 -0400 Subject: [PATCH] Changes action create/update http apis to return bodies (#42493) Prior to this PR, the create and update http apis for actions returned an object with the shape `{ id: }`. This PR changes the responses to be the complete body of the action that was created / updated, not just the `id`. I believe this is the final piece of the "fix http apis for actions / alerts" issue https://github.com/elastic/kibana/issues/41828 --- .../actions/server/routes/create.test.ts | 9 ++++++--- .../plugins/actions/server/routes/create.ts | 8 +++----- .../actions/server/routes/update.test.ts | 7 ++++++- .../plugins/actions/server/routes/update.ts | 3 +-- .../apis/actions/builtin_action_types/email.ts | 9 +++++++++ .../actions/builtin_action_types/es_index.ts | 18 ++++++++++++++++-- .../actions/builtin_action_types/server_log.ts | 3 +++ .../apis/actions/builtin_action_types/slack.ts | 3 +++ .../api_integration/apis/actions/create.ts | 10 ++++++++++ .../test/api_integration/apis/actions/fire.ts | 5 +++++ .../api_integration/apis/actions/update.ts | 15 +++++++++++++++ 11 files changed, 77 insertions(+), 13 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/routes/create.test.ts b/x-pack/legacy/plugins/actions/server/routes/create.test.ts index ec5714925650..442af6b88d16 100644 --- a/x-pack/legacy/plugins/actions/server/routes/create.test.ts +++ b/x-pack/legacy/plugins/actions/server/routes/create.test.ts @@ -27,18 +27,21 @@ it('creates an action with proper parameters', async () => { }; const createResult = { id: '1', - type: 'action', description: 'My description', actionTypeId: 'abc', config: { foo: true }, - secrets: {}, }; actionsClient.create.mockResolvedValueOnce(createResult); const { payload, statusCode } = await server.inject(request); expect(statusCode).toBe(200); const response = JSON.parse(payload); - expect(response).toEqual({ id: '1' }); + expect(response).toEqual({ + id: '1', + description: 'My description', + actionTypeId: 'abc', + config: { foo: true }, + }); expect(actionsClient.create).toHaveBeenCalledTimes(1); expect(actionsClient.create.mock.calls[0]).toMatchInlineSnapshot(` Array [ diff --git a/x-pack/legacy/plugins/actions/server/routes/create.ts b/x-pack/legacy/plugins/actions/server/routes/create.ts index 6e4034d3258a..e1367cdf15ba 100644 --- a/x-pack/legacy/plugins/actions/server/routes/create.ts +++ b/x-pack/legacy/plugins/actions/server/routes/create.ts @@ -6,7 +6,7 @@ import Joi from 'joi'; import Hapi from 'hapi'; -import { WithoutQueryAndParams } from '../types'; +import { ActionResult, WithoutQueryAndParams } from '../types'; interface CreateRequest extends WithoutQueryAndParams { query: { @@ -42,13 +42,11 @@ export function createRoute(server: Hapi.Server) { .required(), }, }, - async handler(request: CreateRequest) { + async handler(request: CreateRequest): Promise { const actionsClient = request.getActionsClient!(); const action = request.payload; - const createdAction = await actionsClient.create({ action }); - - return { id: createdAction.id }; + return await actionsClient.create({ action }); }, }); } diff --git a/x-pack/legacy/plugins/actions/server/routes/update.test.ts b/x-pack/legacy/plugins/actions/server/routes/update.test.ts index 1d7699b8a163..a08522ccb981 100644 --- a/x-pack/legacy/plugins/actions/server/routes/update.test.ts +++ b/x-pack/legacy/plugins/actions/server/routes/update.test.ts @@ -35,7 +35,12 @@ it('calls the update function with proper parameters', async () => { const { payload, statusCode } = await server.inject(request); expect(statusCode).toBe(200); const response = JSON.parse(payload); - expect(response).toEqual({ id: '1' }); + expect(response).toEqual({ + id: '1', + actionTypeId: 'my-action-type-id', + description: 'My description', + config: { foo: true }, + }); expect(actionsClient.update).toHaveBeenCalledTimes(1); expect(actionsClient.update.mock.calls[0]).toMatchInlineSnapshot(` Array [ diff --git a/x-pack/legacy/plugins/actions/server/routes/update.ts b/x-pack/legacy/plugins/actions/server/routes/update.ts index 45933109a8bd..f8c2e7059f78 100644 --- a/x-pack/legacy/plugins/actions/server/routes/update.ts +++ b/x-pack/legacy/plugins/actions/server/routes/update.ts @@ -42,8 +42,7 @@ export function updateRoute(server: Hapi.Server) { const { id } = request.params; const { description, config, secrets } = request.payload; const actionsClient = request.getActionsClient!(); - await actionsClient.update({ id, action: { description, config, secrets } }); - return { id }; + return await actionsClient.update({ id, action: { description, config, secrets } }); }, }); } diff --git a/x-pack/test/api_integration/apis/actions/builtin_action_types/email.ts b/x-pack/test/api_integration/apis/actions/builtin_action_types/email.ts index 7eefbe46175b..bbe512700be0 100644 --- a/x-pack/test/api_integration/apis/actions/builtin_action_types/email.ts +++ b/x-pack/test/api_integration/apis/actions/builtin_action_types/email.ts @@ -38,6 +38,15 @@ export default function emailTest({ getService }: FtrProviderContext) { createdActionId = createdAction.id; expect(createdAction).to.eql({ id: createdActionId, + description: 'An email action', + actionTypeId: '.email', + config: { + service: '__json', + host: null, + port: null, + secure: null, + from: 'bob@example.com', + }, }); expect(typeof createdActionId).to.be('string'); diff --git a/x-pack/test/api_integration/apis/actions/builtin_action_types/es_index.ts b/x-pack/test/api_integration/apis/actions/builtin_action_types/es_index.ts index 86fe8222678a..9f77be490bb9 100644 --- a/x-pack/test/api_integration/apis/actions/builtin_action_types/es_index.ts +++ b/x-pack/test/api_integration/apis/actions/builtin_action_types/es_index.ts @@ -35,7 +35,14 @@ export default function indexTest({ getService }: FtrProviderContext) { }) .expect(200); - expect(createdAction).to.eql({ id: createdAction.id }); + expect(createdAction).to.eql({ + id: createdAction.id, + description: 'An index action', + actionTypeId: '.index', + config: { + index: null, + }, + }); createdActionID = createdAction.id; expect(typeof createdActionID).to.be('string'); @@ -63,7 +70,14 @@ export default function indexTest({ getService }: FtrProviderContext) { }) .expect(200); - expect(createdActionWithIndex).to.eql({ id: createdActionWithIndex.id }); + expect(createdActionWithIndex).to.eql({ + id: createdActionWithIndex.id, + description: 'An index action with index config', + actionTypeId: '.index', + config: { + index: ES_TEST_INDEX_NAME, + }, + }); createdActionIDWithIndex = createdActionWithIndex.id; expect(typeof createdActionIDWithIndex).to.be('string'); diff --git a/x-pack/test/api_integration/apis/actions/builtin_action_types/server_log.ts b/x-pack/test/api_integration/apis/actions/builtin_action_types/server_log.ts index 6f2e1c46d296..e56555e1094d 100644 --- a/x-pack/test/api_integration/apis/actions/builtin_action_types/server_log.ts +++ b/x-pack/test/api_integration/apis/actions/builtin_action_types/server_log.ts @@ -30,6 +30,9 @@ export default function serverLogTest({ getService }: FtrProviderContext) { serverLogActionId = createdAction.id; expect(createdAction).to.eql({ id: createdAction.id, + description: 'A server.log action', + actionTypeId: '.server-log', + config: {}, }); expect(typeof createdAction.id).to.be('string'); diff --git a/x-pack/test/api_integration/apis/actions/builtin_action_types/slack.ts b/x-pack/test/api_integration/apis/actions/builtin_action_types/slack.ts index 4eea6adb6758..8a6f0a7e2710 100644 --- a/x-pack/test/api_integration/apis/actions/builtin_action_types/slack.ts +++ b/x-pack/test/api_integration/apis/actions/builtin_action_types/slack.ts @@ -42,6 +42,9 @@ export default function slackTest({ getService }: FtrProviderContext) { expect(createdAction).to.eql({ id: createdAction.id, + description: 'A slack action', + actionTypeId: '.slack', + config: {}, }); expect(typeof createdAction.id).to.be('string'); diff --git a/x-pack/test/api_integration/apis/actions/create.ts b/x-pack/test/api_integration/apis/actions/create.ts index a443cab072c5..670ccdc31092 100644 --- a/x-pack/test/api_integration/apis/actions/create.ts +++ b/x-pack/test/api_integration/apis/actions/create.ts @@ -32,6 +32,11 @@ export default function createActionTests({ getService }: FtrProviderContext) { .then((resp: any) => { expect(resp.body).to.eql({ id: resp.body.id, + description: 'My action', + actionTypeId: 'test.index-record', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); expect(typeof resp.body.id).to.be('string'); }); @@ -54,6 +59,11 @@ export default function createActionTests({ getService }: FtrProviderContext) { .expect(200); expect(createdAction).to.eql({ id: createdAction.id, + description: 'My action', + actionTypeId: 'test.index-record', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); expect(typeof createdAction.id).to.be('string'); await supertest.get(`/s/space_1/api/action/${createdAction.id}`).expect(200); diff --git a/x-pack/test/api_integration/apis/actions/fire.ts b/x-pack/test/api_integration/apis/actions/fire.ts index 8bdf0abd9fe1..5525f0db89a3 100644 --- a/x-pack/test/api_integration/apis/actions/fire.ts +++ b/x-pack/test/api_integration/apis/actions/fire.ts @@ -196,6 +196,11 @@ export default function({ getService }: FtrProviderContext) { .expect(200); expect(updatedAction).to.eql({ id: ES_ARCHIVER_ACTION_ID, + actionTypeId: 'test.index-record', + description: 'My action updated', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); await supertest .post(`/api/action/${ES_ARCHIVER_ACTION_ID}/_fire`) diff --git a/x-pack/test/api_integration/apis/actions/update.ts b/x-pack/test/api_integration/apis/actions/update.ts index b077ca57ef01..d68d1526d9ee 100644 --- a/x-pack/test/api_integration/apis/actions/update.ts +++ b/x-pack/test/api_integration/apis/actions/update.ts @@ -33,6 +33,11 @@ export default function updateActionTests({ getService }: FtrProviderContext) { .then((resp: any) => { expect(resp.body).to.eql({ id: ES_ARCHIVER_ACTION_ID, + actionTypeId: 'test.index-record', + description: 'My action updated', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); }); }); @@ -54,6 +59,11 @@ export default function updateActionTests({ getService }: FtrProviderContext) { .then((resp: any) => { expect(resp.body).to.eql({ id: SPACE_1_ES_ARCHIVER_ACTION_ID, + actionTypeId: 'test.index-record', + description: 'My action updated', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); }); }); @@ -110,6 +120,11 @@ export default function updateActionTests({ getService }: FtrProviderContext) { .expect(200); expect(updatedAction).to.eql({ id: ES_ARCHIVER_ACTION_ID, + actionTypeId: 'test.index-record', + description: 'My action updated', + config: { + unencrypted: `This value shouldn't get encrypted`, + }, }); const { body: fetchedAction } = await supertest .get(`/api/action/${ES_ARCHIVER_ACTION_ID}`)