Skip to content

Commit

Permalink
[ES|QL] Disable the filter actions for multivalue fields (#193415)
Browse files Browse the repository at this point in the history
## Summary

Part of #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
#193015 (comment)

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 <[email protected]>
Co-authored-by: Davis McPhee <[email protected]>
  • Loading branch information
3 people authored Sep 25, 2024
1 parent 6808f82 commit 2492981
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 46 deletions.
6 changes: 6 additions & 0 deletions packages/kbn-esql-utils/src/utils/append_to_query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
6 changes: 5 additions & 1 deletion packages/kbn-esql-utils/src/utils/append_to_query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand All @@ -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()
Expand All @@ -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
);
Expand All @@ -87,65 +90,97 @@ describe('Default cell actions ', function () {
const component = mountWithIntl(
<UnifiedDataTableContext.Provider value={dataTableContextMock}>
<FilterInBtn
Component={(props: any) => <EuiButton {...props} />}
rowIndex={1}
colIndex={1}
columnId="extension"
isExpanded={false}
cellActionProps={{
Component: (props: any) => <EuiButton {...props} />,
rowIndex: 1,
colIndex: 1,
columnId: 'extension',
isExpanded: false,
}}
field={{ name: 'extension', filterable: true } as DataViewField}
isPlainRecord={false}
/>
</UnifiedDataTableContext.Provider>
);
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(
<UnifiedDataTableContext.Provider value={dataTableContextMock}>
<FilterInBtn
Component={(props: any) => <EuiButton {...props} />}
rowIndex={0}
colIndex={1}
columnId="extension"
isExpanded={false}
cellActionProps={{
Component: (props: any) => <EuiButton {...props} />,
rowIndex: 0,
colIndex: 1,
columnId: 'extension',
isExpanded: false,
}}
field={{ name: 'extension', filterable: true } as DataViewField}
isPlainRecord={false}
/>
</UnifiedDataTableContext.Provider>
);
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(
<UnifiedDataTableContext.Provider value={dataTableContextMock}>
<FilterInBtn
Component={(props: any) => <EuiButton {...props} />}
rowIndex={4}
colIndex={1}
columnId="message"
isExpanded={false}
cellActionProps={{
Component: (props: any) => <EuiButton {...props} />,
rowIndex: 4,
colIndex: 1,
columnId: 'message',
isExpanded: false,
}}
field={{ name: 'message', filterable: true } as DataViewField}
isPlainRecord={false}
/>
</UnifiedDataTableContext.Provider>
);
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(
<UnifiedDataTableContext.Provider value={dataTableContextMock}>
<FilterOutBtn
Component={(props: any) => <EuiButton {...props} />}
rowIndex={1}
colIndex={1}
columnId="extension"
isExpanded={false}
cellActionProps={{
Component: (props: any) => <EuiButton {...props} />,
rowIndex: 1,
colIndex: 1,
columnId: 'extension',
isExpanded: false,
}}
field={{ name: 'extension', filterable: true } as DataViewField}
isPlainRecord={false}
/>
</UnifiedDataTableContext.Provider>
);
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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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', {
Expand All @@ -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 },
Expand All @@ -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', {
Expand Down Expand Up @@ -120,23 +143,26 @@ export function buildCopyValueButton(

export function buildCellActions(
field: DataViewField,
isPlainRecord: boolean | undefined,
toastNotifications: ToastsStart,
valueToStringConverter: ValueToStringConverter,
onFilter?: DocViewFilterFn
) {
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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
getOperator(fieldName, values, operation),
fieldType
);
if (!updatedQuery) {
return;
}
data.query.queryString.setQuery({
esql: updatedQuery,
});
Expand Down

0 comments on commit 2492981

Please sign in to comment.