Skip to content

Commit

Permalink
[8.x] [ES|QL] Disable the filter actions for multivalue fields (elast…
Browse files Browse the repository at this point in the history
…ic#193415) (elastic#193962)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Disable the filter actions for multivalue fields
(elastic#193415)](elastic#193415)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-25T10:14:34Z","message":"[ES|QL]
Disable the filter actions for multivalue fields (elastic#193415)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/193015\r\n\r\nIt not allows the
creation of where clause filters in case of multi\r\nvalue fields as
this is not supported yet in ES|QL. Check my
comment\r\nhere\r\nhttps://github.com/elastic/issues/193015#issuecomment-2360704651\r\n\r\nIt
might be possible with full text search but I need to talk to
the\r\nteam first. For now we disable it as it creates a wrong
filter.\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Davis McPhee
<[email protected]>","sha":"249298166fe1499b4aea501056a2291711427c5b","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL","v8.16.0"],"title":"[ES|QL]
Disable the filter actions for multivalue
fields","number":193415,"url":"https://github.com/elastic/kibana/pull/193415","mergeCommit":{"message":"[ES|QL]
Disable the filter actions for multivalue fields (elastic#193415)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/193015\r\n\r\nIt not allows the
creation of where clause filters in case of multi\r\nvalue fields as
this is not supported yet in ES|QL. Check my
comment\r\nhere\r\nhttps://github.com/elastic/issues/193015#issuecomment-2360704651\r\n\r\nIt
might be possible with full text search but I need to talk to
the\r\nteam first. For now we disable it as it creates a wrong
filter.\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Davis McPhee
<[email protected]>","sha":"249298166fe1499b4aea501056a2291711427c5b"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193415","number":193415,"mergeCommit":{"message":"[ES|QL]
Disable the filter actions for multivalue fields (elastic#193415)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/kibana/issues/193015\r\n\r\nIt not allows the
creation of where clause filters in case of multi\r\nvalue fields as
this is not supported yet in ES|QL. Check my
comment\r\nhere\r\nhttps://github.com/elastic/issues/193015#issuecomment-2360704651\r\n\r\nIt
might be possible with full text search but I need to talk to
the\r\nteam first. For now we disable it as it creates a wrong
filter.\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Davis McPhee
<[email protected]>","sha":"249298166fe1499b4aea501056a2291711427c5b"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
kibanamachine and stratoula authored Sep 25, 2024
1 parent 668f212 commit c7b153c
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.getRowByIndex(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.getRowByIndex(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 @@ -211,6 +211,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 c7b153c

Please sign in to comment.