From 4ec560bf9af11c2e4db18eb7c79491f9c557bb0a Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 23 Apr 2024 11:30:23 +0200 Subject: [PATCH 01/24] [ES|QL] Propagate the query with where clasue --- packages/kbn-esql-utils/index.ts | 1 + packages/kbn-esql-utils/src/index.ts | 2 +- .../src/utils/append_to_query.ts | 11 +++++ .../components/layout/discover_layout.tsx | 43 +++++++++++++++---- .../layout/discover_main_content.tsx | 2 +- 5 files changed, 48 insertions(+), 11 deletions(-) diff --git a/packages/kbn-esql-utils/index.ts b/packages/kbn-esql-utils/index.ts index 0d04f40b85612..cdd512c2d92f3 100644 --- a/packages/kbn-esql-utils/index.ts +++ b/packages/kbn-esql-utils/index.ts @@ -16,6 +16,7 @@ export { getInitialESQLQuery, getESQLWithSafeLimit, appendToESQLQuery, + appendWhereClauseToESQLQuery, TextBasedLanguages, } from './src'; diff --git a/packages/kbn-esql-utils/src/index.ts b/packages/kbn-esql-utils/src/index.ts index 92a39a4a6f793..adbfe6b23970a 100644 --- a/packages/kbn-esql-utils/src/index.ts +++ b/packages/kbn-esql-utils/src/index.ts @@ -16,4 +16,4 @@ export { getLimitFromESQLQuery, removeDropCommandsFromESQLQuery, } from './utils/query_parsing_helpers'; -export { appendToESQLQuery } from './utils/append_to_query'; +export { appendToESQLQuery, appendWhereClauseToESQLQuery } from './utils/append_to_query'; 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 efb43951bd082..40a41d9e12013 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -11,3 +11,14 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): string { return `${baseESQLQuery}\n${appendedText}`; } + +export function appendWhereClauseToESQLQuery( + baseESQLQuery: string, + field: string, + value: string, + operation: '+' | '-' +): string { + const operator = operation === '+' ? '==' : '!='; + const whereClause = `| where ${field}${operator}"${value}"`; + return appendToESQLQuery(baseESQLQuery, whereClause); +} 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 344e0b36c9d0e..271875963942f 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 @@ -17,6 +17,8 @@ import { } from '@elastic/eui'; import { css } from '@emotion/react'; import { i18n } from '@kbn/i18n'; +import { isOfAggregateQueryType } from '@kbn/es-query'; +import { appendWhereClauseToESQLQuery } from '@kbn/esql-utils'; import { METRIC_TYPE } from '@kbn/analytics'; import classNames from 'classnames'; import { generateFilters } from '@kbn/data-plugin/public'; @@ -145,6 +147,24 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { [filterManager, dataView, dataViews, trackUiMetric, capabilities] ); + const onPopulateWhereClause = useCallback( + (field: DataViewField | string, values: unknown, operation: '+' | '-') => { + if (query && isOfAggregateQueryType(query) && 'esql' in query) { + const fieldName = typeof field === 'string' ? field : field.name; + const updatedQuery = appendWhereClauseToESQLQuery( + query.esql, + fieldName, + String(values), + operation + ); + data.query.queryString.setQuery({ + esql: updatedQuery, + }); + } + }, + [data, query] + ); + const onFieldEdited = useCallback( async ({ removedFieldName }: { removedFieldName?: string } = {}) => { if (removedFieldName && currentColumns.includes(removedFieldName)) { @@ -223,7 +243,11 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { stateContainer={stateContainer} columns={currentColumns} viewMode={viewMode} - onAddFilter={onAddFilter as DocViewFilterFn} + onAddFilter={ + isPlainRecord + ? (onPopulateWhereClause as DocViewFilterFn) + : (onAddFilter as DocViewFilterFn) + } onFieldEdited={onFieldEdited} container={mainContainer} onDropFieldToTable={onDropFieldToTable} @@ -233,16 +257,17 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { ); }, [ - currentColumns, - dataView, - isPlainRecord, - mainContainer, - onAddFilter, - onDropFieldToTable, - onFieldEdited, resultState, + isPlainRecord, + dataView, stateContainer, + currentColumns, viewMode, + onPopulateWhereClause, + onAddFilter, + onFieldEdited, + mainContainer, + onDropFieldToTable, panelsToggle, ]); @@ -318,7 +343,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { documents$={stateContainer.dataState.data$.documents$} onAddField={onAddColumn} columns={currentColumns} - onAddFilter={!isPlainRecord ? onAddFilter : undefined} + onAddFilter={!isPlainRecord ? onAddFilter : onPopulateWhereClause} onRemoveField={onRemoveColumn} onChangeDataView={stateContainer.actions.onChangeDataView} selectedDataView={dataView} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx index 092279d4243b5..a12d5c8f8c375 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx @@ -125,7 +125,7 @@ export const DiscoverMainContent = ({ From 199c8b2abb862dc9100c611ff1ce89aa2e979b87 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 23 Apr 2024 13:20:41 +0200 Subject: [PATCH 02/24] Take under consideration the last command --- packages/kbn-esql-utils/src/utils/append_to_query.ts | 12 ++++++++++-- .../main/components/layout/discover_layout.tsx | 4 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) 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 40a41d9e12013..e1a3eb13d0369 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -12,13 +12,21 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): return `${baseESQLQuery}\n${appendedText}`; } -export function appendWhereClauseToESQLQuery( +export async function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, value: string, operation: '+' | '-' -): string { +): Promise { const operator = operation === '+' ? '==' : '!='; + + const { getAstAndSyntaxErrors } = await import('@kbn/esql-ast'); + const { ast } = await getAstAndSyntaxErrors(baseESQLQuery); + const lastCommandIsWhere = ast[ast.length - 1].name === 'where'; + if (lastCommandIsWhere) { + const whereClause = `and ${field}${operator}"${value}"`; + return appendToESQLQuery(baseESQLQuery, whereClause); + } const whereClause = `| where ${field}${operator}"${value}"`; return appendToESQLQuery(baseESQLQuery, whereClause); } 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 271875963942f..6f5bf5e1d6125 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 @@ -148,10 +148,10 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { ); const onPopulateWhereClause = useCallback( - (field: DataViewField | string, values: unknown, operation: '+' | '-') => { + async (field: DataViewField | string, values: unknown, operation: '+' | '-') => { if (query && isOfAggregateQueryType(query) && 'esql' in query) { const fieldName = typeof field === 'string' ? field : field.name; - const updatedQuery = appendWhereClauseToESQLQuery( + const updatedQuery = await appendWhereClauseToESQLQuery( query.esql, fieldName, String(values), From 628190188fda81da2497499dd86c591db4e9231d Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 24 Apr 2024 13:06:33 +0200 Subject: [PATCH 03/24] Take under cosideration the filter existence --- .../src/utils/append_to_query.ts | 44 ++++++++++++++++--- .../src/components/data_table.tsx | 2 + .../src/components/data_table_columns.tsx | 22 +++++++++- .../src/components/default_cell_actions.tsx | 10 ++++- .../src/table_context.tsx | 1 + .../components/layout/discover_layout.tsx | 7 ++- .../components/sidebar/lib/get_field_list.ts | 3 +- 7 files changed, 77 insertions(+), 12 deletions(-) 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 e1a3eb13d0369..707ad5bfb958f 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -5,7 +5,6 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ - // Append in a new line the appended text to take care of the case where the user adds a comment at the end of the query // in these cases a base query such as "from index // comment" will result in errors or wrong data if we don't append in a new line export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): string { @@ -15,18 +14,51 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): export async function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, - value: string, - operation: '+' | '-' + value: unknown, + operation: '+' | '-', + fieldType?: string ): Promise { - const operator = operation === '+' ? '==' : '!='; + let operator = operation === '+' ? '==' : '!='; + let filterValue = typeof value === 'string' ? `"${value.replace(/"/g, '\\"')}"` : value; + let fieldName = field; + + // casting + if (fieldType !== 'string' && fieldType !== 'number') { + fieldName = `${fieldName}::string`; + } + + // checking that the value is not null + if (field === '_exists_') { + fieldName = String(value); + operator = ' is not null'; + filterValue = ''; + } const { getAstAndSyntaxErrors } = await import('@kbn/esql-ast'); const { ast } = await getAstAndSyntaxErrors(baseESQLQuery); + const lastCommandIsWhere = ast[ast.length - 1].name === 'where'; if (lastCommandIsWhere) { - const whereClause = `and ${field}${operator}"${value}"`; + const whereCommand = ast[ast.length - 1]; + const whereAstText = whereCommand.text; + if (whereAstText.includes(fieldName) && whereAstText.includes(String(filterValue))) { + const pipesArray = baseESQLQuery.split('|'); + const whereClause = pipesArray[pipesArray.length - 1]; + const matches = whereClause.match(new RegExp(fieldName + '(.*)' + String(filterValue))); + if (matches) { + const existingOperator = matches[1]?.trim(); + if (existingOperator === operator) { + return baseESQLQuery; + } else { + const existingFilter = `${fieldName}${existingOperator}${filterValue}`; + const newFilter = `${fieldName}${operator}${filterValue}`; + return baseESQLQuery.replace(existingFilter, newFilter); + } + } + } + const whereClause = `and ${fieldName}${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } - const whereClause = `| where ${field}${operator}"${value}"`; + const whereClause = `| where ${fieldName}${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 799b950b558bc..0394a9f2c9b50 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -508,6 +508,7 @@ export const UnifiedDataTable = ({ }, valueToStringConverter, componentsTourSteps, + isPlainRecord, }), [ componentsTourSteps, @@ -516,6 +517,7 @@ export const UnifiedDataTable = ({ displayedRows, expandedDoc, isFilterActive, + isPlainRecord, onFilter, setExpandedDoc, usedSelectedDocs, 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 45ae96ac42b00..9837e3b6ae4f4 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 @@ -13,7 +13,7 @@ import { type EuiDataGridColumnCellAction, EuiScreenReaderOnly, } from '@elastic/eui'; -import type { DataView } from '@kbn/data-views-plugin/public'; +import { type DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { ToastsStart, IUiSettingsClient } from '@kbn/core/public'; import { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; import { ExpandButton } from './data_table_expand_button'; @@ -119,6 +119,18 @@ function buildEuiGridColumn({ customGridColumnsConfiguration?: CustomGridColumnsConfiguration; }) { const dataViewField = dataView.getFieldByName(columnName); + const textBasedField = + isPlainRecord && !dataViewField + ? new DataViewField({ + name: columnName, + type: columnsMeta?.[columnName]?.type ?? 'unknown', + esTypes: columnsMeta?.[columnName]?.type + ? ([columnsMeta[columnName].esType] as string[]) + : undefined, + searchable: true, + aggregatable: false, + }) + : undefined; const editFieldButton = editField && dataViewField && @@ -137,6 +149,14 @@ function buildEuiGridColumn({ } else { cellActions = dataViewField ? buildCellActions(dataViewField, toastNotifications, valueToStringConverter, onFilter) + : textBasedField + ? buildCellActions( + textBasedField, + toastNotifications, + valueToStringConverter, + onFilter, + isPlainRecord + ) : []; } 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 6005d75ea6632..a4cdef6129dd8 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 @@ -25,10 +25,15 @@ function onFilterCell( const row = context.rows[rowIndex]; const value = row.flattened[columnId]; const field = context.dataView.fields.getByName(columnId); + const isPlainRecord = Boolean(context.isPlainRecord); - if (field && context.onFilter) { + if (field && context.onFilter && !isPlainRecord) { context.onFilter(field, value, mode); } + + if (context.onFilter && isPlainRecord) { + context.onFilter(columnId, value, mode); + } } export const FilterInBtn = ({ @@ -123,7 +128,8 @@ export function buildCellActions( field: DataViewField, toastNotifications: ToastsStart, valueToStringConverter: ValueToStringConverter, - onFilter?: DocViewFilterFn + onFilter?: DocViewFilterFn, + isPlainRecord?: boolean ) { return [ ...(onFilter && field.filterable ? [FilterInBtn, FilterOutBtn] : []), diff --git a/packages/kbn-unified-data-table/src/table_context.tsx b/packages/kbn-unified-data-table/src/table_context.tsx index 2a1d4656d4a65..cc6c63ab87682 100644 --- a/packages/kbn-unified-data-table/src/table_context.tsx +++ b/packages/kbn-unified-data-table/src/table_context.tsx @@ -23,6 +23,7 @@ export interface DataTableContext { setSelectedDocs: (selected: string[]) => void; valueToStringConverter: ValueToStringConverter; componentsTourSteps?: Record; + isPlainRecord?: boolean; } const defaultContext = {} as unknown as DataTableContext; 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 6f5bf5e1d6125..0eae4cd5b4bb8 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 @@ -151,11 +151,14 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { async (field: DataViewField | string, values: unknown, operation: '+' | '-') => { if (query && isOfAggregateQueryType(query) && 'esql' in query) { const fieldName = typeof field === 'string' ? field : field.name; + // send the field type for casting + const fieldType = typeof field !== 'string' ? field.type : undefined; const updatedQuery = await appendWhereClauseToESQLQuery( query.esql, fieldName, - String(values), - operation + values, + operation, + fieldType ); data.query.queryString.setQuery({ esql: updatedQuery, diff --git a/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts b/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts index 770de2e3b434b..dc48d7943931d 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts +++ b/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts @@ -73,7 +73,8 @@ export function getTextBasedQueryFieldList( name: column.name, type: column.meta?.type ?? 'unknown', esTypes: column.meta?.esType ? [column.meta?.esType] : undefined, - searchable: false, + // ToDo: check first if the result is aggregated + searchable: true, aggregatable: false, isNull: Boolean(column?.isNull), }) From d4f6fc36d8d3626820c3d79529ed45154d8811d6 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 25 Apr 2024 12:39:06 +0200 Subject: [PATCH 04/24] Disable filter actions when the table is embedded --- .../src/components/data_table.tsx | 2 +- .../src/components/data_table_columns.tsx | 2 +- .../doc_table/components/table_row.tsx | 4 +- .../doc_table/doc_table_wrapper.tsx | 2 +- .../embeddable/saved_search_embeddable.tsx | 40 ++++++++++--------- .../discover/public/embeddable/types.ts | 1 - 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 0394a9f2c9b50..c1e785e585e2d 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -153,7 +153,7 @@ export interface UnifiedDataTableProps { /** * Function to add a filter in the grid cell or document flyout */ - onFilter: DocViewFilterFn; + onFilter?: DocViewFilterFn; /** * Function triggered when a column is resized by the user */ 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 9837e3b6ae4f4..3c8b4c80bff55 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 @@ -283,7 +283,7 @@ export function getEuiGridColumns({ }; hasEditDataViewPermission: () => boolean; valueToStringConverter: ValueToStringConverter; - onFilter: DocViewFilterFn; + onFilter?: DocViewFilterFn; editField?: (fieldName: string) => void; visibleCellActions?: number; columnsMeta?: DataTableColumnsMeta; diff --git a/src/plugins/discover/public/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/components/doc_table/components/table_row.tsx index 6057cd64aa473..7c9d55e7049bf 100644 --- a/src/plugins/discover/public/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/components/doc_table/components/table_row.tsx @@ -32,7 +32,7 @@ export type DocTableRow = EsHitRecord & { export interface TableRowProps { columns: string[]; - filter: DocViewFilterFn; + filter?: DocViewFilterFn; filters?: Filter[]; isPlainRecord?: boolean; savedSearchId?: string; @@ -105,7 +105,7 @@ export const TableRow = ({ const inlineFilter = useCallback( (column: string, type: '+' | '-') => { const field = dataView.fields.getByName(column); - filter(field!, row.flattened[column], type); + filter?.(field!, row.flattened[column], type); }, [filter, dataView.fields, row.flattened] ); diff --git a/src/plugins/discover/public/components/doc_table/doc_table_wrapper.tsx b/src/plugins/discover/public/components/doc_table/doc_table_wrapper.tsx index a61523b6647c3..5ad815d7c1cc6 100644 --- a/src/plugins/discover/public/components/doc_table/doc_table_wrapper.tsx +++ b/src/plugins/discover/public/components/doc_table/doc_table_wrapper.tsx @@ -72,7 +72,7 @@ export interface DocTableProps { /** * Filter callback */ - onFilter: DocViewFilterFn; + onFilter?: DocViewFilterFn; /** * Sorting callback */ diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index 5ce82d642fb62..1e086dcac2233 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -459,25 +459,27 @@ export class SavedSearchEmbeddable }); this.updateInput({ sort: sortOrderArr }); }, - onFilter: async (field, value, operator) => { - let filters = generateFilters( - this.services.filterManager, - // @ts-expect-error - field, - value, - operator, - dataView - ); - filters = filters.map((filter) => ({ - ...filter, - $state: { store: FilterStateStore.APP_STATE }, - })); - - await this.executeTriggerActions(APPLY_FILTER_TRIGGER, { - embeddable: this, - filters, - }); - }, + ...(!this.isTextBasedSearch(savedSearch) && { + onFilter: async (field, value, operator) => { + let filters = generateFilters( + this.services.filterManager, + // @ts-expect-error + field, + value, + operator, + dataView + ); + filters = filters.map((filter) => ({ + ...filter, + $state: { store: FilterStateStore.APP_STATE }, + })); + + await this.executeTriggerActions(APPLY_FILTER_TRIGGER, { + embeddable: this, + filters, + }); + }, + }), useNewFieldsApi: !this.services.uiSettings.get(SEARCH_FIELDS_FROM_SOURCE, false), showTimeCol: !this.services.uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false), ariaLabelledBy: 'documentsAriaLabel', diff --git a/src/plugins/discover/public/embeddable/types.ts b/src/plugins/discover/public/embeddable/types.ts index bc99e29d71756..a09a82ee83af1 100644 --- a/src/plugins/discover/public/embeddable/types.ts +++ b/src/plugins/discover/public/embeddable/types.ts @@ -16,7 +16,6 @@ import type { import type { Adapters } from '@kbn/embeddable-plugin/public'; import { EmbeddableApiContext } from '@kbn/presentation-publishing'; - import type { DiscoverServices } from '../build_services'; import type { DocTableEmbeddableSearchProps } from '../components/doc_table/doc_table_embeddable'; import type { DiscoverGridEmbeddableSearchProps } from './saved_search_grid'; From 07a207500db37d9d694acec69d69ff136139c2c7 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:50:22 +0000 Subject: [PATCH 05/24] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- packages/kbn-esql-utils/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kbn-esql-utils/tsconfig.json b/packages/kbn-esql-utils/tsconfig.json index b604fa84c1de3..5a494e9929d7b 100644 --- a/packages/kbn-esql-utils/tsconfig.json +++ b/packages/kbn-esql-utils/tsconfig.json @@ -19,5 +19,6 @@ "@kbn/data-views-plugin", "@kbn/crypto-browser", "@kbn/data-view-utils", + "@kbn/esql-ast", ] } From c995cc0f68696f7620d40b16d9b4113005fdf223 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 25 Apr 2024 13:09:31 +0200 Subject: [PATCH 06/24] Simplifications --- .../src/components/data_table.tsx | 2 -- .../src/components/data_table_columns.tsx | 8 +------- .../src/components/default_cell_actions.tsx | 10 ++-------- packages/kbn-unified-data-table/src/table_context.tsx | 1 - 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index c1e785e585e2d..2fba594e9f902 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -508,7 +508,6 @@ export const UnifiedDataTable = ({ }, valueToStringConverter, componentsTourSteps, - isPlainRecord, }), [ componentsTourSteps, @@ -517,7 +516,6 @@ export const UnifiedDataTable = ({ displayedRows, expandedDoc, isFilterActive, - isPlainRecord, onFilter, setExpandedDoc, usedSelectedDocs, 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 3c8b4c80bff55..e3b93dc222682 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 @@ -150,13 +150,7 @@ function buildEuiGridColumn({ cellActions = dataViewField ? buildCellActions(dataViewField, toastNotifications, valueToStringConverter, onFilter) : textBasedField - ? buildCellActions( - textBasedField, - toastNotifications, - valueToStringConverter, - onFilter, - isPlainRecord - ) + ? buildCellActions(textBasedField, toastNotifications, valueToStringConverter, onFilter) : []; } 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 a4cdef6129dd8..6005d75ea6632 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 @@ -25,15 +25,10 @@ function onFilterCell( const row = context.rows[rowIndex]; const value = row.flattened[columnId]; const field = context.dataView.fields.getByName(columnId); - const isPlainRecord = Boolean(context.isPlainRecord); - if (field && context.onFilter && !isPlainRecord) { + if (field && context.onFilter) { context.onFilter(field, value, mode); } - - if (context.onFilter && isPlainRecord) { - context.onFilter(columnId, value, mode); - } } export const FilterInBtn = ({ @@ -128,8 +123,7 @@ export function buildCellActions( field: DataViewField, toastNotifications: ToastsStart, valueToStringConverter: ValueToStringConverter, - onFilter?: DocViewFilterFn, - isPlainRecord?: boolean + onFilter?: DocViewFilterFn ) { return [ ...(onFilter && field.filterable ? [FilterInBtn, FilterOutBtn] : []), diff --git a/packages/kbn-unified-data-table/src/table_context.tsx b/packages/kbn-unified-data-table/src/table_context.tsx index cc6c63ab87682..2a1d4656d4a65 100644 --- a/packages/kbn-unified-data-table/src/table_context.tsx +++ b/packages/kbn-unified-data-table/src/table_context.tsx @@ -23,7 +23,6 @@ export interface DataTableContext { setSelectedDocs: (selected: string[]) => void; valueToStringConverter: ValueToStringConverter; componentsTourSteps?: Record; - isPlainRecord?: boolean; } const defaultContext = {} as unknown as DataTableContext; From 11639c5e813f1ce603180b2e97f2dd5b464bc9bd Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 25 Apr 2024 13:37:37 +0200 Subject: [PATCH 07/24] Fixes --- .../src/components/data_table_columns.tsx | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) 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 e3b93dc222682..13ff758d878a3 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 @@ -118,19 +118,17 @@ function buildEuiGridColumn({ headerRowHeight?: number; customGridColumnsConfiguration?: CustomGridColumnsConfiguration; }) { - const dataViewField = dataView.getFieldByName(columnName); - const textBasedField = - isPlainRecord && !dataViewField - ? new DataViewField({ - name: columnName, - type: columnsMeta?.[columnName]?.type ?? 'unknown', - esTypes: columnsMeta?.[columnName]?.type - ? ([columnsMeta[columnName].esType] as string[]) - : undefined, - searchable: true, - aggregatable: false, - }) - : undefined; + const dataViewField = !isPlainRecord + ? dataView.getFieldByName(columnName) + : new DataViewField({ + name: columnName, + type: columnsMeta?.[columnName]?.type ?? 'unknown', + esTypes: columnsMeta?.[columnName]?.esType + ? ([columnsMeta[columnName].esType] as string[]) + : undefined, + searchable: true, + aggregatable: false, + }); const editFieldButton = editField && dataViewField && @@ -149,8 +147,6 @@ function buildEuiGridColumn({ } else { cellActions = dataViewField ? buildCellActions(dataViewField, toastNotifications, valueToStringConverter, onFilter) - : textBasedField - ? buildCellActions(textBasedField, toastNotifications, valueToStringConverter, onFilter) : []; } From dfbe8ddc0987511f8228d09d26e9c5e7dfcf12b9 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 26 Apr 2024 11:01:31 +0200 Subject: [PATCH 08/24] More fixes --- .../src/utils/append_to_query.ts | 14 ++++--- .../src/components/default_cell_actions.tsx | 41 ++++++++++++------- .../components/layout/discover_layout.tsx | 4 +- 3 files changed, 36 insertions(+), 23 deletions(-) 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 707ad5bfb958f..76a91887ac395 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -5,19 +5,21 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ +import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; + // Append in a new line the appended text to take care of the case where the user adds a comment at the end of the query // in these cases a base query such as "from index // comment" will result in errors or wrong data if we don't append in a new line export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): string { return `${baseESQLQuery}\n${appendedText}`; } -export async function appendWhereClauseToESQLQuery( +export function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, value: unknown, operation: '+' | '-', fieldType?: string -): Promise { +): string { let operator = operation === '+' ? '==' : '!='; let filterValue = typeof value === 'string' ? `"${value.replace(/"/g, '\\"')}"` : value; let fieldName = field; @@ -34,10 +36,10 @@ export async function appendWhereClauseToESQLQuery( filterValue = ''; } - const { getAstAndSyntaxErrors } = await import('@kbn/esql-ast'); - const { ast } = await getAstAndSyntaxErrors(baseESQLQuery); + const { ast } = getAstAndSyntaxErrors(baseESQLQuery); const lastCommandIsWhere = ast[ast.length - 1].name === 'where'; + // if where command already exists in the end of the query if (lastCommandIsWhere) { const whereCommand = ast[ast.length - 1]; const whereAstText = whereCommand.text; @@ -56,9 +58,9 @@ export async function appendWhereClauseToESQLQuery( } } } - const whereClause = `and ${fieldName}${operator}${filterValue}`; + const whereClause = `and \`${fieldName}\`${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } - const whereClause = `| where ${fieldName}${operator}${filterValue}`; + const whereClause = `| where \`${fieldName}\`${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } 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 6005d75ea6632..109d289eea84a 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 @@ -20,22 +20,21 @@ function onFilterCell( context: DataTableContext, rowIndex: EuiDataGridColumnCellActionProps['rowIndex'], columnId: EuiDataGridColumnCellActionProps['columnId'], - mode: '+' | '-' + mode: '+' | '-', + field: DataViewField ) { const row = context.rows[rowIndex]; const value = row.flattened[columnId]; - const field = context.dataView.fields.getByName(columnId); if (field && context.onFilter) { context.onFilter(field, value, mode); } } -export const FilterInBtn = ({ - Component, - rowIndex, - columnId, -}: EuiDataGridColumnCellActionProps) => { +export const FilterInBtn = ( + { Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps, + field: DataViewField +) => { const context = useContext(UnifiedDataTableContext); const buttonTitle = i18n.translate('unifiedDataTable.grid.filterForAria', { defaultMessage: 'Filter for this {value}', @@ -45,7 +44,7 @@ export const FilterInBtn = ({ return ( { - onFilterCell(context, rowIndex, columnId, '+'); + onFilterCell(context, rowIndex, columnId, '+', field); }} iconType="plusInCircle" aria-label={buttonTitle} @@ -59,11 +58,10 @@ export const FilterInBtn = ({ ); }; -export const FilterOutBtn = ({ - Component, - rowIndex, - columnId, -}: EuiDataGridColumnCellActionProps) => { +export const FilterOutBtn = ( + { Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps, + field: DataViewField +) => { const context = useContext(UnifiedDataTableContext); const buttonTitle = i18n.translate('unifiedDataTable.grid.filterOutAria', { defaultMessage: 'Filter out this {value}', @@ -73,7 +71,7 @@ export const FilterOutBtn = ({ return ( { - onFilterCell(context, rowIndex, columnId, '-'); + onFilterCell(context, rowIndex, columnId, '-', field); }} iconType="minusInCircle" aria-label={buttonTitle} @@ -126,7 +124,20 @@ export function buildCellActions( onFilter?: DocViewFilterFn ) { return [ - ...(onFilter && field.filterable ? [FilterInBtn, FilterOutBtn] : []), + ...(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 + ), + ] + : []), ({ Component, rowIndex, columnId }: EuiDataGridColumnCellActionProps) => buildCopyValueButton( { Component, rowIndex, columnId } as 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 0eae4cd5b4bb8..893fac85809ea 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 @@ -148,12 +148,12 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { ); const onPopulateWhereClause = useCallback( - async (field: DataViewField | string, values: unknown, operation: '+' | '-') => { + (field: DataViewField | string, values: unknown, operation: '+' | '-') => { if (query && isOfAggregateQueryType(query) && 'esql' in query) { const fieldName = typeof field === 'string' ? field : field.name; // send the field type for casting const fieldType = typeof field !== 'string' ? field.type : undefined; - const updatedQuery = await appendWhereClauseToESQLQuery( + const updatedQuery = appendWhereClauseToESQLQuery( query.esql, fieldName, values, From 149c88809bcfe2d1afe2f56d26f4306b2654b442 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 26 Apr 2024 11:04:25 +0200 Subject: [PATCH 09/24] Fix quotes --- packages/kbn-esql-utils/src/utils/append_to_query.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 76a91887ac395..c2edfdd1f3037 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -22,7 +22,7 @@ export function appendWhereClauseToESQLQuery( ): string { let operator = operation === '+' ? '==' : '!='; let filterValue = typeof value === 'string' ? `"${value.replace(/"/g, '\\"')}"` : value; - let fieldName = field; + let fieldName = `\`${field}\``; // casting if (fieldType !== 'string' && fieldType !== 'number') { @@ -58,9 +58,9 @@ export function appendWhereClauseToESQLQuery( } } } - const whereClause = `and \`${fieldName}\`${operator}${filterValue}`; + const whereClause = `and ${fieldName}${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } - const whereClause = `| where \`${fieldName}\`${operator}${filterValue}`; + const whereClause = `| where ${fieldName}${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } From 4cf0952af43de8b68c0ebe2153258f1a2549075b Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 26 Apr 2024 11:42:55 +0200 Subject: [PATCH 10/24] Fix jest tests --- .../data_table_columns.test.tsx.snap | 517 +----------------- .../components/data_table_columns.test.tsx | 2 +- .../sidebar/lib/sidebar_reducer.test.ts | 4 +- 3 files changed, 21 insertions(+), 502 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap index 28d43dc69b6cd..b8f707ba27c73 100644 --- a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap +++ b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap @@ -43,8 +43,6 @@ Array [ }, "cellActions": Array [ [Function], - [Function], - [Function], ], "display": test @@ -52,7 +50,7 @@ Array [ "displayAsText": "extension", "id": "extension", "isSortable": true, - "schema": "string", + "schema": "kibana-json", "visibleCellActions": undefined, }, Object { @@ -177,39 +175,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], - Array [ - "extension", - ], - Array [ - "message", - ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "timestamp", - ], - Array [ - "var_test", - ], - Array [ - "extension", - ], - Array [ - "message", - ], Array [ "extension", ], @@ -305,120 +270,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": undefined, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, Object { "type": "return", "value": Object { @@ -468,7 +319,7 @@ Array [ "displayAsText": "message", "id": "message", "isSortable": true, - "schema": "string", + "schema": "kibana-json", "visibleCellActions": undefined, }, ] @@ -595,15 +446,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], ], "results": Array [ Object { @@ -693,39 +535,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, ], }, "getFormatterForField": [MockFunction], @@ -893,15 +702,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], ], "results": Array [ Object { @@ -991,39 +791,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, ], }, "getFormatterForField": [MockFunction], @@ -1178,15 +945,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], ], "results": Array [ Object { @@ -1276,39 +1034,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, ], }, "getFormatterForField": [MockFunction], @@ -1481,15 +1206,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], Array [ "extension", ], @@ -1585,39 +1301,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, Object { "type": "return", "value": Object { @@ -1804,15 +1487,6 @@ Array [ Array [ "message", ], - Array [ - "timestamp", - ], - Array [ - "extension", - ], - Array [ - "message", - ], Array [ "extension", ], @@ -1908,39 +1582,6 @@ Array [ "type": "string", }, }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "timestamp", - "filterable": true, - "name": "timestamp", - "scripted": false, - "sortable": true, - "type": "date", - }, - }, - Object { - "type": "return", - "value": Object { - "aggregatable": true, - "displayName": "extension", - "filterable": true, - "name": "extension", - "scripted": false, - "type": "string", - }, - }, - Object { - "type": "return", - "value": Object { - "displayName": "message", - "filterable": false, - "name": "message", - "scripted": false, - "type": "string", - }, - }, Object { "type": "return", "value": Object { @@ -2970,8 +2611,6 @@ Array [ }, "cellActions": Array [ [Function], - [Function], - [Function], ], "display": { diff --git a/src/plugins/discover/public/application/main/components/sidebar/lib/sidebar_reducer.test.ts b/src/plugins/discover/public/application/main/components/sidebar/lib/sidebar_reducer.test.ts index 9f886a08c1142..8c6ce4888fb47 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/lib/sidebar_reducer.test.ts +++ b/src/plugins/discover/public/application/main/components/sidebar/lib/sidebar_reducer.test.ts @@ -131,7 +131,7 @@ describe('sidebar reducer', function () { esTypes: undefined, aggregatable: false, isNull: true, - searchable: false, + searchable: true, }), new DataViewField({ name: 'text2', @@ -139,7 +139,7 @@ describe('sidebar reducer', function () { esTypes: ['keyword'], aggregatable: false, isNull: false, - searchable: false, + searchable: true, }), ], fieldCounts: {}, From 6d726f3150bc79099784624594ce1fded0c1747f Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 26 Apr 2024 14:23:06 +0200 Subject: [PATCH 11/24] Fix jest tests --- packages/kbn-esql-utils/README.md | 2 + .../src/utils/append_to_query.test.ts | 119 ++++++++++++++++-- .../src/utils/append_to_query.ts | 12 +- .../src/components/data_table.tsx | 2 + .../components/default_cell_actions.test.tsx | 3 +- .../src/components/default_cell_actions.tsx | 6 +- .../src/table_context.tsx | 1 + 7 files changed, 123 insertions(+), 22 deletions(-) diff --git a/packages/kbn-esql-utils/README.md b/packages/kbn-esql-utils/README.md index b1cbf685d66b8..8dc31041e86da 100644 --- a/packages/kbn-esql-utils/README.md +++ b/packages/kbn-esql-utils/README.md @@ -6,4 +6,6 @@ This package contains utilities for ES|QL. - *getIndexPatternFromESQLQuery*: Use this to retrieve the index pattern from the `from` command. - *getLimitFromESQLQuery*: Use this function to get the limit for a given query. The limit can be either set from the `limit` command or can be a default value set in ES. - *removeDropCommandsFromESQLQuery*: Use this function to remove all the occurences of the `drop` command from the query. +- *appendToESQLQuery*: Use this function to append more pipes in an existing ES|QL query. It adds the additional commands in a new line. +- *appendWhereClauseToESQLQuery*: Use this function to append where clause in an existing query. 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 414b9729af03b..e73427f341c9f 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 @@ -5,21 +5,116 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { appendToESQLQuery } from './append_to_query'; +import { appendToESQLQuery, appendWhereClauseToESQLQuery } from './append_to_query'; -describe('appendToESQLQuery', () => { - it('append the text on a new line after the query', () => { - expect(appendToESQLQuery('from logstash-* // meow', '| stats var = avg(woof)')).toBe( - `from logstash-* // meow +describe('appendToQuery', () => { + describe('appendToESQLQuery', () => { + it('append the text on a new line after the query', () => { + expect(appendToESQLQuery('from logstash-* // meow', '| stats var = avg(woof)')).toBe( + `from logstash-* // meow | stats var = avg(woof)` - ); - }); + ); + }); - it('append the text on a new line after the query for text with variables', () => { - const limit = 10; - expect(appendToESQLQuery('from logstash-*', `| limit ${limit}`)).toBe( - `from logstash-* + it('append the text on a new line after the query for text with variables', () => { + const limit = 10; + expect(appendToESQLQuery('from logstash-*', `| limit ${limit}`)).toBe( + `from logstash-* | limit 10` - ); + ); + }); + }); + + describe('appendWhereClauseToESQLQuery', () => { + it('appends a filter in where clause in an existing query', () => { + expect( + appendWhereClauseToESQLQuery('from logstash-* // meow', 'dest', 'tada!', '+', 'string') + ).toBe( + `from logstash-* // meow +| where \`dest\`=="tada!"` + ); + }); + it('appends a filter out where clause in an existing query', () => { + expect( + appendWhereClauseToESQLQuery('from logstash-* // meow', 'dest', 'tada!', '-', 'string') + ).toBe( + `from logstash-* // meow +| where \`dest\`!="tada!"` + ); + }); + + it('appends a where clause in an existing query with casting to string when the type is not string or number', () => { + expect( + appendWhereClauseToESQLQuery('from logstash-* // meow', 'dest', 'tada!', '-', 'ip') + ).toBe( + `from logstash-* // meow +| where \`dest\`::string!="tada!"` + ); + }); + + it('appends a where clause in an existing query with casting to string when the type is not given', () => { + expect(appendWhereClauseToESQLQuery('from logstash-* // meow', 'dest', 'tada!', '-')).toBe( + `from logstash-* // meow +| where \`dest\`::string!="tada!"` + ); + }); + + it('appends a where clause in an existing query checking that the value is not null if the user asks for existence', () => { + expect(appendWhereClauseToESQLQuery('from logstash-* // meow', '_exists_', 'dest', '+')).toBe( + `from logstash-* // meow +| where \`dest\` is not null` + ); + }); + + it('appends an and clause in an existing query with where command as the last pipe', () => { + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* | where country == "GR"', + 'dest', + 'Crete', + '+', + 'string' + ) + ).toBe( + `from logstash-* | where country == "GR" +and \`dest\`=="Crete"` + ); + }); + + it('doesnt append anything in an existing query with where command as the last pipe if the filter preexists', () => { + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* | where country == "GR"', + 'country', + 'GR', + '+', + 'string' + ) + ).toBe(`from logstash-* | where country == "GR"`); + }); + + it('changes the operator in an existing query with where command as the last pipe if the filter preexists but has the opposite operator', () => { + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* | where country == "GR"', + 'country', + 'GR', + '-', + 'string' + ) + ).toBe(`from logstash-* | where country != "GR"`); + }); + + it('changes the operator in an existing query with where command as the last pipe if the filter preexists but has the opposite operator, the field has backticks', () => { + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* | where `country` == "GR"', + 'country', + 'GR', + '-', + 'string' + ) + ).toBe(`from logstash-* | where \`country\`!= "GR"`); + }); }); }); 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 c2edfdd1f3037..1764ec7c36929 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -31,7 +31,7 @@ export function appendWhereClauseToESQLQuery( // checking that the value is not null if (field === '_exists_') { - fieldName = String(value); + fieldName = `\`${String(value)}\``; operator = ' is not null'; filterValue = ''; } @@ -43,17 +43,17 @@ export function appendWhereClauseToESQLQuery( if (lastCommandIsWhere) { const whereCommand = ast[ast.length - 1]; const whereAstText = whereCommand.text; - if (whereAstText.includes(fieldName) && whereAstText.includes(String(filterValue))) { + if (whereAstText.includes(field) && whereAstText.includes(String(filterValue))) { const pipesArray = baseESQLQuery.split('|'); const whereClause = pipesArray[pipesArray.length - 1]; - const matches = whereClause.match(new RegExp(fieldName + '(.*)' + String(filterValue))); + const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); if (matches) { - const existingOperator = matches[1]?.trim(); + const existingOperator = matches[1]?.trim().replace('`', ''); if (existingOperator === operator) { return baseESQLQuery; } else { - const existingFilter = `${fieldName}${existingOperator}${filterValue}`; - const newFilter = `${fieldName}${operator}${filterValue}`; + const existingFilter = matches[0].trim(); + const newFilter = existingFilter.replace(existingOperator, operator); return baseESQLQuery.replace(existingFilter, newFilter); } } diff --git a/packages/kbn-unified-data-table/src/components/data_table.tsx b/packages/kbn-unified-data-table/src/components/data_table.tsx index 2fba594e9f902..fea6f7c594697 100644 --- a/packages/kbn-unified-data-table/src/components/data_table.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table.tsx @@ -508,11 +508,13 @@ export const UnifiedDataTable = ({ }, valueToStringConverter, componentsTourSteps, + isPlainRecord, }), [ componentsTourSteps, darkMode, dataView, + isPlainRecord, displayedRows, expandedDoc, isFilterActive, 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 6ecaa850da6a1..613c5f644c273 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 @@ -63,8 +63,7 @@ describe('Default cell actions ', function () { dataTableContextMock.valueToStringConverter, jest.fn() ); - expect(cellActions).toContain(FilterInBtn); - expect(cellActions).toContain(FilterOutBtn); + expect(cellActions).toHaveLength(3); }); it('should show Copy action for _source field', async () => { 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 109d289eea84a..35f5ec58fe38a 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 @@ -26,8 +26,10 @@ function onFilterCell( const row = context.rows[rowIndex]; const value = row.flattened[columnId]; - if (field && context.onFilter) { - context.onFilter(field, value, mode); + const filterField = context.isPlainRecord ? field : context.dataView.fields.getByName(columnId); + + if (filterField && context.onFilter) { + context.onFilter(filterField, value, mode); } } diff --git a/packages/kbn-unified-data-table/src/table_context.tsx b/packages/kbn-unified-data-table/src/table_context.tsx index 2a1d4656d4a65..cc6c63ab87682 100644 --- a/packages/kbn-unified-data-table/src/table_context.tsx +++ b/packages/kbn-unified-data-table/src/table_context.tsx @@ -23,6 +23,7 @@ export interface DataTableContext { setSelectedDocs: (selected: string[]) => void; valueToStringConverter: ValueToStringConverter; componentsTourSteps?: Record; + isPlainRecord?: boolean; } const defaultContext = {} as unknown as DataTableContext; From 92b505083d2ecf06ffef76a37ec45f6adbb743dc Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 29 Apr 2024 10:32:52 +0200 Subject: [PATCH 12/24] Some cleanuo --- .../kbn-esql-utils/src/utils/append_to_query.ts | 15 +++++++++++++-- .../src/components/data_table_columns.test.tsx | 4 ++-- .../main/components/sidebar/lib/get_field_list.ts | 1 - .../public/embeddable/saved_search_embeddable.tsx | 1 + src/plugins/discover/public/embeddable/types.ts | 1 + 5 files changed, 17 insertions(+), 5 deletions(-) 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 1764ec7c36929..cee6164703c3a 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -22,14 +22,18 @@ export function appendWhereClauseToESQLQuery( ): string { let operator = operation === '+' ? '==' : '!='; let filterValue = typeof value === 'string' ? `"${value.replace(/"/g, '\\"')}"` : value; + // Adding the backticks here are they are needed for special char fields let fieldName = `\`${field}\``; - // casting + // casting to string + // there are some field types such as the ip that need + // to cast in string first otherwise ES will fail if (fieldType !== 'string' && fieldType !== 'number') { fieldName = `${fieldName}::string`; } // checking that the value is not null + // this is the existence filter if (field === '_exists_') { fieldName = `\`${String(value)}\``; operator = ' is not null'; @@ -39,18 +43,24 @@ export function appendWhereClauseToESQLQuery( const { ast } = getAstAndSyntaxErrors(baseESQLQuery); const lastCommandIsWhere = ast[ast.length - 1].name === 'where'; - // if where command already exists in the end of the query + // if where command already exists in the end of the query: + // - we need to append with and if the filter doesnt't exist + // - we need to change the filter operator if the filter exists with different operator + // - we do nothing if the filter exists with the same operator if (lastCommandIsWhere) { const whereCommand = ast[ast.length - 1]; const whereAstText = whereCommand.text; + // the filter already exists in the where clause if (whereAstText.includes(field) && whereAstText.includes(String(filterValue))) { const pipesArray = baseESQLQuery.split('|'); const whereClause = pipesArray[pipesArray.length - 1]; const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); if (matches) { const existingOperator = matches[1]?.trim().replace('`', ''); + // the filter is the same if (existingOperator === operator) { return baseESQLQuery; + // the filter has different operator } else { const existingFilter = matches[0].trim(); const newFilter = existingFilter.replace(existingOperator, operator); @@ -58,6 +68,7 @@ export function appendWhereClauseToESQLQuery( } } } + // filter does not exist in the where clause const whereClause = `and ${fieldName}${operator}${filterValue}`; return appendToESQLQuery(baseESQLQuery, whereClause); } diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx index d67c50877debf..7032454de8c2b 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx @@ -348,10 +348,10 @@ describe('Data table columns', function () { servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, columnsMeta: { - var_test: { type: 'number' }, + extension: { type: 'string' }, }, }); - expect(gridColumns[1].schema).toBe('kibana-json'); + expect(gridColumns[1].schema).toBe('string'); }); it('returns eui grid with in memory sorting for text based languages and columns not on the columnsMeta', async () => { diff --git a/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts b/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts index dc48d7943931d..071454b0bd579 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts +++ b/src/plugins/discover/public/application/main/components/sidebar/lib/get_field_list.ts @@ -73,7 +73,6 @@ export function getTextBasedQueryFieldList( name: column.name, type: column.meta?.type ?? 'unknown', esTypes: column.meta?.esType ? [column.meta?.esType] : undefined, - // ToDo: check first if the result is aggregated searchable: true, aggregatable: false, isNull: Boolean(column?.isNull), diff --git a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx index 159541c486ed9..fecd5e4a95de8 100644 --- a/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/saved_search_embeddable.tsx @@ -459,6 +459,7 @@ export class SavedSearchEmbeddable }); this.updateInput({ sort: sortOrderArr }); }, + // I don't want to create filters when is embedded ...(!this.isTextBasedSearch(savedSearch) && { onFilter: async (field, value, operator) => { let filters = generateFilters( diff --git a/src/plugins/discover/public/embeddable/types.ts b/src/plugins/discover/public/embeddable/types.ts index a09a82ee83af1..bc99e29d71756 100644 --- a/src/plugins/discover/public/embeddable/types.ts +++ b/src/plugins/discover/public/embeddable/types.ts @@ -16,6 +16,7 @@ import type { import type { Adapters } from '@kbn/embeddable-plugin/public'; import { EmbeddableApiContext } from '@kbn/presentation-publishing'; + import type { DiscoverServices } from '../build_services'; import type { DocTableEmbeddableSearchProps } from '../components/doc_table/doc_table_embeddable'; import type { DiscoverGridEmbeddableSearchProps } from './saved_search_grid'; From deee9d43285fccb369ad03fde1dc50919af7f17f Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 29 Apr 2024 11:59:01 +0200 Subject: [PATCH 13/24] Add functional tests --- .../src/utils/append_to_query.ts | 2 +- .../apps/discover/group4/_esql_view.ts | 50 +++++++++++++++++ .../discover/group6/_sidebar_field_stats.ts | 56 +++++++++++++++++-- .../page_objects/unified_field_list.ts | 11 ++++ test/functional/services/data_grid.ts | 5 ++ 5 files changed, 119 insertions(+), 5 deletions(-) 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 cee6164703c3a..52223979d86a6 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -28,7 +28,7 @@ export function appendWhereClauseToESQLQuery( // casting to string // there are some field types such as the ip that need // to cast in string first otherwise ES will fail - if (fieldType !== 'string' && fieldType !== 'number') { + if (fieldType !== 'string' && fieldType !== 'number' && fieldType !== 'boolean') { fieldName = `${fieldName}::string`; } diff --git a/test/functional/apps/discover/group4/_esql_view.ts b/test/functional/apps/discover/group4/_esql_view.ts index 31f323500b119..216ba84042512 100644 --- a/test/functional/apps/discover/group4/_esql_view.ts +++ b/test/functional/apps/discover/group4/_esql_view.ts @@ -527,5 +527,55 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); }); }); + + describe('filtering by clicking on the table', () => { + beforeEach(async () => { + await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultAbsoluteRange(); + }); + + it('should append a where clause by clicking the table', async () => { + await PageObjects.discover.selectTextBaseLang(); + const testQuery = `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB`; + await monacoEditor.setCodeEditorValue(testQuery); + + await testSubjects.click('querySubmitButton'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded(); + + await testSubjects.click('TextBasedLangEditor-expand'); + await dataGrid.clickCellFilterForButton(0, 3); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB\n| where \`geo.dest\`=="BT"` + ); + + // negate + await dataGrid.clickCellFilterOutButton(0, 3); + const newValue = await monacoEditor.getCodeEditorValue(); + expect(newValue).to.eql( + `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB\n| where \`geo.dest\`!="BT"` + ); + }); + + it('should append an end in existing where clause by clicking the table', async () => { + await PageObjects.discover.selectTextBaseLang(); + const testQuery = `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB | where countB > 0`; + await monacoEditor.setCodeEditorValue(testQuery); + + await testSubjects.click('querySubmitButton'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded(); + + await testSubjects.click('TextBasedLangEditor-expand'); + await dataGrid.clickCellFilterForButton(0, 3); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB | where countB > 0\nand \`geo.dest\`=="BT"` + ); + }); + }); }); } diff --git a/test/functional/apps/discover/group6/_sidebar_field_stats.ts b/test/functional/apps/discover/group6/_sidebar_field_stats.ts index 7923bc311dedc..ff5179a492bfe 100644 --- a/test/functional/apps/discover/group6/_sidebar_field_stats.ts +++ b/test/functional/apps/discover/group6/_sidebar_field_stats.ts @@ -145,9 +145,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('text based columns', function () { before(async () => { - const TEST_START_TIME = 'Sep 23, 2015 @ 06:31:44.000'; - const TEST_END_TIME = 'Sep 23, 2015 @ 18:31:44.000'; - await PageObjects.timePicker.setAbsoluteRange(TEST_START_TIME, TEST_END_TIME); + await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); await PageObjects.common.navigateToApp('discover'); await PageObjects.discover.waitUntilSearchingHasFinished(); @@ -169,6 +167,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '42 sample values' ); + + await PageObjects.unifiedFieldList.clickFieldListPlusFilter('bytes', '0'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* [METADATA _index, _id] | sort @timestamp desc | limit 500\n| where \`bytes\`==0` + ); await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -183,6 +188,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '500 sample values' ); + + await PageObjects.unifiedFieldList.clickFieldListPlusFilter('extension.raw', 'css'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* [METADATA _index, _id] | sort @timestamp desc | limit 500\n| where \`extension.raw\`=="css"` + ); + await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -197,6 +210,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '32 sample values' ); + + await PageObjects.unifiedFieldList.clickFieldListPlusFilter('clientip', '216.126.255.31'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* [METADATA _index, _id] | sort @timestamp desc | limit 500\n| where \`clientip\`::string=="216.126.255.31"` + ); + await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -214,8 +235,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.unifiedFieldList.closeFieldPopover(); }); - it('should not have stats for a date field yet', async () => { + it('should not have stats for a date field yet but create an is not null filter', async () => { await PageObjects.unifiedFieldList.clickFieldListItem('@timestamp'); + await PageObjects.unifiedFieldList.clickFieldListExistsFilter('@timestamp'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* [METADATA _index, _id] | sort @timestamp desc | limit 500\n| where \`@timestamp\` is not null` + ); await testSubjects.missingOrFail('dscFieldStats-statsFooter'); await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -245,6 +272,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '100 sample records' ); + + await PageObjects.unifiedFieldList.clickFieldListPlusFilter('extension', 'css'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* [METADATA _index, _id] | sort @timestamp desc | limit 500\n| where \`extension\`=="css"` + ); + await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -277,6 +312,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '3 sample values' ); + + await PageObjects.unifiedFieldList.clickFieldListPlusFilter('avg(bytes)', '5453'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql( + `from logstash-* | sort @timestamp desc | limit 50 | stats avg(bytes) by geo.dest | limit 3\n| where \`avg(bytes)\`==5453` + ); + await PageObjects.unifiedFieldList.closeFieldPopover(); }); @@ -298,6 +341,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await testSubjects.getVisibleText('dscFieldStats-statsFooter')).to.contain( '1 sample value' ); + + await PageObjects.unifiedFieldList.clickFieldListMinusFilter('enabled', 'true'); + await testSubjects.click('TextBasedLangEditor-expand'); + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql(`row enabled = true\n| where \`enabled\`!=true`); await PageObjects.unifiedFieldList.closeFieldPopover(); }); }); diff --git a/test/functional/page_objects/unified_field_list.ts b/test/functional/page_objects/unified_field_list.ts index d09ac99e30790..95644bf7483b1 100644 --- a/test/functional/page_objects/unified_field_list.ts +++ b/test/functional/page_objects/unified_field_list.ts @@ -233,6 +233,17 @@ export class UnifiedFieldListPageObject extends FtrService { await this.header.waitUntilLoadingHasFinished(); } + public async clickFieldListExistsFilter(field: string) { + const existsFilterTestSubj = `discoverFieldListPanelAddExistFilter-${field}`; + if (!(await this.testSubjects.exists(existsFilterTestSubj))) { + // field has to be open + await this.clickFieldListItem(field); + } + // this.testSubjects.find doesn't handle spaces in the data-test-subj value + await this.testSubjects.click(existsFilterTestSubj); + await this.header.waitUntilLoadingHasFinished(); + } + public async openSidebarFieldFilter() { await this.testSubjects.click('fieldListFiltersFieldTypeFilterToggle'); await this.testSubjects.existOrFail('fieldListFiltersFieldTypeFilterOptions'); diff --git a/test/functional/services/data_grid.ts b/test/functional/services/data_grid.ts index c12d34c7d50f3..320e912bfa6a9 100644 --- a/test/functional/services/data_grid.ts +++ b/test/functional/services/data_grid.ts @@ -136,6 +136,11 @@ export class DataGridService extends FtrService { await actionButton.click(); } + public async clickCellFilterOutButton(rowIndex: number = 0, columnIndex: number = 0) { + const actionButton = await this.getCellActionButton(rowIndex, columnIndex, 'filterOutButton'); + await actionButton.click(); + } + /** * The same as getCellElement, but useful when multiple data grids are on the page. */ From 067d5d39e8bf8de855fef571d47ce582457bde0c Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 29 Apr 2024 13:47:16 +0200 Subject: [PATCH 14/24] Fix FTs --- test/functional/apps/discover/group6/_sidebar_field_stats.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/apps/discover/group6/_sidebar_field_stats.ts b/test/functional/apps/discover/group6/_sidebar_field_stats.ts index ff5179a492bfe..b2526445f75bf 100644 --- a/test/functional/apps/discover/group6/_sidebar_field_stats.ts +++ b/test/functional/apps/discover/group6/_sidebar_field_stats.ts @@ -144,7 +144,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); describe('text based columns', function () { - before(async () => { + beforeEach(async () => { await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings(); await PageObjects.common.navigateToApp('discover'); await PageObjects.discover.waitUntilSearchingHasFinished(); From 523d50528bacc2ecb0a98e791961156732d20734 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 30 Apr 2024 11:22:44 +0200 Subject: [PATCH 15/24] Wait for the query to run - test stabilization --- test/functional/apps/discover/group4/_esql_view.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/functional/apps/discover/group4/_esql_view.ts b/test/functional/apps/discover/group4/_esql_view.ts index 216ba84042512..70340a9514e61 100644 --- a/test/functional/apps/discover/group4/_esql_view.ts +++ b/test/functional/apps/discover/group4/_esql_view.ts @@ -546,6 +546,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.click('TextBasedLangEditor-expand'); await dataGrid.clickCellFilterForButton(0, 3); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded(); + const editorValue = await monacoEditor.getCodeEditorValue(); expect(editorValue).to.eql( `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB\n| where \`geo.dest\`=="BT"` @@ -553,6 +557,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // negate await dataGrid.clickCellFilterOutButton(0, 3); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded(); + const newValue = await monacoEditor.getCodeEditorValue(); expect(newValue).to.eql( `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB\n| where \`geo.dest\`!="BT"` @@ -571,6 +579,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.click('TextBasedLangEditor-expand'); await dataGrid.clickCellFilterForButton(0, 3); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.unifiedFieldList.waitUntilSidebarHasLoaded(); + const editorValue = await monacoEditor.getCodeEditorValue(); expect(editorValue).to.eql( `from logstash-* | sort @timestamp desc | limit 10000 | stats countB = count(bytes) by geo.dest | sort countB | where countB > 0\nand \`geo.dest\`=="BT"` From a6ddb094a5490fe1f2255d6cb4961e0817ace9f5 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 1 May 2024 09:22:38 +0200 Subject: [PATCH 16/24] Address some of the comments --- .../src/utils/append_to_query.test.ts | 10 ++++++- .../src/utils/append_to_query.ts | 5 ++-- .../data_table_columns.test.tsx.snap | 19 ++++++++++-- .../components/data_table_columns.test.tsx | 8 +++++ .../components/layout/discover_layout.tsx | 29 +++++++++++-------- 5 files changed, 53 insertions(+), 18 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 e73427f341c9f..e074cb1a05630 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 @@ -60,7 +60,15 @@ describe('appendToQuery', () => { }); it('appends a where clause in an existing query checking that the value is not null if the user asks for existence', () => { - expect(appendWhereClauseToESQLQuery('from logstash-* // meow', '_exists_', 'dest', '+')).toBe( + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* // meow', + 'dest', + undefined, + '_exists_', + 'string' + ) + ).toBe( `from logstash-* // meow | where \`dest\` is not null` ); 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 52223979d86a6..b4b76105818f8 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -17,7 +17,7 @@ export function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, value: unknown, - operation: '+' | '-', + operation: '+' | '-' | '_exists_', fieldType?: string ): string { let operator = operation === '+' ? '==' : '!='; @@ -34,8 +34,7 @@ export function appendWhereClauseToESQLQuery( // checking that the value is not null // this is the existence filter - if (field === '_exists_') { - fieldName = `\`${String(value)}\``; + if (operation === '_exists_') { operator = ' is not null'; filterValue = ''; } diff --git a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap index b8f707ba27c73..3759462c21b7d 100644 --- a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap +++ b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap @@ -43,6 +43,8 @@ Array [ }, "cellActions": Array [ [Function], + [Function], + [Function], ], "display": test @@ -50,7 +52,7 @@ Array [ "displayAsText": "extension", "id": "extension", "isSortable": true, - "schema": "kibana-json", + "schema": "numeric", "visibleCellActions": undefined, }, Object { @@ -94,10 +96,23 @@ Array [ }, "cellActions": Array [ [Function], + [Function], + [Function], ], "display": servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, + columnsMeta: { + extension: { type: 'number' }, + message: { type: 'string', esType: 'keyword' }, + }, }); const extensionGridColumn = gridColumns[0]; @@ -428,6 +432,10 @@ describe('Data table columns', function () { servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, customGridColumnsConfiguration, + columnsMeta: { + extension: { type: 'number' }, + message: { type: 'string', esType: 'keyword' }, + }, }); expect(customizedGridColumns).toMatchSnapshot(); 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 2bff79a255c5b..6b761f919d212 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 @@ -164,21 +164,31 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const fieldName = typeof field === 'string' ? field : field.name; // send the field type for casting const fieldType = typeof field !== 'string' ? field.type : undefined; + // weird existence logic from Discover components + // in the field it comes the operator _exists_ and in the value the field + // I need to take care of it here but I think it should be handled on the fieldlist instead const updatedQuery = appendWhereClauseToESQLQuery( query.esql, - fieldName, - values, - operation, + fieldName === '_exists_' ? String(values) : fieldName, + fieldName === '_exists_' ? undefined : values, + fieldName === '_exists_' ? '_exists_' : operation, fieldType ); data.query.queryString.setQuery({ esql: updatedQuery, }); + if (trackUiMetric) { + trackUiMetric(METRIC_TYPE.CLICK, 'esql_filter_added'); + } } }, - [data, query] + [data.query.queryString, query, trackUiMetric] ); + const onFilter = isPlainRecord + ? (onPopulateWhereClause as DocViewFilterFn) + : (onAddFilter as DocViewFilterFn); + const onFieldEdited = useCallback( async ({ removedFieldName }: { removedFieldName?: string } = {}) => { if (removedFieldName && currentColumns.includes(removedFieldName)) { @@ -257,11 +267,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { stateContainer={stateContainer} columns={currentColumns} viewMode={viewMode} - onAddFilter={ - isPlainRecord - ? (onPopulateWhereClause as DocViewFilterFn) - : (onAddFilter as DocViewFilterFn) - } + onAddFilter={onFilter} onFieldEdited={onFieldEdited} container={mainContainer} onDropFieldToTable={onDropFieldToTable} @@ -277,8 +283,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { stateContainer, currentColumns, viewMode, - onPopulateWhereClause, - onAddFilter, + onFilter, onFieldEdited, mainContainer, onDropFieldToTable, @@ -357,7 +362,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { documents$={stateContainer.dataState.data$.documents$} onAddField={onAddColumn} columns={currentColumns} - onAddFilter={!isPlainRecord ? onAddFilter : onPopulateWhereClause} + onAddFilter={onFilter} onRemoveField={onRemoveColumn} onChangeDataView={stateContainer.actions.onChangeDataView} selectedDataView={dataView} From aca8198188b13a91297d60cd97652170ed728936 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 1 May 2024 10:20:14 +0200 Subject: [PATCH 17/24] Fix case bug --- .../src/utils/append_to_query.test.ts | 15 +++++++++ .../src/utils/append_to_query.ts | 31 +++++++++++++++++++ 2 files changed, 46 insertions(+) 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 e074cb1a05630..a7f94db92e876 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 @@ -124,5 +124,20 @@ and \`dest\`=="Crete"` ) ).toBe(`from logstash-* | where \`country\`!= "GR"`); }); + + it('appends an and clause in an existing query with where command as the last pipe if the filter preexists but the operator is not the correct one', () => { + expect( + appendWhereClauseToESQLQuery( + `from logstash-* | where CIDR_MATCH(ip1, "127.0.0.2/32", "127.0.0.3/32")`, + 'ip', + '127.0.0.2/32', + '-', + 'ip' + ) + ).toBe( + `from logstash-* | where CIDR_MATCH(ip1, "127.0.0.2/32", "127.0.0.3/32") +and \`ip\`::string!="127.0.0.2/32"` + ); + }); }); }); 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 b4b76105818f8..b7c2730b6e8e5 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -13,6 +13,33 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): return `${baseESQLQuery}\n${appendedText}`; } +// function findFunction(id, currentNode) { +// let i; +// let currentChild; +// let result; + +// if (id == currentNode.id) { +// return currentNode; +// } else { +// // Use a for loop instead of forEach to avoid nested functions +// // Otherwise "return" will not work properly +// for (i = 0; i < currentNode.children.length; i += 1) { +// currentChild = currentNode.children[i]; + +// // Search in the current child +// result = findNode(id, currentChild); + +// // Return the result if the node has been found +// if (result !== false) { +// return result; +// } +// } + +// // The node has not been found and we have no more options +// return false; +// } +// } + export function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, @@ -46,6 +73,7 @@ export function appendWhereClauseToESQLQuery( // - we need to append with and if the filter doesnt't exist // - we need to change the filter operator if the filter exists with different operator // - we do nothing if the filter exists with the same operator + console.dir(ast[ast.length - 1]); if (lastCommandIsWhere) { const whereCommand = ast[ast.length - 1]; const whereAstText = whereCommand.text; @@ -56,6 +84,9 @@ export function appendWhereClauseToESQLQuery( const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); if (matches) { const existingOperator = matches[1]?.trim().replace('`', ''); + if (!['==', '!='].includes(existingOperator.trim())) { + return appendToESQLQuery(baseESQLQuery, `and ${fieldName}${operator}${filterValue}`); + } // the filter is the same if (existingOperator === operator) { return baseESQLQuery; From 8c143964924051480ae366d37733fddbedfaf1b3 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 1 May 2024 12:05:13 +0200 Subject: [PATCH 18/24] Use recursive function --- .../src/utils/append_to_query.test.ts | 2 +- .../src/utils/append_to_query.ts | 96 ++++++++++++------- 2 files changed, 63 insertions(+), 35 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 a7f94db92e876..eef5d00750697 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 @@ -122,7 +122,7 @@ and \`dest\`=="Crete"` '-', 'string' ) - ).toBe(`from logstash-* | where \`country\`!= "GR"`); + ).toBe(`from logstash-* | where \`country\` != "GR"`); }); it('appends an and clause in an existing query with where command as the last pipe if the filter preexists but the operator is not the correct one', () => { 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 b7c2730b6e8e5..7468ac604f142 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -5,7 +5,19 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; +import { + type ESQLFunction, + type ESQLSingleAstItem, + type ESQLColumn, + type ESQLLiteral, + getAstAndSyntaxErrors, +} from '@kbn/esql-ast'; + +interface FilterArguments { + fieldName: string; + value: string; + operator: string; +} // Append in a new line the appended text to take care of the case where the user adds a comment at the end of the query // in these cases a base query such as "from index // comment" will result in errors or wrong data if we don't append in a new line @@ -13,32 +25,42 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): return `${baseESQLQuery}\n${appendedText}`; } -// function findFunction(id, currentNode) { -// let i; -// let currentChild; -// let result; - -// if (id == currentNode.id) { -// return currentNode; -// } else { -// // Use a for loop instead of forEach to avoid nested functions -// // Otherwise "return" will not work properly -// for (i = 0; i < currentNode.children.length; i += 1) { -// currentChild = currentNode.children[i]; - -// // Search in the current child -// result = findNode(id, currentChild); - -// // Return the result if the node has been found -// if (result !== false) { -// return result; -// } -// } - -// // The node has not been found and we have no more options -// return false; -// } -// } +function findInAst( + astItem: ESQLFunction, + fieldName: string, + value: string +): FilterArguments | undefined { + const singleAstItem = astItem as ESQLFunction; + const args = singleAstItem.args; + const isLastNode = args?.some((a) => 'type' in a && a.type === 'column'); + if (isLastNode) { + const column = args.find((a) => { + const argument = a as ESQLSingleAstItem; + return argument.type === 'column'; + }) as ESQLColumn; + const literal = args.find((a) => { + const argument = a as ESQLSingleAstItem; + return argument.type === 'literal'; + }) as ESQLLiteral; + if (column?.name === fieldName && literal?.name.includes(value)) { + return { + fieldName, + value, + operator: astItem.name, + }; + } else { + return undefined; + } + } else { + const functions = args?.filter((item) => 'type' in item && item.type === 'function'); + for (const functionAstItem of functions) { + const item = findInAst(functionAstItem as ESQLFunction, fieldName, value); + if (item) { + return item; + } + } + } +} export function appendWhereClauseToESQLQuery( baseESQLQuery: string, @@ -62,6 +84,7 @@ export function appendWhereClauseToESQLQuery( // checking that the value is not null // this is the existence filter if (operation === '_exists_') { + fieldName = `\`${String(field)}\``; operator = ' is not null'; filterValue = ''; } @@ -73,7 +96,6 @@ export function appendWhereClauseToESQLQuery( // - we need to append with and if the filter doesnt't exist // - we need to change the filter operator if the filter exists with different operator // - we do nothing if the filter exists with the same operator - console.dir(ast[ast.length - 1]); if (lastCommandIsWhere) { const whereCommand = ast[ast.length - 1]; const whereAstText = whereCommand.text; @@ -81,9 +103,12 @@ export function appendWhereClauseToESQLQuery( if (whereAstText.includes(field) && whereAstText.includes(String(filterValue))) { const pipesArray = baseESQLQuery.split('|'); const whereClause = pipesArray[pipesArray.length - 1]; - const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); - if (matches) { - const existingOperator = matches[1]?.trim().replace('`', ''); + + const args = ast[ast.length - 1].args[0] as ESQLFunction; + const astItem = findInAst(args, field, String(value)); + + if (astItem) { + const existingOperator = astItem.operator; if (!['==', '!='].includes(existingOperator.trim())) { return appendToESQLQuery(baseESQLQuery, `and ${fieldName}${operator}${filterValue}`); } @@ -92,9 +117,12 @@ export function appendWhereClauseToESQLQuery( return baseESQLQuery; // the filter has different operator } else { - const existingFilter = matches[0].trim(); - const newFilter = existingFilter.replace(existingOperator, operator); - return baseESQLQuery.replace(existingFilter, newFilter); + const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); + if (matches) { + const existingFilter = matches[0].trim(); + const newFilter = existingFilter.replace(existingOperator, operator); + return baseESQLQuery.replace(existingFilter, newFilter); + } } } } From ebad48282f809331f696aa2db559401f1a4773f1 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 2 May 2024 08:26:46 +0200 Subject: [PATCH 19/24] Revert implememtation and rely on regex --- .../src/utils/append_to_query.test.ts | 2 +- .../src/utils/append_to_query.ts | 68 ++----------------- 2 files changed, 8 insertions(+), 62 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 eef5d00750697..a7f94db92e876 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 @@ -122,7 +122,7 @@ and \`dest\`=="Crete"` '-', 'string' ) - ).toBe(`from logstash-* | where \`country\` != "GR"`); + ).toBe(`from logstash-* | where \`country\`!= "GR"`); }); it('appends an and clause in an existing query with where command as the last pipe if the filter preexists but the operator is not the correct one', () => { 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 7468ac604f142..8250d058ebd0a 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -5,19 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { - type ESQLFunction, - type ESQLSingleAstItem, - type ESQLColumn, - type ESQLLiteral, - getAstAndSyntaxErrors, -} from '@kbn/esql-ast'; - -interface FilterArguments { - fieldName: string; - value: string; - operator: string; -} +import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; // Append in a new line the appended text to take care of the case where the user adds a comment at the end of the query // in these cases a base query such as "from index // comment" will result in errors or wrong data if we don't append in a new line @@ -25,43 +13,6 @@ export function appendToESQLQuery(baseESQLQuery: string, appendedText: string): return `${baseESQLQuery}\n${appendedText}`; } -function findInAst( - astItem: ESQLFunction, - fieldName: string, - value: string -): FilterArguments | undefined { - const singleAstItem = astItem as ESQLFunction; - const args = singleAstItem.args; - const isLastNode = args?.some((a) => 'type' in a && a.type === 'column'); - if (isLastNode) { - const column = args.find((a) => { - const argument = a as ESQLSingleAstItem; - return argument.type === 'column'; - }) as ESQLColumn; - const literal = args.find((a) => { - const argument = a as ESQLSingleAstItem; - return argument.type === 'literal'; - }) as ESQLLiteral; - if (column?.name === fieldName && literal?.name.includes(value)) { - return { - fieldName, - value, - operator: astItem.name, - }; - } else { - return undefined; - } - } else { - const functions = args?.filter((item) => 'type' in item && item.type === 'function'); - for (const functionAstItem of functions) { - const item = findInAst(functionAstItem as ESQLFunction, fieldName, value); - if (item) { - return item; - } - } - } -} - export function appendWhereClauseToESQLQuery( baseESQLQuery: string, field: string, @@ -104,11 +55,9 @@ export function appendWhereClauseToESQLQuery( const pipesArray = baseESQLQuery.split('|'); const whereClause = pipesArray[pipesArray.length - 1]; - const args = ast[ast.length - 1].args[0] as ESQLFunction; - const astItem = findInAst(args, field, String(value)); - - if (astItem) { - const existingOperator = astItem.operator; + const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); + if (matches) { + const existingOperator = matches[1]?.trim().replace('`', ''); if (!['==', '!='].includes(existingOperator.trim())) { return appendToESQLQuery(baseESQLQuery, `and ${fieldName}${operator}${filterValue}`); } @@ -117,12 +66,9 @@ export function appendWhereClauseToESQLQuery( return baseESQLQuery; // the filter has different operator } else { - const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); - if (matches) { - const existingFilter = matches[0].trim(); - const newFilter = existingFilter.replace(existingOperator, operator); - return baseESQLQuery.replace(existingFilter, newFilter); - } + const existingFilter = matches[0].trim(); + const newFilter = existingFilter.replace(existingOperator, operator); + return baseESQLQuery.replace(existingFilter, newFilter); } } } From 193e6708d46b590b23b42073188ab1bbddca1e49 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Thu, 2 May 2024 08:35:50 +0200 Subject: [PATCH 20/24] Do not append if filter is not null already exists --- .../src/utils/append_to_query.test.ts | 12 ++++++++++++ .../src/utils/append_to_query.ts | 19 ++++++++++++++----- 2 files changed, 26 insertions(+), 5 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 a7f94db92e876..cb9465cff05b6 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 @@ -101,6 +101,18 @@ and \`dest\`=="Crete"` ).toBe(`from logstash-* | where country == "GR"`); }); + it('doesnt append anything in an existing query with where command as the last pipe if the _exists_ filter preexists', () => { + expect( + appendWhereClauseToESQLQuery( + 'from logstash-* | where country IS NOT NULL', + 'country', + undefined, + '_exists_', + 'string' + ) + ).toBe(`from logstash-* | where country IS NOT NULL`); + }); + it('changes the operator in an existing query with where command as the last pipe if the filter preexists but has the opposite operator', () => { expect( appendWhereClauseToESQLQuery( 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 8250d058ebd0a..d8f6ed3a44073 100644 --- a/packages/kbn-esql-utils/src/utils/append_to_query.ts +++ b/packages/kbn-esql-utils/src/utils/append_to_query.ts @@ -20,7 +20,17 @@ export function appendWhereClauseToESQLQuery( operation: '+' | '-' | '_exists_', fieldType?: string ): string { - let operator = operation === '+' ? '==' : '!='; + let operator; + switch (operation) { + case '_exists_': + operator = ' is not null'; + break; + case '-': + operator = '!='; + break; + default: + operator = '=='; + } let filterValue = typeof value === 'string' ? `"${value.replace(/"/g, '\\"')}"` : value; // Adding the backticks here are they are needed for special char fields let fieldName = `\`${field}\``; @@ -36,7 +46,6 @@ export function appendWhereClauseToESQLQuery( // this is the existence filter if (operation === '_exists_') { fieldName = `\`${String(field)}\``; - operator = ' is not null'; filterValue = ''; } @@ -57,12 +66,12 @@ export function appendWhereClauseToESQLQuery( const matches = whereClause.match(new RegExp(field + '(.*)' + String(filterValue))); if (matches) { - const existingOperator = matches[1]?.trim().replace('`', ''); - if (!['==', '!='].includes(existingOperator.trim())) { + const existingOperator = matches[1]?.trim().replace('`', '').toLowerCase(); + if (!['==', '!=', 'is not null'].includes(existingOperator.trim())) { return appendToESQLQuery(baseESQLQuery, `and ${fieldName}${operator}${filterValue}`); } // the filter is the same - if (existingOperator === operator) { + if (existingOperator === operator.trim()) { return baseESQLQuery; // the filter has different operator } else { From fff036df37c55946dab4a0577c977e8dc2b8a5a6 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 6 May 2024 11:23:20 +0200 Subject: [PATCH 21/24] Correct existing wrong Discover tests --- .../data_table_columns.test.tsx.snap | 48 +++++++++++++++++-- .../components/data_table_columns.test.tsx | 5 ++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap index 3759462c21b7d..09b51e02f4db9 100644 --- a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap +++ b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap @@ -2626,6 +2626,8 @@ Array [ }, "cellActions": Array [ [Function], + [Function], + [Function], ], "display": servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, + columnsMeta: { + extension: { type: 'number' }, + message: { type: 'string', esType: 'keyword' }, + timestamp: { type: 'date', esType: 'dateTime' }, + }, }); expect(actual).toMatchSnapshot(); }); From adc9d6eb8141685a23d9d9d35f65d092dc91ee4e Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 6 May 2024 11:37:34 +0200 Subject: [PATCH 22/24] Pass the correct type for the extension in the test --- .../data_table_columns.test.tsx.snap | 16 ++++++++-------- .../src/components/data_table_columns.test.tsx | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap index 09b51e02f4db9..f7c30484bfc52 100644 --- a/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap +++ b/packages/kbn-unified-data-table/src/components/__snapshots__/data_table_columns.test.tsx.snap @@ -52,7 +52,7 @@ Array [ "displayAsText": "extension", "id": "extension", "isSortable": true, - "schema": "numeric", + "schema": "string", "visibleCellActions": undefined, }, Object { @@ -105,7 +105,7 @@ Array [ columnsMeta={ Object { "extension": Object { - "type": "number", + "type": "string", }, "message": Object { "esType": "keyword", @@ -1136,7 +1136,7 @@ Array [ columnsMeta={ Object { "extension": Object { - "type": "number", + "type": "string", }, "message": Object { "esType": "keyword", @@ -1366,7 +1366,7 @@ Array [ "displayAsText": "extension", "id": "extension", "isSortable": false, - "schema": "numeric", + "schema": "string", "visibleCellActions": undefined, }, Object { @@ -1417,7 +1417,7 @@ Array [ columnsMeta={ Object { "extension": Object { - "type": "number", + "type": "string", }, "message": Object { "esType": "keyword", @@ -2857,7 +2857,7 @@ Array [ columnsMeta={ Object { "extension": Object { - "type": "number", + "type": "string", }, "message": Object { "esType": "keyword", @@ -3021,7 +3021,7 @@ Array [ "displayAsText": "extension", "id": "extension", "isSortable": true, - "schema": "numeric", + "schema": "string", "visibleCellActions": undefined, }, Object { @@ -3074,7 +3074,7 @@ Array [ columnsMeta={ Object { "extension": Object { - "type": "number", + "type": "string", }, "message": Object { "esType": "keyword", diff --git a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx index 070eb2db92576..e17d7d437cded 100644 --- a/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx +++ b/packages/kbn-unified-data-table/src/components/data_table_columns.test.tsx @@ -94,7 +94,7 @@ describe('Data table columns', function () { servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, columnsMeta: { - extension: { type: 'number' }, + extension: { type: 'string' }, message: { type: 'string', esType: 'keyword' }, timestamp: { type: 'date', esType: 'dateTime' }, }, @@ -304,7 +304,7 @@ describe('Data table columns', function () { const actual = getEuiGridColumns({ showColumnTokens: true, columnsMeta: { - extension: { type: 'number' }, + extension: { type: 'string' }, message: { type: 'string', esType: 'keyword' }, }, columns, @@ -408,7 +408,7 @@ describe('Data table columns', function () { servicesMock.dataViewFieldEditor.userPermissions.editIndexPattern(), onFilter: () => {}, columnsMeta: { - extension: { type: 'number' }, + extension: { type: 'string' }, message: { type: 'string', esType: 'keyword' }, }, }); @@ -438,7 +438,7 @@ describe('Data table columns', function () { onFilter: () => {}, customGridColumnsConfiguration, columnsMeta: { - extension: { type: 'number' }, + extension: { type: 'string' }, message: { type: 'string', esType: 'keyword' }, }, }); From 0d16316fbcaac4531798b760ec9174fe8e957dc0 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 6 May 2024 11:44:50 +0200 Subject: [PATCH 23/24] Simoplification --- .../src/components/default_cell_actions.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 35f5ec58fe38a..109d289eea84a 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 @@ -26,10 +26,8 @@ function onFilterCell( const row = context.rows[rowIndex]; const value = row.flattened[columnId]; - const filterField = context.isPlainRecord ? field : context.dataView.fields.getByName(columnId); - - if (filterField && context.onFilter) { - context.onFilter(filterField, value, mode); + if (field && context.onFilter) { + context.onFilter(field, value, mode); } } From 8bc656ebc35a4eabda1184270be929c8f3402943 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 6 May 2024 13:36:28 +0200 Subject: [PATCH 24/24] Update jest test --- .../components/default_cell_actions.test.tsx | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) 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 613c5f644c273..a57c805367ef5 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 @@ -95,11 +95,7 @@ describe('Default cell actions ', function () { ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( - dataTableContextMock.dataView.fields.getByName('extension'), - 'jpg', - '+' - ); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, 'jpg', '+'); }); it('triggers filter function when FilterInBtn is clicked for a non-provided value', async () => { const component = mountWithIntl( @@ -115,11 +111,7 @@ describe('Default cell actions ', function () { ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( - dataTableContextMock.dataView.fields.getByName('extension'), - undefined, - '+' - ); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, undefined, '+'); }); it('triggers filter function when FilterInBtn is clicked for an empty string value', async () => { const component = mountWithIntl( @@ -135,11 +127,7 @@ describe('Default cell actions ', function () { ); const button = findTestSubject(component, 'filterForButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( - dataTableContextMock.dataView.fields.getByName('message'), - '', - '+' - ); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, '', '+'); }); it('triggers filter function when FilterOutBtn is clicked', async () => { const component = mountWithIntl( @@ -155,11 +143,7 @@ describe('Default cell actions ', function () { ); const button = findTestSubject(component, 'filterOutButton'); await button.simulate('click'); - expect(dataTableContextMock.onFilter).toHaveBeenCalledWith( - dataTableContextMock.dataView.fields.getByName('extension'), - 'jpg', - '-' - ); + expect(dataTableContextMock.onFilter).toHaveBeenCalledWith({}, 'jpg', '-'); }); it('triggers clipboard copy when CopyBtn is clicked', async () => { const component = mountWithIntl(