Skip to content

Commit

Permalink
add tests and fix label input
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 committed Nov 26, 2020
1 parent b7bb2bc commit bc5d789
Show file tree
Hide file tree
Showing 9 changed files with 210 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<HTMLInputElement>) => {
const val = String(e.target.value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(<IndexPatternDimensionEditorComponent {...props} />);
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(<IndexPatternDimensionEditorComponent {...props} />);
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(<IndexPatternDimensionEditorComponent {...props} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
})}
</EuiLink>
</EuiText>
Expand All @@ -134,8 +134,8 @@ export function TimeScaling({
>
<span>
{i18n.translate('xpack.lens.indexPattern.timeScale.label', {
defaultMessage: 'Normalize by time unit',
})}
defaultMessage: 'Normalize by unit',
})}{' '}
<EuiIcon type="questionInCircle" color="subdued" size="s" className="eui-alignTop" />
</span>
</EuiToolTip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
dateBasedOperationToExpression,
hasDateField,
} from './utils';
import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils';
import { OperationDefinition } from '..';

const ofName = buildLabelFunction((name?: string) => {
Expand Down Expand Up @@ -85,6 +86,7 @@ export const derivativeOperation: OperationDefinition<
isTransferable: (column, newIndexPattern) => {
return hasDateField(newIndexPattern);
},
onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange,
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
layer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -97,6 +98,7 @@ export const movingAverageOperation: OperationDefinition<
isTransferable: (column, newIndexPattern) => {
return hasDateField(newIndexPattern);
},
onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange,
getErrorMessage: (layer: IndexPatternLayer) => {
return checkForDateHistogram(
layer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -61,6 +64,7 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn, 'field
: undefined,
};
},
onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange,
toEsAggsConfig: (column, columnId) => ({
id: columnId,
enabled: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = FormattedIndexPatternColumn &
FieldBasedIndexPatternColumn & {
Expand Down Expand Up @@ -68,6 +71,8 @@ function buildMetricOperation<T extends MetricColumn<string>>({
(!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 }) => ({
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -28,3 +29,26 @@ export function adjustTimeScaleLabelSuffix(
// add new suffix if column has a time scale now
return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`;
}

export function adjustTimeScaleOnOtherColumnChange<T extends BaseIndexPatternColumn>(
column: T,
columns: Partial<Record<string, BaseIndexPatternColumn>>
) {
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),
};
}

0 comments on commit bc5d789

Please sign in to comment.