[SecuritySolutions] Update CellActions to support all types used by D…

…iscover (#160524) Original issue: #144943 ## Summary * Update CellActions value to be `Serializable`. * Update Default Actions and SecuritySolution Actions to allowlist the supported Kibana types. * Add an extra check to Action's `execute` to ensure the field value is compatible. ### How to test it? * Open Discover and create a saved search with many different field types * Go to Security Solutions dashboards * Create a new dashboard and import the saved search * Test the created dashboard inside Security Solutions ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Jun 30, 2023 · 360c4c3 · 360c4c3
1 parent e7e1932
commit 360c4c3
Show file tree

Hide file tree

Showing 36 changed files with 633 additions and 233 deletions.
diff --git a/packages/kbn-cell-actions/src/actions/copy_to_clipboard/copy_to_clipboard.test.ts b/packages/kbn-cell-actions/src/actions/copy_to_clipboard/copy_to_clipboard.test.ts
@@ -9,21 +9,25 @@
 import { createCopyToClipboardActionFactory } from './copy_to_clipboard';
 import type { CellActionExecutionContext } from '../../types';
 import type { NotificationsStart } from '@kbn/core/public';
+import { KBN_FIELD_TYPES } from '@kbn/field-types';
 
 const mockSuccessToast = jest.fn();
+const mockWarningToast = jest.fn();
 
 const mockCopy = jest.fn((text: string) => true);
 jest.mock('copy-to-clipboard', () => (text: string) => mockCopy(text));
 
 describe('Default createCopyToClipboardActionFactory', () => {
   const copyToClipboardActionFactory = createCopyToClipboardActionFactory({
-    notifications: { toasts: { addSuccess: mockSuccessToast } } as unknown as NotificationsStart,
+    notifications: {
+      toasts: { addSuccess: mockSuccessToast, addWarning: mockWarningToast },
+    } as unknown as NotificationsStart,
   });
   const copyToClipboardAction = copyToClipboardActionFactory({ id: 'testAction' });
   const context = {
     data: [
       {
-        field: { name: 'user.name', type: 'text' },
+        field: { name: 'user.name', type: 'string' },
         value: 'the value',
       },
     ],
@@ -45,6 +49,20 @@ describe('Default createCopyToClipboardActionFactory', () => {
     it('should return true if everything is okay', async () => {
       expect(await copyToClipboardAction.isCompatible(context)).toEqual(true);
     });
+
+    it('should return false if Kbn type is unsupported', async () => {
+      expect(
+        await copyToClipboardAction.isCompatible({
+          ...context,
+          data: [
+            {
+              ...context.data[0],
+              field: { ...context.data[0].field, type: KBN_FIELD_TYPES.NUMBER_RANGE },
+            },
+          ],
+        })
+      ).toEqual(false);
+    });
   });
 
   describe('execute', () => {
@@ -111,5 +129,19 @@ describe('Default createCopyToClipboardActionFactory', () => {
       expect(mockCopy).toHaveBeenCalledWith('user.name: true AND false AND true');
       expect(mockSuccessToast).toHaveBeenCalled();
     });
+
+    it('should notify the user when value type is unsupported', async () => {
+      await copyToClipboardAction.execute({
+        ...context,
+        data: [
+          {
+            ...context.data[0],
+            value: {},
+          },
+        ],
+      });
+      expect(mockCopy).not.toHaveBeenCalled();
+      expect(mockWarningToast).toHaveBeenCalled();
+    });
   });
 });
diff --git a/packages/kbn-cell-actions/src/actions/copy_to_clipboard/copy_to_clipboard.ts b/packages/kbn-cell-actions/src/actions/copy_to_clipboard/copy_to_clipboard.ts
@@ -10,8 +10,16 @@ import copy from 'copy-to-clipboard';
 import { i18n } from '@kbn/i18n';
 import type { NotificationsStart } from '@kbn/core/public';
 import { isString } from 'lodash/fp';
+import { KBN_FIELD_TYPES } from '@kbn/field-types';
 import { COPY_CELL_ACTION_TYPE } from '../../constants';
 import { createCellActionFactory } from '../factory';
+import {
+  filterOutNullableValues,
+  isTypeSupportedByDefaultActions,
+  isValueSupportedByDefaultActions,
+  valueToArray,
+} from '../utils';
+import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '../translations';
 
 const ICON = 'copyClipboard';
 const COPY_TO_CLIPBOARD = i18n.translate('cellActions.actions.copyToClipboard.displayName', {
@@ -37,19 +45,24 @@ export const createCopyToClipboardActionFactory = createCellActionFactory(
 
       return (
         data.length === 1 && // TODO Add support for multiple values
-        field.name != null
+        field.name != null &&
+        isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES)
       );
     },
     execute: async ({ data }) => {
       const field = data[0]?.field;
-      const value = data[0]?.value;
+      const rawValue = data[0]?.value;
+      const value = filterOutNullableValues(valueToArray(rawValue));
 
-      let textValue: undefined | string;
-      if (value != null) {
-        const valuesArray = Array.isArray(value) ? value : [value];
-        textValue = valuesArray.map((v) => (isString(v) ? `"${escapeValue(v)}"` : v)).join(' AND ');
+      if (!isValueSupportedByDefaultActions(value)) {
+        notifications.toasts.addWarning({
+          title: ACTION_INCOMPATIBLE_VALUE_WARNING,
+        });
+        return;
       }
-      const text = textValue ? `${field.name}: ${textValue}` : field.name;
+
+      const textValue = value.map((v) => (isString(v) ? `"${escapeValue(v)}"` : v)).join(' AND ');
+      const text = textValue !== '' ? `${field.name}: ${textValue}` : field.name;
       const isSuccess = copy(text, { debug: true });
 
       if (isSuccess) {

diff --git a/packages/kbn-cell-actions/src/actions/filter/create_filter.test.ts b/packages/kbn-cell-actions/src/actions/filter/create_filter.test.ts
@@ -15,11 +15,8 @@ const booleanValue = true;
 
 describe('createFilter', () => {
   it.each([
-    { caseName: 'string', caseValue: value },
     { caseName: 'string array', caseValue: [value] },
-    { caseName: 'number', caseValue: numberValue, query: numberValue.toString() },
     { caseName: 'number array', caseValue: [numberValue], query: numberValue.toString() },
-    { caseName: 'boolean', caseValue: booleanValue, query: booleanValue.toString() },
     { caseName: 'boolean array', caseValue: [booleanValue], query: booleanValue.toString() },
   ])('should return filter with $caseName value', ({ caseValue, query = value }) => {
     expect(createFilter({ key: field, value: caseValue, negate: false })).toEqual({
@@ -42,11 +39,8 @@ describe('createFilter', () => {
   });
 
   it.each([
-    { caseName: 'string', caseValue: value },
     { caseName: 'string array', caseValue: [value] },
-    { caseName: 'number', caseValue: numberValue, query: numberValue.toString() },
     { caseName: 'number array', caseValue: [numberValue], query: numberValue.toString() },
-    { caseName: 'boolean', caseValue: booleanValue, query: booleanValue.toString() },
     { caseName: 'boolean array', caseValue: [booleanValue], query: booleanValue.toString() },
   ])('should return negate filter with $caseName value', ({ caseValue, query = value }) => {
     expect(createFilter({ key: field, value: caseValue, negate: true })).toEqual({
@@ -93,45 +87,41 @@ describe('createFilter', () => {
     });
   });
 
-  it.each([
-    { caseName: 'null', caseValue: null },
-    { caseName: 'undefined', caseValue: undefined },
-    { caseName: 'empty string', caseValue: '' },
-    { caseName: 'empty array', caseValue: [] },
-  ])('should return exist filter with $caseName value', ({ caseValue }) => {
-    expect(createFilter({ key: field, value: caseValue, negate: false })).toEqual({
-      query: {
-        exists: {
-          field,
+  it.each([{ caseName: 'empty array', caseValue: [] }])(
+    'should return exist filter with $caseName value',
+    ({ caseValue }) => {
+      expect(createFilter({ key: field, value: caseValue, negate: false })).toEqual({
+        query: {
+          exists: {
+            field,
+          },
         },
-      },
-      meta: {
-        key: field,
-        negate: false,
-        type: 'exists',
-        value: 'exists',
-      },
-    });
-  });
+        meta: {
+          key: field,
+          negate: false,
+          type: 'exists',
+          value: 'exists',
+        },
+      });
+    }
+  );
 
-  it.each([
-    { caseName: 'null', caseValue: null },
-    { caseName: 'undefined', caseValue: undefined },
-    { caseName: 'empty string', caseValue: '' },
-    { caseName: 'empty array', caseValue: [] },
-  ])('should return negate exist filter with $caseName value', ({ caseValue }) => {
-    expect(createFilter({ key: field, value: caseValue, negate: true })).toEqual({
-      query: {
-        exists: {
-          field,
+  it.each([{ caseName: 'empty array', caseValue: [] }])(
+    'should return negate exist filter with $caseName value',
+    ({ caseValue }) => {
+      expect(createFilter({ key: field, value: caseValue, negate: true })).toEqual({
+        query: {
+          exists: {
+            field,
+          },
         },
-      },
-      meta: {
-        key: field,
-        negate: true,
-        type: 'exists',
-        value: 'exists',
-      },
-    });
-  });
+        meta: {
+          key: field,
+          negate: true,
+          type: 'exists',
+          value: 'exists',
+        },
+      });
+    }
+  );
 });
diff --git a/packages/kbn-cell-actions/src/actions/filter/create_filter.ts b/packages/kbn-cell-actions/src/actions/filter/create_filter.ts
@@ -13,13 +13,10 @@ import {
   type PhraseFilter,
   type Filter,
 } from '@kbn/es-query';
-import { isArray } from 'lodash/fp';
-import { CellActionFieldValue } from '../../types';
+import { DefaultActionsSupportedValue } from '../types';
 
-export const isEmptyFilterValue = (
-  value: CellActionFieldValue
-): value is null | undefined | never[] =>
-  value == null || value === '' || (isArray(value) && value.length === 0);
+export const isEmptyFilterValue = (value: Array<string | number | boolean>) =>
+  value.length === 0 || value.every((v) => v === '');
 
 const createExistsFilter = ({ key, negate }: { key: string; negate: boolean }): ExistsFilter => ({
   meta: { key, negate, type: FILTERS.EXISTS, value: 'exists' },
@@ -49,7 +46,7 @@ const createCombinedFilter = ({
   key,
   negate,
 }: {
-  values: string[] | number[] | boolean[];
+  values: DefaultActionsSupportedValue;
   key: string;
   negate: boolean;
 }): CombinedFilter => ({
@@ -68,18 +65,16 @@ export const createFilter = ({
   negate,
 }: {
   key: string;
-  value: CellActionFieldValue;
+  value: DefaultActionsSupportedValue;
   negate: boolean;
 }): Filter => {
-  if (isEmptyFilterValue(value)) {
+  if (value.length === 0) {
     return createExistsFilter({ key, negate });
   }
-  if (Array.isArray(value)) {
-    if (value.length > 1) {
-      return createCombinedFilter({ key, negate, values: value });
-    } else {
-      return createPhraseFilter({ key, negate, value: value[0] });
-    }
+
+  if (value.length > 1) {
+    return createCombinedFilter({ key, negate, values: value });
+  } else {
+    return createPhraseFilter({ key, negate, value: value[0] });
   }
-  return createPhraseFilter({ key, negate, value });
 };
diff --git a/packages/kbn-cell-actions/src/actions/filter/filter_in.test.ts b/packages/kbn-cell-actions/src/actions/filter/filter_in.test.ts
@@ -5,9 +5,10 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
-import type { FilterManager } from '@kbn/data-plugin/public';
+import { FilterManager, KBN_FIELD_TYPES } from '@kbn/data-plugin/public';
 import { createFilterInActionFactory } from './filter_in';
 import { makeActionContext } from '../../mocks/helpers';
+import { NotificationsStart } from '@kbn/core-notifications-browser';
 
 const mockFilterManager = { addFilters: jest.fn() } as unknown as FilterManager;
 
@@ -20,15 +21,18 @@ jest.mock('./create_filter', () => ({
 const fieldName = 'user.name';
 const value = 'the value';
 
+const mockWarningToast = jest.fn();
+
 describe('createFilterInActionFactory', () => {
   const filterInActionFactory = createFilterInActionFactory({
     filterManager: mockFilterManager,
+    notifications: { toasts: { addWarning: mockWarningToast } } as unknown as NotificationsStart,
   });
   const filterInAction = filterInActionFactory({ id: 'testAction' });
   const context = makeActionContext({
     data: [
       {
-        field: { name: fieldName, type: 'text', searchable: true, aggregatable: true },
+        field: { name: fieldName, type: 'string', searchable: true, aggregatable: true },
         value,
       },
     ],
@@ -57,12 +61,27 @@ describe('createFilterInActionFactory', () => {
           ...context,
           data: [
             {
+              ...context.data[0],
               field: { ...context.data[0].field, name: '' },
             },
           ],
         })
       ).toEqual(false);
     });
+
+    it('should return false if Kbn type is unsupported', async () => {
+      expect(
+        await filterInAction.isCompatible({
+          ...context,
+          data: [
+            {
+              ...context.data[0],
+              field: { ...context.data[0].field, type: KBN_FIELD_TYPES.MISSING },
+            },
+          ],
+        })
+      ).toEqual(false);
+    });
   });
 
   describe('execute', () => {
@@ -75,7 +94,7 @@ describe('createFilterInActionFactory', () => {
       await filterInAction.execute(context);
       expect(mockCreateFilter).toHaveBeenCalledWith({
         key: fieldName,
-        value,
+        value: [value],
         negate: false,
       });
     });
@@ -107,7 +126,7 @@ describe('createFilterInActionFactory', () => {
           },
         ],
       });
-      expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: null, negate: true });
+      expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [], negate: true });
     });
 
     it('should create negate filter query with undefined value', async () => {
@@ -122,7 +141,7 @@ describe('createFilterInActionFactory', () => {
       });
       expect(mockCreateFilter).toHaveBeenCalledWith({
         key: fieldName,
-        value: undefined,
+        value: [],
         negate: true,
       });
     });
@@ -137,7 +156,7 @@ describe('createFilterInActionFactory', () => {
           },
         ],
       });
-      expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: '', negate: true });
+      expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [''], negate: true });
     });
 
     it('should create negate filter query with empty array value', async () => {
@@ -152,5 +171,19 @@ describe('createFilterInActionFactory', () => {
       });
       expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [], negate: true });
     });
+
+    it('should notify the user when value type is unsupported', async () => {
+      await filterInAction.execute({
+        ...context,
+        data: [
+          {
+            ...context.data[0],
+            value: [{}, {}, {}],
+          },
+        ],
+      });
+      expect(mockCreateFilter).not.toHaveBeenCalled();
+      expect(mockWarningToast).toHaveBeenCalled();
+    });
   });
 });