Index Action - Moved index params fields to connector config (#60349)

* Moved index params fields to connector config * Fixed type check issue * Fixing functional tests * Fixed due to comments * Fixed functional tests * Fixed tests and type check
elastic · Mar 18, 2020 · 2207e0a · 2207e0a
1 parent c1435db
commit 2207e0a
Show file tree

Hide file tree

Showing 19 changed files with 528 additions and 333 deletions.
diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.test.ts
@@ -43,18 +43,46 @@ describe('actionTypeRegistry.get() works', () => {
 
 describe('config validation', () => {
   test('config validation succeeds when config is valid', () => {
-    const config: Record<string, any> = {};
+    const config: Record<string, any> = {
+      index: 'testing-123',
+      refresh: false,
+    };
 
     expect(validateConfig(actionType, config)).toEqual({
       ...config,
-      index: null,
+      index: 'testing-123',
+      refresh: false,
     });
 
-    config.index = 'testing-123';
+    config.executionTimeField = 'field-123';
     expect(validateConfig(actionType, config)).toEqual({
       ...config,
       index: 'testing-123',
+      refresh: false,
+      executionTimeField: 'field-123',
     });
+
+    delete config.index;
+
+    expect(() => {
+      validateConfig(actionType, { index: 666 });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type config: [index]: expected value of type [string] but got [number]"`
+    );
+    delete config.executionTimeField;
+
+    expect(() => {
+      validateConfig(actionType, { index: 'testing-123', executionTimeField: true });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type config: [executionTimeField]: expected value of type [string] but got [boolean]"`
+    );
+
+    delete config.refresh;
+    expect(() => {
+      validateConfig(actionType, { index: 'testing-123', refresh: 'foo' });
+    }).toThrowErrorMatchingInlineSnapshot(
+      `"error validating action type config: [refresh]: expected value of type [boolean] but got [string]"`
+    );
   });
 
   test('config validation fails when config is not valid', () => {
@@ -65,46 +93,16 @@ describe('config validation', () => {
     expect(() => {
       validateConfig(actionType, baseConfig);
     }).toThrowErrorMatchingInlineSnapshot(
-      `"error validating action type config: [indeX]: definition for this key is missing"`
+      `"error validating action type config: [index]: expected value of type [string] but got [undefined]"`
     );
-
-    delete baseConfig.user;
-    baseConfig.index = 666;
-
-    expect(() => {
-      validateConfig(actionType, baseConfig);
-    }).toThrowErrorMatchingInlineSnapshot(`
-"error validating action type config: [index]: types that failed validation:
-- [index.0]: expected value of type [string] but got [number]
-- [index.1]: expected value to equal [null]"
-`);
   });
 });
 
 describe('params validation', () => {
   test('params validation succeeds when params is valid', () => {
     const params: Record<string, any> = {
-      index: 'testing-123',
-      executionTimeField: 'field-used-for-time',
-      refresh: true,
       documents: [{ rando: 'thing' }],
     };
-    expect(validateParams(actionType, params)).toMatchInlineSnapshot(`
-        Object {
-          "documents": Array [
-            Object {
-              "rando": "thing",
-            },
-          ],
-          "executionTimeField": "field-used-for-time",
-          "index": "testing-123",
-          "refresh": true,
-        }
-    `);
-
-    delete params.index;
-    delete params.refresh;
-    delete params.executionTimeField;
     expect(validateParams(actionType, params)).toMatchInlineSnapshot(`
         Object {
           "documents": Array [
@@ -129,24 +127,6 @@ describe('params validation', () => {
       `"error validating action params: [documents]: expected value of type [array] but got [undefined]"`
     );
 
-    expect(() => {
-      validateParams(actionType, { index: 666 });
-    }).toThrowErrorMatchingInlineSnapshot(
-      `"error validating action params: [index]: expected value of type [string] but got [number]"`
-    );
-
-    expect(() => {
-      validateParams(actionType, { executionTimeField: true });
-    }).toThrowErrorMatchingInlineSnapshot(
-      `"error validating action params: [executionTimeField]: expected value of type [string] but got [boolean]"`
-    );
-
-    expect(() => {
-      validateParams(actionType, { refresh: 'foo' });
-    }).toThrowErrorMatchingInlineSnapshot(
-      `"error validating action params: [refresh]: expected value of type [boolean] but got [string]"`
-    );
-
     expect(() => {
       validateParams(actionType, { documents: ['should be an object'] });
     }).toThrowErrorMatchingInlineSnapshot(
@@ -162,13 +142,10 @@ describe('execute()', () => {
     let params: ActionParamsType;
     let executorOptions: ActionTypeExecutorOptions;
 
-    // minimal params, index via param
-    config = { index: null };
+    // minimal params
+    config = { index: 'index-value', refresh: false, executionTimeField: undefined };
     params = {
-      index: 'index-via-param',
       documents: [{ jim: 'bob' }],
-      executionTimeField: undefined,
-      refresh: undefined,
     };
 
     const actionId = 'some-id';
@@ -190,19 +167,17 @@ describe('execute()', () => {
                     "jim": "bob",
                   },
                 ],
-                "index": "index-via-param",
+                "index": "index-value",
+                "refresh": false,
               },
             ],
           ]
     `);
 
-    // full params (except index), index via config
-    config = { index: 'index-via-config' };
+    // full params
+    config = { index: 'index-value', executionTimeField: 'field_to_use_for_time', refresh: true };
     params = {
-      index: undefined,
       documents: [{ jimbob: 'jr' }],
-      executionTimeField: 'field_to_use_for_time',
-      refresh: true,
     };
 
     executorOptions = { actionId, config, secrets, params, services };
@@ -226,20 +201,17 @@ describe('execute()', () => {
                   "jimbob": "jr",
                 },
               ],
-              "index": "index-via-config",
+              "index": "index-value",
               "refresh": true,
             },
           ],
         ]
     `);
 
-    // minimal params, index via config and param
-    config = { index: 'index-via-config' };
+    // minimal params
+    config = { index: 'index-value', executionTimeField: undefined, refresh: false };
     params = {
-      index: 'index-via-param',
       documents: [{ jim: 'bob' }],
-      executionTimeField: undefined,
-      refresh: undefined,
     };
 
     executorOptions = { actionId, config, secrets, params, services };
@@ -259,19 +231,17 @@ describe('execute()', () => {
                 "jim": "bob",
               },
             ],
-            "index": "index-via-config",
+            "index": "index-value",
+            "refresh": false,
           },
         ],
       ]
     `);
 
     // multiple documents
-    config = { index: null };
+    config = { index: 'index-value', executionTimeField: undefined, refresh: false };
     params = {
-      index: 'index-via-param',
       documents: [{ a: 1 }, { b: 2 }],
-      executionTimeField: undefined,
-      refresh: undefined,
     };
 
     executorOptions = { actionId, config, secrets, params, services };
@@ -297,7 +267,8 @@ describe('execute()', () => {
                     "b": 2,
                   },
                 ],
-                "index": "index-via-param",
+                "index": "index-value",
+                "refresh": false,
               },
             ],
           ]

diff --git a/x-pack/plugins/actions/server/builtin_action_types/es_index.ts b/x-pack/plugins/actions/server/builtin_action_types/es_index.ts
@@ -8,7 +8,6 @@ import { curry } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import { schema, TypeOf } from '@kbn/config-schema';
 
-import { nullableType } from './lib/nullable';
 import { Logger } from '../../../../../src/core/server';
 import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
 
@@ -17,7 +16,9 @@ import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from
 export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;
 
 const ConfigSchema = schema.object({
-  index: nullableType(schema.string()),
+  index: schema.string(),
+  refresh: schema.boolean({ defaultValue: false }),
+  executionTimeField: schema.maybe(schema.string()),
 });
 
 // params definition
@@ -28,9 +29,6 @@ export type ActionParamsType = TypeOf<typeof ParamsSchema>;
 // - timeout not added here, as this seems to be a generic thing we want to do
 //   eventually: https://github.com/elastic/kibana/projects/26#card-24087404
 const ParamsSchema = schema.object({
-  index: schema.maybe(schema.string()),
-  executionTimeField: schema.maybe(schema.string()),
-  refresh: schema.maybe(schema.boolean()),
   documents: schema.arrayOf(schema.recordOf(schema.string(), schema.any())),
 });
 
@@ -60,27 +58,12 @@ async function executor(
   const params = execOptions.params as ActionParamsType;
   const services = execOptions.services;
 
-  if (config.index == null && params.index == null) {
-    const message = i18n.translate('xpack.actions.builtin.esIndex.indexParamRequiredErrorMessage', {
-      defaultMessage: 'index param needs to be set because not set in config for action',
-    });
-    return {
-      status: 'error',
-      actionId,
-      message,
-    };
-  }
-
-  if (config.index != null && params.index != null) {
-    logger.debug(`index passed in params overridden by index set in config for action ${actionId}`);
-  }
-
-  const index = config.index || params.index;
+  const index = config.index;
 
   const bulkBody = [];
   for (const document of params.documents) {
-    if (params.executionTimeField != null) {
-      document[params.executionTimeField] = new Date();
+    if (config.executionTimeField != null) {
+      document[config.executionTimeField] = new Date();
     }
 
     bulkBody.push({ index: {} });
@@ -92,9 +75,7 @@ async function executor(
     body: bulkBody,
   };
 
-  if (params.refresh != null) {
-    bulkParams.refresh = params.refresh;
-  }
+  bulkParams.refresh = config.refresh;
 
   let result;
   try {
@@ -103,6 +84,7 @@ async function executor(
     const message = i18n.translate('xpack.actions.builtin.esIndex.errorIndexingErrorMessage', {
       defaultMessage: 'error indexing documents',
     });
+    logger.error(`error indexing documents: ${err.message}`);
     return {
       status: 'error',
       actionId,

diff --git a/x-pack/plugins/triggers_actions_ui/public/application/boot.tsx b/x-pack/plugins/triggers_actions_ui/public/application/boot.tsx
@@ -9,7 +9,7 @@ import { render, unmountComponentAtNode } from 'react-dom';
 import { SavedObjectsClientContract } from 'src/core/public';
 
 import { App, AppDeps } from './app';
-import { setSavedObjectsClient } from '../application/components/builtin_alert_types/threshold/lib/api';
+import { setSavedObjectsClient } from '../common/lib/index_threshold_api';
 
 interface BootDeps extends AppDeps {
   element: HTMLElement;