Add request flyout to Threshold Watch edit form.

- Move action models into common directory. - Remove boom error reporting from action models because this is an unnecessary level of defensiveness since we control the UI that consumes this API. - Make changes to action model valiation which may or may not break the UX. - Add serializeJsonWatch function and consume it within the JSON Watch edit form.
elastic · Aug 14, 2019 · 215cd03 · 215cd03
1 parent 2901eab
commit 215cd03
Show file tree

Hide file tree

Showing 26 changed files with 210 additions and 185 deletions.
diff --git a/x-pack/legacy/plugins/watcher/common/constants/watch_types.ts b/x-pack/legacy/plugins/watcher/common/constants/watch_types.ts
@@ -6,8 +6,6 @@
 
 export const WATCH_TYPES: { [key: string]: string } = {
   JSON: 'json',
-
   THRESHOLD: 'threshold',
-
   MONITORING: 'monitoring',
 };
diff --git a/x-pack/legacy/plugins/watcher/common/lib/serialization/index.js b/x-pack/legacy/plugins/watcher/common/lib/serialization/index.js
@@ -4,5 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+export { serializeJsonWatch } from './serialize_json_watch';
 export { serializeThresholdWatch } from './serialize_threshold_watch';
 export { buildInput } from './serialization_helpers';
+
+// TODO: Address this stub
+export function serializeMonitoringWatch() {
+  return {};
+}
diff --git a/...ck/legacy/plugins/watcher/common/lib/serialization/serialization_helpers/build_actions.js b/...ck/legacy/plugins/watcher/common/lib/serialization/serialization_helpers/build_actions.js
@@ -5,6 +5,7 @@
  */
 
 import { forEach } from 'lodash';
+import { Action } from '../../../models/action';
 
 /*
 watch.actions
@@ -13,7 +14,8 @@ export function buildActions(actions) {
   const result = {};
 
   forEach(actions, (action) => {
-    Object.assign(result, action.upstreamJson);
+    const actionModel = Action.fromDownstreamJson(action);
+    Object.assign(result, actionModel.upstreamJson);
   });
 
   return result;

diff --git a/x-pack/legacy/plugins/watcher/common/lib/serialization/serialize_json_watch.js b/x-pack/legacy/plugins/watcher/common/lib/serialization/serialize_json_watch.js
@@ -0,0 +1,24 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { WATCH_TYPES } from '../../constants';
+
+export function serializeJsonWatch(name, json) {
+  const serializedWatch = {
+    ...json,
+    metadata: {
+      xpack: {
+        type: WATCH_TYPES.JSON,
+      }
+    },
+  };
+
+  if (name) {
+    serializedWatch.metadata.name = name;
+  }
+
+  return serializedWatch;
+}
diff --git a/x-pack/legacy/plugins/watcher/common/lib/serialization/serialize_threshold_watch.js b/x-pack/legacy/plugins/watcher/common/lib/serialization/serialize_threshold_watch.js
@@ -14,24 +14,31 @@ import {
   buildTrigger,
 } from './serialization_helpers';
 
-export function serializeThresholdWatch(watch) {
-  const {
-    name,
-    triggerIntervalSize,
-    triggerIntervalUnit,
-    index, timeWindowSize, timeWindowUnit, timeField, aggType, aggField, termField, termSize, termOrder,
-    thresholdComparator, hasTermsAgg, threshold,
-    actions,
-  } = watch;
-
+export function serializeThresholdWatch({
+  name,
+  triggerIntervalSize,
+  triggerIntervalUnit,
+  index,
+  timeWindowSize,
+  timeWindowUnit,
+  timeField,
+  aggType,
+  aggField,
+  termField,
+  termSize,
+  termOrder,
+  thresholdComparator,
+  hasTermsAgg,
+  threshold,
+  actions,
+}) {
   const serializedWatch = {
     trigger: buildTrigger(triggerIntervalSize, triggerIntervalUnit),
     input: buildInput({ index, timeWindowSize, timeWindowUnit, timeField, aggType, aggField, termField, termSize, termOrder }),
     condition: buildCondition({ aggType, thresholdComparator, hasTermsAgg, threshold }),
     transform: buildTransform({ aggType, thresholdComparator, hasTermsAgg, threshold }),
     actions: buildActions(actions),
     metadata: {
-      name,
       xpack: {
         type: WATCH_TYPES.THRESHOLD,
       },
@@ -52,5 +59,9 @@ export function serializeThresholdWatch(watch) {
     },
   };
 
+  if (name) {
+    serializedWatch.metadata.name = name;
+  }
+
   return serializedWatch;
 }
diff --git a/...ns/watcher/server/models/action/action.js → ...ns/watcher/common/models/action/action.js b/...ns/watcher/server/models/action/action.js → ...ns/watcher/common/models/action/action.js
@@ -5,7 +5,6 @@
  */
 
 import { set } from 'lodash';
