From 3c51e10d9ceaa94385e4e3b0b8ffa319504b3987 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Tue, 5 Nov 2024 18:04:04 +0100 Subject: [PATCH 01/17] [Discover] Breakdown support for fieldstats --- .../src/utils/esql_fields_utils.ts | 26 ++++++++------ packages/kbn-field-utils/index.ts | 1 + .../utils/field_supports_breakdown.test.ts | 0 .../src}/utils/field_supports_breakdown.ts | 12 +++++-- .../field_popover/field_popover_header.tsx | 29 +++++++++++++++ .../field_list_item.tsx | 17 +++++++-- .../field_list_sidebar.tsx | 36 ++++++++++--------- .../components/layout/discover_layout.tsx | 29 ++++++++++----- .../sidebar/discover_sidebar_responsive.tsx | 28 +++++++++------ .../public/chart/breakdown_field_selector.tsx | 8 +++-- src/plugins/unified_histogram/public/index.ts | 1 - .../public/services/lens_vis_service.ts | 2 +- .../public/hooks/use_degraded_docs_chart.ts | 2 +- 13 files changed, 135 insertions(+), 56 deletions(-) rename {src/plugins/unified_histogram/public => packages/kbn-field-utils/src}/utils/field_supports_breakdown.test.ts (100%) rename {src/plugins/unified_histogram/public => packages/kbn-field-utils/src}/utils/field_supports_breakdown.ts (65%) diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index e2c5c785f8f58..8a9efe926448f 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -7,6 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { DataViewField } from '@kbn/data-views-plugin/common'; import type { DatatableColumn } from '@kbn/expressions-plugin/common'; const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape']; @@ -40,22 +41,27 @@ export const isESQLColumnSortable = (column: DatatableColumn): boolean => { return true; }; +// Helper function to check if a field is groupable based on its type and esType +const isGroupable = (type: string | undefined, esType: string | undefined): boolean => { + if (type === UNKNOWN_FIELD) { + return false; + } + if (esType && esType.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { + return false; + } + return true; +}; + /** * Check if a column is groupable (| STATS ... BY ). * * @param column The DatatableColumn of the field. * @returns True if the column is groupable, false otherwise. */ - export const isESQLColumnGroupable = (column: DatatableColumn): boolean => { - // we don't allow grouping on the unknown field types - if (column.meta?.type === UNKNOWN_FIELD) { - return false; - } - // we don't allow grouping on tsdb counter fields - if (column.meta?.esType && column.meta?.esType?.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { - return false; - } + return isGroupable(column.meta?.type, column.meta?.esType); +}; - return true; +export const isESQLFieldGroupable = (field: DataViewField): boolean => { + return isGroupable(field.type, field.esTypes?.[0]); }; diff --git a/packages/kbn-field-utils/index.ts b/packages/kbn-field-utils/index.ts index ae77f0d1524fd..d3ce774177dcc 100644 --- a/packages/kbn-field-utils/index.ts +++ b/packages/kbn-field-utils/index.ts @@ -25,6 +25,7 @@ export { comboBoxFieldOptionMatcher, getFieldSearchMatchingHighlight, } from './src/utils/field_name_wildcard_matcher'; +export { fieldSupportsBreakdown } from './src/utils/field_supports_breakdown'; export { FieldIcon, type FieldIconProps, getFieldIconProps } from './src/components/field_icon'; export { FieldDescription, type FieldDescriptionProps } from './src/components/field_description'; diff --git a/src/plugins/unified_histogram/public/utils/field_supports_breakdown.test.ts b/packages/kbn-field-utils/src/utils/field_supports_breakdown.test.ts similarity index 100% rename from src/plugins/unified_histogram/public/utils/field_supports_breakdown.test.ts rename to packages/kbn-field-utils/src/utils/field_supports_breakdown.test.ts diff --git a/src/plugins/unified_histogram/public/utils/field_supports_breakdown.ts b/packages/kbn-field-utils/src/utils/field_supports_breakdown.ts similarity index 65% rename from src/plugins/unified_histogram/public/utils/field_supports_breakdown.ts rename to packages/kbn-field-utils/src/utils/field_supports_breakdown.ts index 12fdfbf20aa3b..3177d40012b57 100644 --- a/src/plugins/unified_histogram/public/utils/field_supports_breakdown.ts +++ b/packages/kbn-field-utils/src/utils/field_supports_breakdown.ts @@ -7,12 +7,18 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { DataViewField } from '@kbn/data-views-plugin/public'; +import { type DataViewField } from '@kbn/data-views-plugin/common'; +import { KNOWN_FIELD_TYPES } from './field_types'; -const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); +const supportedTypes = new Set([ + KNOWN_FIELD_TYPES.STRING, + KNOWN_FIELD_TYPES.BOOLEAN, + KNOWN_FIELD_TYPES.NUMBER, + KNOWN_FIELD_TYPES.IP, +]); export const fieldSupportsBreakdown = (field: DataViewField) => - supportedTypes.has(field.type) && + supportedTypes.has(field.type as KNOWN_FIELD_TYPES) && field.aggregatable && !field.scripted && field.timeSeriesMetric !== 'counter'; diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx index 65d5a1da98034..069b73ca3f21c 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx @@ -27,10 +27,12 @@ import type { AddFieldFilterHandler } from '../../types'; export interface FieldPopoverHeaderProps { field: DataViewField; closePopover: EuiPopoverProps['closePopover']; + buttonAddBreakdownFieldProps?: Partial; buttonAddFieldToWorkspaceProps?: Partial; buttonAddFilterProps?: Partial; buttonEditFieldProps?: Partial; buttonDeleteFieldProps?: Partial; + onAddBreakdownField?: (field: DataViewField | undefined) => void; onAddFieldToWorkspace?: (field: DataViewField) => unknown; onAddFilter?: AddFieldFilterHandler; onEditField?: (fieldName: string) => unknown; @@ -43,10 +45,12 @@ export interface FieldPopoverHeaderProps { export const FieldPopoverHeader: React.FC = ({ field, closePopover, + buttonAddBreakdownFieldProps, buttonAddFieldToWorkspaceProps, buttonAddFilterProps, buttonEditFieldProps, buttonDeleteFieldProps, + onAddBreakdownField, onAddFieldToWorkspace, onAddFilter, onEditField, @@ -82,6 +86,13 @@ export const FieldPopoverHeader: React.FC = ({ defaultMessage: 'Delete data view field', }); + const addBreakdownFieldTooltip = i18n.translate( + 'unifiedFieldList.fieldPopover.addBreakdownFieldLabel', + { + defaultMessage: 'Add breakdown', + } + ); + return ( <> @@ -90,6 +101,24 @@ export const FieldPopoverHeader: React.FC = ({
{field.displayName}
+ {onAddBreakdownField && ( + + + { + closePopover(); + onAddBreakdownField(field); + }} + /> + + + )} {onAddFieldToWorkspace && ( void; /** * Callback to add/select the field */ @@ -215,6 +221,7 @@ function UnifiedFieldListItemComponent({ field, highlight, dataView, + onAddBreakdownField, onAddFieldToWorkspace, onRemoveFieldFromWorkspace, onAddFilter, @@ -232,6 +239,9 @@ function UnifiedFieldListItemComponent({ }: UnifiedFieldListItemProps) { const [infoIsOpen, setOpen] = useState(false); + const isBreakdownSupported = + searchMode === 'documents' ? fieldSupportsBreakdown(field) : isESQLFieldGroupable(field); + const addFilterAndClosePopover: typeof onAddFilter | undefined = useMemo( () => onAddFilter @@ -394,13 +404,14 @@ function UnifiedFieldListItemComponent({ data-test-subj={stateService.creationOptions.dataTestSubj?.fieldListItemPopoverDataTestSubj} renderHeader={() => ( )} diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx index a7d8fb4616cb0..2d23903c34ea3 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx @@ -48,6 +48,7 @@ export type UnifiedFieldListSidebarCustomizableProps = Pick< | 'dataView' | 'trackUiMetric' | 'onAddFilter' + | 'onAddBreakdownField' | 'onAddFieldToWorkspace' | 'onRemoveFieldFromWorkspace' | 'additionalFilters' @@ -161,6 +162,7 @@ export const UnifiedFieldListSidebarComponent: React.FC (
    • ), @@ -298,6 +301,7 @@ export const UnifiedFieldListSidebarComponent: React.FC (isOfAggregateQueryType(query) ? !hasTransformationalCommand(query.esql) : true), + [query] + ); + + const onAddBreakdownField = useCallback( + (field: DataViewField | undefined) => { + stateContainer.appState.update({ breakdownField: field?.name }); + }, + [stateContainer] + ); + const onFieldEdited = useCallback( async ({ removedFieldName }: { removedFieldName?: string } = {}) => { if (removedFieldName && currentColumns.includes(removedFieldName)) { @@ -423,18 +435,19 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { sidebarToggleState$={sidebarToggleState$} sidebarPanel={ } mainPanel={ diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx index 80a3b9d412c76..17a3e9def8b23 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx @@ -87,6 +87,10 @@ export interface DiscoverSidebarResponsiveProps { * hits fetched from ES, displayed in the doc table */ documents$: DataDocuments$; + /** + * Callback to update breakdown field + */ + onAddBreakdownField?: (breakdownField: DataViewField | undefined) => void; /** * Callback function when selecting a field */ @@ -151,6 +155,7 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps) selectedDataView, columns, trackUiMetric, + onAddBreakdownField, onAddFilter, onFieldEdited, onDataViewCreated, @@ -373,23 +378,24 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps) {selectedDataView ? ( ) : null} 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 0d6aa1e75fe80..418892bd5434f 100644 --- a/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx +++ b/src/plugins/unified_histogram/public/chart/breakdown_field_selector.tsx @@ -9,7 +9,12 @@ import React, { useCallback, useMemo } from 'react'; import { EuiSelectableOption } from '@elastic/eui'; -import { FieldIcon, getFieldIconProps, comboBoxFieldOptionMatcher } from '@kbn/field-utils'; +import { + FieldIcon, + getFieldIconProps, + comboBoxFieldOptionMatcher, + fieldSupportsBreakdown, +} from '@kbn/field-utils'; import { css } from '@emotion/react'; import { isESQLColumnGroupable } from '@kbn/esql-utils'; import { type DataView, DataViewField } from '@kbn/data-views-plugin/common'; @@ -17,7 +22,6 @@ 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'; import { ToolbarSelector, ToolbarSelectorProps, diff --git a/src/plugins/unified_histogram/public/index.ts b/src/plugins/unified_histogram/public/index.ts index fda59c78819da..f79db6ce8a51a 100644 --- a/src/plugins/unified_histogram/public/index.ts +++ b/src/plugins/unified_histogram/public/index.ts @@ -36,6 +36,5 @@ export type { } from './types'; export { UnifiedHistogramFetchStatus, UnifiedHistogramExternalVisContextStatus } from './types'; export { canImportVisContext } from './utils/external_vis_context'; -export { fieldSupportsBreakdown } from './utils/field_supports_breakdown'; export const plugin = () => new UnifiedHistogramPublicPlugin(); 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 e48ebc6459071..c846977dfee35 100644 --- a/src/plugins/unified_histogram/public/services/lens_vis_service.ts +++ b/src/plugins/unified_histogram/public/services/lens_vis_service.ts @@ -32,6 +32,7 @@ import { LegendSize } from '@kbn/visualizations-plugin/public'; import { XYConfiguration } from '@kbn/visualizations-plugin/common'; import type { Datatable, DatatableColumn } from '@kbn/expressions-plugin/common'; import type { DataPublicPluginStart } from '@kbn/data-plugin/public'; +import { fieldSupportsBreakdown } from '@kbn/field-utils'; import { UnifiedHistogramExternalVisContextStatus, UnifiedHistogramSuggestionContext, @@ -44,7 +45,6 @@ import { type QueryParams, } from '../utils/external_vis_context'; import { computeInterval } from '../utils/compute_interval'; -import { fieldSupportsBreakdown } from '../utils/field_supports_breakdown'; import { shouldDisplayHistogram } from '../layout/helpers'; import { enrichLensAttributesWithTablesData } from '../utils/lens_vis_from_table'; diff --git a/x-pack/plugins/observability_solution/dataset_quality/public/hooks/use_degraded_docs_chart.ts b/x-pack/plugins/observability_solution/dataset_quality/public/hooks/use_degraded_docs_chart.ts index d3fad141335de..90779613f0c06 100644 --- a/x-pack/plugins/observability_solution/dataset_quality/public/hooks/use_degraded_docs_chart.ts +++ b/x-pack/plugins/observability_solution/dataset_quality/public/hooks/use_degraded_docs_chart.ts @@ -7,10 +7,10 @@ import { useCallback, useEffect, useMemo, useState } from 'react'; import type { Action } from '@kbn/ui-actions-plugin/public'; -import { fieldSupportsBreakdown } from '@kbn/unified-histogram-plugin/public'; import { i18n } from '@kbn/i18n'; import { useEuiTheme } from '@elastic/eui'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/common'; +import { fieldSupportsBreakdown } from '@kbn/field-utils'; import { DEFAULT_LOGS_DATA_VIEW } from '../../common/constants'; import { useCreateDataView } from './use_create_dataview'; import { useKibanaContextForPlugin } from '../utils'; From 3fad54870906ac07d496725bd8dcecb348722997 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:15:01 +0000 Subject: [PATCH 02/17] [CI] Auto-commit changed files from 'node scripts/yarn_deduplicate' --- .../observability_solution/dataset_quality/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/observability_solution/dataset_quality/tsconfig.json b/x-pack/plugins/observability_solution/dataset_quality/tsconfig.json index f0d82fadb54ad..8576b08bd9479 100644 --- a/x-pack/plugins/observability_solution/dataset_quality/tsconfig.json +++ b/x-pack/plugins/observability_solution/dataset_quality/tsconfig.json @@ -60,7 +60,8 @@ "@kbn/usage-collection-plugin", "@kbn/rison", "@kbn/task-manager-plugin", - "@kbn/core-application-browser" + "@kbn/core-application-browser", + "@kbn/field-utils" ], "exclude": [ "target/**/*" From 2c66f0d47ea654119801eff639bdf5ba05c093da Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Wed, 6 Nov 2024 10:35:53 +0100 Subject: [PATCH 03/17] code refactor --- packages/kbn-data-view-utils/index.ts | 1 + .../convert_to_data_table_column.test.ts | 61 +++++++++++++++++++ .../src/utils/convert_to_data_table_column.ts | 30 +++++++++ .../src/utils/esql_fields_utils.ts | 26 +++----- .../field_popover/field_popover_header.tsx | 2 +- .../field_list_item.tsx | 7 ++- .../components/layout/discover_layout.tsx | 8 +-- 7 files changed, 112 insertions(+), 23 deletions(-) create mode 100644 packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts create mode 100644 packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts diff --git a/packages/kbn-data-view-utils/index.ts b/packages/kbn-data-view-utils/index.ts index ae013a12a4715..ecb6775cf4caf 100644 --- a/packages/kbn-data-view-utils/index.ts +++ b/packages/kbn-data-view-utils/index.ts @@ -9,5 +9,6 @@ export * from './src/constants'; export { convertDatatableColumnToDataViewFieldSpec } from './src/utils/convert_to_data_view_field_spec'; +export { convertDataViewFieldToDatatableColumn } from './src/utils/convert_to_data_table_column'; export { createRegExpPatternFrom } from './src/utils/create_regexp_pattern_from'; export { testPatternAgainstAllowedList } from './src/utils/test_pattern_against_allowed_list'; diff --git a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts new file mode 100644 index 0000000000000..c1cf4aa229c2f --- /dev/null +++ b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { FieldSpec } from '@kbn/data-views-plugin/common'; +import { convertDataViewFieldToDatatableColumn } from './convert_to_data_table_column'; + +describe('convertDataViewFieldToDatatableColumn', () => { + it('should return a correct DatatableColumn object for a counter timeseries field', () => { + const field: FieldSpec = { + name: 'bytes_counter', + timeSeriesMetric: 'counter', + type: 'number', + esTypes: ['long'], + searchable: true, + aggregatable: false, + isNull: false, + }; + const result = convertDataViewFieldToDatatableColumn(field); + expect(result).toEqual( + expect.objectContaining({ + id: 'bytes_counter', + name: 'bytes_counter', + meta: { + type: 'number', + esType: 'counter_long', + }, + isNull: false, + }) + ); + }); + + it('should return a correct DatatableColumn object for a non-counter timeseries field', () => { + const field: FieldSpec = { + name: 'bytes', + timeSeriesMetric: 'summary', + type: 'number', + esTypes: ['long'], + searchable: true, + aggregatable: false, + isNull: false, + }; + const result = convertDataViewFieldToDatatableColumn(field); + expect(result).toEqual( + expect.objectContaining({ + id: 'bytes', + name: 'bytes', + meta: { + type: 'number', + esType: 'long', + }, + isNull: false, + }) + ); + }); +}); diff --git a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts new file mode 100644 index 0000000000000..742a682d34ad0 --- /dev/null +++ b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { DatatableColumn } from '@kbn/expressions-plugin/common'; +import type { FieldSpec } from '@kbn/data-views-plugin/common'; + +/** + * Convert a DataViewField to a DatatableColumn + */ +export function convertDataViewFieldToDatatableColumn(field: FieldSpec): DatatableColumn { + const isCounterTimeSeries = field.timeSeriesMetric === 'counter'; + const esType = + isCounterTimeSeries && field.esTypes?.[0] ? `counter_${field.esTypes[0]}` : field.esTypes?.[0]; + + return { + id: field.name, + name: field.name, + meta: { + type: field.type, + esType, + }, + isNull: Boolean(field?.isNull), + } as DatatableColumn; +} diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index 8a9efe926448f..e2c5c785f8f58 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -7,7 +7,6 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { DataViewField } from '@kbn/data-views-plugin/common'; import type { DatatableColumn } from '@kbn/expressions-plugin/common'; const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape']; @@ -41,27 +40,22 @@ export const isESQLColumnSortable = (column: DatatableColumn): boolean => { return true; }; -// Helper function to check if a field is groupable based on its type and esType -const isGroupable = (type: string | undefined, esType: string | undefined): boolean => { - if (type === UNKNOWN_FIELD) { - return false; - } - if (esType && esType.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { - return false; - } - return true; -}; - /** * Check if a column is groupable (| STATS ... BY ). * * @param column The DatatableColumn of the field. * @returns True if the column is groupable, false otherwise. */ + export const isESQLColumnGroupable = (column: DatatableColumn): boolean => { - return isGroupable(column.meta?.type, column.meta?.esType); -}; + // we don't allow grouping on the unknown field types + if (column.meta?.type === UNKNOWN_FIELD) { + return false; + } + // we don't allow grouping on tsdb counter fields + if (column.meta?.esType && column.meta?.esType?.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { + return false; + } -export const isESQLFieldGroupable = (field: DataViewField): boolean => { - return isGroupable(field.type, field.esTypes?.[0]); + return true; }; diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx index 069b73ca3f21c..97c9161f1b321 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx @@ -110,7 +110,7 @@ export const FieldPopoverHeader: React.FC = ({ data-test-subj={`fieldPopoverHeader_addBreakdownField-${field.name}`} aria-label={addBreakdownFieldTooltip} {...(buttonAddBreakdownFieldProps || {})} - iconType="visPie" + iconType="visBarVertical" onClick={() => { closePopover(); onAddBreakdownField(field); diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx index 60effeadde859..a79cacb167708 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx @@ -16,7 +16,8 @@ import { Draggable } from '@kbn/dom-drag-drop'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { Filter } from '@kbn/es-query'; import { fieldSupportsBreakdown } from '@kbn/field-utils'; -import { isESQLFieldGroupable } from '@kbn/esql-utils/src/utils/esql_fields_utils'; +import { isESQLColumnGroupable } from '@kbn/esql-utils'; +import { convertDataViewFieldToDatatableColumn } from '@kbn/data-view-utils'; import type { SearchMode } from '../../types'; import { FieldItemButton, type FieldItemButtonProps } from '../../components/field_item_button'; import { @@ -240,7 +241,9 @@ function UnifiedFieldListItemComponent({ const [infoIsOpen, setOpen] = useState(false); const isBreakdownSupported = - searchMode === 'documents' ? fieldSupportsBreakdown(field) : isESQLFieldGroupable(field); + searchMode === 'documents' + ? fieldSupportsBreakdown(field) + : isESQLColumnGroupable(convertDataViewFieldToDatatableColumn(field.spec)); const addFilterAndClosePopover: typeof onAddFilter | undefined = useMemo( () => 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 ed95edb20183b..87d016c5750d0 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 @@ -25,7 +25,7 @@ import { METRIC_TYPE } from '@kbn/analytics'; import classNames from 'classnames'; import { generateFilters } from '@kbn/data-plugin/public'; import { useDragDropContext } from '@kbn/dom-drag-drop'; -import { DataViewField, DataViewType } from '@kbn/data-views-plugin/public'; +import { type DataViewField, DataViewType } from '@kbn/data-views-plugin/public'; import { SEARCH_FIELDS_FROM_SOURCE, SHOW_FIELD_STATISTICS, @@ -256,12 +256,12 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const onFilter = isEsqlMode ? onPopulateWhereClause : onAddFilter; - const isBreakdownSupported = useMemo( + const canSetBreakdownField = useMemo( () => (isOfAggregateQueryType(query) ? !hasTransformationalCommand(query.esql) : true), [query] ); - const onAddBreakdownField = useCallback( + const onSetBreakdownField = useCallback( (field: DataViewField | undefined) => { stateContainer.appState.update({ breakdownField: field?.name }); }, @@ -438,7 +438,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { additionalFilters={customFilters} columns={currentColumns} documents$={stateContainer.dataState.data$.documents$} - onAddBreakdownField={isBreakdownSupported ? onAddBreakdownField : undefined} + onAddBreakdownField={canSetBreakdownField ? onSetBreakdownField : undefined} onAddField={onAddColumnWithTracking} onAddFilter={onFilter} onChangeDataView={stateContainer.actions.onChangeDataView} From 93ffa66fcdacfe24ac34bde49d97e8b86a8bd3df Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Wed, 6 Nov 2024 10:59:21 +0100 Subject: [PATCH 04/17] bug fix --- .../application/main/components/layout/discover_layout.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 87d016c5750d0..8963ceff32d05 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 @@ -257,8 +257,11 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { const onFilter = isEsqlMode ? onPopulateWhereClause : onAddFilter; const canSetBreakdownField = useMemo( - () => (isOfAggregateQueryType(query) ? !hasTransformationalCommand(query.esql) : true), - [query] + () => + isOfAggregateQueryType(query) + ? dataView?.isTimeBased() && !hasTransformationalCommand(query.esql) + : true, + [dataView, query] ); const onSetBreakdownField = useCallback( From 96848c1e40d1d10a834d00306a3280a471e662a3 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 6 Nov 2024 10:46:51 +0000 Subject: [PATCH 05/17] [CI] Auto-commit changed files from 'node scripts/notice' --- packages/kbn-unified-field-list/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kbn-unified-field-list/tsconfig.json b/packages/kbn-unified-field-list/tsconfig.json index 830e56ac6ab00..0afc0164eec64 100644 --- a/packages/kbn-unified-field-list/tsconfig.json +++ b/packages/kbn-unified-field-list/tsconfig.json @@ -35,7 +35,8 @@ "@kbn/esql-utils", "@kbn/search-types", "@kbn/fields-metadata-plugin", - "@kbn/ui-theme" + "@kbn/ui-theme", + "@kbn/data-view-utils" ], "exclude": ["target/**/*"] } From 93b98944a7e67ebb6762c19a32b245b824ecfdc6 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Wed, 6 Nov 2024 17:57:17 +0100 Subject: [PATCH 06/17] revert code --- packages/kbn-data-view-utils/index.ts | 1 - .../convert_to_data_table_column.test.ts | 61 ------------------- .../src/utils/convert_to_data_table_column.ts | 30 --------- .../src/utils/esql_fields_utils.ts | 26 +++++--- .../field_list_item.tsx | 7 +-- 5 files changed, 18 insertions(+), 107 deletions(-) delete mode 100644 packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts delete mode 100644 packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts diff --git a/packages/kbn-data-view-utils/index.ts b/packages/kbn-data-view-utils/index.ts index ecb6775cf4caf..ae013a12a4715 100644 --- a/packages/kbn-data-view-utils/index.ts +++ b/packages/kbn-data-view-utils/index.ts @@ -9,6 +9,5 @@ export * from './src/constants'; export { convertDatatableColumnToDataViewFieldSpec } from './src/utils/convert_to_data_view_field_spec'; -export { convertDataViewFieldToDatatableColumn } from './src/utils/convert_to_data_table_column'; export { createRegExpPatternFrom } from './src/utils/create_regexp_pattern_from'; export { testPatternAgainstAllowedList } from './src/utils/test_pattern_against_allowed_list'; diff --git a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts deleted file mode 100644 index c1cf4aa229c2f..0000000000000 --- a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import type { FieldSpec } from '@kbn/data-views-plugin/common'; -import { convertDataViewFieldToDatatableColumn } from './convert_to_data_table_column'; - -describe('convertDataViewFieldToDatatableColumn', () => { - it('should return a correct DatatableColumn object for a counter timeseries field', () => { - const field: FieldSpec = { - name: 'bytes_counter', - timeSeriesMetric: 'counter', - type: 'number', - esTypes: ['long'], - searchable: true, - aggregatable: false, - isNull: false, - }; - const result = convertDataViewFieldToDatatableColumn(field); - expect(result).toEqual( - expect.objectContaining({ - id: 'bytes_counter', - name: 'bytes_counter', - meta: { - type: 'number', - esType: 'counter_long', - }, - isNull: false, - }) - ); - }); - - it('should return a correct DatatableColumn object for a non-counter timeseries field', () => { - const field: FieldSpec = { - name: 'bytes', - timeSeriesMetric: 'summary', - type: 'number', - esTypes: ['long'], - searchable: true, - aggregatable: false, - isNull: false, - }; - const result = convertDataViewFieldToDatatableColumn(field); - expect(result).toEqual( - expect.objectContaining({ - id: 'bytes', - name: 'bytes', - meta: { - type: 'number', - esType: 'long', - }, - isNull: false, - }) - ); - }); -}); diff --git a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts b/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts deleted file mode 100644 index 742a682d34ad0..0000000000000 --- a/packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import type { DatatableColumn } from '@kbn/expressions-plugin/common'; -import type { FieldSpec } from '@kbn/data-views-plugin/common'; - -/** - * Convert a DataViewField to a DatatableColumn - */ -export function convertDataViewFieldToDatatableColumn(field: FieldSpec): DatatableColumn { - const isCounterTimeSeries = field.timeSeriesMetric === 'counter'; - const esType = - isCounterTimeSeries && field.esTypes?.[0] ? `counter_${field.esTypes[0]}` : field.esTypes?.[0]; - - return { - id: field.name, - name: field.name, - meta: { - type: field.type, - esType, - }, - isNull: Boolean(field?.isNull), - } as DatatableColumn; -} diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index e2c5c785f8f58..8a9efe926448f 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -7,6 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { DataViewField } from '@kbn/data-views-plugin/common'; import type { DatatableColumn } from '@kbn/expressions-plugin/common'; const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape']; @@ -40,22 +41,27 @@ export const isESQLColumnSortable = (column: DatatableColumn): boolean => { return true; }; +// Helper function to check if a field is groupable based on its type and esType +const isGroupable = (type: string | undefined, esType: string | undefined): boolean => { + if (type === UNKNOWN_FIELD) { + return false; + } + if (esType && esType.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { + return false; + } + return true; +}; + /** * Check if a column is groupable (| STATS ... BY ). * * @param column The DatatableColumn of the field. * @returns True if the column is groupable, false otherwise. */ - export const isESQLColumnGroupable = (column: DatatableColumn): boolean => { - // we don't allow grouping on the unknown field types - if (column.meta?.type === UNKNOWN_FIELD) { - return false; - } - // we don't allow grouping on tsdb counter fields - if (column.meta?.esType && column.meta?.esType?.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { - return false; - } + return isGroupable(column.meta?.type, column.meta?.esType); +}; - return true; +export const isESQLFieldGroupable = (field: DataViewField): boolean => { + return isGroupable(field.type, field.esTypes?.[0]); }; diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx index a79cacb167708..60effeadde859 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx @@ -16,8 +16,7 @@ import { Draggable } from '@kbn/dom-drag-drop'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { Filter } from '@kbn/es-query'; import { fieldSupportsBreakdown } from '@kbn/field-utils'; -import { isESQLColumnGroupable } from '@kbn/esql-utils'; -import { convertDataViewFieldToDatatableColumn } from '@kbn/data-view-utils'; +import { isESQLFieldGroupable } from '@kbn/esql-utils/src/utils/esql_fields_utils'; import type { SearchMode } from '../../types'; import { FieldItemButton, type FieldItemButtonProps } from '../../components/field_item_button'; import { @@ -241,9 +240,7 @@ function UnifiedFieldListItemComponent({ const [infoIsOpen, setOpen] = useState(false); const isBreakdownSupported = - searchMode === 'documents' - ? fieldSupportsBreakdown(field) - : isESQLColumnGroupable(convertDataViewFieldToDatatableColumn(field.spec)); + searchMode === 'documents' ? fieldSupportsBreakdown(field) : isESQLFieldGroupable(field); const addFilterAndClosePopover: typeof onAddFilter | undefined = useMemo( () => From 6f06144d5cf9930b1bc53b3709bcbc8df30e4d24 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:11:26 +0000 Subject: [PATCH 07/17] [CI] Auto-commit changed files from 'node scripts/notice' --- packages/kbn-unified-field-list/tsconfig.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kbn-unified-field-list/tsconfig.json b/packages/kbn-unified-field-list/tsconfig.json index 0afc0164eec64..50ecc4480f89a 100644 --- a/packages/kbn-unified-field-list/tsconfig.json +++ b/packages/kbn-unified-field-list/tsconfig.json @@ -36,7 +36,6 @@ "@kbn/search-types", "@kbn/fields-metadata-plugin", "@kbn/ui-theme", - "@kbn/data-view-utils" ], "exclude": ["target/**/*"] } From 7297442d1da558174206c3767d2912378d3fe29e Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Wed, 6 Nov 2024 19:38:42 +0100 Subject: [PATCH 08/17] add unit tests --- .../src/utils/esql_fields_utils.test.ts | 49 ++++++++++++++++++- .../src/utils/esql_fields_utils.ts | 9 ++-- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts index bfc3a4d6708f0..5bf9980d4ae78 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.test.ts @@ -7,7 +7,12 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ import type { DatatableColumn } from '@kbn/expressions-plugin/common'; -import { isESQLColumnSortable, isESQLColumnGroupable } from './esql_fields_utils'; +import { + isESQLColumnSortable, + isESQLColumnGroupable, + isESQLFieldGroupable, +} from './esql_fields_utils'; +import type { FieldSpec } from '@kbn/data-views-plugin/common'; describe('esql fields helpers', () => { describe('isESQLColumnSortable', () => { @@ -104,4 +109,46 @@ describe('esql fields helpers', () => { expect(isESQLColumnGroupable(keywordField)).toBeTruthy(); }); }); + + describe('isESQLFieldGroupable', () => { + it('returns false for unsupported fields', () => { + const fieldSpec: FieldSpec = { + name: 'unsupported', + type: 'unknown', + esTypes: ['unknown'], + searchable: true, + aggregatable: false, + isNull: false, + }; + + expect(isESQLFieldGroupable(fieldSpec)).toBeFalsy(); + }); + + it('returns false for counter fields', () => { + const fieldSpec: FieldSpec = { + name: 'tsbd_counter', + type: 'number', + esTypes: ['long'], + timeSeriesMetric: 'counter', + searchable: true, + aggregatable: false, + isNull: false, + }; + + expect(isESQLFieldGroupable(fieldSpec)).toBeFalsy(); + }); + + it('returns true for everything else', () => { + const fieldSpec: FieldSpec = { + name: 'sortable', + type: 'string', + esTypes: ['keyword'], + searchable: true, + aggregatable: false, + isNull: false, + }; + + expect(isESQLFieldGroupable(fieldSpec)).toBeTruthy(); + }); + }); }); diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index 8a9efe926448f..d8f19b0a4377b 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { DataViewField } from '@kbn/data-views-plugin/common'; +import type { FieldSpec } from '@kbn/data-views-plugin/common'; import type { DatatableColumn } from '@kbn/expressions-plugin/common'; const SPATIAL_FIELDS = ['geo_point', 'geo_shape', 'point', 'shape']; @@ -62,6 +62,9 @@ export const isESQLColumnGroupable = (column: DatatableColumn): boolean => { return isGroupable(column.meta?.type, column.meta?.esType); }; -export const isESQLFieldGroupable = (field: DataViewField): boolean => { - return isGroupable(field.type, field.esTypes?.[0]); +export const isESQLFieldGroupable = (field: FieldSpec): boolean => { + const isCounterTimeSeries = field.timeSeriesMetric === 'counter'; + const esType = + isCounterTimeSeries && field.esTypes?.[0] ? `counter_${field.esTypes[0]}` : field.esTypes?.[0]; + return isGroupable(field.type, esType); }; From 26c21e9794b34974c905dbeade84c304fb25b63c Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Wed, 6 Nov 2024 21:11:50 +0100 Subject: [PATCH 09/17] added component tests --- .../field_popover_header.test.tsx | 28 ++++++++++++++- .../field_list_item.test.tsx | 36 +++++++++++++++++++ .../discover_sidebar_responsive.test.tsx | 14 ++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.test.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.test.tsx index 19fee33bc6eaf..a4e322f2b681c 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.test.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.test.tsx @@ -38,6 +38,7 @@ describe('UnifiedFieldList ', () => { field={field} closePopover={mockClose} onAddFieldToWorkspace={jest.fn()} + onAddBreakdownField={jest.fn()} onAddFilter={jest.fn()} onEditField={jest.fn()} onDeleteField={jest.fn()} @@ -45,6 +46,9 @@ describe('UnifiedFieldList ', () => { ); expect(wrapper.text()).toBe(fieldName); + expect( + wrapper.find(`[data-test-subj="fieldPopoverHeader_addBreakdownField-${fieldName}"]`).exists() + ).toBeTruthy(); expect( wrapper.find(`[data-test-subj="fieldPopoverHeader_addField-${fieldName}"]`).exists() ).toBeTruthy(); @@ -57,7 +61,29 @@ describe('UnifiedFieldList ', () => { expect( wrapper.find(`[data-test-subj="fieldPopoverHeader_deleteField-${fieldName}"]`).exists() ).toBeTruthy(); - expect(wrapper.find(EuiButtonIcon)).toHaveLength(4); + expect(wrapper.find(EuiButtonIcon)).toHaveLength(5); + }); + + it('should correctly handle add-breakdown-field action', async () => { + const mockClose = jest.fn(); + const mockAddBreakdownField = jest.fn(); + const fieldName = 'extension'; + const field = dataView.fields.find((f) => f.name === fieldName)!; + const wrapper = mountWithIntl( + + ); + + wrapper + .find(`[data-test-subj="fieldPopoverHeader_addBreakdownField-${fieldName}"]`) + .first() + .simulate('click'); + + expect(mockClose).toHaveBeenCalled(); + expect(mockAddBreakdownField).toHaveBeenCalledWith(field); }); it('should correctly handle add-field action', async () => { diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.test.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.test.tsx index a5b1955aeca2e..671a4175ad10a 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.test.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.test.tsx @@ -43,10 +43,12 @@ async function getComponent({ selected = false, field, canFilter = true, + isBreakdownSupported = true, }: { selected?: boolean; field?: DataViewField; canFilter?: boolean; + isBreakdownSupported?: boolean; }) { const finalField = field ?? @@ -76,6 +78,7 @@ async function getComponent({ dataView: stubDataView, field: finalField, ...(canFilter && { onAddFilter: jest.fn() }), + ...(isBreakdownSupported && { onAddBreakdownField: jest.fn() }), onAddFieldToWorkspace: jest.fn(), onRemoveFieldFromWorkspace: jest.fn(), onEditField: jest.fn(), @@ -137,6 +140,34 @@ describe('UnifiedFieldListItem', function () { expect(comp.find(FieldItemButton).prop('onClick')).toBeUndefined(); }); + + it('should not show addBreakdownField action button if not supported', async function () { + const field = new DataViewField({ + name: 'extension.keyword', + type: 'string', + esTypes: ['keyword'], + aggregatable: true, + searchable: true, + }); + const { comp } = await getComponent({ + field, + isBreakdownSupported: false, + }); + + await act(async () => { + const fieldItem = findTestSubject(comp, 'field-extension.keyword-showDetails'); + await fieldItem.simulate('click'); + await comp.update(); + }); + + await comp.update(); + + expect( + comp + .find('[data-test-subj="fieldPopoverHeader_addBreakdownField-extension.keyword"]') + .exists() + ).toBeFalsy(); + }); it('should request field stats', async function () { const field = new DataViewField({ name: 'machine.os.raw', @@ -189,6 +220,11 @@ describe('UnifiedFieldListItem', function () { await comp.update(); expect(comp.find(EuiPopover).prop('isOpen')).toBe(true); + expect( + comp + .find('[data-test-subj="fieldPopoverHeader_addBreakdownField-extension.keyword"]') + .exists() + ).toBeTruthy(); expect( comp.find('[data-test-subj="fieldPopoverHeader_addField-extension.keyword"]').exists() ).toBeTruthy(); diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx index 6147bb916dad4..296d403da6901 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx @@ -162,6 +162,7 @@ function getCompProps(options?: { hits?: DataTableRecord[] }): DiscoverSidebarRe result: hits, }) as DataDocuments$, onChangeDataView: jest.fn(), + onAddBreakdownField: jest.fn(), onAddFilter: jest.fn(), onAddField: jest.fn(), onRemoveField: jest.fn(), @@ -397,6 +398,19 @@ describe('discover responsive sidebar', function () { expect(ExistingFieldsServiceApi.loadFieldExisting).not.toHaveBeenCalled(); }); + it('should allow adding breakdown field', async function () { + const comp = await mountComponent(props); + const availableFields = findTestSubject(comp, 'fieldListGroupedAvailableFields'); + await act(async () => { + const button = findTestSubject(availableFields, 'field-extension-showDetails'); + button.simulate('click'); + comp.update(); + }); + + comp.update(); + findTestSubject(comp, 'fieldPopoverHeader_addBreakdownField-extension').simulate('click'); + expect(props.onAddBreakdownField).toHaveBeenCalled(); + }); it('should allow selecting fields', async function () { const comp = await mountComponent(props); const availableFields = findTestSubject(comp, 'fieldListGroupedAvailableFields'); From 2e13155bf940b6b0e00b9e4b4eba05085f6ec464 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Thu, 7 Nov 2024 15:00:52 +0100 Subject: [PATCH 10/17] code refactor --- packages/kbn-esql-utils/src/utils/esql_fields_utils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index d8f19b0a4377b..a8a27de7e5dcc 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -63,8 +63,6 @@ export const isESQLColumnGroupable = (column: DatatableColumn): boolean => { }; export const isESQLFieldGroupable = (field: FieldSpec): boolean => { - const isCounterTimeSeries = field.timeSeriesMetric === 'counter'; - const esType = - isCounterTimeSeries && field.esTypes?.[0] ? `counter_${field.esTypes[0]}` : field.esTypes?.[0]; - return isGroupable(field.type, esType); + if (field.timeSeriesMetric === 'counter') return false; + return isGroupable(field.type, field.esTypes?.[0]); }; From e491c5403b3866b66b6bd98c193e4ffa4f4433c6 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Fri, 8 Nov 2024 08:49:40 +0100 Subject: [PATCH 11/17] code refactor --- packages/kbn-esql-utils/src/index.ts | 6 +++++- packages/kbn-esql-utils/src/utils/esql_fields_utils.ts | 2 ++ .../containers/unified_field_list_item/field_list_item.tsx | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/kbn-esql-utils/src/index.ts b/packages/kbn-esql-utils/src/index.ts index cf530be20d7ae..6cf2dd031bf07 100644 --- a/packages/kbn-esql-utils/src/index.ts +++ b/packages/kbn-esql-utils/src/index.ts @@ -31,4 +31,8 @@ export { getStartEndParams, hasStartEndParams, } from './utils/run_query'; -export { isESQLColumnSortable, isESQLColumnGroupable } from './utils/esql_fields_utils'; +export { + isESQLColumnSortable, + isESQLColumnGroupable, + isESQLFieldGroupable, +} from './utils/esql_fields_utils'; diff --git a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts index a8a27de7e5dcc..fa0ae534197f6 100644 --- a/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts +++ b/packages/kbn-esql-utils/src/utils/esql_fields_utils.ts @@ -43,9 +43,11 @@ export const isESQLColumnSortable = (column: DatatableColumn): boolean => { // Helper function to check if a field is groupable based on its type and esType const isGroupable = (type: string | undefined, esType: string | undefined): boolean => { + // we don't allow grouping on the unknown field types if (type === UNKNOWN_FIELD) { return false; } + // we don't allow grouping on tsdb counter fields if (esType && esType.indexOf(TSDB_COUNTER_FIELDS_PREFIX) !== -1) { return false; } diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx index 60effeadde859..67923aecd3bea 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx @@ -16,7 +16,7 @@ import { Draggable } from '@kbn/dom-drag-drop'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { Filter } from '@kbn/es-query'; import { fieldSupportsBreakdown } from '@kbn/field-utils'; -import { isESQLFieldGroupable } from '@kbn/esql-utils/src/utils/esql_fields_utils'; +import { isESQLFieldGroupable } from '@kbn/esql-utils'; import type { SearchMode } from '../../types'; import { FieldItemButton, type FieldItemButtonProps } from '../../components/field_item_button'; import { From 263eaab6bc27b87873fb412cc1e12a2c8974db71 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Fri, 8 Nov 2024 09:09:11 +0100 Subject: [PATCH 12/17] export fix --- packages/kbn-esql-utils/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kbn-esql-utils/index.ts b/packages/kbn-esql-utils/index.ts index ee47c0321e2e3..9519e84624cbe 100644 --- a/packages/kbn-esql-utils/index.ts +++ b/packages/kbn-esql-utils/index.ts @@ -31,6 +31,7 @@ export { getQueryColumnsFromESQLQuery, isESQLColumnSortable, isESQLColumnGroupable, + isESQLFieldGroupable, TextBasedLanguages, } from './src'; From 98ed6474762ae45d5273ec04728efdaee5393a12 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Fri, 8 Nov 2024 09:40:54 +0100 Subject: [PATCH 13/17] icon change --- .../src/components/field_popover/field_popover_header.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx index 97c9161f1b321..e83035f30c9f8 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx @@ -110,7 +110,7 @@ export const FieldPopoverHeader: React.FC = ({ data-test-subj={`fieldPopoverHeader_addBreakdownField-${field.name}`} aria-label={addBreakdownFieldTooltip} {...(buttonAddBreakdownFieldProps || {})} - iconType="visBarVertical" + iconType="visBarVerticalStacked" onClick={() => { closePopover(); onAddBreakdownField(field); From 923e305b0c3f67f22024e0e39df8dda2d27a1f80 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 11 Nov 2024 09:33:09 +0000 Subject: [PATCH 14/17] [CI] Auto-commit changed files from 'node scripts/notice' --- packages/kbn-unified-field-list/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kbn-unified-field-list/tsconfig.json b/packages/kbn-unified-field-list/tsconfig.json index 53a749a2680de..630ce0540ee1d 100644 --- a/packages/kbn-unified-field-list/tsconfig.json +++ b/packages/kbn-unified-field-list/tsconfig.json @@ -35,6 +35,7 @@ "@kbn/search-types", "@kbn/fields-metadata-plugin", "@kbn/ui-theme", + "@kbn/esql-utils", ], "exclude": ["target/**/*"] } From 62c0e565b455d3b028ae561f0f0d113b4c2e29c9 Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Mon, 11 Nov 2024 12:53:44 +0100 Subject: [PATCH 15/17] code refactor and tests --- .../field_popover/field_popover_header.tsx | 7 +------ .../main/components/layout/discover_layout.tsx | 4 ++-- .../functional/apps/discover/esql/_esql_view.ts | 17 +++++++++++++++++ .../group1/_discover_histogram_breakdown.ts | 10 +++++++++- .../page_objects/unified_field_list.ts | 10 ++++++++++ 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx index e83035f30c9f8..92a3fd3f55e92 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx @@ -27,7 +27,6 @@ import type { AddFieldFilterHandler } from '../../types'; export interface FieldPopoverHeaderProps { field: DataViewField; closePopover: EuiPopoverProps['closePopover']; - buttonAddBreakdownFieldProps?: Partial; buttonAddFieldToWorkspaceProps?: Partial; buttonAddFilterProps?: Partial; buttonEditFieldProps?: Partial; @@ -45,7 +44,6 @@ export interface FieldPopoverHeaderProps { export const FieldPopoverHeader: React.FC = ({ field, closePopover, - buttonAddBreakdownFieldProps, buttonAddFieldToWorkspaceProps, buttonAddFilterProps, buttonEditFieldProps, @@ -103,13 +101,10 @@ export const FieldPopoverHeader: React.FC = ({     {onAddBreakdownField && ( - + { closePopover(); 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 8963ceff32d05..7784b8308053e 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 @@ -264,7 +264,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { [dataView, query] ); - const onSetBreakdownField = useCallback( + const onAddBreakdownField = useCallback( (field: DataViewField | undefined) => { stateContainer.appState.update({ breakdownField: field?.name }); }, @@ -441,7 +441,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) { additionalFilters={customFilters} columns={currentColumns} documents$={stateContainer.dataState.data$.documents$} - onAddBreakdownField={canSetBreakdownField ? onSetBreakdownField : undefined} + onAddBreakdownField={canSetBreakdownField ? onAddBreakdownField : undefined} onAddField={onAddColumnWithTracking} onAddFilter={onFilter} onChangeDataView={stateContainer.actions.onChangeDataView} diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 272de320d2051..4d19f72d6c147 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -795,6 +795,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await discover.waitUntilSearchingHasFinished(); }); + it('should choose breakdown field when selected from field stats', 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 unifiedFieldList.clickFieldListAddBreakdownField('extension'); + await header.waitUntilLoadingHasFinished(); + const list = await discover.getHistogramLegendList(); + expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); + }); + it('should choose breakdown field', async () => { await discover.selectTextBaseLang(); await header.waitUntilLoadingHasFinished(); diff --git a/test/functional/apps/discover/group1/_discover_histogram_breakdown.ts b/test/functional/apps/discover/group1/_discover_histogram_breakdown.ts index 5ce8732e5355a..6f04a6df1b6b4 100644 --- a/test/functional/apps/discover/group1/_discover_histogram_breakdown.ts +++ b/test/functional/apps/discover/group1/_discover_histogram_breakdown.ts @@ -14,11 +14,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const kibanaServer = getService('kibanaServer'); const filterBar = getService('filterBar'); - const { common, discover, header, timePicker } = getPageObjects([ + const { common, discover, header, timePicker, unifiedFieldList } = getPageObjects([ 'common', 'discover', 'header', 'timePicker', + 'unifiedFieldList', ]); describe('discover unified histogram breakdown', function describeIndexTests() { @@ -36,6 +37,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); }); + it('should apply breakdown when selected from field stats', async () => { + await unifiedFieldList.clickFieldListAddBreakdownField('geo.dest'); + await header.waitUntilLoadingHasFinished(); + const list = await discover.getHistogramLegendList(); + expect(list).to.eql(['CN', 'IN', 'US', 'Other']); + }); + it('should choose breakdown field', async () => { await discover.chooseBreakdownField('extension.raw'); await header.waitUntilLoadingHasFinished(); diff --git a/test/functional/page_objects/unified_field_list.ts b/test/functional/page_objects/unified_field_list.ts index 14006c8c5d62e..4751769f717bd 100644 --- a/test/functional/page_objects/unified_field_list.ts +++ b/test/functional/page_objects/unified_field_list.ts @@ -226,6 +226,16 @@ export class UnifiedFieldListPageObject extends FtrService { await this.testSubjects.missingOrFail(`fieldVisualize-${field}`); } + public async clickFieldListAddBreakdownField(field: string) { + const addBreakdownFieldTestSubj = `fieldPopoverHeader_addBreakdownField-${field}`; + if (!(await this.testSubjects.exists(addBreakdownFieldTestSubj))) { + // field has to be open + await this.clickFieldListItem(field); + } + await this.testSubjects.click(addBreakdownFieldTestSubj); + await this.header.waitUntilLoadingHasFinished(); + } + public async clickFieldListPlusFilter(field: string, value: string) { const plusFilterTestSubj = `plus-${field}-${value}`; if (!(await this.testSubjects.exists(plusFilterTestSubj))) { From 31978ddcf5b7eb55ac3448e4ea8adc114206aa8a Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Mon, 11 Nov 2024 15:21:17 +0100 Subject: [PATCH 16/17] Tests --- .../apps/discover/esql/_esql_view.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/test/functional/apps/discover/esql/_esql_view.ts b/test/functional/apps/discover/esql/_esql_view.ts index 4d19f72d6c147..18841d07dac3d 100644 --- a/test/functional/apps/discover/esql/_esql_view.ts +++ b/test/functional/apps/discover/esql/_esql_view.ts @@ -795,23 +795,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await discover.waitUntilSearchingHasFinished(); }); - it('should choose breakdown field when selected from field stats', 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 unifiedFieldList.clickFieldListAddBreakdownField('extension'); - await header.waitUntilLoadingHasFinished(); - const list = await discover.getHistogramLegendList(); - expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); - }); - it('should choose breakdown field', async () => { await discover.selectTextBaseLang(); await header.waitUntilLoadingHasFinished(); @@ -860,6 +843,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const list = await discover.getHistogramLegendList(); expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); }); + + it('should choose breakdown field when selected from field stats', 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 unifiedFieldList.clickFieldListAddBreakdownField('extension'); + await header.waitUntilLoadingHasFinished(); + const list = await discover.getHistogramLegendList(); + expect(list).to.eql(['css', 'gif', 'jpg', 'php', 'png']); + }); }); }); } From e0a313d90b331f024eff2a94792dca655fa11d2d Mon Sep 17 00:00:00 2001 From: mohamedhamed-ahmed Date: Tue, 12 Nov 2024 11:32:13 +0100 Subject: [PATCH 17/17] swap icon positions --- .../field_popover/field_popover_header.tsx | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx index 92a3fd3f55e92..3babd05871913 100644 --- a/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx +++ b/packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx @@ -99,21 +99,6 @@ export const FieldPopoverHeader: React.FC = ({
    {field.displayName}
     - {onAddBreakdownField && ( - - - { - closePopover(); - onAddBreakdownField(field); - }} - /> - - - )} {onAddFieldToWorkspace && ( = ({ )} + {onAddBreakdownField && ( + + + { + closePopover(); + onAddBreakdownField(field); + }} + /> + + + )} {onAddFilter && field.filterable && !field.scripted && (