From 16cd1037bab2969cc73940e1dd56a469c299337a Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 18 Dec 2020 18:39:26 +0100 Subject: [PATCH 1/5] add percentile --- .../operations/definitions/helpers.tsx | 9 +- .../operations/definitions/index.ts | 25 ++- .../operations/definitions/metrics.tsx | 25 ++- .../operations/definitions/percentile.tsx | 197 ++++++++++++++++++ .../operations/definitions/terms/index.tsx | 23 +- .../indexpattern_datasource/to_expression.ts | 10 +- 6 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx 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 640a357d9a7a4..25956c1b10dbe 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 @@ -7,7 +7,7 @@ import { useRef } from 'react'; import useDebounce from 'react-use/lib/useDebounce'; import { i18n } from '@kbn/i18n'; -import { operationDefinitionMap } from '.'; +import { IndexPatternColumn, operationDefinitionMap } from '.'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPattern } from '../../types'; @@ -62,3 +62,10 @@ export function getInvalidFieldMessage( ] : undefined; } + +export function getEsAggsSuffix(column: IndexPatternColumn) { + const operationDefinition = operationDefinitionMap[column.operationType]; + return operationDefinition.input === 'field' && operationDefinition.getEsAggsSuffix + ? operationDefinition.getEsAggsSuffix(column) + : ''; +} 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 1f19b4e770313..d63576e6c2b7a 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 @@ -9,6 +9,7 @@ import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { termsOperation, TermsIndexPatternColumn } from './terms'; import { filtersOperation, FiltersIndexPatternColumn } from './filters'; import { cardinalityOperation, CardinalityIndexPatternColumn } from './cardinality'; +import { percentileOperation, PercentileIndexPatternColumn } from './percentile'; import { minOperation, MinIndexPatternColumn, @@ -63,6 +64,7 @@ export type IndexPatternColumn = | CardinalityIndexPatternColumn | SumIndexPatternColumn | MedianIndexPatternColumn + | PercentileIndexPatternColumn | CountIndexPatternColumn | LastValueIndexPatternColumn | CumulativeSumIndexPatternColumn @@ -87,6 +89,7 @@ const internalOperationDefinitions = [ cardinalityOperation, sumOperation, medianOperation, + percentileOperation, lastValueOperation, countOperation, rangeOperation, @@ -101,6 +104,7 @@ export { rangeOperation } from './ranges'; export { filtersOperation } from './filters'; export { dateHistogramOperation } from './date_histogram'; export { minOperation, averageOperation, sumOperation, maxOperation } from './metrics'; +export { percentileOperation } from './percentile'; export { countOperation } from './count'; export { lastValueOperation } from './last_value'; export { @@ -227,7 +231,12 @@ interface FieldlessOperationDefinition { * Function turning a column into an agg config passed to the `esaggs` function * together with the agg configs returned from other columns. */ - toEsAggsFn: (column: C, columnId: string, indexPattern: IndexPattern) => ExpressionAstFunction; + toEsAggsFn: ( + column: C, + columnId: string, + indexPattern: IndexPattern, + layer: IndexPatternLayer + ) => ExpressionAstFunction; } interface FieldBasedOperationDefinition { @@ -266,7 +275,19 @@ interface FieldBasedOperationDefinition { * Function turning a column into an agg config passed to the `esaggs` function * together with the agg configs returned from other columns. */ - toEsAggsFn: (column: C, columnId: string, indexPattern: IndexPattern) => ExpressionAstFunction; + toEsAggsFn: ( + column: C, + columnId: string, + indexPattern: IndexPattern, + layer: IndexPatternLayer + ) => ExpressionAstFunction; + /** + * Optional function to return the suffix used for ES bucket paths and esaggs column id. + * This is relevant for multi metrics to pick the right value. + * + * @param column The current column + */ + getEsAggsSuffix?: (column: C) => string; /** * 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/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index a886bfdaad325..45613056b9b66 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -85,17 +85,20 @@ function buildMetricOperation>({ optionalTimeScaling ? adjustTimeScaleOnOtherColumnChange(column, otherColumns) : column, getDefaultLabel: (column, indexPattern, columns) => labelLookup(indexPattern.getFieldByName(column.sourceField)!.displayName, column), - buildColumn: ({ field, previousColumn }) => ({ - label: labelLookup(field.displayName, previousColumn), - dataType: 'number', - operationType: type, - sourceField: field.name, - isBucketed: false, - scale: 'ratio', - timeScale: optionalTimeScaling ? previousColumn?.timeScale : undefined, - params: - previousColumn && previousColumn.dataType === 'number' ? previousColumn.params : undefined, - }), + buildColumn: ({ field, previousColumn }) => + ({ + label: labelLookup(field.displayName, previousColumn), + dataType: 'number', + operationType: type, + sourceField: field.name, + isBucketed: false, + scale: 'ratio', + timeScale: optionalTimeScaling ? previousColumn?.timeScale : undefined, + params: + previousColumn && previousColumn.dataType === 'number' + ? previousColumn.params + : undefined, + } as T), onFieldChange: (oldColumn, field) => { return { ...oldColumn, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx new file mode 100644 index 0000000000000..36cf91f27d192 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -0,0 +1,197 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EuiFieldNumber, EuiFormRow } from '@elastic/eui'; +import React, { useCallback, useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { AggFunctionsMapping } from 'src/plugins/data/public'; +import useDebounce from 'react-use/lib/useDebounce'; +import { buildExpressionFunction } from '../../../../../../../src/plugins/expressions/public'; +import { OperationDefinition } from './index'; +import { getInvalidFieldMessage } from './helpers'; +import { FieldBasedIndexPatternColumn } from './column_types'; +import { mergeLayer } from '../../state_helpers'; + +export interface PercentileIndexPatternColumn extends FieldBasedIndexPatternColumn { + operationType: 'percentile'; + params: { + percentile: number; + format?: { + id: string; + params?: { + decimals: number; + }; + }; + }; +} + +function ofName(name: string, percentile: number) { + return i18n.translate('xpack.lens.indexPattern.percentileOf', { + defaultMessage: + '{percentile, selectordinal, one {#st} two {#nd} few {#rd} other {#th}} percentile of {name}', + values: { name, percentile }, + }); +} + +const DEFAULT_PERCENTILE_VALUE = 93; + +export const percentileOperation: OperationDefinition = { + type: 'percentile', + displayName: i18n.translate('xpack.lens.indexPattern.percentile', { + defaultMessage: 'Percentile', + }), + input: 'field', + getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { + if ( + fieldType === 'number' && + aggregatable && + (!aggregationRestrictions || aggregationRestrictions.percentile) + ) { + return { + dataType: 'number', + isBucketed: false, + scale: 'ratio', + }; + } + }, + isTransferable: (column, newIndexPattern) => { + const newField = newIndexPattern.getFieldByName(column.sourceField); + + return Boolean( + newField && + newField.type === 'number' && + newField.aggregatable && + (!newField.aggregationRestrictions || newField.aggregationRestrictions.percentile) + ); + }, + getDefaultLabel: (column, indexPattern, columns) => + ofName(indexPattern.getFieldByName(column.sourceField)!.displayName, column.params.percentile), + buildColumn: ({ field, previousColumn }) => { + const existingFormat = + previousColumn?.params && 'format' in previousColumn?.params + ? previousColumn?.params?.format + : undefined; + const existingPercentileParam = + previousColumn?.operationType === 'percentile' && previousColumn?.params.percentile; + const newPercentileParam = existingPercentileParam || DEFAULT_PERCENTILE_VALUE; + return { + label: ofName(field.displayName, newPercentileParam), + dataType: 'number', + operationType: 'percentile', + sourceField: field.name, + isBucketed: false, + scale: 'ratio', + params: { + format: existingFormat, + percentile: newPercentileParam, + }, + }; + }, + onFieldChange: (oldColumn, field) => { + return { + ...oldColumn, + label: ofName(field.displayName, oldColumn.params.percentile), + sourceField: field.name, + }; + }, + toEsAggsFn: (column, columnId, _indexPattern) => { + return buildExpressionFunction('aggPercentiles', { + id: columnId, + enabled: true, + schema: 'metric', + field: column.sourceField, + percents: [column.params.percentile], + }).toAst(); + }, + getEsAggsSuffix: (column) => { + const value = column.params.percentile; + return `.${value}`; + }, + getErrorMessage: (layer, columnId, indexPattern) => + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), + paramEditor: function PercentileParamEditor({ + state, + setState, + currentColumn, + layerId, + columnId, + }) { + const currentIndexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; + const layer = state.layers[layerId]; + const [inputValue, setInputValue] = useState(String(currentColumn.params.percentile)); + + const inputValueAsNumber = Number(inputValue); + const inputValueIsValid = + !Number.isNaN(inputValueAsNumber) && + inputValueAsNumber >= 0 && + inputValueAsNumber <= 100 && + inputValueAsNumber % 1 === 0; + + useDebounce( + () => { + if (!inputValueIsValid) return; + setState( + mergeLayer({ + state, + layerId, + newLayer: { + columns: { + ...layer.columns, + [columnId]: { + ...currentColumn, + label: currentColumn.customLabel + ? currentColumn.label + : ofName( + currentIndexPattern.getFieldByName(currentColumn.sourceField) + ?.displayName || currentColumn.sourceField, + inputValueAsNumber + ), + params: { + ...currentColumn.params, + percentile: inputValueAsNumber, + }, + }, + }, + }, + }) + ); + }, + 256, + [inputValue] + ); + + const handleInputChange = useCallback((e: React.ChangeEvent) => { + const val = String(e.target.value); + setInputValue(val); + }, []); + return ( + <> + + + + + ); + }, +}; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx index 1e2d800cb3e80..aa77c15314ca0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx @@ -24,7 +24,7 @@ import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesRangeInput } from './values_range_input'; -import { getInvalidFieldMessage } from '../helpers'; +import { getEsAggsSuffix, getInvalidFieldMessage } from '../helpers'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { @@ -88,11 +88,9 @@ export const termsOperation: OperationDefinition column && !column.isBucketed && !isReferenced(layer, columnId) - ) - .map(([id]) => id)[0]; + const [existingMetricColumn] = Object.entries(layer.columns).filter( + ([columnId, column]) => column && !column.isBucketed && !isReferenced(layer, columnId) + )[0]; const previousBucketsLength = Object.values(layer.columns).filter( (col) => col && col.isBucketed @@ -108,7 +106,10 @@ export const termsOperation: OperationDefinition { + toEsAggsFn: (column, columnId, _indexPattern, layer) => { return buildExpressionFunction('aggTerms', { id: columnId, enabled: true, schema: 'segment', field: column.sourceField, orderBy: - column.params.orderBy.type === 'alphabetical' ? '_key' : column.params.orderBy.columnId, + column.params.orderBy.type === 'alphabetical' + ? '_key' + : `${column.params.orderBy.columnId}${getEsAggsSuffix( + layer.columns[column.params.orderBy.columnId] + )}`, order: column.params.orderDirection, size: column.params.size, otherBucket: Boolean(column.params.otherBucket), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index a85b8920366b5..574aee862af1a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -20,6 +20,7 @@ import { operationDefinitionMap } from './operations'; import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types'; import { OriginalColumn } from './rename_columns'; import { dateHistogramOperation } from './operations/definitions'; +import { getEsAggsSuffix } from './operations/definitions/helpers'; function getExpressionForLayer( layer: IndexPatternLayer, @@ -41,15 +42,20 @@ function getExpressionForLayer( expressions.push(...def.toExpression(layer, colId, indexPattern)); } else { aggs.push( - buildExpression({ type: 'expression', chain: [def.toEsAggsFn(col, colId, indexPattern)] }) + buildExpression({ + type: 'expression', + chain: [def.toEsAggsFn(col, colId, indexPattern, layer)], + }) ); } }); const idMap = columnEntries.reduce((currentIdMap, [colId, column], index) => { + const esAggsId = `col-${columnEntries.length === 1 ? 0 : index}-${colId}`; + const suffix = getEsAggsSuffix(column); return { ...currentIdMap, - [`col-${columnEntries.length === 1 ? 0 : index}-${colId}`]: { + [`${esAggsId}${suffix}`]: { ...column, id: colId, }, From f108001c0a17beb976f0266724998f4073589b4e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 21 Dec 2020 15:02:08 +0100 Subject: [PATCH 2/5] add tests and clean up --- .../dimension_panel/dimension_panel.test.tsx | 2 + .../indexpattern.test.ts | 47 ++++ .../definitions/date_histogram.test.tsx | 6 +- .../definitions/filters/filters.test.tsx | 3 +- .../definitions/last_value.test.tsx | 3 +- .../definitions/percentile.test.tsx | 255 ++++++++++++++++++ .../operations/definitions/percentile.tsx | 12 +- .../definitions/ranges/ranges.test.tsx | 12 +- .../definitions/terms/terms.test.tsx | 45 +++- .../operations/operations.test.ts | 5 + 10 files changed, 373 insertions(+), 17 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx index 6bfeafd41c6b4..b4fb1a0b452e2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx @@ -1321,8 +1321,10 @@ describe('IndexPatternDimensionEditorPanel', () => { 'Maximum', 'Median', 'Minimum', + 'Percentile', 'Sum', 'Unique count', + '\u00a0', ]); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts index 042ea0353ac63..4a29716f465d6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -474,6 +474,53 @@ describe('IndexPattern Data Source', () => { expect(ast.chain[0].arguments.timeFields).toEqual(['timestamp', 'another_datefield']); }); + it('should add the suffix to the remap column id if provided by the operation', async () => { + const queryBaseState: IndexPatternBaseState = { + currentIndexPatternId: '1', + layers: { + first: { + indexPatternId: '1', + columnOrder: ['def', 'abc'], + columns: { + abc: { + label: '23rd percentile', + dataType: 'number', + isBucketed: false, + sourceField: 'bytes', + operationType: 'percentile', + params: { + percentile: 23, + }, + }, + def: { + label: 'Terms', + dataType: 'string', + isBucketed: true, + operationType: 'terms', + sourceField: 'source', + params: { + size: 5, + orderBy: { + type: 'alphabetical', + }, + orderDirection: 'asc', + }, + }, + }, + }, + }, + }; + + const state = enrichBaseState(queryBaseState); + + const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; + expect(Object.keys(JSON.parse(ast.chain[1].arguments.idMap[0] as string))).toEqual([ + 'col-0-def', + // col-1 is the auto naming of esasggs, abc is the specified column id, .23 is the generated suffix + 'col-1-abc.23', + ]); + }); + it('should add time_scale and format function if time scale is set and supported', async () => { const queryBaseState: IndexPatternBaseState = { currentIndexPatternId: '1', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx index e40b9ccf89c1a..097455896803d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx @@ -248,7 +248,8 @@ describe('date_histogram', () => { const esAggsFn = dateHistogramOperation.toEsAggsFn( state.layers.first.columns.col1 as DateHistogramIndexPatternColumn, 'col1', - state.indexPatterns['1'] + state.indexPatterns['1'], + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ @@ -299,7 +300,8 @@ describe('date_histogram', () => { }, }, ]), - } + }, + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx index bb8e52ba443a2..c24a58c06577e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx @@ -90,7 +90,8 @@ describe('filters', () => { const esAggsFn = filtersOperation.toEsAggsFn( state.layers.first.columns.col1 as FiltersIndexPatternColumn, 'col1', - state.indexPatterns['1'] + state.indexPatterns['1'], + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 6c896adfce9b1..6ebe7926c70c5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -80,7 +80,8 @@ describe('last_value', () => { const esAggsFn = lastValueOperation.toEsAggsFn( { ...lastValueColumn, params: { ...lastValueColumn.params } }, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx new file mode 100644 index 0000000000000..1ca537b86f83a --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx @@ -0,0 +1,255 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { shallow, mount } from 'enzyme'; +import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import { dataPluginMock } from '../../../../../../../src/plugins/data/public/mocks'; +import { createMockedIndexPattern } from '../../mocks'; +import { percentileOperation } from './index'; +import { IndexPatternPrivateState, IndexPattern } from '../../types'; +import { PercentileIndexPatternColumn } from './percentile'; +import { EuiFieldNumber } from '@elastic/eui'; +import { act } from 'react-dom/test-utils'; +import { EuiFormRow } from '@elastic/eui'; + +const defaultProps = { + storage: {} as IStorageWrapper, + uiSettings: {} as IUiSettingsClient, + savedObjectsClient: {} as SavedObjectsClientContract, + dateRange: { fromDate: 'now-1d', toDate: 'now' }, + data: dataPluginMock.createStartContract(), + http: {} as HttpSetup, +}; + +describe('percentile', () => { + let state: IndexPatternPrivateState; + const InlineOptions = percentileOperation.paramEditor!; + + beforeEach(() => { + const indexPattern = createMockedIndexPattern(); + state = { + indexPatternRefs: [], + indexPatterns: { + '1': { + ...indexPattern, + hasRestrictions: false, + } as IndexPattern, + }, + existingFields: {}, + currentIndexPatternId: '1', + isFirstExistenceFetch: false, + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, + operationType: 'terms', + params: { + orderBy: { type: 'alphabetical' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }, + col2: { + label: '23rd percentile of a', + dataType: 'number', + isBucketed: false, + sourceField: 'a', + operationType: 'percentile', + params: { + percentile: 23, + }, + }, + }, + }, + }, + }; + }); + + describe('toEsAggsFn', () => { + it('should reflect params correctly', () => { + const percentileColumn = state.layers.first.columns.col2 as PercentileIndexPatternColumn; + const esAggsFn = percentileOperation.toEsAggsFn( + percentileColumn, + 'col1', + {} as IndexPattern, + state.layers.first + ); + expect(esAggsFn).toEqual( + expect.objectContaining({ + arguments: expect.objectContaining({ + percents: [23], + field: ['a'], + }), + }) + ); + }); + }); + + describe('onFieldChange', () => { + it('should change correctly to new field', () => { + const oldColumn: PercentileIndexPatternColumn = { + operationType: 'percentile', + sourceField: 'bytes', + label: '23rd percentile of bytes', + isBucketed: true, + dataType: 'number', + params: { + percentile: 23, + }, + }; + const indexPattern = createMockedIndexPattern(); + const newNumberField = indexPattern.getFieldByName('memory')!; + const column = percentileOperation.onFieldChange(oldColumn, newNumberField); + + expect(column).toEqual( + expect.objectContaining({ + dataType: 'number', + sourceField: 'memory', + params: expect.objectContaining({ + percentile: 23, + }), + }) + ); + expect(column.label).toContain('memory'); + }); + }); + + describe('buildColumn', () => { + it('should set default percentile', () => { + const percentileColumn = percentileOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + field: { + aggregatable: true, + searchable: true, + type: 'number', + name: 'bytes', + displayName: 'test', + }, + layer: { columns: {}, columnOrder: [], indexPatternId: '' }, + }); + expect(percentileColumn.dataType).toEqual('number'); + expect(percentileColumn.params.percentile).toEqual(95); + expect(percentileColumn.label).toEqual('95th percentile of test'); + }); + }); + + describe('param editor', () => { + it('should render current percentile', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + + const input = instance.find('[data-test-subj="lns-indexPattern-percentile-input"]'); + + expect(input.prop('value')).toEqual('23'); + }); + + it('should update state on change', async () => { + jest.useFakeTimers(); + const setStateSpy = jest.fn(); + const instance = mount( + + ); + + const input = instance + .find('[data-test-subj="lns-indexPattern-percentile-input"]') + .find(EuiFieldNumber); + + await act(async () => { + input.prop('onChange')!({ target: { value: '27' } } as React.ChangeEvent); + }); + + instance.update(); + + jest.runAllTimers(); + + expect(setStateSpy).toHaveBeenCalledWith({ + ...state, + layers: { + first: { + ...state.layers.first, + columns: { + ...state.layers.first.columns, + col2: { + ...state.layers.first.columns.col2, + params: { + percentile: 27, + }, + label: '27th percentile of a', + }, + }, + }, + }, + }); + }); + + it('should not update on invalid input, but show invalid value locally', async () => { + const setStateSpy = jest.fn(); + const instance = mount( + + ); + + const input = instance + .find('[data-test-subj="lns-indexPattern-percentile-input"]') + .find(EuiFieldNumber); + + await act(async () => { + input.prop('onChange')!({ + target: { value: '12.12' }, + } as React.ChangeEvent); + }); + + instance.update(); + + jest.runAllTimers(); + + expect(setStateSpy).not.toHaveBeenCalled(); + + expect( + instance + .find('[data-test-subj="lns-indexPattern-percentile-form"]') + .find(EuiFormRow) + .prop('isInvalid') + ).toEqual(true); + expect( + instance + .find('[data-test-subj="lns-indexPattern-percentile-input"]') + .find(EuiFieldNumber) + .prop('value') + ).toEqual('12.12'); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index 36cf91f27d192..411b8bec1877d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -36,7 +36,7 @@ function ofName(name: string, percentile: number) { }); } -const DEFAULT_PERCENTILE_VALUE = 93; +const DEFAULT_PERCENTILE_VALUE = 95; export const percentileOperation: OperationDefinition = { type: 'percentile', @@ -45,11 +45,7 @@ export const percentileOperation: OperationDefinition { - if ( - fieldType === 'number' && - aggregatable && - (!aggregationRestrictions || aggregationRestrictions.percentile) - ) { + if (fieldType === 'number' && aggregatable && !aggregationRestrictions) { return { dataType: 'number', isBucketed: false, @@ -64,7 +60,7 @@ export const percentileOperation: OperationDefinition @@ -173,6 +169,7 @@ export const percentileOperation: OperationDefinition { const esAggsFn = rangeOperation.toEsAggsFn( state.layers.first.columns.col1 as RangeIndexPatternColumn, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toMatchInlineSnapshot(` Object { @@ -194,7 +195,8 @@ describe('ranges', () => { const esAggsFn = rangeOperation.toEsAggsFn( state.layers.first.columns.col1 as RangeIndexPatternColumn, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toEqual( @@ -213,7 +215,8 @@ describe('ranges', () => { const esAggsFn = rangeOperation.toEsAggsFn( state.layers.first.columns.col1 as RangeIndexPatternColumn, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toEqual( @@ -232,7 +235,8 @@ describe('ranges', () => { const esAggsFn = rangeOperation.toEsAggsFn( state.layers.first.columns.col1 as RangeIndexPatternColumn, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect((esAggsFn as { arguments: unknown }).arguments).toEqual( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index fc8f8534bcfb2..a18eaf3e3cbb5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -77,7 +77,8 @@ describe('terms', () => { const esAggsFn = termsOperation.toEsAggsFn( { ...termsColumn, params: { ...termsColumn.params, otherBucket: true } }, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ @@ -99,7 +100,8 @@ describe('terms', () => { params: { ...termsColumn.params, otherBucket: false, missingBucket: true }, }, 'col1', - {} as IndexPattern + {} as IndexPattern, + state.layers.first ); expect(esAggsFn).toEqual( expect.objectContaining({ @@ -110,6 +112,45 @@ describe('terms', () => { }) ); }); + + it('should include esaggs suffix from other columns in orderby argument', () => { + const termsColumn = state.layers.first.columns.col1 as TermsIndexPatternColumn; + const esAggsFn = termsOperation.toEsAggsFn( + { + ...termsColumn, + params: { + ...termsColumn.params, + otherBucket: true, + orderBy: { type: 'column', columnId: 'abcde' }, + }, + }, + 'col1', + {} as IndexPattern, + { + ...state.layers.first, + columns: { + ...state.layers.first.columns, + abcde: { + dataType: 'number', + isBucketed: false, + operationType: 'percentile', + sourceField: 'abc', + label: '', + params: { + percentile: 12, + }, + }, + }, + } + ); + expect(esAggsFn).toEqual( + expect.objectContaining({ + arguments: expect.objectContaining({ + orderBy: ['abcde.12'], + }), + }) + ); + }); }); describe('onFieldChange', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index 9f2b8eab4e09b..882252132c5b3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -293,6 +293,11 @@ describe('getOperationTypesForField', () => { "operationType": "median", "type": "field", }, + Object { + "field": "bytes", + "operationType": "percentile", + "type": "field", + }, Object { "field": "bytes", "operationType": "last_value", From 1c0c4d6effcbfeb2e1a71957f39011c3cecaf7ae Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 22 Dec 2020 18:21:28 +0100 Subject: [PATCH 3/5] code review --- .../calculations/moving_average.tsx | 17 ++------------ .../operations/definitions/helpers.tsx | 21 ++++++++++++++++++ .../operations/definitions/percentile.tsx | 22 ++++++++----------- .../definitions/ranges/advanced_editor.tsx | 4 ++-- .../operations/definitions/ranges/ranges.tsx | 8 +++---- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx index e1bc378635f1d..d9805b337c000 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx @@ -19,7 +19,7 @@ import { hasDateField, } from './utils'; import { updateColumnParam } from '../../layer_helpers'; -import { useDebounceWithOptions } from '../helpers'; +import { isValidNumber, useDebounceWithOptions } from '../helpers'; import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils'; import type { OperationDefinition, ParamEditorProps } from '..'; @@ -122,19 +122,6 @@ export const movingAverageOperation: OperationDefinition< timeScalingMode: 'optional', }; -function isValidNumber(input: string) { - if (input === '') return false; - try { - const val = parseFloat(input); - if (isNaN(val)) return false; - if (val < 1) return false; - if (val.toString().includes('.')) return false; - } catch (e) { - return false; - } - return true; -} - function MovingAverageParamEditor({ layer, updateLayer, @@ -145,7 +132,7 @@ function MovingAverageParamEditor({ useDebounceWithOptions( () => { - if (!isValidNumber(inputValue)) return; + if (!isValidNumber(inputValue, true, undefined, 1)) return; const inputNumber = parseInt(inputValue, 10); updateLayer( updateColumnParam({ 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 25ac7a54fbd04..f79c1753aa833 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 @@ -78,3 +78,24 @@ export function getSafeName(name: string, indexPattern: IndexPattern): string { defaultMessage: 'Missing field', }); } + +export function isValidNumber( + inputValue: string | number | null | undefined, + integer?: boolean, + upperBound?: number, + lowerBound?: number +) { + const inputValueAsNumber = Number(inputValue); + // an input is value if it's not an empty string, parses to a valid number, is between 0 and 100 (inclusive) + // and is an integer + return ( + inputValue !== '' && + inputValue !== null && + inputValue !== undefined && + !Number.isNaN(inputValueAsNumber) && + Number.isFinite(inputValueAsNumber) && + (!integer || Number.isInteger(inputValueAsNumber)) && + (upperBound === undefined || inputValueAsNumber <= upperBound) && + (lowerBound === undefined || inputValueAsNumber >= lowerBound) + ); +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index e30441b05f077..73eca12a01736 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -11,7 +11,7 @@ import { AggFunctionsMapping } from 'src/plugins/data/public'; import useDebounce from 'react-use/lib/useDebounce'; import { buildExpressionFunction } from '../../../../../../../src/plugins/expressions/public'; import { OperationDefinition } from './index'; -import { getInvalidFieldMessage } from './helpers'; +import { getInvalidFieldMessage, getSafeName, isValidNumber } from './helpers'; import { FieldBasedIndexPatternColumn } from './column_types'; export interface PercentileIndexPatternColumn extends FieldBasedIndexPatternColumn { @@ -63,8 +63,8 @@ export const percentileOperation: OperationDefinition - ofName(indexPattern.getFieldByName(column.sourceField)!.displayName, column.params.percentile), - buildColumn: ({ field, previousColumn }) => { + ofName(getSafeName(column.sourceField, indexPattern), column.params.percentile), + buildColumn: ({ field, previousColumn, indexPattern }) => { const existingFormat = previousColumn?.params && 'format' in previousColumn?.params ? previousColumn?.params?.format @@ -73,7 +73,7 @@ export const percentileOperation: OperationDefinition= 0 && - inputValueAsNumber <= 100 && - inputValueAsNumber % 1 === 0; + const inputValueIsValid = isValidNumber(inputValue, true, 99, 1); useDebounce( () => { @@ -171,7 +166,7 @@ export const percentileOperation: OperationDefinition @@ -179,8 +174,9 @@ export const percentileOperation: OperationDefinition diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx index 9ab677bf68f62..420846f7fc801 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx @@ -22,7 +22,7 @@ import { keys, } from '@elastic/eui'; import { IFieldFormat } from '../../../../../../../../src/plugins/data/common'; -import { RangeTypeLens, isValidRange, isValidNumber } from './ranges'; +import { RangeTypeLens, isValidRange } from './ranges'; import { FROM_PLACEHOLDER, TO_PLACEHOLDER, TYPING_DEBOUNCE_TIME } from './constants'; import { NewBucketButton, @@ -30,7 +30,7 @@ import { DraggableBucketContainer, LabelInput, } from '../shared_components'; -import { useDebounceWithOptions } from '../helpers'; +import { isValidNumber, useDebounceWithOptions } from '../helpers'; const generateId = htmlIdGenerator(); 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 b687e6fe3da50..98d9a239f5dd7 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 @@ -19,7 +19,7 @@ import { updateColumnParam } from '../../layer_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; -import { getInvalidFieldMessage } from '../helpers'; +import { getInvalidFieldMessage, isValidNumber } from '../helpers'; type RangeType = Omit; // Try to cover all possible serialized states for ranges @@ -54,8 +54,6 @@ export type UpdateParamsFnType = ( // on initialization values can be null (from the Infinity serialization), so handle it correctly // or they will be casted to 0 by the editor ( see #78867 ) -export const isValidNumber = (value: number | '' | null): value is number => - value != null && value !== '' && !isNaN(value) && isFinite(value); export const isRangeWithin = (range: RangeType): boolean => range.from <= range.to; const isFullRange = (range: RangeTypeLens): range is FullRangeTypeLens => isValidNumber(range.from) && isValidNumber(range.to); @@ -152,10 +150,10 @@ export const rangeOperation: OperationDefinition = { label: range.label }; // be careful with the fields to set on partial ranges if (isValidNumber(range.from)) { - partialRange.from = range.from; + partialRange.from = Number(range.from); } if (isValidNumber(range.to)) { - partialRange.to = range.to; + partialRange.to = Number(range.to); } return partialRange; }) From 17764140674db3bfc95c4c6ea9225138f658b403 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 23 Dec 2020 09:32:02 +0100 Subject: [PATCH 4/5] fix test --- .../operations/definitions/percentile.test.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx index eec253a252889..38ae7da069afa 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx @@ -116,15 +116,12 @@ describe('percentile', () => { describe('buildColumn', () => { it('should set default percentile', () => { + const indexPattern = createMockedIndexPattern(); + const bytesField = indexPattern.fields.find(({ name }) => name === 'bytes')!; + bytesField.displayName = 'test'; const percentileColumn = percentileOperation.buildColumn({ - indexPattern: createMockedIndexPattern(), - field: { - aggregatable: true, - searchable: true, - type: 'number', - name: 'bytes', - displayName: 'test', - }, + indexPattern, + field: bytesField, layer: { columns: {}, columnOrder: [], indexPatternId: '' }, }); expect(percentileColumn.dataType).toEqual('number'); From d9f9d39099bd88cae93b56b3bd09a6abf7b023aa Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 23 Dec 2020 15:56:17 +0100 Subject: [PATCH 5/5] clean up things --- .../operations/definitions/helpers.tsx | 2 - .../definitions/percentile.test.tsx | 4 ++ .../operations/definitions/percentile.tsx | 65 ++++++++++--------- .../operations/definitions/ranges/ranges.tsx | 2 - 4 files changed, 38 insertions(+), 35 deletions(-) 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 f79c1753aa833..29148052cee8e 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 @@ -86,8 +86,6 @@ export function isValidNumber( lowerBound?: number ) { const inputValueAsNumber = Number(inputValue); - // an input is value if it's not an empty string, parses to a valid number, is between 0 and 100 (inclusive) - // and is an integer return ( inputValue !== '' && inputValue !== null && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx index 38ae7da069afa..c22eec62ea1ab 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx @@ -161,6 +161,8 @@ describe('percentile', () => { /> ); + jest.runAllTimers(); + const input = instance .find('[data-test-subj="lns-indexPattern-percentile-input"]') .find(EuiFieldNumber); @@ -200,6 +202,8 @@ describe('percentile', () => { /> ); + jest.runAllTimers(); + const input = instance .find('[data-test-subj="lns-indexPattern-percentile-input"]') .find(EuiFieldNumber); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx index 73eca12a01736..b381a0ecb664a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx @@ -8,10 +8,14 @@ import { EuiFieldNumber, EuiFormRow } from '@elastic/eui'; import React, { useCallback, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { AggFunctionsMapping } from 'src/plugins/data/public'; -import useDebounce from 'react-use/lib/useDebounce'; import { buildExpressionFunction } from '../../../../../../../src/plugins/expressions/public'; import { OperationDefinition } from './index'; -import { getInvalidFieldMessage, getSafeName, isValidNumber } from './helpers'; +import { + getInvalidFieldMessage, + getSafeName, + isValidNumber, + useDebounceWithOptions, +} from './helpers'; import { FieldBasedIndexPatternColumn } from './column_types'; export interface PercentileIndexPatternColumn extends FieldBasedIndexPatternColumn { @@ -117,11 +121,11 @@ export const percentileOperation: OperationDefinition { if (!inputValueIsValid) return; updateLayer({ @@ -145,6 +149,7 @@ export const percentileOperation: OperationDefinition - - - - + + + ); }, }; 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 98d9a239f5dd7..aa5cc8255a584 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 @@ -52,8 +52,6 @@ export type UpdateParamsFnType = ( value: RangeColumnParams[K] ) => void; -// on initialization values can be null (from the Infinity serialization), so handle it correctly -// or they will be casted to 0 by the editor ( see #78867 ) export const isRangeWithin = (range: RangeType): boolean => range.from <= range.to; const isFullRange = (range: RangeTypeLens): range is FullRangeTypeLens => isValidNumber(range.from) && isValidNumber(range.to);