From 1d043563cb2070b816d1c5c9e20e223eba26719d Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 22 Aug 2019 15:47:30 +0200 Subject: [PATCH 1/4] Add auto date histogram --- .../indexpattern_plugin/__mocks__/loader.ts | 119 +-------------- .../dimension_panel/dimension_panel.tsx | 40 ++++-- .../dimension_panel/popover_editor.tsx | 58 ++++++-- .../lens/public/indexpattern_plugin/mocks.ts | 117 +++++++++++++++ .../operation_definitions/count.tsx | 10 +- .../date_histogram.test.tsx | 22 ++- .../operation_definitions/date_histogram.tsx | 136 +++++++++++------- .../filter_ratio.test.tsx | 2 + .../operation_definitions/filter_ratio.tsx | 10 +- .../operation_definitions/metrics.tsx | 7 + .../operation_definitions/terms.test.tsx | 3 + .../operation_definitions/terms.tsx | 9 +- .../public/indexpattern_plugin/operations.ts | 16 +++ .../indexpattern_plugin/state_helpers.ts | 4 +- 14 files changed, 355 insertions(+), 198 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/__mocks__/loader.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/__mocks__/loader.ts index cf7012650b318..87a6dd522ec63 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/__mocks__/loader.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/__mocks__/loader.ts @@ -4,123 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ +import { createMockedIndexPattern, createMockedRestrictedIndexPattern } from '../mocks'; + export function getIndexPatterns() { return new Promise(resolve => { - resolve([ - { - id: '1', - title: 'my-fake-index-pattern', - timeFieldName: 'timestamp', - fields: [ - { - name: 'timestamp', - type: 'date', - aggregatable: true, - searchable: true, - }, - { - name: 'start_date', - type: 'date', - aggregatable: true, - searchable: true, - }, - { - name: 'bytes', - type: 'number', - aggregatable: true, - searchable: true, - }, - { - name: 'memory', - type: 'number', - aggregatable: true, - searchable: true, - }, - { - name: 'source', - type: 'string', - aggregatable: true, - searchable: true, - }, - { - name: 'dest', - type: 'string', - aggregatable: true, - searchable: true, - }, - ], - }, - { - id: '2', - title: 'my-fake-restricted-pattern', - timeFieldName: 'timestamp', - fields: [ - { - name: 'timestamp', - type: 'date', - aggregatable: true, - searchable: true, - }, - { - name: 'bytes', - type: 'number', - aggregatable: true, - searchable: true, - }, - { - name: 'source', - type: 'string', - aggregatable: true, - searchable: true, - }, - ], - typeMeta: { - params: { - rollup_index: 'my-fake-index-pattern', - }, - aggs: { - terms: { - source: { - agg: 'terms', - }, - }, - date_histogram: { - timestamp: { - agg: 'date_histogram', - fixed_interval: '1d', - delay: '7d', - time_zone: 'UTC', - }, - }, - histogram: { - bytes: { - agg: 'histogram', - interval: 1000, - }, - }, - avg: { - bytes: { - agg: 'avg', - }, - }, - max: { - bytes: { - agg: 'max', - }, - }, - min: { - bytes: { - agg: 'min', - }, - }, - sum: { - bytes: { - agg: 'sum', - }, - }, - }, - }, - }, - ]); + resolve([createMockedIndexPattern(), createMockedRestrictedIndexPattern()]); }); } diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx index 689e5a0f52c26..6e8bcba5eae3f 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx @@ -18,11 +18,16 @@ import { OperationType, } from '../indexpattern'; -import { getAvailableOperationsByMetadata, buildColumn } from '../operations'; +import { + getAvailableOperationsByMetadata, + buildColumn, + operationDefinitionMap, + OperationDefinition, +} from '../operations'; import { PopoverEditor } from './popover_editor'; import { DragContextState, ChildDragDropProvider, DragDrop } from '../../drag_drop'; import { changeColumn, deleteColumn } from '../state_helpers'; -import { isDraggedField } from '../utils'; +import { isDraggedField, hasField } from '../utils'; export type IndexPatternDimensionPanelProps = DatasourceDimensionPanelProps & { state: IndexPatternPrivateState; @@ -109,18 +114,35 @@ export const IndexPatternDimensionPanel = memo(function IndexPatternDimensionPan return; } - props.setState( - changeColumn({ - state: props.state, - layerId, - columnId: props.columnId, - newColumn: buildColumn({ + const operationsForNewField = + operationFieldSupportMatrix.operationByField[droppedItem.field.name]; + + const hasFieldChanged = + selectedColumn && + hasField(selectedColumn) && + selectedColumn.sourceField !== droppedItem.field.name && + operationsForNewField && + operationsForNewField.includes(selectedColumn.operationType); + + const newColumn = hasFieldChanged + ? (operationDefinitionMap[selectedColumn.operationType] as OperationDefinition< + IndexPatternColumn + >).onFieldChange(selectedColumn, currentIndexPattern, droppedItem.field) + : buildColumn({ columns: props.state.layers[props.layerId].columns, indexPattern: currentIndexPattern, layerId, suggestedPriority: props.suggestedPriority, field: droppedItem.field, - }), + }); + + props.setState( + changeColumn({ + state: props.state, + layerId, + columnId: props.columnId, + newColumn, + keepParams: !hasFieldChanged, }) ); }} diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx index ebf87fb1bdebb..9918e1f0479b6 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx @@ -28,7 +28,12 @@ import { IndexPatternField, } from '../indexpattern'; import { IndexPatternDimensionPanelProps, OperationFieldSupportMatrix } from './dimension_panel'; -import { operationDefinitionMap, getOperationDisplay, buildColumn } from '../operations'; +import { + operationDefinitionMap, + getOperationDisplay, + buildColumn, + OperationDefinition, +} from '../operations'; import { deleteColumn, changeColumn } from '../state_helpers'; import { FieldSelect } from './field_select'; import { hasField } from '../utils'; @@ -271,17 +276,45 @@ export function PopoverEditor(props: PopoverEditorProps) { ); }} onChoose={choice => { - const column = buildColumn({ - columns: props.state.layers[props.layerId].columns, - field: 'field' in choice ? fieldMap[choice.field] : undefined, - indexPattern: currentIndexPattern, - layerId: props.layerId, - suggestedPriority: props.suggestedPriority, - op: - incompatibleSelectedOperationType || - ('field' in choice ? choice.operationType : undefined), - asDocumentOperation: choice.type === 'document', - }); + let column: IndexPatternColumn; + if ( + !incompatibleSelectedOperationType && + selectedColumn && + ('field' in choice && choice.operationType === selectedColumn.operationType) + ) { + const operation = operationDefinitionMap[ + choice.operationType + ] as OperationDefinition; + if (operation.onFieldChange) { + column = operation.onFieldChange( + selectedColumn, + currentIndexPattern, + fieldMap[choice.field] + ); + } else { + column = { ...selectedColumn, sourceField: choice.field } as IndexPatternColumn; + } + // Only simple field change + // don't call buildColumn + // if (onChangeField?) { column = onChangeField() } + // else { column = { field: newField, ...oldParams } } + // changeColumn(keepParams = false) + } else { + // Operation change + // buildColumn() + // changeColumn(keepparams = false) + column = buildColumn({ + columns: props.state.layers[props.layerId].columns, + field: 'field' in choice ? fieldMap[choice.field] : undefined, + indexPattern: currentIndexPattern, + layerId: props.layerId, + suggestedPriority: props.suggestedPriority, + op: + incompatibleSelectedOperationType || + ('field' in choice ? choice.operationType : undefined), + asDocumentOperation: choice.type === 'document', + }); + } setState( changeColumn({ @@ -289,6 +322,7 @@ export function PopoverEditor(props: PopoverEditorProps) { layerId, columnId, newColumn: column, + keepParams: false, }) ); setInvalidOperationType(null); diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/mocks.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/mocks.ts index b24d53e0f552f..79d88136b5fa1 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/mocks.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/mocks.ts @@ -5,6 +5,123 @@ */ import { DragContextState } from '../drag_drop'; +import { IndexPattern } from './indexpattern'; + +export const createMockedIndexPattern = (): IndexPattern => ({ + id: '1', + title: 'my-fake-index-pattern', + timeFieldName: 'timestamp', + fields: [ + { + name: 'timestamp', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'start_date', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'memory', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'source', + type: 'string', + aggregatable: true, + searchable: true, + }, + { + name: 'dest', + type: 'string', + aggregatable: true, + searchable: true, + }, + ], +}); + +export const createMockedRestrictedIndexPattern = () => ({ + id: '2', + title: 'my-fake-restricted-pattern', + timeFieldName: 'timestamp', + fields: [ + { + name: 'timestamp', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'source', + type: 'string', + aggregatable: true, + searchable: true, + }, + ], + typeMeta: { + params: { + rollup_index: 'my-fake-index-pattern', + }, + aggs: { + terms: { + source: { + agg: 'terms', + }, + }, + date_histogram: { + timestamp: { + agg: 'date_histogram', + fixed_interval: '1d', + delay: '7d', + time_zone: 'UTC', + }, + }, + histogram: { + bytes: { + agg: 'histogram', + interval: 1000, + }, + }, + avg: { + bytes: { + agg: 'avg', + }, + }, + max: { + bytes: { + agg: 'max', + }, + }, + min: { + bytes: { + agg: 'min', + }, + }, + sum: { + bytes: { + agg: 'sum', + }, + }, + }, + }, +}); export function createMockedDragDropContext(): jest.Mocked { return { diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/count.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/count.tsx index 0cb4838faa12c..767e6373f4858 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/count.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/count.tsx @@ -8,6 +8,10 @@ import { i18n } from '@kbn/i18n'; import { CountIndexPatternColumn } from '../indexpattern'; import { OperationDefinition } from '../operations'; +const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { + defaultMessage: 'Count of documents', +}); + export const countOperation: OperationDefinition = { type: 'count', displayName: i18n.translate('xpack.lens.indexPattern.count', { @@ -25,9 +29,7 @@ export const countOperation: OperationDefinition = { }, buildColumn({ suggestedPriority }) { return { - label: i18n.translate('xpack.lens.indexPattern.countOf', { - defaultMessage: 'Count of documents', - }), + label: countLabel, dataType: 'number', operationType: 'count', suggestedPriority, @@ -35,6 +37,8 @@ export const countOperation: OperationDefinition = { scale: 'ratio', }; }, + // This cannot be called practically, since this is a fieldless operation + onFieldChange: oldColumn => ({ ...oldColumn }), toEsAggsConfig: (column, columnId) => ({ id: columnId, enabled: true, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx index 8e94087f4a5fb..07cf7e6dae1c5 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx @@ -11,6 +11,7 @@ import { DateHistogramIndexPatternColumn, IndexPatternPrivateState } from '../in import { EuiRange } from '@elastic/eui'; import { UiSettingsClientContract } from 'src/core/public'; import { Storage } from 'ui/storage'; +import { createMockedIndexPattern } from '../mocks'; jest.mock('ui/new_platform'); @@ -91,11 +92,12 @@ describe('date_histogram', () => { }); describe('buildColumn', () => { - it('should create column object with default params', () => { + it('should create column object with auto interval for primary time field', () => { const column = dateHistogramOperation.buildColumn({ columns: {}, suggestedPriority: 0, layerId: 'first', + indexPattern: createMockedIndexPattern(), field: { name: 'timestamp', type: 'date', @@ -104,6 +106,23 @@ describe('date_histogram', () => { searchable: true, }, }); + expect(column.params.interval).toEqual('auto'); + }); + + it('should create column object with manual interval for non-primary time fields', () => { + const column = dateHistogramOperation.buildColumn({ + columns: {}, + suggestedPriority: 0, + layerId: 'first', + indexPattern: createMockedIndexPattern(), + field: { + name: 'start_date', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + }, + }); expect(column.params.interval).toEqual('d'); }); @@ -112,6 +131,7 @@ describe('date_histogram', () => { columns: {}, suggestedPriority: 0, layerId: 'first', + indexPattern: createMockedIndexPattern(), field: { name: 'timestamp', type: 'date', diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx index e454c700bb8db..b21858ff7e144 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx @@ -7,15 +7,16 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiForm, EuiFormRow, EuiRange } from '@elastic/eui'; -import { IndexPatternField, DateHistogramIndexPatternColumn } from '../indexpattern'; -import { DimensionPriority } from '../../types'; +import { EuiForm, EuiFormRow, EuiRange, EuiSwitch } from '@elastic/eui'; +import { DateHistogramIndexPatternColumn, IndexPattern } from '../indexpattern'; import { OperationDefinition } from '../operations'; import { updateColumnParam } from '../state_helpers'; type PropType = C extends React.ComponentType ? P : unknown; +const autoInterval = 'auto'; const supportedIntervals = ['M', 'w', 'd', 'h']; +const defaultCustomInterval = supportedIntervals[2]; // Add ticks to EuiRange component props const FixedEuiRange = (EuiRange as unknown) as React.ComponentType< @@ -34,6 +35,10 @@ function ofName(name: string) { }); } +function supportsAutoInterval(fieldName: string, indexPattern: IndexPattern): boolean { + return indexPattern.timeFieldName ? indexPattern.timeFieldName === fieldName : false; +} + export const dateHistogramOperation: OperationDefinition = { type: 'date_histogram', displayName: i18n.translate('xpack.lens.indexPattern.dateHistogram', { @@ -56,17 +61,11 @@ export const dateHistogramOperation: OperationDefinition { + return { + ...oldColumn, + label: ofName(field.name), + sourceField: field.name, + params: { + ...oldColumn.params, + interval: + oldColumn.params.interval === 'auto' && !supportsAutoInterval(field.name, indexPattern) + ? defaultCustomInterval + : oldColumn.params.interval, + }, + }; + }, toEsAggsConfig: (column, columnId) => ({ id: columnId, enabled: true, @@ -157,6 +170,8 @@ export const dateHistogramOperation: OperationDefinition) { + const interval = ev.target.checked ? defaultCustomInterval : autoInterval; + setState(updateColumnParam(state, layerId, column, 'interval', interval)); + } + return ( - - {intervalIsRestricted ? ( - - ) : ( - ({ - label: interval, - value: index, - }))} - onChange={(e: React.ChangeEvent) => - setState( - updateColumnParam( - state, - layerId, - column, - 'interval', - numericToInterval(Number(e.target.value)) - ) - ) - } - aria-label={i18n.translate('xpack.lens.indexPattern.dateHistogram.interval', { - defaultMessage: 'Level of detail', + {fieldAllowsAutoInterval && ( + + - )} - + + )} + {column.params.interval !== autoInterval && ( + + {intervalIsRestricted ? ( + + ) : ( + ({ + label: interval, + value: index, + }))} + onChange={(e: React.ChangeEvent) => + setState( + updateColumnParam( + state, + layerId, + column, + 'interval', + numericToInterval(Number(e.target.value)) + ) + ) + } + aria-label={i18n.translate('xpack.lens.indexPattern.dateHistogram.interval', { + defaultMessage: 'Level of detail', + })} + /> + )} + + )} ); }, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.test.tsx index 43d17c96a9810..81cda0ced4dbe 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.test.tsx @@ -12,6 +12,7 @@ import { FilterRatioIndexPatternColumn, IndexPatternPrivateState } from '../inde import { Storage } from 'ui/storage'; import { UiSettingsClientContract } from 'src/core/public'; import { QueryBarInput } from '../../../../../../../src/legacy/core_plugins/data/public/query'; +import { createMockedIndexPattern } from '../mocks'; jest.mock('ui/new_platform'); @@ -61,6 +62,7 @@ describe('filter_ratio', () => { layerId: 'first', columns: {}, suggestedPriority: undefined, + indexPattern: createMockedIndexPattern(), }); expect(column.params.numerator).toEqual({ query: '', language: 'kuery' }); expect(column.params.denominator).toEqual({ query: '', language: 'kuery' }); diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.tsx index 3f4e7c3dc407d..07fec23462b52 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/filter_ratio.tsx @@ -16,6 +16,10 @@ import { FilterRatioIndexPatternColumn } from '../indexpattern'; import { OperationDefinition } from '../operations'; import { updateColumnParam } from '../state_helpers'; +const filterRatioLabel = i18n.translate('xpack.lens.indexPattern.filterRatio', { + defaultMessage: 'Filter Ratio', +}); + export const filterRatioOperation: OperationDefinition = { type: 'filter_ratio', displayName: i18n.translate('xpack.lens.indexPattern.filterRatio', { @@ -33,9 +37,7 @@ export const filterRatioOperation: OperationDefinition ({ ...oldColumn }), toEsAggsConfig: (column, columnId) => ({ id: columnId, enabled: true, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/metrics.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/metrics.tsx index 47ab939336899..55851d2714b01 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/metrics.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/metrics.tsx @@ -63,6 +63,13 @@ function buildMetricOperation( scale: 'ratio', } as T; }, + onFieldChange: (oldColumn, indexPattern, field) => { + return { + ...oldColumn, + label: ofName(field.name), + sourceField: field.name, + }; + }, toEsAggsConfig: (column, columnId) => ({ id: columnId, enabled: true, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx index 1f639907b79d0..d309e0cecf149 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx @@ -11,6 +11,7 @@ import { IndexPatternPrivateState, TermsIndexPatternColumn } from '../indexpatte import { EuiRange, EuiSelect } from '@elastic/eui'; import { UiSettingsClientContract } from 'src/core/public'; import { Storage } from 'ui/storage'; +import { createMockedIndexPattern } from '../mocks'; jest.mock('ui/new_platform'); @@ -138,6 +139,7 @@ describe('terms', () => { const termsColumn = termsOperation.buildColumn({ layerId: 'first', suggestedPriority: undefined, + indexPattern: createMockedIndexPattern(), field: { aggregatable: true, searchable: true, @@ -153,6 +155,7 @@ describe('terms', () => { const termsColumn = termsOperation.buildColumn({ layerId: 'first', suggestedPriority: undefined, + indexPattern: createMockedIndexPattern(), columns: { col1: { label: 'Count', diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.tsx index ca0455c9372da..a187e5390e4b7 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.tsx @@ -26,7 +26,7 @@ const FixedEuiRange = (EuiRange as unknown) as React.ComponentType< function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { - defaultMessage: 'Top Values of {name}', + defaultMessage: 'Top values of {name}', values: { name }, }); } @@ -111,6 +111,13 @@ export const termsOperation: OperationDefinition = { missingBucketLabel: 'Missing', }, }), + onFieldChange: (oldColumn, indexPattern, field) => { + return { + ...oldColumn, + label: ofName(field.name), + sourceField: field.name, + }; + }, onOtherColumnChanged: (currentColumn, columns) => { if (currentColumn.params.orderBy.type === 'column') { // check whether the column is still there and still a metric diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operations.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operations.ts index e7110364020d1..cc47b6dc4298a 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operations.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operations.ts @@ -77,6 +77,7 @@ export interface OperationDefinition { layerId: string; columns: Partial>; field?: IndexPatternField; + indexPattern: IndexPattern; }) => C; onOtherColumnChanged?: ( currentColumn: C, @@ -86,6 +87,20 @@ export interface OperationDefinition { toEsAggsConfig: (column: C, columnId: string) => unknown; isTransferable: (column: C, newIndexPattern: IndexPattern) => boolean; transfer?: (column: C, newIndexPattern: IndexPattern) => C; + /** + * This method will be called if the user changes the field of an operation. + * If this isn't implemented the new column will just be the old column with the new field name, + * and everything else will stay the same. You can implement this method to modify the logic + * by which the new column will be built. This will only be called for switching the field, + * not for initially selecting a field. + * + * See {@link #transfer} for controlling column building when switching an index pattern not just a field. + * + * @param oldColumn The column before the user changed the field. + * @param indexPattern The index pattern that field is on. + * @param field The field that the user changed to. + */ + onFieldChange: (oldColumn: C, indexPattern: IndexPattern, field: IndexPatternField) => C; } export function isColumnTransferable(column: IndexPatternColumn, newIndexPattern: IndexPattern) { @@ -196,5 +211,6 @@ export function buildColumn({ suggestedPriority, field, layerId, + indexPattern, }); } diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/state_helpers.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/state_helpers.ts index e4031071e448c..4a17f7774664b 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/state_helpers.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/state_helpers.ts @@ -77,7 +77,7 @@ export function changeColumn({ layerId, columnId, newColumn, - keepParams, + keepParams = true, }: { state: IndexPatternPrivateState; layerId: string; @@ -88,7 +88,7 @@ export function changeColumn({ const oldColumn = state.layers[layerId].columns[columnId]; const updatedColumn = - (typeof keepParams === 'boolean' ? keepParams : true) && + keepParams && oldColumn && oldColumn.operationType === newColumn.operationType && 'params' in oldColumn From a1ea70e66768615663373ede418cfe0f76ab58e5 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 22 Aug 2019 16:42:41 +0200 Subject: [PATCH 2/4] Improve documentation and cleanup --- .../dimension_panel/dimension_panel.tsx | 7 ++++++ .../dimension_panel/popover_editor.tsx | 25 ++++++------------- .../operation_definitions/date_histogram.tsx | 2 ++ .../public/indexpattern_plugin/operations.ts | 13 ++++++---- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx index 6e8bcba5eae3f..175ac5cbcc94e 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/dimension_panel.tsx @@ -117,6 +117,9 @@ export const IndexPatternDimensionPanel = memo(function IndexPatternDimensionPan const operationsForNewField = operationFieldSupportMatrix.operationByField[droppedItem.field.name]; + // We need to check if dragging in a new field, was just a field change on the same + // index pattern and on the same operations (therefore checking if the new field supports + // our previous operation) const hasFieldChanged = selectedColumn && hasField(selectedColumn) && @@ -124,6 +127,8 @@ export const IndexPatternDimensionPanel = memo(function IndexPatternDimensionPan operationsForNewField && operationsForNewField.includes(selectedColumn.operationType); + // If only the field has changed use the onFieldChange method on the operation to get the + // new column, otherwise use the regular buildColumn to get a new column. const newColumn = hasFieldChanged ? (operationDefinitionMap[selectedColumn.operationType] as OperationDefinition< IndexPatternColumn @@ -142,6 +147,8 @@ export const IndexPatternDimensionPanel = memo(function IndexPatternDimensionPan layerId, columnId: props.columnId, newColumn, + // If the field has changed, the onFieldChange method needs to take care of everything including moving + // over params. If we create a new column above we want changeColumn to move over params. keepParams: !hasFieldChanged, }) ); diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx index 9918e1f0479b6..4b476936f1f76 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/dimension_panel/popover_editor.tsx @@ -282,27 +282,18 @@ export function PopoverEditor(props: PopoverEditorProps) { selectedColumn && ('field' in choice && choice.operationType === selectedColumn.operationType) ) { + // If we just changed the field are not in an error state and the operation didn't change, + // we use the operations onFieldChange method to calculate the new column. const operation = operationDefinitionMap[ choice.operationType ] as OperationDefinition; - if (operation.onFieldChange) { - column = operation.onFieldChange( - selectedColumn, - currentIndexPattern, - fieldMap[choice.field] - ); - } else { - column = { ...selectedColumn, sourceField: choice.field } as IndexPatternColumn; - } - // Only simple field change - // don't call buildColumn - // if (onChangeField?) { column = onChangeField() } - // else { column = { field: newField, ...oldParams } } - // changeColumn(keepParams = false) + column = operation.onFieldChange( + selectedColumn, + currentIndexPattern, + fieldMap[choice.field] + ); } else { - // Operation change - // buildColumn() - // changeColumn(keepparams = false) + // Otherwise we'll use the buildColumn method to calculate a new column column = buildColumn({ columns: props.state.layers[props.layerId].columns, field: 'field' in choice ? fieldMap[choice.field] : undefined, diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx index b21858ff7e144..50f41b46250a7 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx @@ -138,6 +138,8 @@ export const dateHistogramOperation: OperationDefinition { transfer?: (column: C, newIndexPattern: IndexPattern) => C; /** * This method will be called if the user changes the field of an operation. - * If this isn't implemented the new column will just be the old column with the new field name, - * and everything else will stay the same. You can implement this method to modify the logic - * by which the new column will be built. This will only be called for switching the field, - * not for initially selecting a field. + * You must implement it and return the new column after the field change. + * The most simple implementation will just change the field on the column, and keep + * the rest the same. Some implementations might want to change labels, or their parameters + * when changing the field. * - * See {@link #transfer} for controlling column building when switching an index pattern not just a field. + * This will only be called for switching the field, not for initially selecting a field. + * + * See {@link OperationDefinition#transfer} for controlling column building when switching an + * index pattern not just a field. * * @param oldColumn The column before the user changed the field. * @param indexPattern The index pattern that field is on. From bc956207dbd3c0ed6f9997bb971ab3cfd4abc376 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 22 Aug 2019 18:04:16 +0200 Subject: [PATCH 3/4] Add tests --- .../date_histogram.test.tsx | 98 ++++++++++++++++++- .../operation_definitions/terms.test.tsx | 28 ++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx index 07cf7e6dae1c5..7920039e68220 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { dateHistogramOperation } from './date_histogram'; import { shallow } from 'enzyme'; import { DateHistogramIndexPatternColumn, IndexPatternPrivateState } from '../indexpattern'; -import { EuiRange } from '@elastic/eui'; +import { EuiRange, EuiSwitch } from '@elastic/eui'; import { UiSettingsClientContract } from 'src/core/public'; import { Storage } from 'ui/storage'; import { createMockedIndexPattern } from '../mocks'; @@ -26,6 +26,7 @@ describe('date_histogram', () => { 1: { id: '1', title: 'Mock Indexpattern', + timeFieldName: 'timestamp', fields: [ { name: 'timestamp', @@ -87,6 +88,24 @@ describe('date_histogram', () => { }, }, }, + third: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + params: { + interval: 'auto', + }, + sourceField: 'timestamp', + }, + }, + }, }, }; }); @@ -169,6 +188,48 @@ describe('date_histogram', () => { }); }); + describe('onFieldChange', () => { + it('should change correctly without auto interval', () => { + const oldColumn: DateHistogramIndexPatternColumn = { + operationType: 'date_histogram', + sourceField: 'timestamp', + label: 'Date over timestamp', + isBucketed: true, + dataType: 'date', + params: { + interval: 'd', + }, + }; + const indexPattern = createMockedIndexPattern(); + const newDateField = indexPattern.fields.find(i => i.name === 'start_date')!; + + const column = dateHistogramOperation.onFieldChange(oldColumn, indexPattern, newDateField); + expect(column).toHaveProperty('sourceField', 'start_date'); + expect(column).toHaveProperty('params.interval', 'd'); + expect(column.label).toContain('start_date'); + }); + + it('should change interval from auto when switching to a non primary time field', () => { + const oldColumn: DateHistogramIndexPatternColumn = { + operationType: 'date_histogram', + sourceField: 'timestamp', + label: 'Date over timestamp', + isBucketed: true, + dataType: 'date', + params: { + interval: 'auto', + }, + }; + const indexPattern = createMockedIndexPattern(); + const newDateField = indexPattern.fields.find(i => i.name === 'start_date')!; + + const column = dateHistogramOperation.onFieldChange(oldColumn, indexPattern, newDateField); + expect(column).toHaveProperty('sourceField', 'start_date'); + expect(column).toHaveProperty('params.interval', 'd'); + expect(column.label).toContain('start_date'); + }); + }); + describe('transfer', () => { it('should adjust interval and time zone params if that is necessary due to restrictions', () => { const transferedColumn = dateHistogramOperation.transfer!( @@ -281,6 +342,41 @@ describe('date_histogram', () => { expect(instance.find(EuiRange).prop('value')).toEqual(2); }); + it('should render disabled switch and no level of detail control for auto interval', () => { + const instance = shallow( + + ); + expect(instance.find(EuiRange).exists()).toBe(false); + expect(instance.find(EuiSwitch).prop('checked')).toBe(false); + }); + + it('should allow switching to manual interval', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + instance.find(EuiSwitch).prop('onChange')!({ + target: { checked: true }, + } as React.ChangeEvent); + expect(setStateSpy).toHaveBeenCalled(); + const newState = setStateSpy.mock.calls[0][0]; + expect(newState).toHaveProperty('layers.third.columns.col1.params.interval', 'd'); + }); + it('should update state with the interval value', () => { const setStateSpy = jest.fn(); const instance = shallow( diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx index d309e0cecf149..5967104459b29 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx @@ -74,6 +74,34 @@ describe('terms', () => { }); }); + describe('onFieldChange', () => { + it('should change correctly without auto interval', () => { + const oldColumn: TermsIndexPatternColumn = { + operationType: 'terms', + sourceField: 'source', + label: 'Top values of source', + isBucketed: true, + dataType: 'string', + params: { + size: 5, + orderBy: { + type: 'alphabetical', + }, + orderDirection: 'asc', + }, + }; + const indexPattern = createMockedIndexPattern(); + const newDateField = indexPattern.fields.find(i => i.name === 'dest')!; + + const column = termsOperation.onFieldChange(oldColumn, indexPattern, newDateField); + expect(column).toHaveProperty('sourceField', 'dest'); + expect(column).toHaveProperty('params.size', 5); + expect(column).toHaveProperty('params.orderBy.type', 'alphabetical'); + expect(column).toHaveProperty('params.orderDirection', 'asc'); + expect(column.label).toContain('dest'); + }); + }); + describe('getPossibleOperationsForField', () => { it('should return operation with the right type', () => { expect( From 374857f944380cc9d14d2de83bdc024e3610e454 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 23 Aug 2019 10:42:42 +0200 Subject: [PATCH 4/4] Change test name --- .../indexpattern_plugin/operation_definitions/terms.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx index 5967104459b29..5226ac9c97d8f 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/terms.test.tsx @@ -75,7 +75,7 @@ describe('terms', () => { }); describe('onFieldChange', () => { - it('should change correctly without auto interval', () => { + it('should change correctly to new field', () => { const oldColumn: TermsIndexPatternColumn = { operationType: 'terms', sourceField: 'source',