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 index b9292a4c82701..9e51775429a91 100644 --- 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 index 80cb7dd19c0b3..8805b02ed9cfc 100644 --- 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 index 846d30a237fcf..d38b25f93e929 100644 --- 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 index 483d7709aebd5..8f0fee5a5fa03 100644 --- 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) => + 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 index 546881072aed9..b1006e21c01f5 100644 --- 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(); + }); }); }); diff --git a/packages/kbn-cell-actions/src/actions/filter/filter_in.ts b/packages/kbn-cell-actions/src/actions/filter/filter_in.ts index 91ca6cdcf5c9e..419d25079a704 100644 --- a/packages/kbn-cell-actions/src/actions/filter/filter_in.ts +++ b/packages/kbn-cell-actions/src/actions/filter/filter_in.ts @@ -6,11 +6,19 @@ * Side Public License, v 1. */ import { i18n } from '@kbn/i18n'; -import type { FilterManager } from '@kbn/data-plugin/public'; +import type { FilterManager, KBN_FIELD_TYPES } from '@kbn/data-plugin/public'; +import { NotificationsStart } from '@kbn/core-notifications-browser'; import { createFilter, isEmptyFilterValue } from './create_filter'; import { FILTER_CELL_ACTION_TYPE } from '../../constants'; import { createCellActionFactory } from '../factory'; -import { CellActionFieldValue } from '../../types'; +import { + filterOutNullableValues, + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + valueToArray, +} from '../utils'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '../translations'; +import { DefaultActionsSupportedValue } from '../types'; const ICON = 'plusInCircle'; const FILTER_IN = i18n.translate('cellActions.actions.filterIn', { @@ -18,7 +26,13 @@ const FILTER_IN = i18n.translate('cellActions.actions.filterIn', { }); export const createFilterInActionFactory = createCellActionFactory( - ({ filterManager }: { filterManager: FilterManager }) => ({ + ({ + filterManager, + notifications: { toasts }, + }: { + filterManager: FilterManager; + notifications: NotificationsStart; + }) => ({ type: FILTER_CELL_ACTION_TYPE, getIconType: () => ICON, getDisplayName: () => FILTER_IN, @@ -28,13 +42,22 @@ export const createFilterInActionFactory = createCellActionFactory( return ( data.length === 1 && // TODO Add support for multiple values - !!field.name + !!field.name && + isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES) ); }, execute: async ({ data }) => { const field = data[0]?.field; - const value = data[0]?.value; - addFilterIn({ filterManager, fieldName: field.name, value }); + const rawValue = data[0]?.value; + const value = filterOutNullableValues(valueToArray(rawValue)); + + if (isValueSupportedByDefaultActions(value)) { + addFilterIn({ filterManager, fieldName: field.name, value }); + } else { + toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + } }, }) ); @@ -46,7 +69,7 @@ export const addFilterIn = ({ }: { filterManager: FilterManager | undefined; fieldName: string; - value: CellActionFieldValue; + value: DefaultActionsSupportedValue; }) => { if (filterManager != null) { const filter = createFilter({ diff --git a/packages/kbn-cell-actions/src/actions/filter/filter_out.test.ts b/packages/kbn-cell-actions/src/actions/filter/filter_out.test.ts index d42b8bc7a26a4..2d56a93ee420e 100644 --- a/packages/kbn-cell-actions/src/actions/filter/filter_out.test.ts +++ b/packages/kbn-cell-actions/src/actions/filter/filter_out.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 { createFilterOutActionFactory } from './filter_out'; import { makeActionContext } from '../../mocks/helpers'; +import { NotificationsStart } from '@kbn/core-notifications-browser'; const mockFilterManager = { addFilters: jest.fn() } as unknown as FilterManager; @@ -20,13 +21,18 @@ jest.mock('./create_filter', () => ({ const fieldName = 'user.name'; const value = 'the value'; +const mockWarningToast = jest.fn(); + describe('createFilterOutAction', () => { - const filterOutActionFactory = createFilterOutActionFactory({ filterManager: mockFilterManager }); + const filterOutActionFactory = createFilterOutActionFactory({ + filterManager: mockFilterManager, + notifications: { toasts: { addWarning: mockWarningToast } } as unknown as NotificationsStart, + }); const filterOutAction = filterOutActionFactory({ 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, }, ], @@ -61,6 +67,20 @@ describe('createFilterOutAction', () => { }) ).toEqual(false); }); + + it('should return false if Kbn type is unsupported', async () => { + expect( + await filterOutAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, type: KBN_FIELD_TYPES._SOURCE }, + }, + ], + }) + ).toEqual(false); + }); }); describe('execute', () => { @@ -71,7 +91,11 @@ describe('createFilterOutAction', () => { it('should create negate filter query with value', async () => { await filterOutAction.execute(context); - expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value, negate: true }); + expect(mockCreateFilter).toHaveBeenCalledWith({ + key: fieldName, + value: [value], + negate: true, + }); }); it('should create negate filter query with array value', async () => { @@ -101,7 +125,7 @@ describe('createFilterOutAction', () => { }, ], }); - expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: null, negate: false }); + expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [], negate: false }); }); it('should create filter query with undefined value', async () => { @@ -116,7 +140,7 @@ describe('createFilterOutAction', () => { }); expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, - value: undefined, + value: [], negate: false, }); }); @@ -131,7 +155,7 @@ describe('createFilterOutAction', () => { }, ], }); - expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: '', negate: false }); + expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [''], negate: false }); }); it('should create negate filter query with empty array value', async () => { @@ -146,5 +170,19 @@ describe('createFilterOutAction', () => { }); expect(mockCreateFilter).toHaveBeenCalledWith({ key: fieldName, value: [], negate: false }); }); + + it('should notify the user when value type is unsupported', async () => { + await filterOutAction.execute({ + ...context, + data: [ + { + ...context.data[0], + value: { a: {} }, + }, + ], + }); + expect(mockCreateFilter).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); }); }); diff --git a/packages/kbn-cell-actions/src/actions/filter/filter_out.ts b/packages/kbn-cell-actions/src/actions/filter/filter_out.ts index 2a4622de97286..237a4f11746af 100644 --- a/packages/kbn-cell-actions/src/actions/filter/filter_out.ts +++ b/packages/kbn-cell-actions/src/actions/filter/filter_out.ts @@ -6,11 +6,19 @@ * Side Public License, v 1. */ import { i18n } from '@kbn/i18n'; -import type { FilterManager } from '@kbn/data-plugin/public'; +import type { FilterManager, KBN_FIELD_TYPES } from '@kbn/data-plugin/public'; +import { NotificationsStart } from '@kbn/core-notifications-browser'; import { createFilter, isEmptyFilterValue } from './create_filter'; import { FILTER_CELL_ACTION_TYPE } from '../../constants'; import { createCellActionFactory } from '../factory'; -import { CellActionFieldValue } from '../../types'; +import { + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + valueToArray, + filterOutNullableValues, +} from '../utils'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '../translations'; +import { DefaultActionsSupportedValue } from '../types'; const ICON = 'minusInCircle'; const FILTER_OUT = i18n.translate('cellActions.actions.filterOut', { @@ -18,7 +26,13 @@ const FILTER_OUT = i18n.translate('cellActions.actions.filterOut', { }); export const createFilterOutActionFactory = createCellActionFactory( - ({ filterManager }: { filterManager: FilterManager }) => ({ + ({ + filterManager, + notifications: { toasts }, + }: { + filterManager: FilterManager; + notifications: NotificationsStart; + }) => ({ type: FILTER_CELL_ACTION_TYPE, getIconType: () => ICON, getDisplayName: () => FILTER_OUT, @@ -28,18 +42,27 @@ export const createFilterOutActionFactory = createCellActionFactory( return ( data.length === 1 && // TODO Add support for multiple values - !!field.name + !!field.name && + 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)); - addFilterOut({ - filterManager, - fieldName: field.name, - value, - }); + if (isValueSupportedByDefaultActions(value)) { + addFilterOut({ + filterManager, + fieldName: field.name, + value, + }); + } else { + toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + } }, }) ); @@ -51,7 +74,7 @@ export const addFilterOut = ({ }: { filterManager: FilterManager | undefined; fieldName: string; - value: CellActionFieldValue; + value: DefaultActionsSupportedValue; }) => { if (filterManager != null) { const filter = createFilter({ diff --git a/packages/kbn-cell-actions/src/utils.ts b/packages/kbn-cell-actions/src/actions/translations.ts similarity index 57% rename from packages/kbn-cell-actions/src/utils.ts rename to packages/kbn-cell-actions/src/actions/translations.ts index ef0a7c8dfb004..a8e9828959eee 100644 --- a/packages/kbn-cell-actions/src/utils.ts +++ b/packages/kbn-cell-actions/src/actions/translations.ts @@ -6,8 +6,11 @@ * Side Public License, v 1. */ -import { KBN_FIELD_TYPES } from '@kbn/field-types'; -import { SUPPORTED_KBN_TYPES } from './constants'; +import { i18n } from '@kbn/i18n'; -export const isTypeSupportedByCellActions = (kbnFieldType: KBN_FIELD_TYPES) => - SUPPORTED_KBN_TYPES.includes(kbnFieldType); +export const ACTION_INCOMPATIBLE_VALUE_WARNING = i18n.translate( + 'cellActions.actions.incompatibility.warningMessage', + { + defaultMessage: 'The action can not be executed because the value and type are incompatible', + } +); diff --git a/packages/kbn-cell-actions/src/actions/types.ts b/packages/kbn-cell-actions/src/actions/types.ts new file mode 100644 index 0000000000000..f7700e3c5edaf --- /dev/null +++ b/packages/kbn-cell-actions/src/actions/types.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { SerializableRecord } from '@kbn/utility-types'; +import { SerializableArray } from '@kbn/utility-types/src/serializable'; + +export type DefaultActionsSupportedValue = string[] | number[] | boolean[]; + +export type NonNullableSerializable = + | string + | number + | boolean + | SerializableArray + | SerializableRecord; diff --git a/packages/kbn-cell-actions/src/actions/utils.test.ts b/packages/kbn-cell-actions/src/actions/utils.test.ts new file mode 100644 index 0000000000000..28ad6e8bd02d2 --- /dev/null +++ b/packages/kbn-cell-actions/src/actions/utils.test.ts @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { + filterOutNullableValues, + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, +} from './utils'; + +describe('utils', () => { + describe('isTypeSupportedByDefaultActions', () => { + it('returns true when the type is number', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.NUMBER)).toBe(true); + }); + + it('returns true when the type is string', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.STRING)).toBe(true); + }); + + it('returns true when the type is ip', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.IP)).toBe(true); + }); + + it('returns true when the type is date', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.DATE)).toBe(true); + }); + + it('returns true when the type is boolean', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.BOOLEAN)).toBe(true); + }); + + it('returns false when the type is unknown', () => { + expect(isTypeSupportedByDefaultActions(KBN_FIELD_TYPES.UNKNOWN)).toBe(false); + }); + }); + + describe('isValueSupportedByDefaultActions', () => { + it('returns true when the value is an array of strings', () => { + expect(isValueSupportedByDefaultActions(['string', 'string'])).toBe(true); + }); + + it('returns true when the value is an array of number', () => { + expect(isValueSupportedByDefaultActions([2, 2])).toBe(true); + }); + + it('returns true when the value is an empty array', () => { + expect(isValueSupportedByDefaultActions([])).toBe(true); + }); + + it('returns true when the value is an array of booleans', () => { + expect(isValueSupportedByDefaultActions([false, true])).toBe(true); + }); + + it('returns false when the value is an mixed-type array', () => { + expect(isValueSupportedByDefaultActions([2, 'string', false])).toBe(false); + }); + }); + + describe('filterOutNullableValues', () => { + it('returns empty array when all values are nullable', () => { + expect(filterOutNullableValues([null, undefined, null, undefined])).toEqual([]); + }); + + it('returns the same elements when they are all non-nullable', () => { + expect(filterOutNullableValues([2, 'string', true])).toEqual([2, 'string', true]); + }); + }); +}); diff --git a/packages/kbn-cell-actions/src/actions/utils.ts b/packages/kbn-cell-actions/src/actions/utils.ts new file mode 100644 index 0000000000000..6282f6a7772e3 --- /dev/null +++ b/packages/kbn-cell-actions/src/actions/utils.ts @@ -0,0 +1,39 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { isBoolean, isNumber, isString } from 'lodash/fp'; +import { Serializable, SerializableArray } from '@kbn/utility-types/src/serializable'; +import { DefaultActionsSupportedValue, NonNullableSerializable } from './types'; + +export const SUPPORTED_KBN_TYPES = [ + KBN_FIELD_TYPES.DATE, + KBN_FIELD_TYPES.IP, + KBN_FIELD_TYPES.STRING, + KBN_FIELD_TYPES.NUMBER, + KBN_FIELD_TYPES.BOOLEAN, +]; + +export const isTypeSupportedByDefaultActions = (kbnFieldType: KBN_FIELD_TYPES) => + SUPPORTED_KBN_TYPES.includes(kbnFieldType); + +const isNonMixedTypeArray = ( + value: Array +): value is string[] | number[] | boolean[] => value.every((v) => typeof v === typeof value[0]); + +export const isValueSupportedByDefaultActions = ( + value: NonNullableSerializable[] +): value is DefaultActionsSupportedValue => + value.every((v): v is string | number | boolean => isString(v) || isNumber(v) || isBoolean(v)) && + isNonMixedTypeArray(value); + +export const filterOutNullableValues = (value: SerializableArray): NonNullableSerializable[] => + value.filter((v): v is NonNullableSerializable => v != null); + +export const valueToArray = (value: Serializable): SerializableArray => + Array.isArray(value) ? value : [value]; diff --git a/packages/kbn-cell-actions/src/constants.ts b/packages/kbn-cell-actions/src/constants.ts index 4e6b1d0f36235..70b9862a933de 100644 --- a/packages/kbn-cell-actions/src/constants.ts +++ b/packages/kbn-cell-actions/src/constants.ts @@ -6,8 +6,6 @@ * Side Public License, v 1. */ -import { KBN_FIELD_TYPES } from '@kbn/field-types'; - export const FILTER_CELL_ACTION_TYPE = 'cellAction-filter'; export const COPY_CELL_ACTION_TYPE = 'cellAction-copy'; @@ -16,11 +14,3 @@ export enum CellActionsMode { HOVER_RIGHT = 'hover-right', INLINE = 'inline', } - -export const SUPPORTED_KBN_TYPES = [ - KBN_FIELD_TYPES.DATE, - KBN_FIELD_TYPES.IP, - KBN_FIELD_TYPES.STRING, - KBN_FIELD_TYPES.NUMBER, - KBN_FIELD_TYPES.BOOLEAN, -]; diff --git a/packages/kbn-cell-actions/src/types.ts b/packages/kbn-cell-actions/src/types.ts index 142144b13b72a..bd7cb69c38aac 100644 --- a/packages/kbn-cell-actions/src/types.ts +++ b/packages/kbn-cell-actions/src/types.ts @@ -11,6 +11,7 @@ import type { UiActionsService, } from '@kbn/ui-actions-plugin/public'; import type { FieldSpec } from '@kbn/data-views-plugin/common'; +import { Serializable } from '@kbn/utility-types'; import type { CellActionsMode } from './constants'; export interface CellActionsProviderProps { @@ -24,14 +25,14 @@ export interface CellActionsProviderProps { type Metadata = Record; export type CellActionFieldValue = - | string - | number - | boolean + | Serializable + // Add primitive array types to allow type guards to work. + // Because SerializableArray is a cyclic self referenced Array. | string[] | number[] | boolean[] - | null - | undefined; + | null[] + | undefined[]; export interface CellActionsData { /** diff --git a/packages/kbn-cell-actions/src/utils.test.ts b/packages/kbn-cell-actions/src/utils.test.ts deleted file mode 100644 index 36cf1dfa4cccb..0000000000000 --- a/packages/kbn-cell-actions/src/utils.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { KBN_FIELD_TYPES } from '@kbn/field-types'; -import { isTypeSupportedByCellActions } from './utils'; - -describe('isTypeSupportedByCellActions', () => { - it('returns true if the type is number', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.NUMBER)).toBe(true); - }); - - it('returns true if the type is string', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.STRING)).toBe(true); - }); - - it('returns true if the type is ip', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.IP)).toBe(true); - }); - - it('returns true if the type is date', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.DATE)).toBe(true); - }); - - it('returns true if the type is boolean', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.BOOLEAN)).toBe(true); - }); - - it('returns false if the type is unknown', () => { - expect(isTypeSupportedByCellActions(KBN_FIELD_TYPES.UNKNOWN)).toBe(false); - }); -}); diff --git a/packages/kbn-cell-actions/tsconfig.json b/packages/kbn-cell-actions/tsconfig.json index c504b32bac30d..f7ac4890347af 100644 --- a/packages/kbn-cell-actions/tsconfig.json +++ b/packages/kbn-cell-actions/tsconfig.json @@ -21,6 +21,8 @@ "@kbn/ui-actions-plugin", "@kbn/field-types", "@kbn/data-views-plugin", + "@kbn/core-notifications-browser", + "@kbn/utility-types", ], "exclude": ["target/**/*"] } diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx index c6d0bb4e0f48b..bd6529dd44314 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.test.tsx @@ -275,42 +275,6 @@ describe('DiscoverGrid', () => { }) ); }); - - it('should call useDataGridColumnsCellActions with empty field name and type for unsupported field types', async () => { - await getComponent({ - ...getProps(), - columns: ['message', '_source'], - onFieldEdited: jest.fn(), - cellActionsTriggerId: 'test', - }); - - expect(mockUseDataGridColumnsCellActions).toHaveBeenCalledWith( - expect.objectContaining({ - triggerId: 'test', - getCellValue: expect.any(Function), - fields: [ - { - name: '@timestamp', - type: 'date', - aggregatable: true, - searchable: undefined, - }, - { - name: 'message', - type: 'string', - aggregatable: false, - searchable: undefined, - }, - { - searchable: false, - aggregatable: false, - name: '', - type: '', - }, - ], - }) - ); - }); }); describe('sorting', () => { diff --git a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx index 49d4f61def707..3d4acfc34d925 100644 --- a/src/plugins/discover/public/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/components/discover_grid/discover_grid.tsx @@ -29,14 +29,12 @@ import type { SortOrder } from '@kbn/saved-search-plugin/public'; import { useDataGridColumnsCellActions, type UseDataGridColumnsCellActionsProps, - type CellActionFieldValue, } from '@kbn/cell-actions'; import type { AggregateQuery, Filter, Query } from '@kbn/es-query'; import { FieldFormatsStart } from '@kbn/field-formats-plugin/public'; import type { ToastsStart, IUiSettingsClient, HttpStart, CoreStart } from '@kbn/core/public'; import { DataViewFieldEditorStart } from '@kbn/data-view-field-editor-plugin/public'; -import { KBN_FIELD_TYPES } from '@kbn/data-plugin/common'; -import { isTypeSupportedByCellActions } from '@kbn/cell-actions/src/utils'; +import { Serializable } from '@kbn/utility-types'; import { DocViewFilterFn } from '../../services/doc_views/doc_views_types'; import { getSchemaDetectors } from './discover_grid_schema'; import { DiscoverGridFlyout } from './discover_grid_flyout'; @@ -451,7 +449,7 @@ export const DiscoverGrid = ({ const getCellValue = useCallback( (fieldName, rowIndex) => - displayedRows[rowIndex % displayedRows.length].flattened[fieldName] as CellActionFieldValue, + displayedRows[rowIndex % displayedRows.length].flattened[fieldName] as Serializable, [displayedRows] ); @@ -460,8 +458,7 @@ export const DiscoverGrid = ({ cellActionsTriggerId && !isPlainRecord ? visibleColumns.map((columnName) => { const field = dataView.getFieldByName(columnName); - if (!field || !isTypeSupportedByCellActions(field.type as KBN_FIELD_TYPES)) { - // disable custom actions on object columns + if (!field) { return { name: '', type: '', diff --git a/x-pack/packages/security-solution/data_table/components/data_table/index.tsx b/x-pack/packages/security-solution/data_table/components/data_table/index.tsx index b11c932b60b58..6c71d796dc9ea 100644 --- a/x-pack/packages/security-solution/data_table/components/data_table/index.tsx +++ b/x-pack/packages/security-solution/data_table/components/data_table/index.tsx @@ -338,7 +338,7 @@ export const DataTableComponent = React.memo( ? // TODO use FieldSpec object instead of column columnHeaders.map((column) => ({ name: column.id, - type: column.type ?? 'keyword', + type: column.type ?? '', // When type is an empty string all cell actions are incompatible aggregatable: column.aggregatable ?? false, searchable: column.searchable ?? false, esTypes: column.esTypes ?? [], diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.test.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.test.ts index 3eeb09d1b8188..dade311fa6b49 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.test.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.test.ts @@ -13,6 +13,7 @@ import type { CellActionExecutionContext } from '@kbn/cell-actions'; import { GEO_FIELD_TYPE } from '../../../timelines/components/timeline/body/renderers/constants'; import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; import { set } from 'lodash/fp'; +import { KBN_FIELD_TYPES } from '@kbn/field-types'; const services = createStartServicesMock(); const mockWarningToast = services.notifications.toasts.addWarning; @@ -27,7 +28,7 @@ const value = 'the-value'; const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value, }, ], @@ -75,6 +76,7 @@ describe('createAddToTimelineCellAction', () => { it('should return true if everything is okay', async () => { expect(await addToTimelineAction.isCompatible(context)).toEqual(true); }); + it('should return false if field not allowed', async () => { expect( await addToTimelineAction.isCompatible({ @@ -88,6 +90,20 @@ describe('createAddToTimelineCellAction', () => { }) ).toEqual(false); }); + + it('should return false if Kbn type is unsupported', async () => { + expect( + await addToTimelineAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, type: KBN_FIELD_TYPES.DATE_RANGE }, + }, + ], + }) + ).toEqual(false); + }); }); describe('execute', () => { @@ -190,6 +206,20 @@ describe('createAddToTimelineCellAction', () => { expect(mockWarningToast).toHaveBeenCalled(); }); + it('should show warning if value type is unsupported', async () => { + await addToTimelineAction.execute({ + ...context, + data: [ + { + ...context.data[0], + value: {}, + }, + ], + }); + expect(mockDispatch).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); + describe('should execute correctly when negateFilters is provided', () => { it('should not exclude if negateFilters is false', async () => { await addToTimelineAction.execute({ diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.ts index 0f5dd25ed6b64..9ce248701a75d 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/add_to_timeline.ts @@ -7,6 +7,14 @@ import { createCellActionFactory } from '@kbn/cell-actions'; import type { CellActionTemplate } from '@kbn/cell-actions'; +import { + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + filterOutNullableValues, + valueToArray, +} from '@kbn/cell-actions/src/actions/utils'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '@kbn/cell-actions/src/actions/translations'; +import type { KBN_FIELD_TYPES } from '@kbn/field-types'; import { addProvider } from '../../../timelines/store/timeline/actions'; import { TimelineId } from '../../../../common/types'; import type { SecurityAppStore } from '../../../common/store'; @@ -44,15 +52,24 @@ export const createAddToTimelineCellActionFactory = createCellActionFactory( return ( data.length === 1 && // TODO Add support for multiple values fieldHasCellActions(field.name) && - isValidDataProviderField(field.name, field.type) + isValidDataProviderField(field.name, field.type) && + isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES) ); }, + execute: async ({ data, metadata }) => { const { name, type } = data[0]?.field; - const value = data[0]?.value; + const rawValue = data[0]?.value; + const value = filterOutNullableValues(valueToArray(rawValue)); + + if (!isValueSupportedByDefaultActions(value)) { + notificationsService.toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + return; + } - const values = Array.isArray(value) ? value : [value]; - const [firstValue, ...andValues] = values; + const [firstValue, ...andValues] = value; const [dataProvider] = createDataProviders({ contextId: TimelineId.active, @@ -81,10 +98,7 @@ export const createAddToTimelineCellActionFactory = createCellActionFactory( if (dataProvider) { store.dispatch(addProvider({ id: TimelineId.active, providers: [dataProvider] })); - let messageValue = ''; - if (value != null) { - messageValue = Array.isArray(value) ? value.join(', ') : value.toString(); - } + const messageValue = value.join(', '); notificationsService.toasts.addSuccess({ title: ADD_TO_TIMELINE_SUCCESS_TITLE(messageValue), }); diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.test.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.test.ts index 97edad91e5a01..d8c83e6a336d9 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.test.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.test.ts @@ -13,6 +13,7 @@ import type { CellActionExecutionContext } from '@kbn/cell-actions'; import { GEO_FIELD_TYPE } from '../../../timelines/components/timeline/body/renderers/constants'; import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; import { timelineActions } from '../../../timelines/store/timeline'; +import { KBN_FIELD_TYPES } from '@kbn/field-types'; const services = createStartServicesMock(); const mockWarningToast = services.notifications.toasts.addWarning; @@ -27,7 +28,7 @@ const value = 'the-value'; const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value, }, ], @@ -78,6 +79,7 @@ describe('createAddToNewTimelineCellAction', () => { it('should return true if everything is okay', async () => { expect(await addToTimelineAction.isCompatible(context)).toEqual(true); }); + it('should return false if field not allowed', async () => { expect( await addToTimelineAction.isCompatible({ @@ -91,6 +93,20 @@ describe('createAddToNewTimelineCellAction', () => { }) ).toEqual(false); }); + + it('should return false if Kbn type is unsupported', async () => { + expect( + await addToTimelineAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, type: KBN_FIELD_TYPES.NESTED }, + }, + ], + }) + ).toEqual(false); + }); }); describe('execute', () => { @@ -114,6 +130,20 @@ describe('createAddToNewTimelineCellAction', () => { expect(mockWarningToast).toHaveBeenCalled(); }); + it('should show warning if value type is unsupported', async () => { + await addToTimelineAction.execute({ + ...context, + data: [ + { + ...context.data[0], + value: [[[]]], + }, + ], + }); + expect(mockDispatch).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); + describe('should execute correctly when negateFilters is provided', () => { it('should not exclude if negateFilters is false', async () => { await addToTimelineAction.execute({ diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.ts index b24c0c39ce365..0ba3cf3ffc8a8 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/cell_action/investigate_in_new_timeline.ts @@ -6,6 +6,14 @@ */ import { createCellActionFactory, type CellActionTemplate } from '@kbn/cell-actions'; +import type { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + valueToArray, + filterOutNullableValues, +} from '@kbn/cell-actions/src/actions/utils'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '@kbn/cell-actions/src/actions/translations'; import { timelineActions } from '../../../timelines/store/timeline'; import { addProvider, showTimeline } from '../../../timelines/store/timeline/actions'; import { TimelineId } from '../../../../common/types'; @@ -44,12 +52,21 @@ export const createInvestigateInNewTimelineCellActionFactory = createCellActionF return ( data.length === 1 && // TODO Add support for multiple values fieldHasCellActions(field.name) && - isValidDataProviderField(field.name, field.type) + isValidDataProviderField(field.name, field.type) && + isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES) ); }, execute: async ({ data, metadata }) => { const field = data[0]?.field; - const value = data[0]?.value; + const rawValue = data[0]?.value; + const value = filterOutNullableValues(valueToArray(rawValue)); + + if (!isValueSupportedByDefaultActions(value)) { + notificationsService.toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + return; + } const dataProviders = createDataProviders({ diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts index 4c79ed3cc56cb..ac50c5de28ef5 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts @@ -5,7 +5,6 @@ * 2.0. */ -import type { CellActionFieldValue } from '@kbn/cell-actions/src/types'; import { escapeDataProviderId } from '@kbn/securitysolution-t-grid'; import type { Serializable } from '@kbn/utility-types'; @@ -61,7 +60,7 @@ export interface CreateDataProviderParams { field?: string; fieldFormat?: string; fieldType?: string; - values: CellActionFieldValue; + values: string | string[] | number | number[] | boolean | boolean[]; sourceParamType?: Serializable; negate?: boolean; } @@ -78,7 +77,11 @@ export const createDataProviders = ({ }: CreateDataProviderParams) => { if (field == null) return null; - const arrayValues = Array.isArray(values) ? (values.length > 0 ? values : [null]) : [values]; + const arrayValues: Array = Array.isArray(values) + ? values.length > 0 + ? values + : [null] + : [values]; return arrayValues.reduce((dataProviders, rawValue, index) => { let id: string = ''; diff --git a/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts index d61a1ab64bdb8..f8c62857e3e24 100644 --- a/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts +++ b/x-pack/plugins/security_solution/public/actions/add_to_timeline/discover/add_to_timeline.test.ts @@ -29,7 +29,7 @@ const store = { const value = 'the-value'; const context = { - data: [{ field: { name: 'user.name', type: 'text' }, value }], + data: [{ field: { name: 'user.name', type: 'string' }, value }], } as CellActionExecutionContext; const defaultDataProvider = { diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts index 6df6f6b00b177..ce4655bd6afe2 100644 --- a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.test.ts @@ -19,7 +19,7 @@ describe('createCopyToClipboardCellActionFactory', () => { const copyToClipboardActionFactory = createCopyToClipboardCellActionFactory({ services }); const copyToClipboardAction = copyToClipboardActionFactory({ id: 'testAction' }); const context = { - data: [{ field: { name: 'user.name', type: 'text' }, value: 'the value' }], + data: [{ field: { name: 'user.name', type: 'string' }, value: 'the value' }], } as CellActionExecutionContext; beforeEach(() => { diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.ts index 39d39ea8b4910..e4e30a0576cbb 100644 --- a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.ts +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/cell_action/copy_to_clipboard.ts @@ -25,10 +25,7 @@ export const createCopyToClipboardCellActionFactory = ({ isCompatible: async ({ data }) => { const field = data[0]?.field; - return ( - data.length === 1 && // TODO Add support for multiple values - fieldHasCellActions(field.name) - ); + return fieldHasCellActions(field.name); }, }); }; diff --git a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts index 23ca470e11d22..0a4032e9b0136 100644 --- a/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts +++ b/x-pack/plugins/security_solution/public/actions/copy_to_clipboard/discover/copy_to_clipboard.test.ts @@ -26,7 +26,7 @@ describe('createCopyToClipboardDiscoverCellActionFactory', () => { const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value: 'the value', }, ], diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts index 2e2a511933476..01650820294bd 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.test.ts @@ -18,10 +18,12 @@ import type { SecurityCellActionExecutionContext } from '../../types'; import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; import { TableId } from '@kbn/securitysolution-data-table'; import { TimelineId } from '../../../../common/types'; +import { KBN_FIELD_TYPES } from '@kbn/field-types'; const services = createStartServicesMock(); const mockGlobalFilterManager = services.data.query.filterManager; const mockTimelineFilterManager = createFilterManagerMock(); +const mockWarningToast = services.notifications.toasts.addWarning; const mockState = { ...mockGlobalState, @@ -57,7 +59,7 @@ describe('createFilterInCellActionFactory', () => { const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value: 'the value', }, ], @@ -75,18 +77,34 @@ describe('createFilterInCellActionFactory', () => { it('should return true if everything is okay', async () => { expect(await filterInAction.isCompatible(context)).toEqual(true); }); + it('should return false if field not allowed', async () => { expect( await filterInAction.isCompatible({ ...context, data: [ { + ...context.data[0], field: { ...context.data[0].field, name: 'signal.reason' }, }, ], }) ).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.HISTOGRAM }, + }, + ], + }) + ).toEqual(false); + }); }); describe('execute', () => { @@ -101,6 +119,21 @@ describe('createFilterInCellActionFactory', () => { expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); }); + + it('should show warning if value type is unsupported', async () => { + await filterInAction.execute({ + ...dataTableContext, + data: [ + { + ...context.data[0], + value: { test: '123' }, + }, + ], + }); + expect(mockGlobalFilterManager.addFilters).not.toHaveBeenCalled(); + expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); }); describe('timeline scope execution', () => { diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.ts index 6285d662fff93..905da379d01ff 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_in.ts @@ -6,6 +6,14 @@ */ import { addFilterIn, addFilterOut, createFilterInActionFactory } from '@kbn/cell-actions'; +import { + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + valueToArray, + filterOutNullableValues, +} from '@kbn/cell-actions/src/actions/utils'; +import type { KBN_FIELD_TYPES } from '@kbn/field-types'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '@kbn/cell-actions/src/actions/translations'; import type { SecurityAppStore } from '../../../common/store'; import { timelineSelectors } from '../../../timelines/store/timeline'; import { fieldHasCellActions } from '../../utils'; @@ -25,7 +33,11 @@ export const createFilterInCellActionFactory = ({ const getTimelineById = timelineSelectors.getTimelineByIdSelector(); const { filterManager } = services.data.query; - const genericFilterInActionFactory = createFilterInActionFactory({ filterManager }); + const { notifications } = services; + const genericFilterInActionFactory = createFilterInActionFactory({ + filterManager, + notifications, + }); return genericFilterInActionFactory.combine({ type: SecurityCellActionType.FILTER, @@ -34,12 +46,21 @@ export const createFilterInCellActionFactory = ({ return ( data.length === 1 && // TODO Add support for multiple values - fieldHasCellActions(field.name) + fieldHasCellActions(field.name) && + isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES) ); }, execute: async ({ data, metadata }) => { const field = data[0]?.field; - const value = data[0]?.value; + const rawValue = data[0]?.value; + const value = filterOutNullableValues(valueToArray(rawValue)); + + if (!isValueSupportedByDefaultActions(value)) { + notifications.toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + return; + } if (!field) return; diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts index b4c855c3a4509..088cc944cb1c7 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.test.ts @@ -18,10 +18,12 @@ import type { SecurityCellActionExecutionContext } from '../../types'; import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock'; import { TimelineId } from '../../../../common/types'; import { TableId } from '@kbn/securitysolution-data-table'; +import { KBN_FIELD_TYPES } from '@kbn/field-types'; const services = createStartServicesMock(); const mockGlobalFilterManager = services.data.query.filterManager; const mockTimelineFilterManager = createFilterManagerMock(); +const mockWarningToast = services.notifications.toasts.addWarning; const mockState = { ...mockGlobalState, @@ -51,7 +53,7 @@ describe('createFilterOutCellActionFactory', () => { const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value: 'the value', }, ], @@ -69,6 +71,7 @@ describe('createFilterOutCellActionFactory', () => { it('should return true if everything is okay', async () => { expect(await filterOutAction.isCompatible(context)).toEqual(true); }); + it('should return false if field not allowed', async () => { expect( await filterOutAction.isCompatible({ @@ -81,6 +84,20 @@ describe('createFilterOutCellActionFactory', () => { }) ).toEqual(false); }); + + it('should return false if Kbn type is unsupported', async () => { + expect( + await filterOutAction.isCompatible({ + ...context, + data: [ + { + ...context.data[0], + field: { ...context.data[0].field, type: KBN_FIELD_TYPES.OBJECT }, + }, + ], + }) + ).toEqual(false); + }); }); describe('execute', () => { @@ -95,6 +112,21 @@ describe('createFilterOutCellActionFactory', () => { expect(mockGlobalFilterManager.addFilters).toHaveBeenCalled(); expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); }); + + it('should show warning if value type is unsupported', async () => { + await filterOutAction.execute({ + ...dataTableContext, + data: [ + { + ...context.data[0], + value: [{ test: 'value' }], + }, + ], + }); + expect(mockGlobalFilterManager.addFilters).not.toHaveBeenCalled(); + expect(mockTimelineFilterManager.addFilters).not.toHaveBeenCalled(); + expect(mockWarningToast).toHaveBeenCalled(); + }); }); describe('timeline scope execution', () => { diff --git a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.ts b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.ts index faa591b2c1617..38c2e52b7c7c7 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/cell_action/filter_out.ts @@ -6,6 +6,14 @@ */ import { addFilterIn, addFilterOut, createFilterOutActionFactory } from '@kbn/cell-actions'; +import { + isTypeSupportedByDefaultActions, + isValueSupportedByDefaultActions, + valueToArray, + filterOutNullableValues, +} from '@kbn/cell-actions/src/actions/utils'; +import { ACTION_INCOMPATIBLE_VALUE_WARNING } from '@kbn/cell-actions/src/actions/translations'; +import type { KBN_FIELD_TYPES } from '@kbn/field-types'; import { fieldHasCellActions } from '../../utils'; import type { SecurityAppStore } from '../../../common/store'; import type { StartServices } from '../../../types'; @@ -25,7 +33,12 @@ export const createFilterOutCellActionFactory = ({ const getTimelineById = timelineSelectors.getTimelineByIdSelector(); const { filterManager } = services.data.query; - const genericFilterOutActionFactory = createFilterOutActionFactory({ filterManager }); + const { notifications } = services; + + const genericFilterOutActionFactory = createFilterOutActionFactory({ + filterManager, + notifications, + }); return genericFilterOutActionFactory.combine({ type: SecurityCellActionType.FILTER, @@ -34,14 +47,24 @@ export const createFilterOutCellActionFactory = ({ return ( data.length === 1 && // TODO Add support for multiple values - fieldHasCellActions(field.name) + fieldHasCellActions(field.name) && + isTypeSupportedByDefaultActions(field.type as KBN_FIELD_TYPES) ); }, execute: async ({ data, metadata }) => { const field = data[0]?.field; - const value = data[0]?.value; + const rawValue = data[0]?.value; + const value = filterOutNullableValues(valueToArray(rawValue)); + + if (!isValueSupportedByDefaultActions(value)) { + notifications.toasts.addWarning({ + title: ACTION_INCOMPATIBLE_VALUE_WARNING, + }); + return; + } if (!field) return; + // if negateFilters is true we have to perform the opposite operation, we can just execute filterIn with the same params const addFilter = metadata?.negateFilters === true ? addFilterIn : addFilterOut; diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts index aefe4698ed09e..a221210ce63d4 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_in.test.ts @@ -46,7 +46,7 @@ describe('createFilterInDiscoverCellActionFactory', () => { }); const context = { - data: [{ field: { name: 'user.name', type: 'text' }, value: 'the value' }], + data: [{ field: { name: 'user.name', type: 'string' }, value: 'the value' }], } as SecurityCellActionExecutionContext; it('should return display name', () => { diff --git a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts index 42f6bad3040b8..bcbc4dcbbad39 100644 --- a/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts +++ b/x-pack/plugins/security_solution/public/actions/filter/discover/filter_out.test.ts @@ -48,7 +48,7 @@ describe('createFilterOutDiscoverCellActionFactory', () => { const context = { data: [ { - field: { name: 'user.name', type: 'text' }, + field: { name: 'user.name', type: 'string' }, value: 'the value', }, ], diff --git a/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx b/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx index cc7d8458e06c4..0e1c419b0449e 100644 --- a/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx +++ b/x-pack/plugins/security_solution/public/actions/show_top_n/cell_action/show_top_n.tsx @@ -29,7 +29,7 @@ const SHOW_TOP = (fieldName: string) => }); const ICON = 'visBarVertical'; -const UNSUPPORTED_FIELD_TYPES = [ES_FIELD_TYPES.DATE, ES_FIELD_TYPES.TEXT]; +const UNSUPPORTED_ES_FIELD_TYPES = [ES_FIELD_TYPES.DATE, ES_FIELD_TYPES.TEXT]; export const createShowTopNCellActionFactory = createCellActionFactory( ({ @@ -52,7 +52,7 @@ export const createShowTopNCellActionFactory = createCellActionFactory( data.length === 1 && fieldHasCellActions(field.name) && (field.esTypes ?? []).every( - (esType) => !UNSUPPORTED_FIELD_TYPES.includes(esType as ES_FIELD_TYPES) + (esType) => !UNSUPPORTED_ES_FIELD_TYPES.includes(esType as ES_FIELD_TYPES) ) && !!field.aggregatable ); diff --git a/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx b/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx index 393232924ed3c..b6987f2fe29f6 100644 --- a/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx +++ b/x-pack/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_cell_actions.tsx @@ -72,7 +72,7 @@ export const getUseCellActionsHook = (tableId: TableId) => { const browserField: Partial | undefined = browserFieldsByName[column.id]; return { name: column.id, - type: browserField?.type ?? 'keyword', + type: browserField?.type ?? '', // When type is an empty string all cell actions are incompatible esTypes: browserField?.esTypes ?? [], aggregatable: browserField?.aggregatable ?? false, searchable: browserField?.searchable ?? false,