From 28b6179aece526bd767bb190d696736d65d31cd5 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Mon, 29 Jul 2024 10:01:53 -0600 Subject: [PATCH] [Controls] Fix error thrown on numeric options list (#188789) ## Summary This PR makes the suggestions returned from the options list route more type safe by ensuring that strings are **always** returned - previously, it sometimes returned numbers, which was inconsistent with our defined types. Unfortunately, this messed with the field formatter for date fields specifically - so, to get around this, I've had to convert date fields back to a number specifically for the formatter **only**. This resolves the error that was getting thrown for numeric options list controls, which was happening because we were returning **numbers** from the suggestions route rather than strings and some recent changes to the `EuiSelectable` component require strings: **Before:** https://github.com/user-attachments/assets/0e723e2f-e8f0-4466-b857-8164088cd1e7 **After** https://github.com/user-attachments/assets/d9b138b9-de27-4e14-8c85-0ce4bfde16ce ### Checklist - [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 - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../options_list/options_list_selections.ts | 16 ++++ .../controls/common/options_list/types.ts | 22 +++--- .../public/hooks/use_field_formatter.ts | 8 +- .../components/options_list_control.tsx | 16 ++-- .../components/options_list_popover.test.tsx | 65 ++++++++++++++-- ...ptions_list_popover_invalid_selections.tsx | 12 ++- .../options_list_popover_suggestions.tsx | 40 +++++++--- .../components/options_list_strings.ts | 4 + .../embeddable/options_list_embeddable.tsx | 16 ++-- .../options_list/options_list_reducers.ts | 22 ++++-- .../controls/public/options_list/types.ts | 12 +-- .../options_list_suggestions_route.ts | 1 + .../options_list_validation_queries.test.ts | 1 + .../options_list_validation_queries.ts | 16 ++-- .../options_list_all_suggestions.test.ts | 12 +-- .../options_list_exact_match.test.ts | 77 ++++++++++++------- .../options_list_search_suggestions.test.ts | 30 ++++++-- .../controls/server/options_list/types.ts | 7 +- src/plugins/controls/tsconfig.json | 8 +- .../options_list/options_list_suggestions.ts | 33 ++++++++ .../alerts/detection_page_filters.cy.ts | 2 +- 21 files changed, 305 insertions(+), 115 deletions(-) create mode 100644 src/plugins/controls/common/options_list/options_list_selections.ts diff --git a/src/plugins/controls/common/options_list/options_list_selections.ts b/src/plugins/controls/common/options_list/options_list_selections.ts new file mode 100644 index 0000000000000..0f635ad9301e3 --- /dev/null +++ b/src/plugins/controls/common/options_list/options_list_selections.ts @@ -0,0 +1,16 @@ +/* + * 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 { FieldSpec } from '@kbn/data-views-plugin/common'; + +export type OptionsListSelection = string | number; + +export const getSelectionAsFieldType = (field: FieldSpec, key: string): OptionsListSelection => { + const storeAsNumber = field.type === 'number' || field.type === 'date'; + return storeAsNumber ? +key : key; +}; diff --git a/src/plugins/controls/common/options_list/types.ts b/src/plugins/controls/common/options_list/types.ts index 56085d7d5848f..1242e866ca089 100644 --- a/src/plugins/controls/common/options_list/types.ts +++ b/src/plugins/controls/common/options_list/types.ts @@ -10,6 +10,7 @@ import { DataView, FieldSpec, RuntimeFieldSpec } from '@kbn/data-views-plugin/co import type { BoolQuery, Filter, Query, TimeRange } from '@kbn/es-query'; import type { DataControlInput } from '../types'; +import { OptionsListSelection } from './options_list_selections'; import { OptionsListSearchTechnique } from './suggestions_searching'; import type { OptionsListSortingType } from './suggestions_sorting'; @@ -18,7 +19,7 @@ export const OPTIONS_LIST_CONTROL = 'optionsListControl'; // TODO: Replace with export interface OptionsListEmbeddableInput extends DataControlInput { searchTechnique?: OptionsListSearchTechnique; sort?: OptionsListSortingType; - selectedOptions?: string[]; + selectedOptions?: OptionsListSelection[]; existsSelected?: boolean; runPastTimeout?: boolean; singleSelect?: boolean; @@ -30,7 +31,7 @@ export interface OptionsListEmbeddableInput extends DataControlInput { exclude?: boolean; } -export type OptionsListSuggestions = Array<{ value: string; docCount?: number }>; +export type OptionsListSuggestions = Array<{ value: OptionsListSelection; docCount?: number }>; /** * The Options list response is returned from the serverside Options List route. @@ -38,7 +39,7 @@ export type OptionsListSuggestions = Array<{ value: string; docCount?: number }> export interface OptionsListSuccessResponse { suggestions: OptionsListSuggestions; totalCardinality?: number; // total cardinality will be undefined when `useExpensiveQueries` is `false` - invalidSelections?: string[]; + invalidSelections?: OptionsListSelection[]; } /** @@ -61,12 +62,9 @@ export type OptionsListResponse = OptionsListSuccessResponse | OptionsListFailur */ export type OptionsListRequest = Omit< OptionsListRequestBody, - 'filters' | 'fieldName' | 'fieldSpec' | 'textFieldName' + 'filters' | 'fieldName' | 'fieldSpec' > & { - searchTechnique?: OptionsListSearchTechnique; - allowExpensiveQueries: boolean; timeRange?: TimeRange; - runPastTimeout?: boolean; dataView: DataView; filters?: Filter[]; field: FieldSpec; @@ -76,16 +74,16 @@ export type OptionsListRequest = Omit< /** * The Options list request body is sent to the serverside Options List route and is used to create the ES query. */ -export interface OptionsListRequestBody { +export interface OptionsListRequestBody + extends Pick< + OptionsListEmbeddableInput, + 'fieldName' | 'searchTechnique' | 'sort' | 'selectedOptions' + > { runtimeFieldMap?: Record; - searchTechnique?: OptionsListSearchTechnique; allowExpensiveQueries: boolean; - sort?: OptionsListSortingType; filters?: Array<{ bool: BoolQuery }>; - selectedOptions?: Array; runPastTimeout?: boolean; searchString?: string; fieldSpec?: FieldSpec; - fieldName: string; size: number; } diff --git a/src/plugins/controls/public/hooks/use_field_formatter.ts b/src/plugins/controls/public/hooks/use_field_formatter.ts index 5485be0bb1b23..0a775384f9595 100644 --- a/src/plugins/controls/public/hooks/use_field_formatter.ts +++ b/src/plugins/controls/public/hooks/use_field_formatter.ts @@ -5,8 +5,10 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { FieldSpec } from '@kbn/data-views-plugin/common'; import { useEffect, useState } from 'react'; + +import { FieldSpec } from '@kbn/data-views-plugin/common'; + import { pluginServices } from '../services'; export const useFieldFormatter = ({ @@ -19,7 +21,7 @@ export const useFieldFormatter = ({ const { dataViews: { get: getDataViewById }, } = pluginServices.getServices(); - const [fieldFormatter, setFieldFormatter] = useState(() => (toFormat: string) => toFormat); + const [fieldFormatter, setFieldFormatter] = useState(() => (toFormat: any) => String(toFormat)); /** * derive field formatter from fieldSpec and dataViewId @@ -32,7 +34,7 @@ export const useFieldFormatter = ({ setFieldFormatter( () => dataView?.getFormatterForField(fieldSpec).getConverterFor('text') ?? - ((toFormat: string) => toFormat) + ((toFormat: any) => String(toFormat)) ); })(); }, [fieldSpec, dataViewId, getDataViewById]); diff --git a/src/plugins/controls/public/options_list/components/options_list_control.tsx b/src/plugins/controls/public/options_list/components/options_list_control.tsx index 4d2e41c3f7fa7..ef3aa89bcc2cc 100644 --- a/src/plugins/controls/public/options_list/components/options_list_control.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_control.tsx @@ -6,10 +6,10 @@ * Side Public License, v 1. */ -import { Subject } from 'rxjs'; import classNames from 'classnames'; import { debounce, isEmpty } from 'lodash'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { Subject } from 'rxjs'; import { EuiFilterButton, @@ -22,15 +22,16 @@ import { htmlIdGenerator, } from '@elastic/eui'; +import { OptionsListSelection } from '../../../common/options_list/options_list_selections'; +import { MIN_POPOVER_WIDTH } from '../../constants'; +import { ControlError } from '../../control_group/component/control_error_component'; +import { useFieldFormatter } from '../../hooks/use_field_formatter'; +import { useOptionsList } from '../embeddable/options_list_embeddable'; import { MAX_OPTIONS_LIST_REQUEST_SIZE } from '../types'; -import { OptionsListStrings } from './options_list_strings'; import { OptionsListPopover } from './options_list_popover'; -import { useOptionsList } from '../embeddable/options_list_embeddable'; +import { OptionsListStrings } from './options_list_strings'; import './options_list.scss'; -import { ControlError } from '../../control_group/component/control_error_component'; -import { MIN_POPOVER_WIDTH } from '../../constants'; -import { useFieldFormatter } from '../../hooks/use_field_formatter'; export const OptionsListControl = ({ typeaheadSubject, @@ -128,13 +129,14 @@ export const OptionsListControl = ({ ) : ( <> {selectedOptions?.length - ? selectedOptions.map((value: string, i, { length }) => { + ? selectedOptions.map((value: OptionsListSelection, i, { length }) => { const text = `${fieldFormatter(value)}${ i + 1 === length ? '' : delimiter } `; const isInvalid = invalidSelections?.includes(value); return ( { @@ -168,6 +169,7 @@ describe('Options list popover', () => { test('clicking another option unselects "Exists"', async () => { const popover = await mountComponent({ explicitInput: { existsSelected: true }, + componentState: { field: { type: 'string' } as FieldSpec }, }); const woofOption = popover.getByTestId('optionsList-control-selection-woof'); userEvent.click(woofOption); @@ -185,6 +187,7 @@ describe('Options list popover', () => { const selections = ['woof', 'bark']; const popover = await mountComponent({ explicitInput: { existsSelected: false, selectedOptions: selections }, + componentState: { field: { type: 'number' } as FieldSpec }, }); const existsOption = popover.getByTestId('optionsList-control-selection-exists'); let availableOptionsDiv = popover.getByTestId('optionsList-control-available-options'); @@ -363,4 +366,56 @@ describe('Options list popover', () => { }); }); }); + + describe('field formatter', () => { + const mockedFormatter = jest.fn().mockImplementation((value: unknown) => `formatted:${value}`); + + beforeAll(() => { + stubDataView.getFormatterForField = jest.fn().mockReturnValue({ + getConverterFor: () => mockedFormatter, + }); + pluginServices.getServices().dataViews.get = jest.fn().mockResolvedValue(stubDataView); + }); + + afterEach(() => { + mockedFormatter.mockClear(); + }); + + test('uses field formatter on suggestions', async () => { + const popover = await mountComponent({ + componentState: { + field: stubDataView.fields.getByName('bytes')?.toSpec(), + availableOptions: [ + { value: 1000, docCount: 1 }, + { value: 123456789, docCount: 4 }, + ], + }, + }); + + expect(mockedFormatter).toHaveBeenNthCalledWith(1, 1000); + expect(mockedFormatter).toHaveBeenNthCalledWith(2, 123456789); + const options = await popover.findAllByRole('option'); + expect(options[0].textContent).toEqual('Exists'); + expect( + options[1].getElementsByClassName('euiSelectableListItem__text')[0].textContent + ).toEqual('formatted:1000'); + expect( + options[2].getElementsByClassName('euiSelectableListItem__text')[0].textContent + ).toEqual('formatted:123456789'); + }); + + test('converts string to number for date field', async () => { + await mountComponent({ + componentState: { + field: stubDataView.fields.getByName('@timestamp')?.toSpec(), + availableOptions: [ + { value: 1721283696000, docCount: 1 }, + { value: 1721295533000, docCount: 2 }, + ], + }, + }); + expect(mockedFormatter).toHaveBeenNthCalledWith(1, 1721283696000); + expect(mockedFormatter).toHaveBeenNthCalledWith(2, 1721295533000); + }); + }); }); diff --git a/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx b/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx index 4de8a2f483ea0..5041eadcce1ce 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover_invalid_selections.tsx @@ -19,6 +19,7 @@ import { EuiTitle, } from '@elastic/eui'; +import { getSelectionAsFieldType } from '../../../common/options_list/options_list_selections'; import { useFieldFormatter } from '../../hooks/use_field_formatter'; import { useOptionsList } from '../embeddable/options_list_embeddable'; import { OptionsListStrings } from './options_list_strings'; @@ -39,7 +40,7 @@ export const OptionsListPopoverInvalidSelections = () => { /* This useEffect makes selectableOptions responsive to unchecking options */ const options: EuiSelectableOption[] = (invalidSelections ?? []).map((key) => { return { - key, + key: String(key), label: fieldFormatter(key), checked: 'on', className: 'optionsList__selectionInvalid', @@ -91,8 +92,15 @@ export const OptionsListPopoverInvalidSelections = () => { options={selectableOptions} listProps={{ onFocusBadge: false, isVirtualized: false }} onChange={(newSuggestions, _, changedOption) => { + if (!fieldSpec || !changedOption.key) { + // this should never happen, but early return for type safety + // eslint-disable-next-line no-console + console.warn(OptionsListStrings.popover.getInvalidSelectionMessage()); + return; + } setSelectableOptions(newSuggestions); - optionsList.dispatch.deselectOption(changedOption.key ?? changedOption.label); + const key = getSelectionAsFieldType(fieldSpec, changedOption.key); + optionsList.dispatch.deselectOption(key); }} > {(list) => list} diff --git a/src/plugins/controls/public/options_list/components/options_list_popover_suggestions.tsx b/src/plugins/controls/public/options_list/components/options_list_popover_suggestions.tsx index 097660a55a540..486978b63ec5e 100644 --- a/src/plugins/controls/public/options_list/components/options_list_popover_suggestions.tsx +++ b/src/plugins/controls/public/options_list/components/options_list_popover_suggestions.tsx @@ -8,16 +8,20 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { euiThemeVars } from '@kbn/ui-theme'; import { EuiHighlight, EuiSelectable } from '@elastic/eui'; import { EuiSelectableOption } from '@elastic/eui/src/components/selectable/selectable_option'; +import { euiThemeVars } from '@kbn/ui-theme'; -import { MAX_OPTIONS_LIST_REQUEST_SIZE } from '../types'; -import { OptionsListStrings } from './options_list_strings'; +import { + getSelectionAsFieldType, + OptionsListSelection, +} from '../../../common/options_list/options_list_selections'; +import { useFieldFormatter } from '../../hooks/use_field_formatter'; import { useOptionsList } from '../embeddable/options_list_embeddable'; +import { MAX_OPTIONS_LIST_REQUEST_SIZE } from '../types'; import { OptionsListPopoverEmptyMessage } from './options_list_popover_empty_message'; import { OptionsListPopoverSuggestionBadge } from './options_list_popover_suggestion_badge'; -import { useFieldFormatter } from '../../hooks/use_field_formatter'; +import { OptionsListStrings } from './options_list_strings'; interface OptionsListPopoverSuggestionsProps { showOnlySelected: boolean; @@ -64,9 +68,12 @@ export const OptionsListPopoverSuggestions = ({ ); // track selectedOptions and invalidSelections in sets for more efficient lookup - const selectedOptionsSet = useMemo(() => new Set(selectedOptions), [selectedOptions]); + const selectedOptionsSet = useMemo( + () => new Set(selectedOptions), + [selectedOptions] + ); const invalidSelectionsSet = useMemo( - () => new Set(invalidSelections), + () => new Set(invalidSelections), [invalidSelections] ); const suggestions = useMemo(() => { @@ -95,8 +102,8 @@ export const OptionsListPopoverSuggestions = ({ } return { - key: suggestion.value, - label: fieldFormatter(suggestion.value) ?? suggestion.value, + key: String(suggestion.value), + label: fieldFormatter(suggestion.value) ?? String(suggestion.value), checked: selectedOptionsSet?.has(suggestion.value) ? 'on' : undefined, 'data-test-subj': `optionsList-control-selection-${suggestion.value}`, className: @@ -191,12 +198,21 @@ export const OptionsListPopoverSuggestions = ({ )} emptyMessage={} onChange={(newSuggestions, _, changedOption) => { - const key = changedOption.key ?? changedOption.label; + if (!fieldSpec || !changedOption.key) { + // this should never happen, but early return for type safety + // eslint-disable-next-line no-console + console.warn(OptionsListStrings.popover.getInvalidSelectionMessage()); + return; + } setSelectableOptions(newSuggestions); - // the order of these checks matters, so be careful if rearranging them - if (key === 'exists-option') { + if (changedOption.key === 'exists-option') { optionsList.dispatch.selectExists(!Boolean(existsSelected)); - } else if (showOnlySelected || selectedOptionsSet.has(key)) { + return; + } + + const key = getSelectionAsFieldType(fieldSpec, changedOption.key); + // the order of these checks matters, so be careful if rearranging them + if (showOnlySelected || selectedOptionsSet.has(key)) { optionsList.dispatch.deselectOption(key); } else if (singleSelect) { optionsList.dispatch.replaceSelection(key); diff --git a/src/plugins/controls/public/options_list/components/options_list_strings.ts b/src/plugins/controls/public/options_list/components/options_list_strings.ts index a63c8ab57932a..d7606424de4d8 100644 --- a/src/plugins/controls/public/options_list/components/options_list_strings.ts +++ b/src/plugins/controls/public/options_list/components/options_list_strings.ts @@ -147,6 +147,10 @@ export const OptionsListStrings = { i18n.translate('controls.optionsList.popover.selectionsEmpty', { defaultMessage: 'You have no selections', }), + getInvalidSelectionMessage: () => + i18n.translate('controls.optionsList.popover.selectionError', { + defaultMessage: 'There was an error when making your selection', + }), getInvalidSearchMessage: (fieldType: string) => { switch (fieldType) { case 'ip': { diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx index 81dd3ea9c64c9..b14f5dbea79ff 100644 --- a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx +++ b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx @@ -44,6 +44,7 @@ import { OptionsListEmbeddableInput, OPTIONS_LIST_CONTROL, } from '../..'; +import { OptionsListSelection } from '../../../common/options_list/options_list_selections'; import { ControlFilterOutput } from '../../control_group/types'; import { pluginServices } from '../../services'; import { ControlsDataViewsService } from '../../services/data_views/types'; @@ -232,8 +233,8 @@ export class OptionsListEmbeddable this.dispatch.clearValidAndInvalidSelections({}); } else { const { invalidSelections } = this.getState().componentState ?? {}; - const newValidSelections: string[] = []; - const newInvalidSelections: string[] = []; + const newValidSelections: OptionsListSelection[] = []; + const newInvalidSelections: OptionsListSelection[] = []; for (const selectedOption of newSelectedOptions) { if (invalidSelections?.includes(selectedOption)) { newInvalidSelections.push(selectedOption); @@ -369,10 +370,10 @@ export class OptionsListEmbeddable }); this.reportInvalidSelections(false); } else { - const valid: string[] = []; - const invalid: string[] = []; + const valid: OptionsListSelection[] = []; + const invalid: OptionsListSelection[] = []; for (const selectedOption of selectedOptions ?? []) { - if (invalidSelections?.includes(String(selectedOption))) invalid.push(selectedOption); + if (invalidSelections?.includes(selectedOption)) invalid.push(selectedOption); else valid.push(selectedOption); } this.dispatch.updateQueryResults({ @@ -437,14 +438,13 @@ export class OptionsListEmbeddable private buildFilter = async (): Promise => { const { - componentState: { validSelections }, - explicitInput: { existsSelected, exclude }, + explicitInput: { existsSelected, exclude, selectedOptions }, } = this.getState(); return await this.selectionsToFilters({ existsSelected, exclude, - selectedOptions: validSelections, + selectedOptions, }); }; diff --git a/src/plugins/controls/public/options_list/options_list_reducers.ts b/src/plugins/controls/public/options_list/options_list_reducers.ts index 488a20313c8d9..76973e9da1e8e 100644 --- a/src/plugins/controls/public/options_list/options_list_reducers.ts +++ b/src/plugins/controls/public/options_list/options_list_reducers.ts @@ -12,6 +12,7 @@ import { FieldSpec } from '@kbn/data-views-plugin/common'; import { Filter } from '@kbn/es-query'; import { isValidSearch } from '../../common/options_list/is_valid_search'; +import { OptionsListSelection } from '../../common/options_list/options_list_selections'; import { OptionsListSortingType, OPTIONS_LIST_DEFAULT_SORT, @@ -25,8 +26,12 @@ export const getDefaultComponentState = (): OptionsListReduxState['componentStat }); export const optionsListReducers = { - deselectOption: (state: WritableDraft, action: PayloadAction) => { - if (!state.explicitInput.selectedOptions) return; + deselectOption: ( + state: WritableDraft, + action: PayloadAction + ) => { + if (!state.explicitInput.selectedOptions || !state.componentState.field) return; + const itemIndex = state.explicitInput.selectedOptions.indexOf(action.payload); if (itemIndex !== -1) { const newSelections = [...state.explicitInput.selectedOptions]; @@ -76,14 +81,18 @@ export const optionsListReducers = { state.explicitInput.existsSelected = false; } }, - selectOption: (state: WritableDraft, action: PayloadAction) => { + selectOption: ( + state: WritableDraft, + action: PayloadAction + ) => { if (!state.explicitInput.selectedOptions) state.explicitInput.selectedOptions = []; if (state.explicitInput.existsSelected) state.explicitInput.existsSelected = false; + state.explicitInput.selectedOptions?.push(action.payload); }, replaceSelection: ( state: WritableDraft, - action: PayloadAction + action: PayloadAction ) => { state.explicitInput.selectedOptions = [action.payload]; if (state.explicitInput.existsSelected) state.explicitInput.existsSelected = false; @@ -101,10 +110,7 @@ export const optionsListReducers = { }, setValidAndInvalidSelections: ( state: WritableDraft, - action: PayloadAction<{ - validSelections: string[]; - invalidSelections: string[]; - }> + action: PayloadAction> ) => { const { invalidSelections, validSelections } = action.payload; state.componentState.invalidSelections = invalidSelections; diff --git a/src/plugins/controls/public/options_list/types.ts b/src/plugins/controls/public/options_list/types.ts index da3f52a4cb870..e96eb657798b9 100644 --- a/src/plugins/controls/public/options_list/types.ts +++ b/src/plugins/controls/public/options_list/types.ts @@ -6,14 +6,15 @@ * Side Public License, v 1. */ -import { ReduxEmbeddableState } from '@kbn/presentation-util-plugin/public'; import { FieldSpec } from '@kbn/data-views-plugin/common'; +import { ReduxEmbeddableState } from '@kbn/presentation-util-plugin/public'; -import { ControlOutput } from '../types'; +import { OptionsListSelection } from '../../common/options_list/options_list_selections'; import { - OptionsListSuggestions, OptionsListEmbeddableInput, + OptionsListSuggestions, } from '../../common/options_list/types'; +import { ControlOutput } from '../types'; export const MIN_OPTIONS_LIST_REQUEST_SIZE = 10; export const MAX_OPTIONS_LIST_REQUEST_SIZE = 1000; @@ -26,10 +27,11 @@ interface SearchString { // Component state is only used by public components. export interface OptionsListComponentState { availableOptions?: OptionsListSuggestions; + invalidSelections?: OptionsListSelection[]; + validSelections?: OptionsListSelection[]; + allowExpensiveQueries: boolean; - invalidSelections?: string[]; searchString: SearchString; - validSelections?: string[]; totalCardinality?: number; popoverOpen: boolean; field?: FieldSpec; diff --git a/src/plugins/controls/server/options_list/options_list_suggestions_route.ts b/src/plugins/controls/server/options_list/options_list_suggestions_route.ts index 50a4be4013ac9..b31c4feb6bf0f 100644 --- a/src/plugins/controls/server/options_list/options_list_suggestions_route.ts +++ b/src/plugins/controls/server/options_list/options_list_suggestions_route.ts @@ -142,6 +142,7 @@ export const setupOptionsListSuggestionsRoute = ( const results = suggestionBuilder.parse(rawEsResult, request); const totalCardinality = results.totalCardinality; const invalidSelections = validationBuilder.parse(rawEsResult, request); + return { suggestions: results.suggestions, totalCardinality, diff --git a/src/plugins/controls/server/options_list/options_list_validation_queries.test.ts b/src/plugins/controls/server/options_list/options_list_validation_queries.test.ts index 1d96215e167b9..8a263562ea68d 100644 --- a/src/plugins/controls/server/options_list/options_list_validation_queries.test.ts +++ b/src/plugins/controls/server/options_list/options_list_validation_queries.test.ts @@ -151,6 +151,7 @@ describe('options list queries', () => { size: 10, fieldName: 'coolTestField', allowExpensiveQueries: true, + fieldSpec: { type: 'string' } as FieldSpec, }) ).toMatchInlineSnapshot(` Array [ diff --git a/src/plugins/controls/server/options_list/options_list_validation_queries.ts b/src/plugins/controls/server/options_list/options_list_validation_queries.ts index edacacb361d0c..2dcf9ea8ce414 100644 --- a/src/plugins/controls/server/options_list/options_list_validation_queries.ts +++ b/src/plugins/controls/server/options_list/options_list_validation_queries.ts @@ -9,6 +9,10 @@ import { getFieldSubtypeNested } from '@kbn/data-views-plugin/common'; import { get, isEmpty } from 'lodash'; +import { + getSelectionAsFieldType, + OptionsListSelection, +} from '../../common/options_list/options_list_selections'; import { OptionsListRequestBody } from '../../common/options_list/types'; import { OptionsListValidationAggregationBuilder } from './types'; @@ -21,9 +25,9 @@ export const getValidationAggregationBuilder: () => OptionsListValidationAggrega let selectedOptionsFilters; if (selectedOptions) { selectedOptionsFilters = selectedOptions.reduce((acc, currentOption) => { - acc[currentOption] = { match: { [fieldName]: String(currentOption) } }; + acc[currentOption] = { match: { [fieldName]: currentOption } }; return acc; - }, {} as { [key: string]: { match: { [key: string]: string } } }); + }, {} as { [key: string]: { match: { [key: string]: OptionsListSelection } } }); } if (isEmpty(selectedOptionsFilters ?? [])) { @@ -55,6 +59,8 @@ export const getValidationAggregationBuilder: () => OptionsListValidationAggrega return validationAggregation; }, parse: (rawEsResult, { fieldSpec }) => { + if (!fieldSpec) return []; + const isNested = fieldSpec && getFieldSubtypeNested(fieldSpec); const rawInvalidSuggestions = get( rawEsResult, @@ -63,9 +69,9 @@ export const getValidationAggregationBuilder: () => OptionsListValidationAggrega : 'aggregations.validation.buckets' ); return rawInvalidSuggestions && !isEmpty(rawInvalidSuggestions) - ? Object.keys(rawInvalidSuggestions).filter( - (key) => rawInvalidSuggestions[key].doc_count === 0 - ) + ? Object.keys(rawInvalidSuggestions) + .filter((key) => rawInvalidSuggestions[key].doc_count === 0) + .map((key: string): OptionsListSelection => getSelectionAsFieldType(fieldSpec, key)) : []; }, }); diff --git a/src/plugins/controls/server/options_list/suggestion_queries/options_list_all_suggestions.test.ts b/src/plugins/controls/server/options_list/suggestion_queries/options_list_all_suggestions.test.ts index 0c5de40f569de..a85fb78552b18 100644 --- a/src/plugins/controls/server/options_list/suggestion_queries/options_list_all_suggestions.test.ts +++ b/src/plugins/controls/server/options_list/suggestion_queries/options_list_all_suggestions.test.ts @@ -119,9 +119,9 @@ describe('options list fetch all suggestions query', () => { aggregations: { suggestions: { buckets: [ - { doc_count: 5, key: '1' }, - { doc_count: 4, key: '2' }, - { doc_count: 3, key: '3' }, + { doc_count: 5, key: 1 }, + { doc_count: 4, key: 2 }, + { doc_count: 3, key: 3 }, ], }, unique_terms: { @@ -133,9 +133,9 @@ describe('options list fetch all suggestions query', () => { const parsed = aggregationBuilder.parse(searchResponseMock, optionsListRequestBodyMock); expect(parsed).toMatchObject({ suggestions: [ - { value: '1', docCount: 5 }, - { value: '2', docCount: 4 }, - { value: '3', docCount: 3 }, + { value: 1, docCount: 5 }, + { value: 2, docCount: 4 }, + { value: 3, docCount: 3 }, ], totalCardinality: 3, }); diff --git a/src/plugins/controls/server/options_list/suggestion_queries/options_list_exact_match.test.ts b/src/plugins/controls/server/options_list/suggestion_queries/options_list_exact_match.test.ts index 77ec137e0dbe7..23b69bd7aed7e 100644 --- a/src/plugins/controls/server/options_list/suggestion_queries/options_list_exact_match.test.ts +++ b/src/plugins/controls/server/options_list/suggestion_queries/options_list_exact_match.test.ts @@ -6,31 +6,11 @@ * Side Public License, v 1. */ -import { SearchResponse } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { FieldSpec } from '@kbn/data-views-plugin/common'; import { OptionsListRequestBody } from '../../../common/options_list/types'; import { getExactMatchAggregationBuilder } from './options_list_exact_match'; describe('options list exact match search query', () => { - test('returns empty result when given invalid search', () => { - const optionsListRequestBodyMock: OptionsListRequestBody = { - size: 10, - fieldName: 'bytes', - allowExpensiveQueries: true, - sort: { by: '_key', direction: 'desc' }, - searchString: '1a2b3c', - fieldSpec: { type: 'number' } as unknown as FieldSpec, - }; - const aggregationBuilder = getExactMatchAggregationBuilder(); - const aggregation = aggregationBuilder.buildAggregation(optionsListRequestBodyMock); - expect(aggregation).toEqual({}); - const parsed = aggregationBuilder.parse( - {} as any as SearchResponse, - optionsListRequestBodyMock - ); - expect(parsed).toEqual({ suggestions: [], totalCardinality: 0 }); - }); - describe('suggestion aggregation', () => { test('string (keyword, text+keyword) field', () => { const optionsListRequestBodyMock: OptionsListRequestBody = { @@ -107,7 +87,7 @@ describe('options list exact match search query', () => { }); }); - test('number field', () => { + test('numeric field', () => { const optionsListRequestBodyMock: OptionsListRequestBody = { size: 10, fieldName: 'bytes', @@ -140,14 +120,57 @@ describe('options list exact match search query', () => { }); }); - test('suggestion parsing', () => { + describe('parsing', () => { + test('parses keyword result', () => { + const optionsListRequestBodyMock: OptionsListRequestBody = { + size: 10, + searchString: 'cool', + allowExpensiveQueries: true, + fieldName: 'coolTestField.keyword', + fieldSpec: { type: 'string' } as unknown as FieldSpec, + }; + const aggregationBuilder = getExactMatchAggregationBuilder(); + + const searchResponseMock = { + hits: { + total: 1, + max_score: 1, + hits: [], + }, + took: 10, + timed_out: false, + _shards: { + failed: 0, + successful: 1, + total: 1, + skipped: 0, + }, + aggregations: { + suggestions: { + filteredSuggestions: { + buckets: [{ doc_count: 5, key: 'cool1' }], + }, + }, + }, + }; + expect( + aggregationBuilder.parse(searchResponseMock, optionsListRequestBodyMock) + ).toMatchObject({ + suggestions: [{ docCount: 5, value: 'cool1' }], + totalCardinality: 1, + }); + }); + }); + + test('parses numeric field result', () => { const optionsListRequestBodyMock: OptionsListRequestBody = { size: 10, - searchString: 'cool', + fieldName: 'bytes', allowExpensiveQueries: true, - fieldName: 'coolTestField.keyword', - fieldSpec: { type: 'string' } as unknown as FieldSpec, + searchString: '12345', + fieldSpec: { type: 'number' } as unknown as FieldSpec, }; + const aggregationBuilder = getExactMatchAggregationBuilder(); const searchResponseMock = { @@ -167,13 +190,13 @@ describe('options list exact match search query', () => { aggregations: { suggestions: { filteredSuggestions: { - buckets: [{ doc_count: 5, key: 'cool1' }], + buckets: [{ doc_count: 5, key: 12345 }], }, }, }, }; expect(aggregationBuilder.parse(searchResponseMock, optionsListRequestBodyMock)).toMatchObject({ - suggestions: [{ docCount: 5, value: 'cool1' }], + suggestions: [{ docCount: 5, value: 12345 }], totalCardinality: 1, }); }); diff --git a/src/plugins/controls/server/options_list/suggestion_queries/options_list_search_suggestions.test.ts b/src/plugins/controls/server/options_list/suggestion_queries/options_list_search_suggestions.test.ts index b2ddb699ebaad..2788b804e050c 100644 --- a/src/plugins/controls/server/options_list/suggestion_queries/options_list_search_suggestions.test.ts +++ b/src/plugins/controls/server/options_list/suggestion_queries/options_list_search_suggestions.test.ts @@ -10,13 +10,9 @@ import { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; import { FieldSpec } from '@kbn/data-views-plugin/common'; import { OptionsListRequestBody } from '../../../common/options_list/types'; -import { getExactMatchAggregationBuilder } from './options_list_exact_match'; +import * as ExactMatch from './options_list_exact_match'; import { getSearchSuggestionsAggregationBuilder } from './options_list_search_suggestions'; -jest.mock('./options_list_exact_match', () => ({ - getExactMatchAggregationBuilder: jest.fn(), -})); - describe('options list type-specific search queries', () => { let rawSearchResponseMock: SearchResponse = {} as SearchResponse; @@ -41,6 +37,7 @@ describe('options list type-specific search queries', () => { describe('suggestion aggregation', () => { test('for unsupported field types, return exact match search instead', () => { + const exactMatchSpy = jest.spyOn(ExactMatch, 'getExactMatchAggregationBuilder'); const optionsListRequestBodyMock: OptionsListRequestBody = { size: 10, fieldName: 'success', @@ -49,7 +46,7 @@ describe('options list type-specific search queries', () => { fieldSpec: { type: 'boolean' } as unknown as FieldSpec, }; getSearchSuggestionsAggregationBuilder(optionsListRequestBodyMock); - expect(getExactMatchAggregationBuilder).toBeCalled(); + expect(exactMatchSpy).toBeCalled(); }); describe('string (keyword, text+keyword, or nested) field', () => { @@ -459,6 +456,25 @@ describe('options list type-specific search queries', () => { `); }); }); + + describe('numeric field', () => { + test('handles an invalid search', () => { + const optionsListRequestBodyMock: OptionsListRequestBody = { + size: 10, + fieldName: 'bytes', + allowExpensiveQueries: true, + sort: { by: '_key', direction: 'asc' }, + searchString: '123a', + fieldSpec: { type: 'number' } as unknown as FieldSpec, + }; + const suggestionAggBuilder = getSearchSuggestionsAggregationBuilder( + optionsListRequestBodyMock + ); + expect(suggestionAggBuilder.buildAggregation(optionsListRequestBodyMock)).toEqual({}); + }); + + // for tests related to searching numeric fields, refer to './options_list_exact_match.test.ts` + }); }); describe('suggestion parsing', () => { @@ -667,5 +683,7 @@ describe('options list type-specific search queries', () => { ] `); }); + + // for tests related to parsing numeric suggestions, refer to './options_list_exact_match.test.ts` }); }); diff --git a/src/plugins/controls/server/options_list/types.ts b/src/plugins/controls/server/options_list/types.ts index ed906d06fb65e..7ee81c251b7f8 100644 --- a/src/plugins/controls/server/options_list/types.ts +++ b/src/plugins/controls/server/options_list/types.ts @@ -7,20 +7,21 @@ */ import { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; +import { OptionsListSelection } from '../../common/options_list/options_list_selections'; import { - OptionsListRequestBody, OptionsListParsedSuggestions, + OptionsListRequestBody, } from '../../common/options_list/types'; export interface EsBucket { - key: any; + key: OptionsListSelection; key_as_string?: string; doc_count: number; } export interface OptionsListValidationAggregationBuilder { buildAggregation: (req: OptionsListRequestBody) => unknown; - parse: (response: SearchResponse, req: OptionsListRequestBody) => string[]; + parse: (response: SearchResponse, req: OptionsListRequestBody) => OptionsListSelection[]; } export interface OptionsListSuggestionAggregationBuilder { diff --git a/src/plugins/controls/tsconfig.json b/src/plugins/controls/tsconfig.json index 3a97d130e2902..2fc9038dafe6c 100644 --- a/src/plugins/controls/tsconfig.json +++ b/src/plugins/controls/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../../tsconfig.base.json", "compilerOptions": { - "outDir": "target/types", + "outDir": "target/types" }, "extraPublicDirs": ["common"], "include": [ @@ -40,9 +40,7 @@ "@kbn/shared-ux-markdown", "@kbn/react-kibana-context-render", "@kbn/presentation-containers", - "@kbn/presentation-publishing", + "@kbn/presentation-publishing" ], - "exclude": [ - "target/**/*", - ] + "exclude": ["target/**/*"] } diff --git a/test/functional/apps/dashboard_elements/controls/options_list/options_list_suggestions.ts b/test/functional/apps/dashboard_elements/controls/options_list/options_list_suggestions.ts index 0b2dda536d41c..220b9819f4466 100644 --- a/test/functional/apps/dashboard_elements/controls/options_list/options_list_suggestions.ts +++ b/test/functional/apps/dashboard_elements/controls/options_list/options_list_suggestions.ts @@ -119,6 +119,27 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardControls.optionsListPopoverSetSort({ by: '_count', direction: 'desc' }); await testSubjects.missingOrFail('dashboardUnsavedChangesBadge'); }); + + it('can sort numeric options lists suggestions', async () => { + await dashboardControls.editExistingControl(controlId); + await dashboardControls.controlsEditorSetfield('weightLbs'); + await dashboardControls.controlEditorSave(); + + await dashboardControls.optionsListOpenPopover(controlId); + await dashboardControls.optionsListPopoverSetSort({ by: '_key', direction: 'asc' }); + const sortedSuggestions = Object.keys( + (await dashboardControls.optionsListPopoverGetAvailableOptions()).suggestions + ).map((key) => parseInt(key, 10)); + for (let i = 0; i < sortedSuggestions.length - 1; i++) { + expect(sortedSuggestions[i]).to.be.lessThan(sortedSuggestions[i + 1]); + } + + // revert to the old field name to keep state consistent for other tests + await dashboardControls.editExistingControl(controlId); + await dashboardControls.controlsEditorSetfield('sound.keyword'); + await dashboardControls.optionsListSetAdditionalSettings({ searchTechnique: 'prefix' }); + await dashboardControls.controlEditorSave(); + }); }); describe('searching', () => { @@ -195,6 +216,18 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardControls.controlEditorSave(); await testSubjects.missingOrFail('dashboardUnsavedChangesBadge'); }); + + it('can search numeric options list', async () => { + await dashboardControls.editExistingControl(controlId); + await dashboardControls.controlsEditorSetfield('weightLbs'); + await dashboardControls.controlEditorSave(); + + await dashboardControls.optionsListOpenPopover(controlId); + await dashboardControls.optionsListPopoverSearchForOption('4'); + expect(await dashboardControls.optionsListPopoverGetAvailableOptionsCount()).to.be(0); + await dashboardControls.optionsListPopoverSearchForOption('45'); // only supports exact match + expect(await dashboardControls.optionsListPopoverGetAvailableOptionsCount()).to.be(1); + }); }); }); } diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/detection_page_filters.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/detection_page_filters.cy.ts index 142ae6efbd439..aeba2d5128e2f 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/detection_page_filters.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/detection_page_filters.cy.ts @@ -99,7 +99,7 @@ const assertFilterControlsWithFilterObject = ( cy.get(OPTION_LIST_VALUES(idx)).should((sub) => { const controlText = sub.text(); filter.selectedOptions?.forEach((option) => { - expect(controlText).to.have.string(option); + expect(controlText).to.have.string(String(option)); }); }); });