From 7c79925bb3f25ddb25de933868250ac5ce45e050 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 23 Sep 2024 10:10:41 +0200 Subject: [PATCH 01/14] [ES|QL][Discover] Enables the breakdown in the histogram --- .../public/container/hooks/use_state_props.ts | 16 +++++++-- .../public/services/lens_vis_service.ts | 35 +++++++++++++++---- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts index bb0e4acc81740..cc07bbd345adb 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts @@ -9,6 +9,7 @@ import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/common'; import { AggregateQuery, isOfAggregateQueryType, Query } from '@kbn/es-query'; +import { hasTransformationalCommand } from '@kbn/esql-utils'; import type { RequestAdapter } from '@kbn/inspector-plugin/public'; import { useCallback, useEffect, useMemo } from 'react'; import { @@ -86,14 +87,25 @@ export const useStateProps = ({ }, [chartHidden, isPlainRecord, isTimeBased, timeInterval]); const breakdown = useMemo(() => { - if (isPlainRecord || !isTimeBased) { + if (!isTimeBased) { return undefined; } + // hide the breakdown field selector when the ES|QL query has a transformational command (STATS, KEEP etc) + if (query && isOfAggregateQueryType(query) && hasTransformationalCommand(query.esql)) { + return undefined; + } + + // if (isPlainRecord) { + // return { + // field: breakdownField ?? , + // }; + // } + return { field: breakdownField ? dataView?.getFieldByName(breakdownField) : undefined, }; - }, [breakdownField, dataView, isPlainRecord, isTimeBased]); + }, [breakdownField, dataView, query, isTimeBased]); const request = useMemo(() => { return { diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index caadc3506ccbe..fbb1e300da195 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -245,7 +245,10 @@ export class LensVisService { if (queryParams.isPlainRecord) { // appends an ES|QL histogram - const histogramSuggestionForESQL = this.getHistogramSuggestionForESQL({ queryParams }); + const histogramSuggestionForESQL = this.getHistogramSuggestionForESQL({ + queryParams, + breakdownField, + }); if (histogramSuggestionForESQL) { availableSuggestionsWithType.push({ suggestion: histogramSuggestionForESQL, @@ -452,16 +455,25 @@ export class LensVisService { private getHistogramSuggestionForESQL = ({ queryParams, + breakdownField, }: { queryParams: QueryParams; + breakdownField?: DataViewField; }): Suggestion | undefined => { - const { dataView, query, timeRange } = queryParams; + const { dataView, query, timeRange, columns } = queryParams; + const breakdownColumn = columns?.find((column) => column.name === breakdownField?.name); if (dataView.isTimeBased() && query && isOfAggregateQueryType(query) && timeRange) { const isOnHistogramMode = shouldDisplayHistogram(query); if (!isOnHistogramMode) return undefined; const interval = computeInterval(timeRange, this.services.data); - const esqlQuery = this.getESQLHistogramQuery({ dataView, query, timeRange, interval }); + const esqlQuery = this.getESQLHistogramQuery({ + dataView, + query, + timeRange, + interval, + breakdownColumn, + }); const context = { dataViewSpec: dataView?.toSpec(), fieldName: '', @@ -485,6 +497,10 @@ export class LensVisService { esql: esqlQuery, }, }; + + if (breakdownColumn) { + context.textBasedColumns.push(breakdownColumn); + } const suggestions = this.lensSuggestionsApi(context, dataView, ['lnsDatatable']) ?? []; if (suggestions.length) { return suggestions[0]; @@ -499,18 +515,21 @@ export class LensVisService { timeRange, query, interval, + breakdownColumn, }: { dataView: DataView; timeRange: TimeRange; query: AggregateQuery; interval?: string; + breakdownColumn?: DatatableColumn; }): string => { const queryInterval = interval ?? computeInterval(timeRange, this.services.data); const language = getAggregateQueryMode(query); const safeQuery = removeDropCommandsFromESQLQuery(query[language]); + const breakdown = breakdownColumn ? `, ${breakdownColumn.name}` : ''; return appendToESQLQuery( safeQuery, - `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\`` + `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp ${breakdown} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\`` ); }; @@ -548,7 +567,7 @@ export class LensVisService { externalVisContextStatus: UnifiedHistogramExternalVisContextStatus; visContext: UnifiedHistogramVisContext | undefined; } => { - const { dataView, query, filters, timeRange } = queryParams; + const { dataView, query, filters, timeRange, columns } = queryParams; const { type: suggestionType, suggestion } = currentSuggestionContext; if (!suggestion || !suggestion.datasourceId || !query || !filters) { @@ -563,13 +582,15 @@ export class LensVisService { dataViewId: dataView.id, timeField: dataView.timeFieldName, timeInterval: isTextBased ? undefined : timeInterval, - breakdownField: isTextBased ? undefined : breakdownField?.name, + breakdownField: breakdownField?.name, }; + const breakdownColumn = columns?.find((column) => column.name === breakdownField?.name); + const currentQuery = suggestionType === UnifiedHistogramSuggestionType.histogramForESQL && isTextBased && timeRange ? { - esql: this.getESQLHistogramQuery({ dataView, query, timeRange }), + esql: this.getESQLHistogramQuery({ dataView, query, timeRange, breakdownColumn }), } : query; From ef0e4e62a51318ea711f7e9162428b96e71535cb Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Mon, 23 Sep 2024 11:39:45 +0200 Subject: [PATCH 02/14] Get fields from the query --- .../public/chart/breakdown_field_selector.tsx | 56 +++++++++++++++---- .../unified_histogram/public/chart/chart.tsx | 9 ++- .../public/container/container.tsx | 1 + .../container/hooks/use_state_props.test.ts | 8 +++ .../public/container/hooks/use_state_props.ts | 20 +++++-- .../public/layout/layout.tsx | 3 +- 6 files changed, 77 insertions(+), 20 deletions(-) diff --git a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx index 7d29827a1389b..9c8c82dcf34b9 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx @@ -11,7 +11,9 @@ import React, { useCallback, useMemo } from 'react'; import { EuiSelectableOption } from '@elastic/eui'; import { FieldIcon, getFieldIconProps, comboBoxFieldOptionMatcher } from '@kbn/field-utils'; import { css } from '@emotion/react'; -import type { DataView, DataViewField } from '@kbn/data-views-plugin/common'; +import { type DataView, DataViewField } from '@kbn/data-views-plugin/common'; +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; +import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils'; import { i18n } from '@kbn/i18n'; import { UnifiedHistogramBreakdownContext } from '../types'; import { fieldSupportsBreakdown } from '../utils/field_supports_breakdown'; @@ -25,22 +27,44 @@ import { export interface BreakdownFieldSelectorProps { dataView: DataView; breakdown: UnifiedHistogramBreakdownContext; + esqlColumns?: DatatableColumn[]; onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void; } +const getDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => { + if (esqlColumns) { + return esqlColumns.map((column) => ({ + key: column.id, + value: column.id, + label: column.name, + name: column.name, + type: column.meta?.type, + })); + } + + return dataView.fields.filter(fieldSupportsBreakdown).map((field) => ({ + key: field.name, + value: field.name, + label: field.displayName, + name: field.name, + type: field.type, + })); +}; + export const BreakdownFieldSelector = ({ dataView, breakdown, + esqlColumns, onBreakdownFieldChange, }: BreakdownFieldSelectorProps) => { + const fields = useMemo(() => getDropdownFields(dataView, esqlColumns), [dataView, esqlColumns]); const fieldOptions: SelectableEntry[] = useMemo(() => { - const options: SelectableEntry[] = dataView.fields - .filter(fieldSupportsBreakdown) + const options: SelectableEntry[] = fields .map((field) => ({ - key: field.name, + key: field.key, name: field.name, - label: field.displayName, - value: field.name, + label: field.label, + value: field.value, checked: breakdown?.field?.name === field.name ? ('on' as EuiSelectableOption['checked']) @@ -69,16 +93,24 @@ export const BreakdownFieldSelector = ({ }); return options; - }, [dataView, breakdown.field]); + }, [fields, breakdown?.field]); const onChange = useCallback>( (chosenOption) => { - const field = chosenOption?.value - ? dataView.fields.find((currentField) => currentField.name === chosenOption.value) - : undefined; - onBreakdownFieldChange?.(field); + let breakdownField: DataViewField | undefined; + if (esqlColumns) { + const breakdownColumn = esqlColumns?.find((column) => column.name === chosenOption?.value); + breakdownField = breakdownColumn + ? new DataViewField(convertDatatableColumnToDataViewFieldSpec(breakdownColumn)) + : undefined; + } else { + breakdownField = chosenOption?.value + ? dataView.fields.find((currentField) => currentField.name === chosenOption.value) + : undefined; + } + onBreakdownFieldChange?.(breakdownField); }, - [dataView.fields, onBreakdownFieldChange] + [dataView.fields, esqlColumns, onBreakdownFieldChange] ); return ( diff --git a/src/plugins/unified_histogram/public/chart/chart.tsx b/src/plugins/unified_histogram/public/chart/chart.tsx index 4204658273447..ed433d100f55c 100644 --- a/src/plugins/unified_histogram/public/chart/chart.tsx +++ b/src/plugins/unified_histogram/public/chart/chart.tsx @@ -19,6 +19,7 @@ import type { LensEmbeddableInput, LensEmbeddableOutput, } from '@kbn/lens-plugin/public'; +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import type { TimeRange } from '@kbn/es-query'; import { Histogram } from './histogram'; @@ -73,12 +74,16 @@ export interface ChartProps { isChartLoading?: boolean; onChartHiddenChange?: (chartHidden: boolean) => void; onTimeIntervalChange?: (timeInterval: string) => void; - onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void; + onBreakdownFieldChange?: ( + breakdownField: DataViewField | undefined, + columns?: DatatableColumn[] + ) => void; onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void; onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void; onFilter?: LensEmbeddableInput['onFilter']; onBrushEnd?: LensEmbeddableInput['onBrushEnd']; withDefaultActions: EmbeddableComponentProps['withDefaultActions']; + columns?: DatatableColumn[]; } const HistogramMemoized = memo(Histogram); @@ -114,6 +119,7 @@ export function Chart({ onBrushEnd, withDefaultActions, abortController, + columns, }: ChartProps) { const lensVisServiceCurrentSuggestionContext = useObservable( lensVisService.currentSuggestionContext$ @@ -312,6 +318,7 @@ export function Chart({ dataView={dataView} breakdown={breakdown} onBreakdownFieldChange={onBreakdownFieldChange} + esqlColumns={isPlainRecord ? columns : undefined} /> )} diff --git a/src/plugins/unified_histogram/public/container/container.tsx b/src/plugins/unified_histogram/public/container/container.tsx index a4231529a629b..15367ae51d9b5 100644 --- a/src/plugins/unified_histogram/public/container/container.tsx +++ b/src/plugins/unified_histogram/public/container/container.tsx @@ -147,6 +147,7 @@ export const UnifiedHistogramContainer = forwardRef< query, searchSessionId, requestAdapter, + columns: containerProps.columns, }); const handleVisContextChange: UnifiedHistogramLayoutProps['onVisContextChanged'] | undefined = diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts index e109b5339e728..939f2d3c9f5c9 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts @@ -60,6 +60,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -150,6 +151,7 @@ describe('useStateProps', () => { query: { esql: 'FROM index' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -236,6 +238,7 @@ describe('useStateProps', () => { query: { esql: 'FROM index' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); expect(result.current.chart).toStrictEqual({ hidden: false, timeInterval: 'auto' }); @@ -255,6 +258,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -333,6 +337,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -411,6 +416,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); const { @@ -470,6 +476,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }) ); (stateService.setLensRequestAdapter as jest.Mock).mockClear(); @@ -489,6 +496,7 @@ describe('useStateProps', () => { query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', + columns: undefined, }; const hook = renderHook((props: Parameters[0]) => useStateProps(props), { initialProps, diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts index cc07bbd345adb..fcc19fcd78a00 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts @@ -11,6 +11,8 @@ import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/co import { AggregateQuery, isOfAggregateQueryType, Query } from '@kbn/es-query'; import { hasTransformationalCommand } from '@kbn/esql-utils'; import type { RequestAdapter } from '@kbn/inspector-plugin/public'; +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; +import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils'; import { useCallback, useEffect, useMemo } from 'react'; import { UnifiedHistogramChartLoadEvent, @@ -35,12 +37,14 @@ export const useStateProps = ({ query, searchSessionId, requestAdapter, + columns, }: { stateService: UnifiedHistogramStateService | undefined; dataView: DataView; query: Query | AggregateQuery | undefined; searchSessionId: string | undefined; requestAdapter: RequestAdapter | undefined; + columns: DatatableColumn[] | undefined; }) => { const breakdownField = useStateSelector(stateService?.state$, breakdownFieldSelector); const chartHidden = useStateSelector(stateService?.state$, chartHiddenSelector); @@ -96,16 +100,20 @@ export const useStateProps = ({ return undefined; } - // if (isPlainRecord) { - // return { - // field: breakdownField ?? , - // }; - // } + if (isPlainRecord) { + const breakdownColumn = columns?.find((column) => column.name === breakdownField); + const field = breakdownColumn + ? new DataViewField(convertDatatableColumnToDataViewFieldSpec(breakdownColumn)) + : undefined; + return { + field, + }; + } return { field: breakdownField ? dataView?.getFieldByName(breakdownField) : undefined, }; - }, [breakdownField, dataView, query, isTimeBased]); + }, [isTimeBased, query, isPlainRecord, breakdownField, dataView, columns]); const request = useMemo(() => { return { diff --git a/src/plugins/unified_histogram/public/layout/layout.tsx b/src/plugins/unified_histogram/public/layout/layout.tsx index aac1cfe308c60..60438bfd4cc77 100644 --- a/src/plugins/unified_histogram/public/layout/layout.tsx +++ b/src/plugins/unified_histogram/public/layout/layout.tsx @@ -14,7 +14,7 @@ import useObservable from 'react-use/lib/useObservable'; import { createHtmlPortalNode, InPortal, OutPortal } from 'react-reverse-portal'; import { css } from '@emotion/css'; import type { Datatable, DatatableColumn } from '@kbn/expressions-plugin/common'; -import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; +import { type DataView, DataViewField } from '@kbn/data-views-plugin/public'; import type { EmbeddableComponentProps, LensEmbeddableInput, @@ -374,6 +374,7 @@ export const UnifiedHistogramLayout = ({ lensAdapters={lensAdapters} lensEmbeddableOutput$={lensEmbeddableOutput$} withDefaultActions={withDefaultActions} + columns={columns} /> From 6ec8eb8c1c8343402aeb523b220bacfb4c7f6d93 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 24 Sep 2024 09:03:10 +0200 Subject: [PATCH 03/14] Adjust for numeric fields --- .../public/services/lens_vis_service.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index fbb1e300da195..0058b14669625 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -503,7 +503,32 @@ export class LensVisService { } const suggestions = this.lensSuggestionsApi(context, dataView, ['lnsDatatable']) ?? []; if (suggestions.length) { - return suggestions[0]; + const suggestion = suggestions[0]; + const suggestionVisualizationState = Object.assign({}, suggestion?.visualizationState); + // the suggestions api will suggest a numeric column as a metric and not as a breakdown, + // so we need to adjust it here + if ( + breakdownColumn && + breakdownColumn.meta?.type === 'number' && + suggestion && + 'layers' in suggestionVisualizationState && + Array.isArray(suggestionVisualizationState.layers) + ) { + return { + ...suggestion, + visualizationState: { + ...(suggestionVisualizationState ?? {}), + layers: suggestionVisualizationState.layers.map((layer) => { + return { + ...layer, + accessors: ['results'], + splitAccessor: breakdownColumn.name, + }; + }), + }, + }; + } + return suggestion; } } From 906cc8993d4ac9ff82006671f2d74e84d1a2bdce Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 24 Sep 2024 11:29:35 +0200 Subject: [PATCH 04/14] Add some unit tests --- .../chart/breakdown_field_selector.test.tsx | 109 +++++++++++++++++- .../public/chart/breakdown_field_selector.tsx | 22 ++-- .../container/hooks/use_state_props.test.ts | 90 ++++++++++++++- 3 files changed, 201 insertions(+), 20 deletions(-) diff --git a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.test.tsx b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.test.tsx index 5b39fd0482bb4..4342c00c98854 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.test.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.test.tsx @@ -9,12 +9,15 @@ import { render, act, screen } from '@testing-library/react'; import React from 'react'; +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; +import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils'; +import { DataViewField } from '@kbn/data-views-plugin/common'; import { UnifiedHistogramBreakdownContext } from '../types'; import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; import { BreakdownFieldSelector } from './breakdown_field_selector'; describe('BreakdownFieldSelector', () => { - it('should render correctly', () => { + it('should render correctly for dataview fields', () => { const onBreakdownFieldChange = jest.fn(); const breakdown: UnifiedHistogramBreakdownContext = { field: undefined, @@ -63,6 +66,67 @@ describe('BreakdownFieldSelector', () => { `); }); + it('should render correctly for ES|QL columns', () => { + const onBreakdownFieldChange = jest.fn(); + const breakdown: UnifiedHistogramBreakdownContext = { + field: undefined, + }; + + render( + + ); + + const button = screen.getByTestId('unifiedHistogramBreakdownSelectorButton'); + expect(button.getAttribute('data-selected-value')).toBe(null); + + act(() => { + button.click(); + }); + + const options = screen.getAllByRole('option'); + expect( + options.map((option) => ({ + label: option.getAttribute('title'), + value: option.getAttribute('value'), + checked: option.getAttribute('aria-checked'), + })) + ).toMatchInlineSnapshot(` + Array [ + Object { + "checked": "true", + "label": "No breakdown", + "value": "__EMPTY_SELECTOR_OPTION__", + }, + Object { + "checked": "false", + "label": "bytes", + "value": "bytes", + }, + Object { + "checked": "false", + "label": "extension", + "value": "extension", + }, + ] + `); + }); + it('should mark the option as checked if breakdown.field is defined', () => { const onBreakdownFieldChange = jest.fn(); const field = dataViewWithTimefieldMock.fields.find((f) => f.name === 'extension')!; @@ -111,7 +175,7 @@ describe('BreakdownFieldSelector', () => { `); }); - it('should call onBreakdownFieldChange with the selected field when the user selects a field', () => { + it('should call onBreakdownFieldChange with the selected field when the user selects a dataview field', () => { const onBreakdownFieldChange = jest.fn(); const selectedField = dataViewWithTimefieldMock.fields.find((f) => f.name === 'bytes')!; const breakdown: UnifiedHistogramBreakdownContext = { @@ -135,4 +199,45 @@ describe('BreakdownFieldSelector', () => { expect(onBreakdownFieldChange).toHaveBeenCalledWith(selectedField); }); + + it('should call onBreakdownFieldChange with the selected field when the user selects an ES|QL field', () => { + const onBreakdownFieldChange = jest.fn(); + const esqlColumns = [ + { + name: 'bytes', + meta: { type: 'number' }, + id: 'bytes', + }, + { + name: 'extension', + meta: { type: 'string' }, + id: 'extension', + }, + ] as DatatableColumn[]; + const breakdownColumn = esqlColumns.find((c) => c.name === 'bytes')!; + const selectedField = new DataViewField( + convertDatatableColumnToDataViewFieldSpec(breakdownColumn) + ); + const breakdown: UnifiedHistogramBreakdownContext = { + field: undefined, + }; + render( + + ); + + act(() => { + screen.getByTestId('unifiedHistogramBreakdownSelectorButton').click(); + }); + + act(() => { + screen.getByTitle('bytes').click(); + }); + + expect(onBreakdownFieldChange).toHaveBeenCalledWith(selectedField); + }); }); diff --git a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx index 9c8c82dcf34b9..8ac828c1e8008 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx @@ -31,24 +31,16 @@ export interface BreakdownFieldSelectorProps { onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void; } -const getDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => { +const mapToDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => { if (esqlColumns) { return esqlColumns.map((column) => ({ - key: column.id, - value: column.id, - label: column.name, name: column.name, + displayName: column.name, type: column.meta?.type, })); } - return dataView.fields.filter(fieldSupportsBreakdown).map((field) => ({ - key: field.name, - value: field.name, - label: field.displayName, - name: field.name, - type: field.type, - })); + return dataView.fields.filter(fieldSupportsBreakdown); }; export const BreakdownFieldSelector = ({ @@ -57,14 +49,14 @@ export const BreakdownFieldSelector = ({ esqlColumns, onBreakdownFieldChange, }: BreakdownFieldSelectorProps) => { - const fields = useMemo(() => getDropdownFields(dataView, esqlColumns), [dataView, esqlColumns]); + const fields = useMemo(() => mapToDropdownFields(dataView, esqlColumns), [dataView, esqlColumns]); const fieldOptions: SelectableEntry[] = useMemo(() => { const options: SelectableEntry[] = fields .map((field) => ({ - key: field.key, + key: field.name, name: field.name, - label: field.label, - value: field.value, + label: field.displayName, + value: field.name, checked: breakdown?.field?.name === field.name ? ('on' as EuiSelectableOption['checked']) diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts index 939f2d3c9f5c9..44a36be34d1ab 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts @@ -11,6 +11,8 @@ import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/co import { RequestAdapter } from '@kbn/inspector-plugin/common'; import { renderHook } from '@testing-library/react-hooks'; import { act } from 'react-test-renderer'; +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; +import { convertDatatableColumnToDataViewFieldSpec } from '@kbn/data-view-utils'; import { UnifiedHistogramFetchStatus, UnifiedHistogramSuggestionContext } from '../../types'; import { dataViewMock } from '../../__mocks__/data_view'; import { dataViewWithTimefieldMock } from '../../__mocks__/data_view_with_timefield'; @@ -156,7 +158,9 @@ describe('useStateProps', () => { ); expect(result.current).toMatchInlineSnapshot(` Object { - "breakdown": undefined, + "breakdown": Object { + "field": undefined, + }, "chart": Object { "hidden": false, "timeInterval": "auto", @@ -222,9 +226,13 @@ describe('useStateProps', () => { }, } `); + + expect(result.current.chart).toStrictEqual({ hidden: false, timeInterval: 'auto' }); + expect(result.current.breakdown).toStrictEqual({ field: undefined }); + expect(result.current.isPlainRecord).toBe(true); }); - it('should return the correct props when a text based language is used', () => { + it('should return the correct props when an ES|QL query is used with transformational commands', () => { const stateService = getStateService({ initialState: { ...initialState, @@ -235,7 +243,7 @@ describe('useStateProps', () => { useStateProps({ stateService, dataView: dataViewWithTimefieldMock, - query: { esql: 'FROM index' }, + query: { esql: 'FROM index | keep field1' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, @@ -246,6 +254,82 @@ describe('useStateProps', () => { expect(result.current.isPlainRecord).toBe(true); }); + it('should return the correct props when an ES|QL query is used with breakdown field', () => { + const breakdownField = 'extension'; + const esqlColumns = [ + { + name: 'bytes', + meta: { type: 'number' }, + id: 'bytes', + }, + { + name: 'extension', + meta: { type: 'string' }, + id: 'extension', + }, + ] as DatatableColumn[]; + const stateService = getStateService({ + initialState: { + ...initialState, + currentSuggestionContext: undefined, + breakdownField, + }, + }); + const { result } = renderHook(() => + useStateProps({ + stateService, + dataView: dataViewWithTimefieldMock, + query: { esql: 'FROM index' }, + requestAdapter: new RequestAdapter(), + searchSessionId: '123', + columns: esqlColumns, + }) + ); + + const breakdownColumn = esqlColumns.find((c) => c.name === breakdownField)!; + const selectedField = new DataViewField( + convertDatatableColumnToDataViewFieldSpec(breakdownColumn) + ); + expect(result.current.breakdown).toStrictEqual({ field: selectedField }); + }); + + it('should call the setBreakdown cb when an ES|QL query is used', () => { + const breakdownField = 'extension'; + const esqlColumns = [ + { + name: 'bytes', + meta: { type: 'number' }, + id: 'bytes', + }, + { + name: 'extension', + meta: { type: 'string' }, + id: 'extension', + }, + ] as DatatableColumn[]; + const stateService = getStateService({ + initialState: { + ...initialState, + currentSuggestionContext: undefined, + }, + }); + const { result } = renderHook(() => + useStateProps({ + stateService, + dataView: dataViewWithTimefieldMock, + query: { esql: 'FROM index' }, + requestAdapter: new RequestAdapter(), + searchSessionId: '123', + columns: esqlColumns, + }) + ); + const { onBreakdownFieldChange } = result.current; + act(() => { + onBreakdownFieldChange({ name: breakdownField } as DataViewField); + }); + expect(stateService.setBreakdownField).toHaveBeenLastCalledWith(breakdownField); + }); + it('should return the correct props when a rollup data view is used', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => From 7609ce9161a65e00cadccbb36a178acfdfe3e59f Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 24 Sep 2024 12:35:38 +0200 Subject: [PATCH 05/14] More unit tests --- .../lens_vis_service.suggestions.test.ts | 52 +++++++++++++++++++ .../public/services/lens_vis_service.ts | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts index f4128146c9f34..c9ed0fc2c7903 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts @@ -8,6 +8,7 @@ */ import type { AggregateQuery, Query } from '@kbn/es-query'; +import { DataViewField } from '@kbn/data-views-plugin/common'; import { deepMockedFields, buildDataViewMock } from '@kbn/discover-utils/src/__mocks__'; import { allSuggestionsMock } from '../__mocks__/suggestions'; import { getLensVisMock } from '../__mocks__/lens_vis'; @@ -195,4 +196,55 @@ describe('LensVisService suggestions', () => { expect(lensVis.currentSuggestionContext?.type).toBe(UnifiedHistogramSuggestionType.unsupported); expect(lensVis.currentSuggestionContext?.suggestion).not.toBeDefined(); }); + + test('should return histogramSuggestion if no suggestions returned by the api with the breakdown field if it is given', async () => { + const lensVis = await getLensVisMock({ + filters: [], + query: { esql: 'from the-data-view | limit 100' }, + dataView: dataViewMock, + timeInterval: 'auto', + timeRange: { + from: '2023-09-03T08:00:00.000Z', + to: '2023-09-04T08:56:28.274Z', + }, + breakdownField: { name: 'var0' } as DataViewField, + columns: [ + { + id: 'var0', + name: 'var0', + meta: { + type: 'number', + }, + }, + ], + isPlainRecord: true, + allSuggestions: [], + hasHistogramSuggestionForESQL: true, + }); + + expect(lensVis.currentSuggestionContext?.type).toBe( + UnifiedHistogramSuggestionType.histogramForESQL + ); + expect(lensVis.currentSuggestionContext?.suggestion).toBeDefined(); + expect(lensVis.currentSuggestionContext?.suggestion?.visualizationState).toHaveProperty( + 'layers', + [ + { + layerId: '662552df-2cdc-4539-bf3b-73b9f827252c', + seriesType: 'bar_stacked', + xAccessor: '@timestamp every 30 second', + accessors: ['results'], + layerType: 'data', + splitAccessor: 'var0', + }, + ] + ); + + const histogramQuery = { + esql: `from the-data-view | limit 100 +| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, var0 | rename timestamp as \`@timestamp every 30 minute\``, + }; + + expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery); + }); }); diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index 0058b14669625..4e53c07592f5c 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -554,7 +554,7 @@ export class LensVisService { const breakdown = breakdownColumn ? `, ${breakdownColumn.name}` : ''; return appendToESQLQuery( safeQuery, - `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp ${breakdown} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\`` + `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\`` ); }; From 128a1a2e1d58d6e6aed2ab8c8fb49a2c5d2fc79e Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 24 Sep 2024 10:47:08 +0000 Subject: [PATCH 06/14] [CI] Auto-commit changed files from 'node scripts/notice' --- src/plugins/unified_histogram/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/unified_histogram/tsconfig.json b/src/plugins/unified_histogram/tsconfig.json index 2f54a5d33797a..d14adf53889b9 100644 --- a/src/plugins/unified_histogram/tsconfig.json +++ b/src/plugins/unified_histogram/tsconfig.json @@ -33,6 +33,7 @@ "@kbn/discover-utils", "@kbn/visualization-utils", "@kbn/search-types", + "@kbn/data-view-utils", ], "exclude": [ "target/**/*", From 59a3d484a6ba8dbeaa898d5d01df0a1d7bf6ea56 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 24 Sep 2024 14:00:46 +0200 Subject: [PATCH 07/14] Adds a FT --- .../lens_vis_service.suggestions.test.ts | 2 +- .../public/services/lens_vis_service.ts | 4 +- .../apps/discover/esql/_esql_view.ts | 40 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts index c9ed0fc2c7903..adcd87fa48e61 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts @@ -242,7 +242,7 @@ describe('LensVisService suggestions', () => { const histogramQuery = { esql: `from the-data-view | limit 100 -| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, var0 | rename timestamp as \`@timestamp every 30 minute\``, +| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, var0 | sort var0 asc | rename timestamp as \`@timestamp every 30 minute\``, }; expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery); diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index 4e53c07592f5c..e08d9dc4cfe03 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -551,7 +551,9 @@ export class LensVisService { const queryInterval = interval ?? computeInterval(timeRange, this.services.data); const language = getAggregateQueryMode(query); const safeQuery = removeDropCommandsFromESQLQuery(query[language]); - const breakdown = breakdownColumn ? `, ${breakdownColumn.name}` : ''; + const breakdown = breakdownColumn + ? `, ${breakdownColumn.name} | sort ${breakdownColumn.name} asc` + : ''; return appendToESQLQuery( safeQuery, `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\`` diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 98bf29b187402..7870667c91ba7 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -675,5 +675,45 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); }); }); + + describe('histogram breakdown', () => { + before(async () => { + await common.navigateToApp('discover'); + await timePicker.setDefaultAbsoluteRange(); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + }); + + it('should choose breakdown field', async () => { + await discover.selectTextBaseLang(); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + + const testQuery = 'from logstash-*'; + await monacoEditor.setCodeEditorValue(testQuery); + await testSubjects.click('querySubmitButton'); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + + await discover.chooseBreakdownField('extension'); + await header.waitUntilLoadingHasFinished(); + const list = await discover.getHistogramLegendList(); + expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); + }); + + it('should save breakdown field in saved search', async () => { + await discover.saveSearch('esql view with breakdown'); + + await discover.clickNewSearchButton(); + await header.waitUntilLoadingHasFinished(); + const prevList = await discover.getHistogramLegendList(); + expect(prevList).to.eql([]); + + await discover.loadSavedSearch('esql view with breakdown'); + await header.waitUntilLoadingHasFinished(); + const list = await discover.getHistogramLegendList(); + expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); + }); + }); }); } From e1698d710a40b6839469fb76232a18188f1e0dd5 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 24 Sep 2024 15:43:37 +0200 Subject: [PATCH 08/14] Fix filtering problem --- test/functional/apps/discover/esql/_esql_view.ts | 11 +++++++++++ x-pack/plugins/lens/public/embeddable/embeddable.tsx | 9 ++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 7870667c91ba7..630b50b9892fa 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -701,6 +701,17 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); }); + it('should add filter using histogram legend values', async () => { + await discover.clickLegendFilter('png', '+'); + await header.waitUntilLoadingHasFinished(); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + await unifiedFieldList.waitUntilSidebarHasLoaded(); + + const editorValue = await monacoEditor.getCodeEditorValue(); + expect(editorValue).to.eql(`from logstash-*\nAND \`extension\`=="png"`); + }); + it('should save breakdown field in saved search', async () => { await discover.saveSearch('esql view with breakdown'); diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 51bcbb4fed635..2d8dad7571c1a 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -24,6 +24,7 @@ import { getAggregateQueryMode, ExecutionContextSearch, getLanguageDisplayName, + isOfAggregateQueryType, } from '@kbn/es-query'; import type { PaletteOutput } from '@kbn/coloring'; import { @@ -1406,7 +1407,13 @@ export class Embeddable } else if (isLensTableRowContextMenuClickEvent(event)) { eventHandler = this.input.onTableRowClick; } - const esqlQuery = this.isTextBasedLanguage() ? this.savedVis?.state.query : undefined; + // if the embeddable is located in an app where there is the Unified search bar with the ES|QL editor, then use this query + // otherwise use the query from the saved object + let esqlQuery: AggregateQuery | Query | undefined; + if (this.isTextBasedLanguage()) { + const query = this.deps.data.query.queryString.getQuery(); + esqlQuery = isOfAggregateQueryType(query) ? query : this.savedVis?.state.query; + } eventHandler?.({ ...event.data, From baf8933c34ceed93db7f31ae78c1361f7516c219 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 25 Sep 2024 09:40:07 +0200 Subject: [PATCH 09/14] Fix functional test --- test/functional/apps/discover/esql/_esql_view.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 630b50b9892fa..dfeb313c7796f 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -709,10 +709,17 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await unifiedFieldList.waitUntilSidebarHasLoaded(); const editorValue = await monacoEditor.getCodeEditorValue(); - expect(editorValue).to.eql(`from logstash-*\nAND \`extension\`=="png"`); + expect(editorValue).to.eql(`from logstash-*\n| WHERE \`extension\`=="png"`); }); it('should save breakdown field in saved search', async () => { + // revert the filter + const testQuery = 'from logstash-*'; + await monacoEditor.setCodeEditorValue(testQuery); + await testSubjects.click('querySubmitButton'); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + await discover.saveSearch('esql view with breakdown'); await discover.clickNewSearchButton(); From 97d8cbd3919a911244f260d01f95d42d72268f90 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 25 Sep 2024 12:21:43 +0200 Subject: [PATCH 10/14] Cleanup --- src/plugins/unified_histogram/public/chart/chart.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/plugins/unified_histogram/public/chart/chart.tsx b/src/plugins/unified_histogram/public/chart/chart.tsx index ed433d100f55c..4fb1b9cbe6471 100644 --- a/src/plugins/unified_histogram/public/chart/chart.tsx +++ b/src/plugins/unified_histogram/public/chart/chart.tsx @@ -74,10 +74,7 @@ export interface ChartProps { isChartLoading?: boolean; onChartHiddenChange?: (chartHidden: boolean) => void; onTimeIntervalChange?: (timeInterval: string) => void; - onBreakdownFieldChange?: ( - breakdownField: DataViewField | undefined, - columns?: DatatableColumn[] - ) => void; + onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void; onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void; onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void; onFilter?: LensEmbeddableInput['onFilter']; From 3fb53197d098f33edc2be5146b5a4a5dd5882bb2 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 27 Sep 2024 09:20:42 +0200 Subject: [PATCH 11/14] Update FT --- .../apps/discover/group3/_lens_vis.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/functional/apps/discover/group3/_lens_vis.ts b/test/functional/apps/discover/group3/_lens_vis.ts index 30918f91ff2fb..a93ed26070190 100644 --- a/test/functional/apps/discover/group3/_lens_vis.ts +++ b/test/functional/apps/discover/group3/_lens_vis.ts @@ -56,7 +56,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await discover.getHitCount()).to.be(totalCount); } - async function checkESQLHistogramVis(timespan: string, totalCount: string) { + async function checkESQLHistogramVis( + timespan: string, + totalCount: string, + hasTransformationalCommand = false + ) { await header.waitUntilLoadingHasFinished(); await discover.waitUntilSearchingHasFinished(); @@ -64,7 +68,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.existOrFail('unifiedHistogramSaveVisualization'); await testSubjects.existOrFail('unifiedHistogramEditFlyoutVisualization'); await testSubjects.missingOrFail('unifiedHistogramEditVisualization'); - await testSubjects.missingOrFail('unifiedHistogramBreakdownSelectorButton'); + if (hasTransformationalCommand) { + await testSubjects.missingOrFail('unifiedHistogramBreakdownSelectorButton'); + } else { + await testSubjects.existOrFail('unifiedHistogramBreakdownSelectorButton'); + } await testSubjects.missingOrFail('unifiedHistogramTimeIntervalSelectorButton'); expect(await discover.getChartTimespan()).to.be(timespan); expect(await discover.getHitCount()).to.be(totalCount); @@ -102,7 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { return seriesType; } - describe('discover lens vis', function () { + describe('discover lens vis-meow', function () { before(async () => { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); @@ -309,7 +317,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await header.waitUntilLoadingHasFinished(); await discover.waitUntilSearchingHasFinished(); - await checkESQLHistogramVis(defaultTimespanESQL, '5'); + await checkESQLHistogramVis(defaultTimespanESQL, '5', true); await discover.chooseLensSuggestion('pie'); await testSubjects.existOrFail('unsavedChangesBadge'); @@ -358,7 +366,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await header.waitUntilLoadingHasFinished(); await discover.waitUntilSearchingHasFinished(); - await checkESQLHistogramVis(defaultTimespanESQL, '5'); + await checkESQLHistogramVis(defaultTimespanESQL, '5', true); await discover.chooseLensSuggestion('pie'); await testSubjects.existOrFail('unsavedChangesBadge'); @@ -411,7 +419,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await header.waitUntilLoadingHasFinished(); await discover.waitUntilSearchingHasFinished(); - await checkESQLHistogramVis(defaultTimespanESQL, '5'); + await checkESQLHistogramVis(defaultTimespanESQL, '5', true); await discover.chooseLensSuggestion('pie'); await testSubjects.existOrFail('unsavedChangesBadge'); @@ -455,7 +463,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await header.waitUntilLoadingHasFinished(); await discover.waitUntilSearchingHasFinished(); - await checkESQLHistogramVis(defaultTimespanESQL, '5'); + await checkESQLHistogramVis(defaultTimespanESQL, '5', true); await discover.chooseLensSuggestion('pie'); await discover.saveSearch('testCustomESQLVis'); From 1b95c94f109e37bb33713962f18fa3a3dd8a16af Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 27 Sep 2024 16:48:25 +0200 Subject: [PATCH 12/14] Address PR comments --- src/plugins/unified_histogram/public/layout/layout.tsx | 2 +- test/functional/apps/discover/group3/_lens_vis.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/unified_histogram/public/layout/layout.tsx b/src/plugins/unified_histogram/public/layout/layout.tsx index 60438bfd4cc77..3e34cf4ee69b3 100644 --- a/src/plugins/unified_histogram/public/layout/layout.tsx +++ b/src/plugins/unified_histogram/public/layout/layout.tsx @@ -14,7 +14,7 @@ import useObservable from 'react-use/lib/useObservable'; import { createHtmlPortalNode, InPortal, OutPortal } from 'react-reverse-portal'; import { css } from '@emotion/css'; import type { Datatable, DatatableColumn } from '@kbn/expressions-plugin/common'; -import { type DataView, DataViewField } from '@kbn/data-views-plugin/public'; +import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import type { EmbeddableComponentProps, LensEmbeddableInput, diff --git a/test/functional/apps/discover/group3/_lens_vis.ts b/test/functional/apps/discover/group3/_lens_vis.ts index a93ed26070190..321486a40238e 100644 --- a/test/functional/apps/discover/group3/_lens_vis.ts +++ b/test/functional/apps/discover/group3/_lens_vis.ts @@ -110,7 +110,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { return seriesType; } - describe('discover lens vis-meow', function () { + describe('discover lens vis', function () { before(async () => { await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); From 9e731dfc361a48659f74ee8b3896cdc0184cbf79 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 1 Oct 2024 08:42:36 +0200 Subject: [PATCH 13/14] Address PR comments --- .../public/chart/breakdown_field_selector.tsx | 24 ++++++------------- .../lens_vis_service.suggestions.test.ts | 2 +- .../public/services/lens_vis_service.ts | 17 +++++++++---- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx index 8ac828c1e8008..c124c341e84bf 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx @@ -33,11 +33,9 @@ export interface BreakdownFieldSelectorProps { const mapToDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => { if (esqlColumns) { - return esqlColumns.map((column) => ({ - name: column.name, - displayName: column.name, - type: column.meta?.type, - })); + return esqlColumns.map( + (column) => new DataViewField(convertDatatableColumnToDataViewFieldSpec(column)) + ); } return dataView.fields.filter(fieldSupportsBreakdown); @@ -89,20 +87,12 @@ export const BreakdownFieldSelector = ({ const onChange = useCallback>( (chosenOption) => { - let breakdownField: DataViewField | undefined; - if (esqlColumns) { - const breakdownColumn = esqlColumns?.find((column) => column.name === chosenOption?.value); - breakdownField = breakdownColumn - ? new DataViewField(convertDatatableColumnToDataViewFieldSpec(breakdownColumn)) - : undefined; - } else { - breakdownField = chosenOption?.value - ? dataView.fields.find((currentField) => currentField.name === chosenOption.value) - : undefined; - } + const breakdownField = chosenOption?.value + ? fields.find((currentField) => currentField.name === chosenOption.value) + : undefined; onBreakdownFieldChange?.(breakdownField); }, - [dataView.fields, esqlColumns, onBreakdownFieldChange] + [fields, onBreakdownFieldChange] ); return ( diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts index adcd87fa48e61..28819f7a5c54b 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.suggestions.test.ts @@ -242,7 +242,7 @@ describe('LensVisService suggestions', () => { const histogramQuery = { esql: `from the-data-view | limit 100 -| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, var0 | sort var0 asc | rename timestamp as \`@timestamp every 30 minute\``, +| EVAL timestamp=DATE_TRUNC(30 minute, @timestamp) | stats results = count(*) by timestamp, \`var0\` | sort \`var0\` asc | rename timestamp as \`@timestamp every 30 minute\``, }; expect(lensVis.visContext?.attributes.state.query).toStrictEqual(histogramQuery); diff --git a/src/plugins/unified_histogram/public/services/lens_vis_service.ts b/src/plugins/unified_histogram/public/services/lens_vis_service.ts index e08d9dc4cfe03..eccfd663b2557 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -461,7 +461,9 @@ export class LensVisService { breakdownField?: DataViewField; }): Suggestion | undefined => { const { dataView, query, timeRange, columns } = queryParams; - const breakdownColumn = columns?.find((column) => column.name === breakdownField?.name); + const breakdownColumn = breakdownField?.name + ? columns?.find((column) => column.name === breakdownField.name) + : undefined; if (dataView.isTimeBased() && query && isOfAggregateQueryType(query) && timeRange) { const isOnHistogramMode = shouldDisplayHistogram(query); if (!isOnHistogramMode) return undefined; @@ -552,7 +554,7 @@ export class LensVisService { const language = getAggregateQueryMode(query); const safeQuery = removeDropCommandsFromESQLQuery(query[language]); const breakdown = breakdownColumn - ? `, ${breakdownColumn.name} | sort ${breakdownColumn.name} asc` + ? `, \`${breakdownColumn.name}\` | sort \`${breakdownColumn.name}\` asc` : ''; return appendToESQLQuery( safeQuery, @@ -612,12 +614,17 @@ export class LensVisService { breakdownField: breakdownField?.name, }; - const breakdownColumn = columns?.find((column) => column.name === breakdownField?.name); - const currentQuery = suggestionType === UnifiedHistogramSuggestionType.histogramForESQL && isTextBased && timeRange ? { - esql: this.getESQLHistogramQuery({ dataView, query, timeRange, breakdownColumn }), + esql: this.getESQLHistogramQuery({ + dataView, + query, + timeRange, + breakdownColumn: breakdownField?.name + ? columns?.find((column) => column.name === breakdownField.name) + : undefined, + }), } : query; From e757a54d206066255e4d70cd3ddeee01a4f5aa8e Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 1 Oct 2024 08:50:22 +0200 Subject: [PATCH 14/14] Filter unsupported fields --- .../public/chart/breakdown_field_selector.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx index c124c341e84bf..b3c49e27c6011 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx @@ -33,8 +33,11 @@ export interface BreakdownFieldSelectorProps { const mapToDropdownFields = (dataView: DataView, esqlColumns?: DatatableColumn[]) => { if (esqlColumns) { - return esqlColumns.map( - (column) => new DataViewField(convertDatatableColumnToDataViewFieldSpec(column)) + return ( + esqlColumns + .map((column) => new DataViewField(convertDatatableColumnToDataViewFieldSpec(column))) + // filter out unsupported field types + .filter((field) => field.type !== 'unknown') ); }