From 249298166fe1499b4aea501056a2291711427c5b Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 25 Sep 2024 12:14:34 +0200 Subject: [PATCH] [ES|QL] Disable the filter actions for multivalue fields (#193415) ## Summary Part of https://github.com/elastic/kibana/issues/193015 It not allows the creation of where clause filters in case of multi value fields as this is not supported yet in ES|QL. Check my comment here https://github.com/elastic/kibana/issues/193015#issuecomment-2360704651 It might be possible with full text search but I need to talk to the team first. For now we disable it as it creates a wrong filter. ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Davis McPhee --- .../src/utils/append_to_query.test.ts | 6 ++ .../src/utils/append_to_query.ts | 6 +- .../src/components/data_table_columns.tsx | 8 +- .../components/default_cell_actions.test.tsx | 83 +++++++++++++------ .../src/components/default_cell_actions.tsx | 66 ++++++++++----- .../components/layout/discover_layout.tsx | 3 + 6 files changed, 126 insertions(+), 46 deletions(-) diff --git a/packages/kbn-esql-utils/src/utils/append_to_query.test.ts b/packages/kbn-esql-utils/src/utils/append_to_query.test.ts index c9db52076222f8..9dc15454cbbdf3 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.test.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.test.ts @@ -168,5 +168,11 @@ AND \`dest\`=="Crete"` and \`ip\`::string!="127.0.0.2/32"` ); }); + + it('returns undefined for multivalue fields', () => { + expect( + appendWhereClauseToESQLQuery('from logstash-*', 'dest', ['meow'], '+', 'string') + ).toBeUndefined(); + }); }); }); diff --git a/packages/kbn-esql-utils/src/utils/append_to_query.ts b/packages/kbn-esql-utils/src/utils/append_to_query.ts index f4161be073a8db..28208818103873 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -21,7 +21,11 @@ export function appendWhereClauseToESQLQuery( value: unknown, operation: '+' | '-' | 'is_not_null' | 'is_null', fieldType?: string -): string { +): string | undefined { + // multivalues filtering is not supported yet + if (Array.isArray(value)) { + return undefined; + } let operator; switch (operation) { case 'is_not_null': diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx index c5bba429ed9345..876db4b0e71498 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.tsx @@ -189,7 +189,13 @@ function buildEuiGridColumn({ cellActions = columnCellActions; } else { cellActions = dataViewField - ? buildCellActions(dataViewField, toastNotifications, valueToStringConverter, onFilter) + ? buildCellActions( + dataViewField, + isPlainRecord, + toastNotifications, + valueToStringConverter, + onFilter + ) : []; if (columnCellActions?.length && cellActionsHandling === 'append') { diff --git a/packages/kbn-unified-data-table/src/components/default_cell_actions.test.tsx b/packages/kbn-unified-data-table/src/components/default_cell_actions.test.tsx index 628d3b5a29697b..d097a2becd0354 100644 --- a/packages/kbn-unified-data-table/src/components/default_cell_actions.test.tsx +++ b/packages/kbn-unified-data-table/src/components/default_cell_actions.test.tsx @@ -45,6 +45,7 @@ describe('Default cell actions ', function () { it('should not show cell actions for unfilterable fields', async () => { const cellActions = buildCellActions( { name: 'foo', filterable: false } as DataViewField, + false, servicesMock.toastNotifications, dataTableContextMock.valueToStringConverter ); @@ -61,6 +62,7 @@ describe('Default cell actions ', function () { it('should show filter actions for filterable fields', async () => { const cellActions = buildCellActions( { name: 'foo', filterable: true } as DataViewField, + false, servicesMock.toastNotifications, dataTableContextMock.valueToStringConverter, jest.fn() @@ -71,6 +73,7 @@ describe('Default cell actions ', function () { it('should show Copy action for _source field', async () => { const cellActions = buildCellActions( { name: '_source', type: '_source', filterable: false } as DataViewField, + false, servicesMock.toastNotifications, dataTableContextMock.valueToStringConverter ); @@ -87,65 +90,97 @@ describe('Default cell actions ', function () { const component = mountWithIntl( } - rowIndex={1} - colIndex={1} - columnId="extension" - isExpanded={false} + cellActionProps={{ + Component: (props: any) => , + rowIndex: 1, + colIndex: 1, + columnId: 'extension', + isExpanded: false, + }} + field={{ name: 'extension', filterable: true } as DataViewField} + isPlainRecord={false} /> ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, 'jpg', '+'); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( + { name: 'extension', filterable: true }, + 'jpg', + '+' + ); }); it('triggers filter function when FilterInBtn is clicked for a non-provided value', async () => { const component = mountWithIntl( } - rowIndex={0} - colIndex={1} - columnId="extension" - isExpanded={false} + cellActionProps={{ + Component: (props: any) => , + rowIndex: 0, + colIndex: 1, + columnId: 'extension', + isExpanded: false, + }} + field={{ name: 'extension', filterable: true } as DataViewField} + isPlainRecord={false} /> ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, undefined, '+'); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( + { name: 'extension', filterable: true }, + undefined, + '+' + ); }); it('triggers filter function when FilterInBtn is clicked for an empty string value', async () => { const component = mountWithIntl( } - rowIndex={4} - colIndex={1} - columnId="message" - isExpanded={false} + cellActionProps={{ + Component: (props: any) => , + rowIndex: 4, + colIndex: 1, + columnId: 'message', + isExpanded: false, + }} + field={{ name: 'message', filterable: true } as DataViewField} + isPlainRecord={false} /> ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, '', '+'); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( + { name: 'message', filterable: true }, + '', + '+' + ); }); it('triggers filter function when FilterOutBtn is clicked', async () => { const component = mountWithIntl( } - rowIndex={1} - colIndex={1} - columnId="extension" - isExpanded={false} + cellActionProps={{ + Component: (props: any) => , + rowIndex: 1, + colIndex: 1, + columnId: 'extension', + isExpanded: false, + }} + field={{ name: 'extension', filterable: true } as DataViewField} + isPlainRecord={false} /> ); const button = findTestSubject(component, 'filterOutButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, 'jpg', '-'); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( + { name: 'extension', filterable: true }, + 'jpg', + '-' + ); }); it('triggers clipboard copy when CopyBtn is clicked', async () => { const component = mountWithIntl( diff --git a/packages/kbn-unified-data-table/src/components/default_cell_actions.tsx b/packages/kbn-unified-data-table/src/components/default_cell_actions.tsx index 0be98036338259..51730aef443497 100644 --- a/packages/kbn-unified-data-table/src/components/default_cell_actions.tsx +++ b/packages/kbn-unified-data-table/src/components/default_cell_actions.tsx @@ -32,11 +32,25 @@ function onFilterCell( } } -export const FilterInBtn = ( - { Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps, - field: DataViewField -) => { +const esqlMultivalueFilteringDisabled = i18n.translate( + 'unifiedDataTable.grid.esqlMultivalueFilteringDisabled', + { + defaultMessage: 'Multivalue filtering is not supported in ES|QL', + } +); + +export const FilterInBtn = ({ + cellActionProps: { Component, rowIndex, columnId }, + field, + isPlainRecord, +}: { + cellActionProps: EuiDataGridColumnCellActionProps; + field: DataViewField; + isPlainRecord: boolean | undefined; +}) => { const context = useContext(UnifiedDataTableContext); + const filteringDisabled = + isPlainRecord && Array.isArray(context.rows[rowIndex]?.flattened[columnId]); const buttonTitle = i18n.translate('unifiedDataTable.grid.filterForAria', { defaultMessage: 'Filter for this {value}', values: { value: columnId }, @@ -49,7 +63,8 @@ export const FilterInBtn = ( }} iconType="plusInCircle" aria-label={buttonTitle} - title={buttonTitle} + title={filteringDisabled ? esqlMultivalueFilteringDisabled : buttonTitle} + disabled={filteringDisabled} data-test-subj="filterForButton" > {i18n.translate('unifiedDataTable.grid.filterFor', { @@ -59,11 +74,18 @@ export const FilterInBtn = ( ); }; -export const FilterOutBtn = ( - { Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps, - field: DataViewField -) => { +export const FilterOutBtn = ({ + cellActionProps: { Component, rowIndex, columnId }, + field, + isPlainRecord, +}: { + cellActionProps: EuiDataGridColumnCellActionProps; + field: DataViewField; + isPlainRecord: boolean | undefined; +}) => { const context = useContext(UnifiedDataTableContext); + const filteringDisabled = + isPlainRecord && Array.isArray(context.rows[rowIndex]?.flattened[columnId]); const buttonTitle = i18n.translate('unifiedDataTable.grid.filterOutAria', { defaultMessage: 'Filter out this {value}', values: { value: columnId }, @@ -76,7 +98,8 @@ export const FilterOutBtn = ( }} iconType="minusInCircle" aria-label={buttonTitle} - title={buttonTitle} + title={filteringDisabled ? esqlMultivalueFilteringDisabled : buttonTitle} + disabled={filteringDisabled} data-test-subj="filterOutButton" > {i18n.translate('unifiedDataTable.grid.filterOut', { @@ -120,6 +143,7 @@ export function buildCopyValueButton( export function buildCellActions( field: DataViewField, + isPlainRecord: boolean | undefined, toastNotifications: ToastsStart, valueToStringConverter: ValueToStringConverter, onFilter?: DocViewFilterFn @@ -127,16 +151,18 @@ export function buildCellActions( return [ ...(onFilter && field.filterable ? [ - ({ Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps) => - FilterInBtn( - { Component, rowIndex, columnId } as EuiDataGridColumnCellActionProps, - field - ), - ({ Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps) => - FilterOutBtn( - { Component, rowIndex, columnId } as EuiDataGridColumnCellActionProps, - field - ), + (cellActionProps: EuiDataGridColumnCellActionProps) => + FilterInBtn({ + cellActionProps, + field, + isPlainRecord, + }), + (cellActionProps: EuiDataGridColumnCellActionProps) => + FilterOutBtn({ + cellActionProps, + field, + isPlainRecord, + }), ] : []), ({ Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps) => diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index 82403ad38c7103..49e645e3f2206b 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -213,6 +213,9 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { getOperator(fieldName, values, operation), fieldType ); + if (!updatedQuery) { + return; + } data.query.queryString.setQuery({ esql: updatedQuery, });