From bc5d789d917c0faa61ae5a06c632a5fd4da52426 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 26 Nov 2020 11:23:54 +0100 Subject: [PATCH] add tests and fix label input --- .../dimension_panel/dimension_editor.tsx | 24 ++++- .../dimension_panel/dimension_panel.test.tsx | 54 +++++++++++ .../dimension_panel/time_scaling.tsx | 6 +- .../definitions/calculations/derivative.tsx | 2 + .../calculations/moving_average.tsx | 2 + .../operations/definitions/count.tsx | 6 +- .../operations/definitions/metrics.tsx | 7 +- .../operations/time_scale_utils.test.ts | 92 +++++++++++++++++++ .../operations/time_scale_utils.ts | 24 +++++ 9 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts 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 696984c19e744..62de601bb7888 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 @@ -6,7 +6,7 @@ import './dimension_editor.scss'; import _ from 'lodash'; -import React, { useState, useMemo } from 'react'; +import React, { useState, useMemo, useEffect, useRef } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiListGroup, @@ -44,10 +44,30 @@ export interface DimensionEditorProps extends IndexPatternDimensionEditorProps { currentIndexPattern: IndexPattern; } +/** + * This component shows a debounced input for the label of a dimension. It will update on root state changes + * if no debounced changes are in flight because the user is currently typing into the input. + */ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => { const [inputValue, setInputValue] = useState(value); + const unflushedChanges = useRef(false); + + const onChangeDebounced = useMemo(() => { + const callback = _.debounce((val: string) => { + onChange(val); + unflushedChanges.current = false; + }, 256); + return (val: string) => { + unflushedChanges.current = true; + callback(val); + }; + }, [onChange]); - const onChangeDebounced = useMemo(() => _.debounce(onChange, 256), [onChange]); + useEffect(() => { + if (!unflushedChanges.current && value !== inputValue) { + setInputValue(value); + } + }, [value, inputValue]); const handleInputChange = (e: React.ChangeEvent) => { const val = String(e.target.value); 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 79102d4e34411..29a0586c92ffe 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 @@ -878,6 +878,60 @@ describe('IndexPatternDimensionEditorPanel', () => { }); }); + it('should carry over time scaling to other operation if possible', () => { + const props = getProps({ + timeScale: 'h', + sourceField: 'bytes', + operationType: 'sum', + label: 'Sum of bytes per hour', + }); + wrapper = mount(); + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-count incompatible"]') + .simulate('click'); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: 'h', + label: 'Count of records per hour', + }), + }, + }, + }, + }); + }); + + it('should not carry over time scaling if the other operation does not support it', () => { + const props = getProps({ + timeScale: 'h', + sourceField: 'bytes', + operationType: 'sum', + label: 'Sum of bytes per hour', + }); + wrapper = mount(); + wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: undefined, + label: 'Average of bytes', + }), + }, + }, + }, + }); + }); + it('should allow to change time scaling', () => { const props = getProps({ timeScale: 's', label: 'Count of records per second' }); wrapper = mount(); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index b8a134d9cb88b..4a0bf39800e4f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -112,7 +112,7 @@ export function TimeScaling({ }} > {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Show with normalized time units', + defaultMessage: 'Show with normalized units', })} @@ -134,8 +134,8 @@ export function TimeScaling({ > {i18n.translate('xpack.lens.indexPattern.timeScale.label', { - defaultMessage: 'Normalize by time unit', - })} + defaultMessage: 'Normalize by unit', + })}{' '} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx index 12ca499f890b3..41fe361c7ba9c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx @@ -13,6 +13,7 @@ import { dateBasedOperationToExpression, hasDateField, } from './utils'; +import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils'; import { OperationDefinition } from '..'; const ofName = buildLabelFunction((name?: string) => { @@ -85,6 +86,7 @@ export const derivativeOperation: OperationDefinition< isTransferable: (column, newIndexPattern) => { return hasDateField(newIndexPattern); }, + onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, getErrorMessage: (layer: IndexPatternLayer) => { return checkForDateHistogram( layer, 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 8e460e86919ed..522899662fbd1 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,6 +19,7 @@ import { } from './utils'; import { updateColumnParam } from '../../layer_helpers'; import { useDebounceWithOptions } from '../helpers'; +import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils'; import type { OperationDefinition, ParamEditorProps } from '..'; const ofName = buildLabelFunction((name?: string) => { @@ -97,6 +98,7 @@ export const movingAverageOperation: OperationDefinition< isTransferable: (column, newIndexPattern) => { return hasDateField(newIndexPattern); }, + onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, getErrorMessage: (layer: IndexPatternLayer) => { return checkForDateHistogram( layer, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index 4156441bc0632..8cb95de72f97e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -8,7 +8,10 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField } from '../../types'; -import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; +import { + adjustTimeScaleLabelSuffix, + adjustTimeScaleOnOtherColumnChange, +} from '../time_scale_utils'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', @@ -61,6 +64,7 @@ export const countOperation: OperationDefinition ({ id: columnId, enabled: true, 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 7697c6928ec51..45ba721981ed5 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 @@ -11,7 +11,10 @@ import { FieldBasedIndexPatternColumn, BaseIndexPatternColumn, } from './column_types'; -import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; +import { + adjustTimeScaleLabelSuffix, + adjustTimeScaleOnOtherColumnChange, +} from '../time_scale_utils'; type MetricColumn = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { @@ -68,6 +71,8 @@ function buildMetricOperation>({ (!newField.aggregationRestrictions || newField.aggregationRestrictions![type]) ); }, + onOtherColumnChanged: (column, otherColumns) => + optionalTimeScaling ? adjustTimeScaleOnOtherColumnChange(column, otherColumns) : column, getDefaultLabel: (column, indexPattern, columns) => labelLookup(indexPattern.getFieldByName(column.sourceField)!.displayName, column), buildColumn: ({ field, previousColumn }) => ({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts new file mode 100644 index 0000000000000..841011c588433 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts @@ -0,0 +1,92 @@ +/* + * 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 { TimeScaleUnit } from '../time_scale'; +import { IndexPatternColumn } from './definitions'; +import { adjustTimeScaleLabelSuffix, adjustTimeScaleOnOtherColumnChange } from './time_scale_utils'; + +export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; + +describe('time scale utils', () => { + describe('adjustTimeScaleLabelSuffix', () => { + it('should should remove existing suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc per second', 's', undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc per hour', 'h', undefined)).toEqual('abc'); + }); + + it('should add suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc', undefined, 's')).toEqual('abc per second'); + expect(adjustTimeScaleLabelSuffix('abc', undefined, 'd')).toEqual('abc per day'); + }); + + it('should change suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc per second', 's', 'd')).toEqual('abc per day'); + expect(adjustTimeScaleLabelSuffix('abc per day', 'd', 's')).toEqual('abc per second'); + }); + + it('should keep current state', () => { + expect(adjustTimeScaleLabelSuffix('abc', undefined, undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc per day', 'd', 'd')).toEqual('abc per day'); + }); + + it('should not fail on inconsistent input', () => { + expect(adjustTimeScaleLabelSuffix('abc', 's', undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc', 's', 'd')).toEqual('abc per day'); + expect(adjustTimeScaleLabelSuffix('abc per day', 's', undefined)).toEqual('abc per day'); + }); + }); + + describe('adjustTimeScaleOnOtherColumnChange', () => { + const baseColumn: IndexPatternColumn = { + operationType: 'count', + sourceField: 'Records', + label: 'Count of records per second', + dataType: 'number', + isBucketed: false, + timeScale: 's', + }; + it('should keep column if there is no time scale', () => { + const column = { ...baseColumn, timeScale: undefined }; + expect(adjustTimeScaleOnOtherColumnChange(column, { col1: column })).toBe(column); + }); + + it('should keep time scale if there is a date histogram', () => { + expect( + adjustTimeScaleOnOtherColumnChange(baseColumn, { + col1: baseColumn, + col2: { + operationType: 'date_histogram', + dataType: 'date', + isBucketed: true, + label: '', + }, + }) + ).toBe(baseColumn); + }); + + it('should remove time scale if there is no date histogram', () => { + expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( + 'timeScale', + undefined + ); + }); + + it('should remove suffix from label', () => { + expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( + 'label', + 'Count of records' + ); + }); + + it('should keep custom label', () => { + const column = { ...baseColumn, label: 'abc', customLabel: true }; + expect(adjustTimeScaleOnOtherColumnChange(column, { col1: column })).toHaveProperty( + 'label', + 'abc' + ); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts index 1d5ed796034d1..5d525e573a617 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts @@ -6,6 +6,7 @@ import { unitSuffixesLong } from '../suffix_formatter'; import { TimeScaleUnit } from '../time_scale'; +import { BaseIndexPatternColumn } from './definitions/column_types'; export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; @@ -28,3 +29,26 @@ export function adjustTimeScaleLabelSuffix( // add new suffix if column has a time scale now return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; } + +export function adjustTimeScaleOnOtherColumnChange( + column: T, + columns: Partial> +) { + if (!column.timeScale) { + return column; + } + const hasDateHistogram = Object.values(columns).some( + (col) => col?.operationType === 'date_histogram' + ); + if (hasDateHistogram) { + return column; + } + if (column.customLabel) { + return column; + } + return { + ...column, + timeScale: undefined, + label: adjustTimeScaleLabelSuffix(column.label, column.timeScale, undefined), + }; +}