From 4329bd14002f59fdd12135b378410f62b67110ac Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 3 May 2022 12:34:33 +0300 Subject: [PATCH 1/6] Enables histogram for histogram fields --- .../data/common/search/aggs/buckets/histogram.ts | 6 +++++- .../definitions/ranges/ranges.test.tsx | 16 ++++++++++++++++ .../operations/definitions/ranges/ranges.tsx | 4 +++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/plugins/data/common/search/aggs/buckets/histogram.ts b/src/plugins/data/common/search/aggs/buckets/histogram.ts index 5231bbbb91b10a..9dc318141a8ea1 100644 --- a/src/plugins/data/common/search/aggs/buckets/histogram.ts +++ b/src/plugins/data/common/search/aggs/buckets/histogram.ts @@ -87,7 +87,11 @@ export const getHistogramBucketAgg = ({ { name: 'field', type: 'field', - filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.NUMBER_RANGE], + filterFieldTypes: [ + KBN_FIELD_TYPES.NUMBER, + KBN_FIELD_TYPES.NUMBER_RANGE, + KBN_FIELD_TYPES.HISTOGRAM, + ], }, { /* diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx index 5f882a3ec21127..b87fa19f95c89e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx @@ -346,6 +346,22 @@ describe('ranges', () => { }); }); + it('should return operation with the right type for histogram', () => { + expect( + rangeOperation.getPossibleOperationForField({ + aggregatable: true, + searchable: true, + name: 'test', + displayName: 'test', + type: 'histogram', + }) + ).toEqual({ + dataType: 'number', + isBucketed: true, + scale: 'interval', + }); + }); + it('should not return operation if field type is not number', () => { expect( rangeOperation.getPossibleOperationForField({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index 11754e1f900050..d29750a60acae0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -71,6 +71,8 @@ function getFieldDefaultFormat(indexPattern: IndexPattern, field: IndexPatternFi return undefined; } +const supportedTypes = ['number', 'histogram']; + export const rangeOperation: OperationDefinition = { type: 'range', displayName: i18n.translate('xpack.lens.indexPattern.intervals', { @@ -82,7 +84,7 @@ export const rangeOperation: OperationDefinition { if ( - type === 'number' && + supportedTypes.includes(type) && aggregatable && (!aggregationRestrictions || aggregationRestrictions.range) ) { From 8c08179c06184575e2cc184752a01a8ad2baa24d Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 3 May 2022 13:33:51 +0300 Subject: [PATCH 2/6] Add support for ranges agg on histogram field --- src/plugins/data/common/search/aggs/buckets/range.ts | 2 +- .../operations/definitions/ranges/ranges.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/data/common/search/aggs/buckets/range.ts b/src/plugins/data/common/search/aggs/buckets/range.ts index e15995be5636b5..433106e1863cd1 100644 --- a/src/plugins/data/common/search/aggs/buckets/range.ts +++ b/src/plugins/data/common/search/aggs/buckets/range.ts @@ -83,7 +83,7 @@ export const getRangeBucketAgg = ({ getFieldFormatsStart }: RangeBucketAggDepend name: 'field', type: 'field', // number_range is not supported by Elasticsearch - filterFieldTypes: [KBN_FIELD_TYPES.NUMBER], + filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM], }, { name: 'ranges', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index d29750a60acae0..42eec75621ac57 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -123,7 +123,7 @@ export const rangeOperation: OperationDefinition Date: Tue, 3 May 2022 13:53:10 +0300 Subject: [PATCH 3/6] Allows auto interval for histogram fields --- .../public/components/controls/number_interval.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/vis_default_editor/public/components/controls/number_interval.tsx b/src/plugins/vis_default_editor/public/components/controls/number_interval.tsx index e1d7a058acd611..3fed3fc2ed4da0 100644 --- a/src/plugins/vis_default_editor/public/components/controls/number_interval.tsx +++ b/src/plugins/vis_default_editor/public/components/controls/number_interval.tsx @@ -74,7 +74,7 @@ function NumberIntervalParamEditor({ setValue, }: AggParamEditorProps) { const field = agg.getField(); - const fieldSupportsAuto = !field || field.type === 'number'; + const fieldSupportsAuto = !field || field.type === 'number' || field.type === 'histogram'; const isAutoChecked = fieldSupportsAuto && isAutoInterval(value); const base: number = get(editorConfig, 'interval.base') as number; const min = base || 0; From 48b41b87ef50b6a3b42d8722231dbd970ede681e Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 4 May 2022 11:14:14 +0300 Subject: [PATCH 4/6] Display a helper error in case a non-count aggregation or a breakdown by bucket is given --- .../dimension_panel/dimension_editor.tsx | 3 +- .../public/indexpattern_datasource/mocks.ts | 7 ++ .../operations/definitions/helpers.test.ts | 109 +++++++++++++++++- .../operations/definitions/helpers.tsx | 39 ++++++- .../operations/definitions/index.ts | 3 +- .../definitions/ranges/ranges.test.tsx | 69 +++++++++++ .../operations/definitions/ranges/ranges.tsx | 20 +++- 7 files changed, 243 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 790873fdc74b2d..18d7fa6909dc22 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -265,7 +265,8 @@ export function DimensionEditor(props: DimensionEditorProps) { definition.getDisabledStatus( state.indexPatterns[state.currentIndexPatternId], state.layers[layerId], - layerType + layerType, + columnId ), }; }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts index 079a866676f04e..6a782357fa6b71 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts @@ -93,6 +93,13 @@ export const createMockedIndexPattern = (): IndexPattern => { lang: 'painless' as const, script: '1234', }, + { + name: 'my_histogram', + displayName: 'my_histogram', + type: 'histogram', + aggregatable: true, + searchable: true, + }, ]; return { id: '1', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts index 616c1f5719cc60..3344b3c775ec67 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts @@ -6,7 +6,8 @@ */ import { createMockedIndexPattern } from '../../mocks'; -import { getInvalidFieldMessage } from './helpers'; +import type { IndexPatternLayer } from '../../types'; +import { getInvalidFieldMessage, getErrorsForHistogramField } from './helpers'; import type { TermsIndexPatternColumn } from './terms'; describe('helpers', () => { @@ -127,4 +128,110 @@ describe('helpers', () => { expect(messages).toBeUndefined(); }); }); + + describe('getErrorsForHistogramField', () => { + const layer = { + columns: { + '731ee6a5-da3d-4b0f-8f37-ffeb539a7980': { + label: 'Count of records', + dataType: 'number', + operationType: 'count', + isBucketed: false, + scale: 'ratio', + sourceField: '___records___', + params: { + emptyAsNull: true, + }, + }, + 'd682b1d9-ce53-4443-a1e6-959197a314a6': { + label: 'my_histogram', + dataType: 'number', + operationType: 'range', + sourceField: 'my_histogram', + isBucketed: true, + scale: 'interval', + params: { + includeEmptyRows: true, + type: 'histogram', + ranges: [ + { + from: 0, + to: 1000, + label: '', + }, + ], + maxBars: 'auto', + }, + }, + }, + columnOrder: ['d682b1d9-ce53-4443-a1e6-959197a314a6', '731ee6a5-da3d-4b0f-8f37-ffeb539a7980'], + incompleteColumns: {}, + indexPatternId: '1', + } as IndexPatternLayer; + it('return no error if a count aggregation is given', () => { + const messages = getErrorsForHistogramField( + layer, + 'd682b1d9-ce53-4443-a1e6-959197a314a6', + createMockedIndexPattern() + ); + expect(messages).toBeUndefined(); + }); + + it('returns an error if a metric is non a count aggregation', () => { + layer.columns['731ee6a5-da3d-4b0f-8f37-ffeb539a7980'].operationType = 'average'; + const messages = getErrorsForHistogramField( + layer, + 'd682b1d9-ce53-4443-a1e6-959197a314a6', + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual( + 'Histogram fields can only be used with a count aggregation. Please remove the histogram field or change the metric to a count or remove the breakdown dimension.' + ); + }); + + it('returns an error if a metric is a count aggregation and a breakdown is also given', () => { + layer.columns['731ee6a5-da3d-4b0f-8f37-ffeb539a7980'].operationType = 'count'; + const newLayer = { + ...layer, + columns: { + ...layer.columns, + 'ef5fa77a-b1f7-405e-9551-6b533aafa114': { + label: 'bytes', + dataType: 'number', + operationType: 'range', + sourceField: 'bytes', + isBucketed: true, + scale: 'interval', + params: { + includeEmptyRows: true, + type: 'histogram', + ranges: [ + { + from: 0, + to: 1000, + label: '', + }, + ], + maxBars: 'auto', + }, + }, + }, + columnOrder: [ + 'd682b1d9-ce53-4443-a1e6-959197a314a6', + '731ee6a5-da3d-4b0f-8f37-ffeb539a7980', + 'ef5fa77a-b1f7-405e-9551-6b533aafa114', + ], + } as IndexPatternLayer; + const messages = getErrorsForHistogramField( + newLayer, + 'd682b1d9-ce53-4443-a1e6-959197a314a6', + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual( + 'Histogram fields can only be used with a count aggregation. Please remove the histogram field or change the metric to a count or remove the breakdown dimension.' + ); + }); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index c464ce0da027c2..203a95431fc135 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -12,7 +12,7 @@ import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn, } from './column_types'; -import { IndexPattern, IndexPatternField } from '../../types'; +import { IndexPattern, IndexPatternField, IndexPatternLayer } from '../../types'; import { hasField } from '../../pure_utils'; export function getInvalidFieldMessage( @@ -87,6 +87,43 @@ export function getInvalidFieldMessage( return undefined; } +export function getErrorsForHistogramField( + layer: IndexPatternLayer, + columnId: string, + indexPattern: IndexPattern +) { + // check if has field of histogram type + const column = layer.columns[columnId]; + const { operationType } = column; + const operationDefinition = operationType ? operationDefinitionMap[operationType] : undefined; + const fieldNames = + hasField(column) && operationDefinition + ? operationDefinition?.getCurrentFields?.(column) ?? [column.sourceField] + : []; + const fields = fieldNames.map((fieldName) => indexPattern.getFieldByName(fieldName)); + const filteredFields = fields.filter(Boolean) as IndexPatternField[]; + const hasHistogramField = filteredFields.some((field) => field.type === 'histogram'); + // check if metric is not count + const metric = layer.columnOrder.filter((colId) => !layer.columns[colId].isBucketed); + const hasCountAggregation = metric.some( + (colId) => layer.columns[colId].operationType === 'count' + ); + + // check if other bucket is present + const otherBucketsExist = layer.columnOrder.some( + (colId) => layer.columns[colId].isBucketed && colId !== columnId + ); + + if (hasHistogramField && (!hasCountAggregation || otherBucketsExist)) { + return [ + i18n.translate('xpack.lens.indexPattern.histogramFieldsWithNoCount', { + defaultMessage: + 'Histogram fields can only be used with a count aggregation. Please remove the histogram field or change the metric to a count or remove the breakdown dimension.', + }), + ]; + } +} + export function combineErrorMessages( errorMessages: Array ): string[] | undefined { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts index 36039a058908be..85dfa9dd821f5d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts @@ -273,7 +273,8 @@ interface BaseOperationDefinitionProps getDisabledStatus?: ( indexPattern: IndexPattern, layer: IndexPatternLayer, - layerType?: LayerType + layerType?: LayerType, + columnId?: string ) => string | undefined; /** * Validate that the operation has the right preconditions in the state. For example: diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx index b87fa19f95c89e..e1bccecf543d1c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx @@ -947,4 +947,73 @@ describe('ranges', () => { }); }); }); + + describe('getErrorMessage', () => { + it('should return no error for valid values', () => { + expect( + rangeOperation.getErrorMessage!(layer, 'col1', defaultOptions.indexPattern) + ).toBeUndefined(); + }); + + it('should return error for histogram with no count aggregation values', () => { + const updatedIndexPattern = { + ...defaultOptions.indexPattern, + fields: [ + { + name: sourceField, + type: 'number', + displayName: sourceField, + searchable: true, + aggregatable: true, + }, + { + name: 'my_histogram', + displayName: 'my_histogram', + type: 'histogram', + aggregatable: true, + searchable: true, + }, + ], + getFieldByName: getFieldByNameFactory([ + { + name: 'my_histogram', + type: 'histogram', + displayName: 'my_histogram', + searchable: true, + aggregatable: true, + }, + ]), + }; + const updatedLayer = { + ...layer, + columns: { + col1: { + label: 'my_histogram', + dataType: 'number', + operationType: 'range', + scale: 'interval', + isBucketed: true, + sourceField: 'my_histogram', + params: { + type: MODES.Histogram, + ranges: [{ from: 0, to: DEFAULT_INTERVAL, label: '' }], + maxBars: 'auto', + }, + } as RangeIndexPatternColumn, + col2: { + label: 'Average', + dataType: 'number', + isBucketed: false, + sourceField, + operationType: 'average', + }, + }, + } as IndexPatternLayer; + expect(rangeOperation.getErrorMessage!(updatedLayer, 'col1', updatedIndexPattern)).toEqual( + expect.arrayContaining([ + expect.stringMatching('Histogram fields can only be used with a count aggregation'), + ]) + ); + }); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index 42eec75621ac57..4846f2fb63bd06 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -18,7 +18,12 @@ import { updateColumnParam } from '../../layer_helpers'; import { supportedFormats } from '../../../../../common/expressions/format_column/supported_formats'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; -import { getInvalidFieldMessage, isValidNumber } from '../helpers'; +import { + getInvalidFieldMessage, + isValidNumber, + combineErrorMessages, + getErrorsForHistogramField, +} from '../helpers'; type RangeType = Omit; // Try to cover all possible serialized states for ranges @@ -80,8 +85,17 @@ export const rangeOperation: OperationDefinition - getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), + getErrorMessage: (layer, columnId, indexPattern) => { + return combineErrorMessages([ + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), + getErrorsForHistogramField(layer, columnId, indexPattern), + ]); + }, + getDisabledStatus(indexPattern, layer, layerType, columnId) { + if (columnId) { + return getErrorsForHistogramField(layer, columnId, indexPattern)?.join(', '); + } + }, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( supportedTypes.includes(type) && From 1c9252b0f8aab14320b40627f167d6eb5f044e9f Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 4 May 2022 11:44:43 +0300 Subject: [PATCH 5/6] Fix jest test --- .../indexpattern_datasource/operations/definitions/helpers.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index 203a95431fc135..fbc0199d4b6ad2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -94,6 +94,7 @@ export function getErrorsForHistogramField( ) { // check if has field of histogram type const column = layer.columns[columnId]; + if (!column) return; const { operationType } = column; const operationDefinition = operationType ? operationDefinitionMap[operationType] : undefined; const fieldNames = From bd124ba7d4a93aa3bf728127d92c91ca366bb179 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 4 May 2022 12:23:48 +0300 Subject: [PATCH 6/6] Refactor helpers jest test --- .../public/indexpattern_datasource/mocks.ts | 7 ----- .../operations/definitions/helpers.test.ts | 31 +++++++++++++++++-- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts index 6a782357fa6b71..079a866676f04e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts @@ -93,13 +93,6 @@ export const createMockedIndexPattern = (): IndexPattern => { lang: 'painless' as const, script: '1234', }, - { - name: 'my_histogram', - displayName: 'my_histogram', - type: 'histogram', - aggregatable: true, - searchable: true, - }, ]; return { id: '1', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts index 3344b3c775ec67..f9d876ce5e728f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts @@ -6,6 +6,7 @@ */ import { createMockedIndexPattern } from '../../mocks'; +import { getFieldByNameFactory } from '../../pure_helpers'; import type { IndexPatternLayer } from '../../types'; import { getInvalidFieldMessage, getErrorsForHistogramField } from './helpers'; import type { TermsIndexPatternColumn } from './terms'; @@ -168,11 +169,35 @@ describe('helpers', () => { incompleteColumns: {}, indexPatternId: '1', } as IndexPatternLayer; + const indexPattern = createMockedIndexPattern(); + const updatedIndexPattern = { + ...indexPattern, + fields: [ + ...indexPattern.fields, + { + name: 'my_histogram', + displayName: 'my_histogram', + type: 'histogram', + aggregatable: true, + searchable: true, + }, + ], + getFieldByName: getFieldByNameFactory([ + { + name: 'my_histogram', + type: 'histogram', + displayName: 'my_histogram', + searchable: true, + aggregatable: true, + }, + ]), + }; + it('return no error if a count aggregation is given', () => { const messages = getErrorsForHistogramField( layer, 'd682b1d9-ce53-4443-a1e6-959197a314a6', - createMockedIndexPattern() + updatedIndexPattern ); expect(messages).toBeUndefined(); }); @@ -182,7 +207,7 @@ describe('helpers', () => { const messages = getErrorsForHistogramField( layer, 'd682b1d9-ce53-4443-a1e6-959197a314a6', - createMockedIndexPattern() + updatedIndexPattern ); expect(messages).toHaveLength(1); expect(messages![0]).toEqual( @@ -226,7 +251,7 @@ describe('helpers', () => { const messages = getErrorsForHistogramField( newLayer, 'd682b1d9-ce53-4443-a1e6-959197a314a6', - createMockedIndexPattern() + updatedIndexPattern ); expect(messages).toHaveLength(1); expect(messages![0]).toEqual(