-import { badRequest } from 'boom';
 import { getActionType } from '../../../common/lib/get_action_type';
 import { ACTION_TYPES } from '../../../common/constants';
 import { LoggingAction } from './logging_action';
@@ -16,7 +15,6 @@ import { WebhookAction } from './webhook_action';
 import { PagerDutyAction } from './pagerduty_action';
 import { JiraAction } from './jira_action';
 import { UnknownAction } from './unknown_action';
-import { i18n } from '@kbn/i18n';
 
 const ActionTypes = {};
 set(ActionTypes, ACTION_TYPES.LOGGING, LoggingAction);
@@ -34,63 +32,17 @@ export class Action {
   }
 
   // From Elasticsearch
-  static fromUpstreamJson(json, options = { throwExceptions: {} }) {
-    if (!json.id) {
-      throw badRequest(
-        i18n.translate('xpack.watcher.models.actionStatus.idPropertyMissingBadRequestMessage', {
-          defaultMessage: 'JSON argument must contain an {id} property',
-          values: {
-            id: 'id'
-          }
-        }),
-      );
-    }
-
-    if (!json.actionJson) {
-      throw badRequest(
-        i18n.translate('xpack.watcher.models.action.actionJsonPropertyMissingBadRequestMessage', {
-          defaultMessage: 'JSON argument must contain an {actionJson} property',
-          values: {
-            actionJson: 'actionJson'
-          }
-        }),
-      );
-    }
-
+  static fromUpstreamJson(json) {
     const type = getActionType(json.actionJson);
     const ActionType = ActionTypes[type] || UnknownAction;
-
-    const { action, errors } = ActionType.fromUpstreamJson(json, options);
-    const doThrowException = options.throwExceptions.Action !== false;
-
-    if (errors && doThrowException) {
-      this.throwErrors(errors);
-    }
-
+    const { action } = ActionType.fromUpstreamJson(json);
     return action;
   }
 
   // From Kibana
-  static fromDownstreamJson(json, options = { throwExceptions: {} }) {
+  static fromDownstreamJson(json) {
     const ActionType = ActionTypes[json.type] || UnknownAction;
-
-    const { action, errors } = ActionType.fromDownstreamJson(json);
-    const doThrowException = options.throwExceptions.Action !== false;
-
-    if (errors && doThrowException) {
-      this.throwErrors(errors);
-    }
-
+    const { action } = ActionType.fromDownstreamJson(json);
     return action;
   }
-
-  static throwErrors(errors) {
-    const allMessages = errors.reduce((message, error) => {
-      if (message) {
-        return `${message}, ${error.message}`;
-      }
-      return error.message;
-    }, '');
-    throw badRequest(allMessages);
-  }
 }
diff --git a/...tcher/server/models/action/action.test.js → ...tcher/common/models/action/action.test.js b/...tcher/server/models/action/action.test.js → ...tcher/common/models/action/action.test.js
diff --git a/...tcher/server/models/action/base_action.js → ...tcher/common/models/action/base_action.js b/...tcher/server/models/action/base_action.js → ...tcher/common/models/action/base_action.js
@@ -4,9 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { badRequest } from 'boom';
-import { i18n } from '@kbn/i18n';
-
 export class BaseAction {
   constructor(props, errors) {
     this.id = props.id;
@@ -35,17 +32,6 @@ export class BaseAction {
   }
 
   static getPropsFromUpstreamJson(json) {
-    if (!json.id) {
-      throw badRequest(
-        i18n.translate('xpack.watcher.models.baseAction.idPropertyMissingBadRequestMessage', {
-          defaultMessage: 'JSON argument must contain an {id} property',
-          values: {
-            id: 'id'
-          }
-        }),
-      );
-    }
-
     return {
       id: json.id
     };

diff --git a/...cher/server/models/action/email_action.js → ...cher/common/models/action/email_action.js b/...cher/server/models/action/email_action.js → ...cher/common/models/action/email_action.js
@@ -108,11 +108,9 @@ export class EmailAction extends BaseAction {
         code: ERROR_CODES.ERR_PROP_MISSING,
         message
       });
-
-      json.email = {};
     }
 
-    if (!json.email.to) {
+    if (json.email && !json.email.to) {
       const message = i18n.translate('xpack.watcher.models.emailAction.actionJsonEmailToPropertyMissingBadRequestMessage', {
         defaultMessage: 'JSON argument must contain an {actionJsonEmailTo} property',
         values: {

diff --git a/...ins/watcher/server/models/action/index.js → ...ins/watcher/common/models/action/index.js b/...ins/watcher/server/models/action/index.js → ...ins/watcher/common/models/action/index.js
diff --git a/...cher/server/models/action/index_action.js → ...cher/common/models/action/index_action.js b/...cher/server/models/action/index_action.js → ...cher/common/models/action/index_action.js
@@ -75,11 +75,9 @@ export class IndexAction extends BaseAction {
           }
         }),
       });
-
-      json.index = {};
     }
 
-    if (!json.index.index) {
+    if (json.index && !json.index.index) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.loggingAction.actionJsonIndexNamePropertyMissingBadRequestMessage', {

diff --git a/...tcher/server/models/action/jira_action.js → ...tcher/common/models/action/jira_action.js b/...tcher/server/models/action/jira_action.js → ...tcher/common/models/action/jira_action.js
@@ -95,8 +95,6 @@ export class JiraAction extends BaseAction {
           }
         }),
       });
-
-      json.jira = {};
     }
 
     if (!get(json, 'jira.fields.project.key')) {
@@ -123,7 +121,7 @@ export class JiraAction extends BaseAction {
       });
     }
 
-    if (!json.jira.fields.summary) {
+    if (!get(json, 'jira.fields.summary')) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.jiraAction.actionJsonJiraSummaryPropertyMissingBadRequestMessage', {

diff --git a/...er/server/models/action/logging_action.js → ...er/common/models/action/logging_action.js b/...er/server/models/action/logging_action.js → ...er/common/models/action/logging_action.js
@@ -78,11 +78,9 @@ export class LoggingAction extends BaseAction {
           }
         }),
       });
-
-      json.logging = {};
     }
 
-    if (!json.logging.text) {
+    if (json.logging && !json.logging.text) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.loggingAction.actionJsonLoggingTextPropertyMissingBadRequestMessage', {

diff --git a/.../server/models/action/pagerduty_action.js → .../common/models/action/pagerduty_action.js b/.../server/models/action/pagerduty_action.js → .../common/models/action/pagerduty_action.js
@@ -78,11 +78,9 @@ export class PagerDutyAction extends BaseAction {
           }
         }),
       });
-
-      json.pagerduty = {};
     }
 
-    if (!json.pagerduty.description) {
+    if (json.pagerduty && !json.pagerduty.description) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.pagerDutyAction.actionJsonPagerDutyDescriptionPropertyMissingBadRequestMessage', {

diff --git a/...cher/server/models/action/slack_action.js → ...cher/common/models/action/slack_action.js b/...cher/server/models/action/slack_action.js → ...cher/common/models/action/slack_action.js
@@ -87,11 +87,9 @@ export class SlackAction extends BaseAction {
           }
         })
       });
-
-      json.slack = {};
     }
 
-    if (!json.slack.message) {
+    if (json.slack && !json.slack.message) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.slackAction.actionJsonSlackMessagePropertyMissingBadRequestMessage', {
@@ -101,8 +99,6 @@ export class SlackAction extends BaseAction {
           }
         }),
       });
-
-      json.slack.message = {};
     }
 
     return { errors: errors.length ? errors : null };

diff --git a/...er/server/models/action/unknown_action.js → ...er/common/models/action/unknown_action.js b/...er/server/models/action/unknown_action.js → ...er/common/models/action/unknown_action.js
diff --git a/...er/server/models/action/webhook_action.js → ...er/common/models/action/webhook_action.js b/...er/server/models/action/webhook_action.js → ...er/common/models/action/webhook_action.js
@@ -145,7 +145,7 @@ export class WebhookAction extends BaseAction {
   static validateJson(json) {
     const errors = [];
 
-    if (!json.webhook.host) {
+    if (json.webhook && !json.webhook.host) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.loggingAction.actionJsonWebhookHostPropertyMissingBadRequestMessage', {
@@ -155,10 +155,9 @@ export class WebhookAction extends BaseAction {
           }
         }),
       });
-      json.webhook = {};
     }
 
-    if (!json.webhook.port) {
+    if (json.webhook && !json.webhook.port) {
       errors.push({
         code: ERROR_CODES.ERR_PROP_MISSING,
         message: i18n.translate('xpack.watcher.models.loggingAction.actionJsonWebhookPortPropertyMissingBadRequestMessage', {

diff --git a/...ns/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit_form.tsx b/...ns/watcher/public/sections/watch_edit/components/json_watch_edit/json_watch_edit_form.tsx
@@ -20,6 +20,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n/react';
+import { serializeJsonWatch } from '../../../../../common/lib/serialization';
 import { ErrableFormRow, SectionError } from '../../../../components';
 import { putWatchApiUrl } from '../../../../lib/documentation_links';
 import { onWatchSave } from '../../watch_edit_actions';
@@ -241,7 +242,11 @@ export const JsonWatchEditForm = () => {
       </EuiForm>
 
       {isRequestVisible ? (
-        <RequestFlyout watch={watch} close={() => setIsRequestVisible(false)} />
+        <RequestFlyout
+          id={watch.id}
+          payload={serializeJsonWatch(watch.name, watch.watch)}
+          close={() => setIsRequestVisible(false)}
+        />
       ) : null}
     </Fragment>
   );

diff --git a/x-pack/legacy/plugins/watcher/public/sections/watch_edit/components/request_flyout.tsx b/x-pack/legacy/plugins/watcher/public/sections/watch_edit/components/request_flyout.tsx
@@ -25,11 +25,14 @@ interface Props {
 }
 
 export class RequestFlyout extends PureComponent<Props> {
+  getEsJson(payload) {
+    return JSON.stringify(payload, null, 2);
+  }
+
   render() {
-    const { watch, close } = this.props;
-    const { id, watchString } = watch;
+    const { id, payload, close } = this.props;
     const endpoint = `PUT _watcher/watch/${id || '<watchId>'}`;
-    const request = `${endpoint}\n${watchString}`;
+    const request = `${endpoint}\n${this.getEsJson(payload)}`;
 
     return (
       <EuiFlyout maxWidth={480} onClose={close}>