From 808a0d06c41e749d8654d2e4f7456eaba4c82224 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Wed, 2 Dec 2020 11:41:38 -0500 Subject: [PATCH 01/10] [Lens] UI for reference-based functions --- .../workspace_panel/workspace_panel.tsx | 2 +- .../dimension_panel/dimension_editor.tsx | 41 ++- .../dimension_panel/dimension_panel.test.tsx | 22 +- .../dimension_panel/operation_support.ts | 2 +- .../dimension_panel/reference_editor.tsx | 319 ++++++++++++++++++ .../indexpattern.test.ts | 4 +- .../indexpattern_datasource/indexpattern.tsx | 4 +- .../indexpattern_suggestions.test.tsx | 6 + .../indexpattern_suggestions.ts | 15 +- .../definitions/calculations/counter_rate.tsx | 5 +- .../calculations/cumulative_sum.tsx | 9 +- .../definitions/calculations/derivative.tsx | 10 +- .../calculations/moving_average.tsx | 2 +- .../operations/layer_helpers.test.ts | 38 +++ .../operations/layer_helpers.ts | 149 +++++--- 15 files changed, 550 insertions(+), 78 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index 6c2c01d944cd92..c9761adf3af67b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -161,7 +161,7 @@ export function WorkspacePanel({ const expression = useMemo( () => { - if (!configurationValidationError || configurationValidationError.length === 0) { + if (!configurationValidationError?.length) { try { return buildExpression({ visualization: activeVisualization, 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 4c3def0e5bc7f9..de4aa83e203cc9 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 @@ -28,6 +28,7 @@ import { updateColumnParam, resetIncomplete, FieldBasedIndexPatternColumn, + OperationType, } from '../operations'; import { mergeLayer } from '../state_helpers'; import { FieldSelect } from './field_select'; @@ -36,6 +37,7 @@ import { BucketNestingEditor } from './bucket_nesting_editor'; import { IndexPattern, IndexPatternLayer } from '../types'; import { trackUiEvent } from '../../lens_ui_telemetry'; import { FormatSelector } from './format_selector'; +import { ReferenceEditor } from './reference_editor'; import { TimeScaling } from './time_scaling'; const operationPanels = getOperationDisplay(); @@ -183,9 +185,12 @@ export function DimensionEditor(props: DimensionEditorProps) { compatibleWithCurrentField ? '' : ' incompatible' }`, onClick() { - if (operationDefinitionMap[operationType].input === 'none') { + if ( + operationDefinitionMap[operationType].input === 'none' || + operationDefinitionMap[operationType].input === 'fullReference' + ) { + // Clear invalid state because we are reseting to a valid column if (selectedColumn?.operationType === operationType) { - // Clear invalid state because we are reseting to a valid column if (incompleteInfo) { setState( mergeLayer({ @@ -293,6 +298,34 @@ export function DimensionEditor(props: DimensionEditorProps) {
+ {!incompleteInfo && + selectedColumn && + 'references' in selectedColumn && + selectedOperationDefinition?.input === 'fullReference' ? ( + <> + {selectedColumn.references.map((referenceId, index) => { + const validation = selectedOperationDefinition.requiredReferences[index]; + + return ( + { + setState(mergeLayer({ state, layerId, newLayer })); + }} + validation={validation} + currentIndexPattern={currentIndexPattern} + existingFields={state.existingFields} + selectionStyle={selectedOperationDefinition.selectionStyle} + /> + ); + })} + + + ) : null} + {!selectedColumn || selectedOperationDefinition?.input === 'field' || (incompleteOperation && operationDefinitionMap[incompleteOperation].input === 'field') ? ( @@ -447,11 +480,11 @@ export function DimensionEditor(props: DimensionEditorProps) { } function getErrorMessage( selectedColumn: IndexPatternColumn | undefined, - incompatibleSelectedOperationType: boolean, + incompleteOperation: boolean, input: 'none' | 'field' | 'fullReference' | undefined, fieldInvalid: boolean ) { - if (selectedColumn && incompatibleSelectedOperationType) { + if (selectedColumn && incompleteOperation) { if (input === 'field') { return i18n.translate('xpack.lens.indexPattern.invalidOperationLabel', { defaultMessage: 'To use this function, select a different field.', 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 d4197f93956608..58c06070f288e8 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 @@ -1068,6 +1068,10 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should support selecting the operation before the field', () => { + setState.mockImplementation((newState) => { + wrapper.setProps({ state: newState }); + }); + wrapper = mount(); wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); @@ -1100,15 +1104,15 @@ describe('IndexPatternDimensionEditorPanel', () => { layers: { first: { ...state.layers.first, + columnOrder: ['col1', 'col2'], columns: { ...state.layers.first.columns, col2: expect.objectContaining({ - sourceField: 'bytes', operationType: 'avg', - // Other parts of this don't matter for this test + sourceField: 'bytes', }), }, - columnOrder: ['col1', 'col2'], + incompleteColumns: {}, }, }, }); @@ -1178,9 +1182,15 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should indicate compatible fields when selecting the operation first', () => { + setState.mockImplementation((newState) => { + wrapper.setProps({ state: newState }); + }); + wrapper = mount(); - wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); + act(() => { + wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); + }); const options = wrapper .find(EuiComboBox) @@ -1242,9 +1252,13 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(items.map(({ label }: { label: React.ReactNode }) => label)).toEqual([ 'Average', 'Count', + 'Counter rate', + 'Cumulative sum', + 'Differences', 'Maximum', 'Median', 'Minimum', + 'Moving average', 'Sum', 'Unique count', '\u00a0', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/operation_support.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/operation_support.ts index 817fdf637f0010..9d55a9d5f75229 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/operation_support.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/operation_support.ts @@ -49,7 +49,7 @@ export const getOperationSupportMatrix = (props: Props): OperationSupportMatrix supportedFieldsByOperation[operation.operationType] = new Set(); } supportedFieldsByOperation[operation.operationType]?.add(operation.field); - } else if (operation.type === 'none') { + } else { supportedOperationsWithoutField.add(operation.operationType); } }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx new file mode 100644 index 00000000000000..a49772a18e09e5 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -0,0 +1,319 @@ +/* + * 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 './dimension_editor.scss'; +import _ from 'lodash'; +import React, { useMemo } from 'react'; +import { i18n } from '@kbn/i18n'; +import { EuiFormRow, EuiSpacer, EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; +import { OperationSupportMatrix } from './operation_support'; +import type { OperationType } from '../indexpattern'; +import { + operationDefinitionMap, + getOperationDisplay, + insertOrReplaceColumn, + replaceColumn, + deleteColumn, + RequiredReference, + isOperationAllowedAsReference, + FieldBasedIndexPatternColumn, +} from '../operations'; +import { FieldSelect } from './field_select'; +import { hasField } from '../utils'; +import type { IndexPattern, IndexPatternLayer, IndexPatternPrivateState } from '../types'; +import { trackUiEvent } from '../../lens_ui_telemetry'; + +const operationPanels = getOperationDisplay(); + +export interface ReferenceEditorProps { + layer: IndexPatternLayer; + parentColumnId: string; + selectionStyle: 'full' | 'field'; + validation: RequiredReference; + columnId: string; + updateLayer: (newLayer: IndexPatternLayer) => void; + currentIndexPattern: IndexPattern; + existingFields: IndexPatternPrivateState['existingFields']; +} + +export function ReferenceEditor(props: ReferenceEditorProps) { + const { + layer, + columnId, + updateLayer, + currentIndexPattern, + existingFields, + validation, + selectionStyle, + } = props; + + const column = layer.columns[columnId]; + const selectedOperationDefinition = column && operationDefinitionMap[column.operationType]; + + const incompleteInfo = layer.incompleteColumns ? layer.incompleteColumns[columnId] : undefined; + const incompleteOperation = incompleteInfo?.operationType; + const incompleteField = incompleteInfo?.sourceField ?? null; + + // Basically the operation support matrix, but different validation + const operationSupportMatrix: OperationSupportMatrix & { + operationTypes: Set; + } = useMemo(() => { + const operationTypes: Set = new Set(); + const operationWithoutField: Set = new Set(); + const operationByField: Partial>> = {}; + const fieldByOperation: Partial>> = {}; + Object.values(operationDefinitionMap) + .sort((op1, op2) => { + return op1.displayName.localeCompare(op2.displayName); + }) + .forEach((op) => { + if (op.input === 'field') { + const allFields = currentIndexPattern.fields.filter((field) => + isOperationAllowedAsReference({ operationType: op.type, validation, field }) + ); + if (allFields.length) { + operationTypes.add(op.type); + fieldByOperation[op.type] = new Set(allFields.map(({ name }) => name)); + allFields.forEach((field) => { + if (!operationByField[field.name]) { + operationByField[field.name] = new Set(); + } + operationByField[field.name]?.add(op.type); + }); + } + } else if (isOperationAllowedAsReference({ operationType: op.type, validation })) { + operationTypes.add(op.type); + operationWithoutField.add(op.type); + } + }); + return { + operationTypes, + operationWithoutField, + operationByField, + fieldByOperation, + }; + }, [currentIndexPattern, validation]); + + const functionOptions: Array> = []; + operationSupportMatrix.operationTypes.forEach((operationType) => { + const label = operationPanels[operationType].displayName; + // const isCompatible = !column || column && hasField(column) && + + functionOptions.push({ + label, + value: operationType, + className: 'lnsIndexPatternDimensionEditor__operation', + // 'data-test-subj': `lns-indexPatternDimension-${operationType}${ + // compatibleWithCurrentField ? '' : ' incompatible' + // }`, + }); + }); + + function onChooseFunction(operationType: OperationType) { + // Clear invalid state because we are creating a valid column + if (column?.operationType === operationType) { + return; + } + const possibleFieldNames = operationSupportMatrix.fieldByOperation[operationType]; + const possibleField = + possibleFieldNames?.size === 1 + ? currentIndexPattern.getFieldByName(possibleFieldNames.values().next().value) + : undefined; + + updateLayer( + insertOrReplaceColumn({ + layer, + columnId, + op: operationType, + indexPattern: currentIndexPattern, + field: possibleField, + }) + ); + trackUiEvent(`indexpattern_dimension_operation_${operationType}`); + return; + } + + const selectedOption = incompleteInfo?.operationType + ? [functionOptions.find(({ value }) => value === incompleteInfo.operationType)!] + : column + ? [functionOptions.find(({ value }) => value === column.operationType)!] + : []; + + // The current operation is invalid if + // const invalidOperation = + // Boolean(incompleteInfo?.operationType) || + // (column && + // selectedOperationDefinition.input === 'field' && + // !operationSupportMatrix.fieldByOperation[column.operationType]?.size); + const invalidField = + column && + selectedOperationDefinition.input === 'field' && + !operationSupportMatrix.fieldByOperation[column.operationType]?.size; + + return ( +
+
+ {selectionStyle !== 'field' ? ( + <> + + { + if (choices.length === 0) { + // onDeleteColumn(); + updateLayer(deleteColumn({ layer, columnId })); + return; + } + + trackUiEvent('indexpattern_dimension_field_changed'); + + onChooseFunction(choices[0].value!); + }} + /> + + + + ) : null} + + {!column || + // (incompatibleSelectedOperationType && + // operationDefinitionMap[incompatibleSelectedOperationType].input === 'field') ? ( + selectedOperationDefinition.input === 'field' ? ( + + { + updateLayer(deleteColumn({ layer, columnId })); + }} + onChoose={(choice) => { + let newLayer: IndexPatternLayer; + if ( + !incompleteInfo?.operationType && + column && + 'field' in choice && + choice.operationType === column.operationType + ) { + // Replaces just the field + newLayer = replaceColumn({ + layer, + columnId, + indexPattern: currentIndexPattern, + op: choice.operationType, + field: currentIndexPattern.getFieldByName(choice.field)!, + }); + } else { + // Finds a new operation + const compatibleOperations = + ('field' in choice && operationSupportMatrix.operationByField[choice.field]) || + new Set(); + let operation; + if (compatibleOperations.size > 0) { + operation = + incompleteInfo?.operationType && + compatibleOperations.has(incompleteInfo.operationType as OperationType) + ? incompleteInfo.operationType + : compatibleOperations.values().next().value; + } else if ('field' in choice) { + operation = choice.operationType; + } + newLayer = insertOrReplaceColumn({ + layer, + columnId, + field: currentIndexPattern.getFieldByName(choice.field), + indexPattern: currentIndexPattern, + op: operation as OperationType, + }); + } + updateLayer(newLayer); + // setInvalidOperationType(null); + }} + /> + + ) : null} +
+
+ ); +} + +{ + /* function getErrorMessage( + column: IndexPatternColumn | undefined, + incompatibleSelectedOperationType: boolean, + input: 'none' | 'field' | 'fullReference' | undefined, + fieldInvalid: boolean +) { + if (column && incompatibleSelectedOperationType) { + if (input === 'field') { + return i18n.translate('xpack.lens.indexPattern.invalidOperationLabel', { + defaultMessage: 'To use this function, select a different field.', + }); + } + return i18n.translate('xpack.lens.indexPattern.chooseFieldLabel', { + defaultMessage: 'To use this function, select a field.', + }); + } + if (fieldInvalid) { + return i18n.translate('xpack.lens.indexPattern.invalidFieldLabel', { + defaultMessage: 'Invalid field. Check your index pattern or pick another field.', + }); + } +} */ +} 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 3c1c8d5f2c0063..eea17613cd2491 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -982,8 +982,8 @@ describe('IndexPattern Data Source', () => { currentIndexPatternId: '1', }; expect(indexPatternDatasource.getErrorMessages(state)).toEqual([ - { shortMessage: 'error 1', longMessage: '' }, - { shortMessage: 'error 2', longMessage: '' }, + { longMessage: 'error 1', shortMessage: '' }, + { longMessage: 'error 2', shortMessage: '' }, ]); expect(getErrorMessages).toHaveBeenCalledTimes(1); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 948619c6b07e5a..badbc7bf5cdf39 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -370,8 +370,8 @@ export function getIndexPatternDatasource({ const layerErrors = Object.values(state.layers).flatMap((layer) => (getErrorMessages(layer) ?? []).map((message) => ({ - shortMessage: message, - longMessage: '', + shortMessage: '', // Not displayed currently + longMessage: message, })) ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx index 9fbad553d441a4..e393dd8ead63fa 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx @@ -1940,6 +1940,12 @@ describe('IndexPattern Data Source suggestions', () => { const suggestions = getDatasourceSuggestionsFromCurrentState(state); expect(suggestions).toEqual([]); }); + + describe('references', () => { + it('does not simplify a reference based operation to an invalid state', () => { + throw new Error('todo: write all the tests for references'); + }); + }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index 263b4646c9feb5..4afaeaa8ee38f5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -17,6 +17,7 @@ import { operationDefinitionMap, IndexPatternColumn, OperationType, + getExistingColumnGroups, } from './operations'; import { hasField, hasInvalidFields } from './utils'; import { @@ -221,7 +222,7 @@ function getExistingLayerSuggestionsForField( ); } - const [, metrics] = separateBucketColumns(layer); + const [, metrics, references] = getExistingColumnGroups(layer); if (metrics.length === 1) { const layerWithReplacedMetric = replaceColumn({ layer, @@ -377,7 +378,7 @@ export function getDatasourceSuggestionsFromCurrentState( .filter(([_id, layer]) => layer.columnOrder.length && layer.indexPatternId) .map(([layerId, layer]) => { const indexPattern = state.indexPatterns[layer.indexPatternId]; - const [buckets, metrics] = separateBucketColumns(layer); + const [buckets, metrics, references] = getExistingColumnGroups(layer); const timeDimension = layer.columnOrder.find( (columnId) => layer.columns[columnId].isBucketed && layer.columns[columnId].dataType === 'date' @@ -570,7 +571,11 @@ function createSuggestionWithDefaultDateHistogram( function createSimplifiedTableSuggestions(state: IndexPatternPrivateState, layerId: string) { const layer = state.layers[layerId]; - const [availableBucketedColumns, availableMetricColumns] = separateBucketColumns(layer); + const [ + availableBucketedColumns, + availableMetricColumns, + availableReferenceColumns, + ] = getExistingColumnGroups(layer); return _.flatten( availableBucketedColumns.map((_col, index) => { @@ -623,7 +628,3 @@ function getMetricSuggestionTitle(layer: IndexPatternLayer, onlyMetric: boolean) 'Title of a suggested chart containing only a single numerical metric calculated over all available data', }); } - -function separateBucketColumns(layer: IndexPatternLayer) { - return partition(layer.columnOrder, (columnId) => layer.columns[columnId].isBucketed); -} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx index 0cfba4cfc739f5..66e09712c564c2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx @@ -60,7 +60,8 @@ export const counterRateOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label, column.timeScale); + const ref = columns[column.references[0]]; + return ofName(ref && 'sourceField' in ref ? ref.sourceField : undefined, column.timeScale); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'lens_counter_rate'); @@ -69,7 +70,7 @@ export const counterRateOperation: OperationDefinition< const metric = layer.columns[referenceIds[0]]; const timeScale = previousColumn?.timeScale || DEFAULT_TIME_SCALE; return { - label: ofName(metric?.label, timeScale), + label: ofName(metric && 'sourceField' in metric ? metric.sourceField : undefined, timeScale), dataType: 'number', operationType: 'counter_rate', isBucketed: false, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx index 9244aaaf90ab76..b0d99934cba744 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx @@ -12,7 +12,7 @@ import { OperationDefinition } from '..'; const ofName = (name?: string) => { return i18n.translate('xpack.lens.indexPattern.cumulativeSumOf', { - defaultMessage: 'Cumulative sum rate of {name}', + defaultMessage: 'Cumulative sum of {name}', values: { name: name ?? @@ -54,15 +54,16 @@ export const cumulativeSumOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label); + const ref = columns[column.references[0]]; + return ofName(ref && 'sourceField' in ref ? ref.sourceField : undefined); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'cumulative_sum'); }, buildColumn: ({ referenceIds, previousColumn, layer }) => { - const metric = layer.columns[referenceIds[0]]; + const ref = layer.columns[referenceIds[0]]; return { - label: ofName(metric?.label), + label: ofName(ref && 'sourceField' in ref ? ref.sourceField : undefined), dataType: 'number', operationType: 'cumulative_sum', isBucketed: false, 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 41fe361c7ba9cf..0b7f0a79f6d117 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 @@ -59,15 +59,19 @@ export const derivativeOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label, column.timeScale); + const ref = columns[column.references[0]]; + return ofName(ref && 'sourceField' in ref ? ref.sourceField : undefined, column.timeScale); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'derivative'); }, buildColumn: ({ referenceIds, previousColumn, layer }) => { - const metric = layer.columns[referenceIds[0]]; + const ref = layer.columns[referenceIds[0]]; return { - label: ofName(metric?.label, previousColumn?.timeScale), + label: ofName( + ref && 'sourceField' in ref ? ref.sourceField : undefined, + previousColumn?.timeScale + ), dataType: 'number', operationType: 'derivative', isBucketed: false, 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 522899662fbd10..1c9bb5533c7b70 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 @@ -50,7 +50,7 @@ export const movingAverageOperation: OperationDefinition< type: 'moving_average', priority: 1, displayName: i18n.translate('xpack.lens.indexPattern.movingAverage', { - defaultMessage: 'Moving Average', + defaultMessage: 'Moving average', }), input: 'fullReference', selectionStyle: 'full', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index 7ffbeac39c6f5b..9acc033e50756a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -190,6 +190,44 @@ describe('state_helpers', () => { ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2'] })); }); + it('should insert a metric after buckets, but before references', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + col3: { + label: 'Reference', + dataType: 'number', + isBucketed: false, + + operationType: 'cumulative_sum', + references: ['col2'], + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'count', + field: documentField, + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2', 'col3'] })); + }); + it('should insert new buckets at the end of previous buckets', () => { const layer: IndexPatternLayer = { indexPatternId: '1', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index b16418d44ba330..b45a0bd73818fb 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -67,9 +67,15 @@ export function insertNewColumn({ const possibleOperation = operationDefinition.getPossibleOperation(); const isBucketed = Boolean(possibleOperation.isBucketed); if (isBucketed) { - return addBucket(layer, operationDefinition.buildColumn({ ...baseOptions, layer }), columnId); + return updateDefaultLabels( + addBucket(layer, operationDefinition.buildColumn({ ...baseOptions, layer }), columnId), + indexPattern + ); } else { - return addMetric(layer, operationDefinition.buildColumn({ ...baseOptions, layer }), columnId); + return updateDefaultLabels( + addMetric(layer, operationDefinition.buildColumn({ ...baseOptions, layer }), columnId), + indexPattern + ); } } @@ -131,7 +137,7 @@ export function insertNewColumn({ const possibleOperation = operationDefinition.getPossibleOperation(); const isBucketed = Boolean(possibleOperation.isBucketed); if (isBucketed) { - return addBucket( + tempLayer = addBucket( tempLayer, operationDefinition.buildColumn({ ...baseOptions, @@ -141,7 +147,7 @@ export function insertNewColumn({ columnId ); } else { - return addMetric( + tempLayer = addMetric( tempLayer, operationDefinition.buildColumn({ ...baseOptions, @@ -151,6 +157,8 @@ export function insertNewColumn({ columnId ); } + + return updateDefaultLabels(tempLayer, indexPattern); } const invalidFieldName = (layer.incompleteColumns ?? {})[columnId]?.sourceField; @@ -165,16 +173,22 @@ export function insertNewColumn({ } const isBucketed = Boolean(possibleOperation.isBucketed); if (isBucketed) { - return addBucket( - layer, - operationDefinition.buildColumn({ ...baseOptions, layer, field: invalidField }), - columnId + return updateDefaultLabels( + addBucket( + layer, + operationDefinition.buildColumn({ ...baseOptions, layer, field: invalidField }), + columnId + ), + indexPattern ); } else { - return addMetric( - layer, - operationDefinition.buildColumn({ ...baseOptions, layer, field: invalidField }), - columnId + return updateDefaultLabels( + addMetric( + layer, + operationDefinition.buildColumn({ ...baseOptions, layer, field: invalidField }), + columnId + ), + indexPattern ); } } else if (!field) { @@ -200,16 +214,14 @@ export function insertNewColumn({ } const isBucketed = Boolean(possibleOperation.isBucketed); if (isBucketed) { - return addBucket( - layer, - operationDefinition.buildColumn({ ...baseOptions, layer, field }), - columnId + return updateDefaultLabels( + addBucket(layer, operationDefinition.buildColumn({ ...baseOptions, layer, field }), columnId), + indexPattern ); } else { - return addMetric( - layer, - operationDefinition.buildColumn({ ...baseOptions, layer, field }), - columnId + return updateDefaultLabels( + addMetric(layer, operationDefinition.buildColumn({ ...baseOptions, layer, field }), columnId), + indexPattern ); } } @@ -251,6 +263,8 @@ export function replaceColumn({ }); } + tempLayer = resetIncomplete(tempLayer, columnId); + if (operationDefinition.input === 'fullReference') { const referenceIds = operationDefinition.requiredReferences.map(() => generateId()); @@ -263,11 +277,14 @@ export function replaceColumn({ previousColumn, }), }; - return { - ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: newColumns, - }; + return updateDefaultLabels( + { + ...tempLayer, + columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), + columns: newColumns, + }, + indexPattern + ); } if (operationDefinition.input === 'none') { @@ -275,11 +292,14 @@ export function replaceColumn({ newColumn = adjustLabel(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; - return { - ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), - }; + return updateDefaultLabels( + { + ...tempLayer, + columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + }, + indexPattern + ); } if (!field) { @@ -296,11 +316,14 @@ export function replaceColumn({ newColumn = adjustLabel(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; - return { - ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), - }; + return updateDefaultLabels( + { + ...tempLayer, + columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + }, + indexPattern + ); } else if ( operationDefinition.input === 'field' && field && @@ -310,12 +333,20 @@ export function replaceColumn({ // Same operation, new field const newColumn = operationDefinition.onFieldChange(previousColumn, field); - const newColumns = { ...layer.columns, [columnId]: adjustLabel(newColumn, previousColumn) }; - return { - ...layer, - columnOrder: getColumnOrder({ ...layer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), - }; + if (previousColumn.customLabel) { + newColumn.customLabel = true; + newColumn.label = previousColumn.label; + } + + const newColumns = { ...layer.columns, [columnId]: newColumn }; + return updateDefaultLabels( + { + ...resetIncomplete(layer, columnId), + columnOrder: getColumnOrder({ ...layer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + }, + indexPattern + ); } else { throw new Error('nothing changed'); } @@ -376,7 +407,6 @@ function addMetric( ...layer.columns, [addedColumnId]: column, }, - columnOrder: [...layer.columnOrder, addedColumnId], }; return { ...tempLayer, columnOrder: getColumnOrder(tempLayer) }; } @@ -478,6 +508,8 @@ export function deleteColumn({ const newIncomplete = { ...(newLayer.incompleteColumns || {}) }; delete newIncomplete[columnId]; + // TODO: Deleting should also update labels, but it's not actually required for now because of the + // reference UI not supporting inner deletion return { ...newLayer, columnOrder: getColumnOrder(newLayer), incompleteColumns: newIncomplete }; } @@ -499,7 +531,7 @@ export function getColumnOrder(layer: IndexPatternLayer): string[] { const [direct, referenceBased] = _.partition( entries, - ([id, col]) => operationDefinitionMap[col.operationType].input !== 'fullReference' + ([, col]) => operationDefinitionMap[col.operationType].input !== 'fullReference' ); // If a reference has another reference as input, put it last in sort order referenceBased.sort(([idA, a], [idB, b]) => { @@ -520,7 +552,7 @@ export function getColumnOrder(layer: IndexPatternLayer): string[] { } // Splits existing columnOrder into the three categories -function getExistingColumnGroups(layer: IndexPatternLayer): [string[], string[], string[]] { +export function getExistingColumnGroups(layer: IndexPatternLayer): [string[], string[], string[]] { const [direct, referenced] = partition( layer.columnOrder, (columnId) => layer.columns[columnId] && !('references' in layer.columns[columnId]) @@ -579,7 +611,7 @@ export function getErrorMessages(layer: IndexPatternLayer): string[] | undefined if (!layer.columns[referenceId]) { errors.push( i18n.translate('xpack.lens.indexPattern.missingReferenceError', { - defaultMessage: 'Dimension {dimensionLabel} is incomplete', + defaultMessage: '"{dimensionLabel}" is missing required configuration', values: { dimensionLabel: column.label, }, @@ -620,7 +652,7 @@ export function isReferenced(layer: IndexPatternLayer, columnId: string): boolea return allReferences.includes(columnId); } -function isColumnValidAsReference({ +export function isColumnValidAsReference({ column, validation, }: { @@ -637,7 +669,7 @@ function isColumnValidAsReference({ ); } -function isOperationAllowedAsReference({ +export function isOperationAllowedAsReference({ operationType, validation, field, @@ -665,6 +697,29 @@ function isOperationAllowedAsReference({ ); } +// Labels need to be updated when columns are added because reference-based column labels +// are sometimes copied into the parents +function updateDefaultLabels( + layer: IndexPatternLayer, + indexPattern: IndexPattern +): IndexPatternLayer { + const copiedColumns = { ...layer.columns }; + layer.columnOrder.forEach((id) => { + const col = copiedColumns[id]; + if (!col.customLabel) { + copiedColumns[id] = { + ...col, + label: operationDefinitionMap[col.operationType].getDefaultLabel( + col, + indexPattern, + copiedColumns + ), + }; + } + }); + return { ...layer, columns: copiedColumns }; +} + export function resetIncomplete(layer: IndexPatternLayer, columnId: string): IndexPatternLayer { const incompleteColumns = { ...(layer.incompleteColumns ?? {}) }; delete incompleteColumns[columnId]; From 706675522655f11aead97da19e966211b0911c5e Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Wed, 2 Dec 2020 19:06:22 -0500 Subject: [PATCH 02/10] Fix tests --- .../dimension_panel/dimension_panel.test.tsx | 10 +- .../indexpattern_suggestions.test.tsx | 192 +++++++++++++++++- .../indexpattern_suggestions.ts | 46 +++-- 3 files changed, 216 insertions(+), 32 deletions(-) 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 ada44d89188b98..6e1f468599c448 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 @@ -827,6 +827,7 @@ describe('IndexPatternDimensionEditorPanel', () => { dataType: 'date', isBucketed: true, label: '', + customLabel: true, operationType: 'date_histogram', sourceField: 'ts', params: { @@ -845,6 +846,7 @@ describe('IndexPatternDimensionEditorPanel', () => { columnId: 'col2', }; } + it('should not show custom options if time scaling is not available', () => { wrapper = mount( { }); it('should support selecting the operation before the field', () => { - setState.mockImplementation((newState) => { - wrapper.setProps({ state: newState }); - }); - wrapper = mount(); wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); @@ -1182,10 +1180,6 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should indicate compatible fields when selecting the operation first', () => { - setState.mockImplementation((newState) => { - wrapper.setProps({ state: newState }); - }); - wrapper = mount(); act(() => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx index e393dd8ead63fa..54fa565562ac99 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx @@ -6,11 +6,12 @@ import { DatasourceSuggestion } from '../types'; import { generateId } from '../id_generator'; -import { IndexPatternPrivateState } from './types'; +import type { IndexPatternPrivateState, IndexPatternLayer } from './types'; import { getDatasourceSuggestionsForField, getDatasourceSuggestionsFromCurrentState, getDatasourceSuggestionsForVisualizeField, + IndexPatternSuggestion, } from './indexpattern_suggestions'; import { documentField } from './document_field'; import { getFieldByNameFactory } from './pure_helpers'; @@ -153,6 +154,7 @@ function testInitialState(): IndexPatternPrivateState { columns: { col1: { label: 'My Op', + customLabel: true, dataType: 'string', isBucketed: true, @@ -172,6 +174,19 @@ function testInitialState(): IndexPatternPrivateState { }; } +// Simplifies the debug output for failed test +function getSuggestionSubset( + suggestions: IndexPatternSuggestion[] +): Array> { + return suggestions.map((s) => { + const newSuggestion = { ...s } as Omit & { + state?: IndexPatternPrivateState; + }; + delete newSuggestion.state; + return newSuggestion; + }); +} + describe('IndexPattern Data Source suggestions', () => { beforeEach(async () => { let count = 0; @@ -698,6 +713,7 @@ describe('IndexPattern Data Source suggestions', () => { isBucketed: true, sourceField: 'source', label: 'values of source', + customLabel: true, operationType: 'terms', params: { orderBy: { type: 'column', columnId: 'colb' }, @@ -710,6 +726,7 @@ describe('IndexPattern Data Source suggestions', () => { isBucketed: false, sourceField: 'bytes', label: 'Avg of bytes', + customLabel: true, operationType: 'avg', }, }, @@ -733,7 +750,7 @@ describe('IndexPattern Data Source suggestions', () => { dataType: 'date', isBucketed: true, sourceField: 'timestamp', - label: 'date histogram of timestamp', + label: 'timestamp', operationType: 'date_histogram', params: { interval: 'w', @@ -744,6 +761,7 @@ describe('IndexPattern Data Source suggestions', () => { isBucketed: false, sourceField: 'bytes', label: 'Avg of bytes', + customLabel: true, operationType: 'avg', }, }, @@ -1008,6 +1026,80 @@ describe('IndexPattern Data Source suggestions', () => { const suggestions = getDatasourceSuggestionsForField(modifiedState, '1', documentField); expect(suggestions).not.toContain(expect.objectContaining({ changeType: 'extended' })); }); + + it('hides any referenced metrics when adding new metrics', () => { + const initialState = stateWithNonEmptyTables(); + const modifiedState: IndexPatternPrivateState = { + ...initialState, + layers: { + // ...initialState.layers, + currentLayer: { + indexPatternId: '1', + columnOrder: ['date', 'metric', 'ref'], + columns: { + date: { + label: '', + customLabel: true, + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { interval: 'auto' }, + }, + metric: { + label: '', + customLabel: true, + dataType: 'number', + isBucketed: false, + operationType: 'avg', + sourceField: 'bytes', + }, + ref: { + label: '', + customLabel: true, + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + }, + }, + }, + }; + const suggestions = getSuggestionSubset( + getDatasourceSuggestionsForField(modifiedState, '1', documentField) + ); + expect(suggestions).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + isMultiRow: true, + changeType: 'extended', + label: undefined, + layerId: 'currentLayer', + columns: [ + { + columnId: 'date', + operation: { dataType: 'date', isBucketed: true, label: '' }, + }, + { + columnId: 'id1', + operation: { + dataType: 'number', + isBucketed: false, + label: 'Count of records', + scale: 'ratio', + }, + }, + { + columnId: 'ref', + operation: { dataType: 'number', isBucketed: false, label: '' }, + }, + ], + }), + keptLayerIds: ['currentLayer'], + }) + ); + }); }); describe('finding the layer that is using the current index pattern', () => { @@ -1121,6 +1213,7 @@ describe('IndexPattern Data Source suggestions', () => { }); }); }); + describe('#getDatasourceSuggestionsForVisualizeField', () => { describe('with no layer', () => { function stateWithoutLayer() { @@ -1218,6 +1311,7 @@ describe('IndexPattern Data Source suggestions', () => { columns: { cola: { label: 'My Op 2', + customLabel: true, dataType: 'string', isBucketed: true, @@ -1305,6 +1399,7 @@ describe('IndexPattern Data Source suggestions', () => { columns: { cola: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: false, operationType: 'avg', @@ -1316,7 +1411,7 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - expect(getDatasourceSuggestionsFromCurrentState(state)).toContainEqual( + expect(getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state))).toContainEqual( expect.objectContaining({ table: { isMultiRow: true, @@ -1359,6 +1454,7 @@ describe('IndexPattern Data Source suggestions', () => { columns: { cola: { label: 'My Terms', + customLabel: true, dataType: 'string', isBucketed: true, operationType: 'terms', @@ -1372,6 +1468,7 @@ describe('IndexPattern Data Source suggestions', () => { }, colb: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: false, operationType: 'avg', @@ -1383,7 +1480,7 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - expect(getDatasourceSuggestionsFromCurrentState(state)).toContainEqual( + expect(getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state))).toContainEqual( expect.objectContaining({ table: { isMultiRow: true, @@ -1442,6 +1539,7 @@ describe('IndexPattern Data Source suggestions', () => { }, colb: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: true, operationType: 'range', @@ -1487,6 +1585,7 @@ describe('IndexPattern Data Source suggestions', () => { }, colb: { label: 'My Custom Range', + customLabel: true, dataType: 'string', isBucketed: true, operationType: 'range', @@ -1503,7 +1602,7 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - expect(getDatasourceSuggestionsFromCurrentState(state)).toContainEqual( + expect(getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state))).toContainEqual( expect.objectContaining({ table: { changeType: 'extended', @@ -1555,6 +1654,7 @@ describe('IndexPattern Data Source suggestions', () => { columns: { id1: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: false, operationType: 'avg', @@ -1631,6 +1731,7 @@ describe('IndexPattern Data Source suggestions', () => { columns: { col1: { label: 'My Op', + customLabel: true, dataType: 'string', isBucketed: true, @@ -1644,6 +1745,7 @@ describe('IndexPattern Data Source suggestions', () => { }, col2: { label: 'My Op', + customLabel: true, dataType: 'string', isBucketed: true, @@ -1657,6 +1759,7 @@ describe('IndexPattern Data Source suggestions', () => { }, col3: { label: 'My Op', + customLabel: true, dataType: 'string', isBucketed: true, @@ -1670,6 +1773,7 @@ describe('IndexPattern Data Source suggestions', () => { }, col4: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: false, @@ -1678,6 +1782,7 @@ describe('IndexPattern Data Source suggestions', () => { }, col5: { label: 'My Op', + customLabel: true, dataType: 'number', isBucketed: false, @@ -1770,7 +1875,7 @@ describe('IndexPattern Data Source suggestions', () => { ...initialState.layers.first, columns: { id1: { - label: 'Date histogram', + label: 'field2', dataType: 'date', isBucketed: true, @@ -1942,8 +2047,79 @@ describe('IndexPattern Data Source suggestions', () => { }); describe('references', () => { - it('does not simplify a reference based operation to an invalid state', () => { - throw new Error('todo: write all the tests for references'); + it('will reduce multiple references correctly to a single reference', () => { + const initialState = testInitialState(); + const state: IndexPatternPrivateState = { + ...initialState, + layers: { + ...initialState.layers, + first: { + ...initialState.layers.first, + columnOrder: ['date', 'metric', 'metric2', 'ref', 'ref2'], + + columns: { + date: { + label: '', + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { interval: 'auto' }, + }, + metric: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'count', + sourceField: 'Records', + }, + ref: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + metric2: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'count', + sourceField: 'Records', + }, + ref2: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric2'], + }, + }, + }, + }, + }; + + const result = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + + expect(result).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + changeType: 'reduced', + layerId: 'first', + columns: [ + { + columnId: 'date', + operation: { dataType: 'date', isBucketed: true, label: '' }, + }, + { + columnId: 'ref', + operation: { dataType: 'number', isBucketed: false, label: '' }, + }, + ], + }), + keptLayerIds: ['first'], + }) + ); }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index 5e311354ef2de0..a2157ff6bf71ba 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import _, { partition } from 'lodash'; +import _ from 'lodash'; import { i18n } from '@kbn/i18n'; import { generateId } from '../id_generator'; import { DatasourceSuggestion, TableChangeType } from '../types'; @@ -18,6 +18,7 @@ import { IndexPatternColumn, OperationType, getExistingColumnGroups, + isReferenced, } from './operations'; import { hasField, hasInvalidColumns } from './utils'; import { @@ -28,7 +29,7 @@ import { } from './types'; import { documentField } from './document_field'; -type IndexPatternSugestion = DatasourceSuggestion; +export type IndexPatternSuggestion = DatasourceSuggestion; function buildSuggestion({ state, @@ -72,10 +73,13 @@ function buildSuggestion({ }, table: { - columns: columnOrder.map((columnId) => ({ - columnId, - operation: columnToOperation(columnMap[columnId]), - })), + columns: columnOrder + // Hide any referenced columns from what visualizations know about + .filter((columnId) => !isReferenced(layers[layerId]!, columnId)) + .map((columnId) => ({ + columnId, + operation: columnToOperation(columnMap[columnId]), + })), isMultiRow, layerId, changeType, @@ -90,7 +94,7 @@ export function getDatasourceSuggestionsForField( state: IndexPatternPrivateState, indexPatternId: string, field: IndexPatternField -): IndexPatternSugestion[] { +): IndexPatternSuggestion[] { if (hasInvalidColumns(state)) return []; const layers = Object.keys(state.layers); const layerIds = layers.filter((id) => state.layers[id].indexPatternId === indexPatternId); @@ -124,7 +128,7 @@ export function getDatasourceSuggestionsForVisualizeField( state: IndexPatternPrivateState, indexPatternId: string, fieldName: string -): IndexPatternSugestion[] { +): IndexPatternSuggestion[] { const layers = Object.keys(state.layers); const layerIds = layers.filter((id) => state.layers[id].indexPatternId === indexPatternId); // Identify the field by the indexPatternId and the fieldName @@ -159,7 +163,7 @@ function getExistingLayerSuggestionsForField( const fieldInUse = Object.values(layer.columns).some( (column) => hasField(column) && column.sourceField === field.name ); - const suggestions: IndexPatternSugestion[] = []; + const suggestions: IndexPatternSuggestion[] = []; if (usableAsBucketOperation && !fieldInUse) { if ( @@ -223,7 +227,8 @@ function getExistingLayerSuggestionsForField( } const [, metrics, references] = getExistingColumnGroups(layer); - if (metrics.length === 1) { + // TODO: Write test for the case where we have exactly one metric and one reference. We shouldn't switch the inner metric. + if (metrics.length === 1 && references.length === 0) { const layerWithReplacedMetric = replaceColumn({ layer, indexPattern, @@ -258,7 +263,7 @@ function getEmptyLayerSuggestionsForField( layerId: string, indexPatternId: string, field: IndexPatternField -): IndexPatternSugestion[] { +): IndexPatternSuggestion[] { const indexPattern = state.indexPatterns[indexPatternId]; let newLayer: IndexPatternLayer | undefined; const bucketOperation = getBucketOperation(field); @@ -586,7 +591,14 @@ function createSimplifiedTableSuggestions(state: IndexPatternPrivateState, layer columnOrder: [...bucketedColumns, ...availableMetricColumns], }; - if (availableMetricColumns.length > 1) { + // Assumes that all references have exactly 1 metric, which might not be true + if (availableReferenceColumns.length > 1) { + return [ + allMetricsSuggestion, + { ...layer, columnOrder: [...bucketedColumns, availableReferenceColumns[0]] }, + ]; + } else if (availableMetricColumns.length > 1 && !availableReferenceColumns.length) { + // Only simplify metrics when there are no references return [ allMetricsSuggestion, { ...layer, columnOrder: [...bucketedColumns, availableMetricColumns[0]] }, @@ -597,10 +609,12 @@ function createSimplifiedTableSuggestions(state: IndexPatternPrivateState, layer }) ) .concat( - availableMetricColumns.map((columnId) => { - // build suggestions with only metrics - return { ...layer, columnOrder: [columnId] }; - }) + availableReferenceColumns.length + ? [] + : availableMetricColumns.map((columnId) => { + // build suggestions with only metrics + return { ...layer, columnOrder: [columnId] }; + }) ) .map((updatedLayer) => { return buildSuggestion({ From 1e4e0eecd8507b74e78aaf4930e0abe86b32cf99 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 3 Dec 2020 18:28:34 -0500 Subject: [PATCH 03/10] Add a few unit tests for reference editor --- .../dimension_panel/dimension_panel.test.tsx | 37 +++++ .../dimension_panel/reference_editor.test.tsx | 128 ++++++++++++++++++ .../dimension_panel/reference_editor.tsx | 108 +++------------ .../operations/__mocks__/index.ts | 2 + 4 files changed, 186 insertions(+), 89 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.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 68a13be2909e5a..e4a2a2de93ddf2 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 @@ -1481,4 +1481,41 @@ describe('IndexPatternDimensionEditorPanel', () => { }, }); }); + + it('should hide the top level field selector when switching from non-reference to reference', () => { + wrapper = mount(); + + expect(wrapper.find('ReferenceEditor')).toHaveLength(0); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-derivative incompatible"]') + .simulate('click'); + + expect(wrapper.find('ReferenceEditor')).toHaveLength(1); + }); + + it('should hide the reference editors when switching from reference to non-reference', () => { + const stateWithReferences: IndexPatternPrivateState = getStateWithColumns({ + col1: { + label: 'Differences of (incomplete)', + dataType: 'number', + isBucketed: false, + operationType: 'derivative', + references: ['col2'], + params: {}, + }, + }); + + wrapper = mount( + + ); + + expect(wrapper.find('ReferenceEditor')).toHaveLength(1); + + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-avg incompatible"]') + .simulate('click'); + + expect(wrapper.find('ReferenceEditor')).toHaveLength(0); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx new file mode 100644 index 00000000000000..3737a50f18364a --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx @@ -0,0 +1,128 @@ +/* + * 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 { ReactWrapper, ShallowWrapper } from 'enzyme'; +import { EuiComboBox } from '@elastic/eui'; +import { mountWithIntl as mount } from '@kbn/test/jest'; +import { OperationMetadata } from '../../types'; +import { createMockedIndexPattern } from '../mocks'; +import { ReferenceEditor, ReferenceEditorProps } from './reference_editor'; + +describe('reference editor', () => { + let wrapper: ReactWrapper | ShallowWrapper; + let updateLayer: jest.Mock; + + function getDefaultArgs() { + return { + layer: { + indexPatternId: '1', + columns: {}, + columnOrder: [], + }, + columnId: 'ref', + updateLayer, + selectionStyle: 'full' as const, + currentIndexPattern: createMockedIndexPattern(), + existingFields: { + 'my-fake-index-pattern': { + timestamp: true, + bytes: true, + memory: true, + source: true, + }, + }, + }; + } + + beforeEach(() => { + updateLayer = jest.fn().mockImplementation((newLayer) => { + if (wrapper instanceof ReactWrapper) { + wrapper.setProps({ layer: newLayer }); + } + }); + + jest.clearAllMocks(); + }); + + afterEach(() => { + if (wrapper) { + wrapper.unmount(); + } + }); + + it('should indicate that all functions and available fields are compatible in the empty state', () => { + wrapper = mount( + meta.dataType === 'number', + }} + /> + ); + + const functions = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]') + .prop('options'); + + expect(functions).not.toContainEqual( + expect.objectContaining({ 'data-test-subj': expect.stringContaining('Incompatible') }) + ); + + const fields = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-dimension-field"]') + .prop('options'); + + expect(fields![0].options).not.toContainEqual( + expect.objectContaining({ 'data-test-subj': expect.stringContaining('Incompatible') }) + ); + }); + + it('should indicate functions and fields that are incompatible with the current', () => { + wrapper = mount( + meta.isBucketed, + }} + /> + ); + + const functions = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]') + .prop('options'); + expect(functions.find(({ label }) => label === 'Date histogram')!['data-test-subj']).toContain( + 'incompatible' + ); + + const fields = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-dimension-field"]') + .prop('options'); + expect( + fields![0].options!.find(({ label }) => label === 'timestampLabel')!['data-test-subj'] + ).toContain('Incompatible'); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index a49772a18e09e5..ef1ed44dfdc9a5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -30,7 +30,6 @@ const operationPanels = getOperationDisplay(); export interface ReferenceEditorProps { layer: IndexPatternLayer; - parentColumnId: string; selectionStyle: 'full' | 'field'; validation: RequiredReference; columnId: string; @@ -99,16 +98,23 @@ export function ReferenceEditor(props: ReferenceEditorProps) { const functionOptions: Array> = []; operationSupportMatrix.operationTypes.forEach((operationType) => { + const def = operationDefinitionMap[operationType]; const label = operationPanels[operationType].displayName; - // const isCompatible = !column || column && hasField(column) && + const isCompatible = + !column || + (column && + hasField(column) && + def.input === 'field' && + operationSupportMatrix.fieldByOperation[operationType]?.has(column.sourceField)) || + (column && !hasField(column) && def.input !== 'field'); functionOptions.push({ label, value: operationType, className: 'lnsIndexPatternDimensionEditor__operation', - // 'data-test-subj': `lns-indexPatternDimension-${operationType}${ - // compatibleWithCurrentField ? '' : ' incompatible' - // }`, + 'data-test-subj': `lns-indexPatternDimension-${operationType}${ + isCompatible ? '' : ' incompatible' + }`, }); }); @@ -142,12 +148,6 @@ export function ReferenceEditor(props: ReferenceEditorProps) { ? [functionOptions.find(({ value }) => value === column.operationType)!] : []; - // The current operation is invalid if - // const invalidOperation = - // Boolean(incompleteInfo?.operationType) || - // (column && - // selectedOperationDefinition.input === 'field' && - // !operationSupportMatrix.fieldByOperation[column.operationType]?.size); const invalidField = column && selectedOperationDefinition.input === 'field' && @@ -159,19 +159,12 @@ export function ReferenceEditor(props: ReferenceEditorProps) { {selectionStyle !== 'field' ? ( <> { @@ -210,19 +203,12 @@ export function ReferenceEditor(props: ReferenceEditorProps) { // operationDefinitionMap[incompatibleSelectedOperationType].input === 'field') ? ( selectedOperationDefinition.input === 'field' ? ( { - let newLayer: IndexPatternLayer; - if ( - !incompleteInfo?.operationType && - column && - 'field' in choice && - choice.operationType === column.operationType - ) { - // Replaces just the field - newLayer = replaceColumn({ + updateLayer( + insertOrReplaceColumn({ layer, columnId, indexPattern: currentIndexPattern, op: choice.operationType, - field: currentIndexPattern.getFieldByName(choice.field)!, - }); - } else { - // Finds a new operation - const compatibleOperations = - ('field' in choice && operationSupportMatrix.operationByField[choice.field]) || - new Set(); - let operation; - if (compatibleOperations.size > 0) { - operation = - incompleteInfo?.operationType && - compatibleOperations.has(incompleteInfo.operationType as OperationType) - ? incompleteInfo.operationType - : compatibleOperations.values().next().value; - } else if ('field' in choice) { - operation = choice.operationType; - } - newLayer = insertOrReplaceColumn({ - layer, - columnId, field: currentIndexPattern.getFieldByName(choice.field), - indexPattern: currentIndexPattern, - op: operation as OperationType, - }); - } - updateLayer(newLayer); - // setInvalidOperationType(null); + }) + ); }} /> @@ -292,28 +247,3 @@ export function ReferenceEditor(props: ReferenceEditorProps) {
); } - -{ - /* function getErrorMessage( - column: IndexPatternColumn | undefined, - incompatibleSelectedOperationType: boolean, - input: 'none' | 'field' | 'fullReference' | undefined, - fieldInvalid: boolean -) { - if (column && incompatibleSelectedOperationType) { - if (input === 'field') { - return i18n.translate('xpack.lens.indexPattern.invalidOperationLabel', { - defaultMessage: 'To use this function, select a different field.', - }); - } - return i18n.translate('xpack.lens.indexPattern.chooseFieldLabel', { - defaultMessage: 'To use this function, select a field.', - }); - } - if (fieldInvalid) { - return i18n.translate('xpack.lens.indexPattern.invalidFieldLabel', { - defaultMessage: 'Invalid field. Check your index pattern or pick another field.', - }); - } -} */ -} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts index ff900134df9a1a..45600e4df556b7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts @@ -42,6 +42,8 @@ export const { getErrorMessages, isReferenced, resetIncomplete, + isColumnValidAsReference, + isOperationAllowedAsReference, } = actualHelpers; export const { adjustTimeScaleLabelSuffix, DEFAULT_TIME_SCALE } = actualTimeScaleUtils; From bd72fa48fd7e18b1d7bf8f0400c5489fac53deb2 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Tue, 15 Dec 2020 18:37:13 -0500 Subject: [PATCH 04/10] Respond to review comments --- .../dimension_panel/dimension_editor.tsx | 25 ++- .../dimension_panel/dimension_panel.test.tsx | 28 +-- .../dimension_panel/reference_editor.test.tsx | 211 ++++++++++++++++++ .../dimension_panel/reference_editor.tsx | 53 +++-- .../indexpattern_suggestions.test.tsx | 2 +- .../definitions/calculations/counter_rate.tsx | 8 + .../calculations/cumulative_sum.tsx | 8 + .../definitions/calculations/derivative.tsx | 8 + .../calculations/moving_average.tsx | 10 +- .../operations/definitions/count.tsx | 7 +- .../operations/definitions/index.ts | 7 +- .../definitions/last_value.test.tsx | 15 +- .../operations/definitions/metrics.tsx | 6 +- .../operations/definitions/terms/index.tsx | 25 ++- .../definitions/terms/terms.test.tsx | 187 +++++++++++----- .../operations/index.ts | 1 + .../operations/layer_helpers.test.ts | 95 ++++++-- .../operations/layer_helpers.ts | 62 ++--- .../operations/time_scale_utils.test.ts | 67 ++++-- .../operations/time_scale_utils.ts | 16 +- 20 files changed, 651 insertions(+), 190 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index e85aa2fe53d663..76d32692848566 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 @@ -29,7 +29,6 @@ import { updateColumnParam, resetIncomplete, FieldBasedIndexPatternColumn, - OperationType, } from '../operations'; import { mergeLayer } from '../state_helpers'; import { FieldSelect } from './field_select'; @@ -150,7 +149,10 @@ export function DimensionEditor(props: DimensionEditorProps) { (selectedColumn && !hasField(selectedColumn) && definition.input === 'none'), disabledStatus: definition.getDisabledStatus && - definition.getDisabledStatus(state.indexPatterns[state.currentIndexPatternId]), + definition.getDisabledStatus( + state.indexPatterns[state.currentIndexPatternId], + state.layers[layerId] + ), }; }); @@ -300,7 +302,6 @@ export function DimensionEditor(props: DimensionEditorProps) { { setState(mergeLayer({ state, layerId, newLayer })); @@ -367,15 +368,6 @@ export function DimensionEditor(props: DimensionEditorProps) { ) : null} - {!currentFieldIsInvalid && !incompleteInfo && selectedColumn && ( - - )} - {!currentFieldIsInvalid && !incompleteInfo && selectedColumn && ParamEditor && ( <> )} + + {!currentFieldIsInvalid && !incompleteInfo && selectedColumn && ( + + )} 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 584e94b8eafd78..65b7bd9a759b8b 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 @@ -1145,20 +1145,22 @@ describe('IndexPatternDimensionEditorPanel', () => { comboBox.prop('onChange')!([options![1].options![0]]); }); - expect(setState).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columnOrder: ['col1', 'col2'], - columns: { - ...state.layers.first.columns, - col2: expect.objectContaining({ - operationType: 'avg', - sourceField: 'bytes', - }), + expect(setState).toHaveBeenCalledWith( + { + ...state, + layers: { + first: { + ...state.layers.first, + columnOrder: ['col1', 'col2'], + columns: { + ...state.layers.first.columns, + col2: expect.objectContaining({ + operationType: 'avg', + sourceField: 'bytes', + }), + }, + incompleteColumns: {}, }, - incompleteColumns: {}, }, }, true diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx index 3737a50f18364a..3f4c5725654363 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx @@ -6,11 +6,16 @@ import React from 'react'; import { ReactWrapper, ShallowWrapper } from 'enzyme'; +import { act } from 'react-dom/test-utils'; import { EuiComboBox } from '@elastic/eui'; import { mountWithIntl as mount } from '@kbn/test/jest'; import { OperationMetadata } from '../../types'; import { createMockedIndexPattern } from '../mocks'; import { ReferenceEditor, ReferenceEditorProps } from './reference_editor'; +import { insertOrReplaceColumn } from '../operations'; +import { FieldSelect } from './field_select'; + +jest.mock('../operations'); describe('reference editor', () => { let wrapper: ReactWrapper | ShallowWrapper; @@ -82,6 +87,9 @@ describe('reference editor', () => { expect(fields![0].options).not.toContainEqual( expect.objectContaining({ 'data-test-subj': expect.stringContaining('Incompatible') }) ); + expect(fields![1].options).not.toContainEqual( + expect.objectContaining({ 'data-test-subj': expect.stringContaining('Incompatible') }) + ); }); it('should indicate functions and fields that are incompatible with the current', () => { @@ -125,4 +133,207 @@ describe('reference editor', () => { fields![0].options!.find(({ label }) => label === 'timestampLabel')!['data-test-subj'] ).toContain('Incompatible'); }); + + it('should not update when selecting the same operation', () => { + wrapper = mount( + meta.dataType === 'number', + }} + /> + ); + + const comboBox = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]'); + const option = comboBox.prop('options')!.find(({ label }) => label === 'Average')!; + + act(() => { + comboBox.prop('onChange')!([option]); + }); + expect(insertOrReplaceColumn).not.toHaveBeenCalled(); + }); + + it('should keep the field when replacing an existing reference with a compatible function', () => { + wrapper = mount( + meta.dataType === 'number', + }} + /> + ); + + const comboBox = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]'); + const option = comboBox.prop('options')!.find(({ label }) => label === 'Maximum')!; + + act(() => { + comboBox.prop('onChange')!([option]); + }); + + expect(insertOrReplaceColumn).toHaveBeenCalledWith( + expect.objectContaining({ + op: 'max', + field: expect.objectContaining({ name: 'bytes' }), + }) + ); + }); + + it('should transition to another function with incompatible field', () => { + wrapper = mount( + true, + }} + /> + ); + + const comboBox = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]'); + const option = comboBox.prop('options')!.find(({ label }) => label === 'Date histogram')!; + + act(() => { + comboBox.prop('onChange')!([option]); + }); + + expect(insertOrReplaceColumn).toHaveBeenCalledWith( + expect.objectContaining({ + op: 'date_histogram', + field: undefined, + }) + ); + }); + + it('should hide the function selector when using a field-only selection style', () => { + wrapper = mount( + true, + }} + /> + ); + + const comboBox = wrapper + .find(EuiComboBox) + .filter('[data-test-subj="indexPattern-reference-function"]'); + expect(comboBox).toHaveLength(0); + }); + + it('should pass the incomplete operation info to FieldSelect', () => { + wrapper = mount( + true, + }} + /> + ); + + const fieldSelect = wrapper.find(FieldSelect); + expect(fieldSelect.prop('fieldIsInvalid')).toEqual(false); + expect(fieldSelect.prop('selectedField')).toEqual('bytes'); + expect(fieldSelect.prop('selectedOperationType')).toEqual('avg'); + expect(fieldSelect.prop('incompleteOperation')).toEqual('max'); + }); + + it('should pass the incomplete field info to FieldSelect', () => { + wrapper = mount( + true, + }} + /> + ); + + const fieldSelect = wrapper.find(FieldSelect); + expect(fieldSelect.prop('fieldIsInvalid')).toEqual(false); + expect(fieldSelect.prop('selectedField')).toEqual('timestamp'); + expect(fieldSelect.prop('selectedOperationType')).toEqual('avg'); + expect(fieldSelect.prop('incompleteOperation')).toBeUndefined(); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index ef1ed44dfdc9a5..a0a15280ca6e2b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -15,11 +15,10 @@ import { operationDefinitionMap, getOperationDisplay, insertOrReplaceColumn, - replaceColumn, deleteColumn, - RequiredReference, isOperationAllowedAsReference, FieldBasedIndexPatternColumn, + RequiredReference, } from '../operations'; import { FieldSelect } from './field_select'; import { hasField } from '../utils'; @@ -119,25 +118,39 @@ export function ReferenceEditor(props: ReferenceEditorProps) { }); function onChooseFunction(operationType: OperationType) { - // Clear invalid state because we are creating a valid column if (column?.operationType === operationType) { return; } const possibleFieldNames = operationSupportMatrix.fieldByOperation[operationType]; - const possibleField = - possibleFieldNames?.size === 1 - ? currentIndexPattern.getFieldByName(possibleFieldNames.values().next().value) - : undefined; - - updateLayer( - insertOrReplaceColumn({ - layer, - columnId, - op: operationType, - indexPattern: currentIndexPattern, - field: possibleField, - }) - ); + if (column && 'sourceField' in column && possibleFieldNames?.has(column.sourceField)) { + // Reuse the current field if possible + updateLayer( + insertOrReplaceColumn({ + layer, + columnId, + op: operationType, + indexPattern: currentIndexPattern, + field: currentIndexPattern.getFieldByName(column.sourceField), + }) + ); + } else { + // If reusing the field is impossible, we generally can't choose for the user. + // The one exception is if the field is the only possible field, like Count of Records. + const possibleField = + possibleFieldNames?.size === 1 + ? currentIndexPattern.getFieldByName(possibleFieldNames.values().next().value) + : undefined; + + updateLayer( + insertOrReplaceColumn({ + layer, + columnId, + op: operationType, + indexPattern: currentIndexPattern, + field: possibleField, + }) + ); + } trackUiEvent(`indexpattern_dimension_operation_${operationType}`); return; } @@ -183,7 +196,6 @@ export function ReferenceEditor(props: ReferenceEditorProps) { singleSelection={{ asPlainText: true }} onChange={(choices) => { if (choices.length === 0) { - // onDeleteColumn(); updateLayer(deleteColumn({ layer, columnId })); return; } @@ -198,10 +210,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) { ) : null} - {!column || - // (incompatibleSelectedOperationType && - // operationDefinitionMap[incompatibleSelectedOperationType].input === 'field') ? ( - selectedOperationDefinition.input === 'field' ? ( + {!column || selectedOperationDefinition.input === 'field' ? ( + adjustTimeScaleOnOtherColumnChange( + layer, + thisColumnId, + changedColumnId + ), toEsAggsFn: (column, columnId) => { return buildExpressionFunction('aggCount', { id: columnId, 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 1f19b4e7703137..ce2b412aceb39f 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 @@ -157,8 +157,9 @@ interface BaseOperationDefinitionProps { * return an updated column. If not implemented, the `id` function is used instead. */ onOtherColumnChanged?: ( - currentColumn: C, - columns: Partial> + layer: IndexPatternLayer, + thisColumnId: string, + changedColumnId: string ) => C; /** * React component for operation specific settings shown in the popover editor @@ -181,7 +182,7 @@ interface BaseOperationDefinitionProps { * but disable it from usage, this function returns the string describing * the status. Otherwise it returns undefined */ - getDisabledStatus?: (indexPattern: IndexPattern) => string | undefined; + getDisabledStatus?: (indexPattern: IndexPattern, layer: IndexPatternLayer) => string | undefined; /** * Validate that the operation has the right preconditions in the state. For example: * diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 6c896adfce9b16..a78e9c5b801182 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 @@ -322,22 +322,27 @@ describe('last_value', () => { it('should return disabledStatus if indexPattern does contain date field', () => { const indexPattern = createMockedIndexPattern(); - expect(lastValueOperation.getDisabledStatus!(indexPattern)).toEqual(undefined); + expect(lastValueOperation.getDisabledStatus!(indexPattern, state.layers.first)).toEqual( + undefined + ); const indexPatternWithoutTimeFieldName = { ...indexPattern, timeFieldName: undefined, }; - expect(lastValueOperation.getDisabledStatus!(indexPatternWithoutTimeFieldName)).toEqual( - undefined - ); + expect( + lastValueOperation.getDisabledStatus!(indexPatternWithoutTimeFieldName, state.layers.first) + ).toEqual(undefined); const indexPatternWithoutTimefields = { ...indexPatternWithoutTimeFieldName, fields: indexPattern.fields.filter((f) => f.type !== 'date'), }; - const disabledStatus = lastValueOperation.getDisabledStatus!(indexPatternWithoutTimefields); + const disabledStatus = lastValueOperation.getDisabledStatus!( + indexPatternWithoutTimefields, + state.layers.first + ); expect(disabledStatus).toEqual( 'This function requires the presence of a date field in your index' ); 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 bd4f36725f37e5..14672eee4d6606 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 @@ -80,8 +80,10 @@ function buildMetricOperation>({ (!newField.aggregationRestrictions || newField.aggregationRestrictions![type]) ); }, - onOtherColumnChanged: (column, otherColumns) => - optionalTimeScaling ? adjustTimeScaleOnOtherColumnChange(column, otherColumns) : column, + onOtherColumnChanged: (layer, thisColumnId, changedColumnId) => + optionalTimeScaling + ? adjustTimeScaleOnOtherColumnChange(layer, thisColumnId, changedColumnId) + : layer.columns[thisColumnId], 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/definitions/terms/index.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx index 1e2d800cb3e80a..154e1086d744ef 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 @@ -18,13 +18,13 @@ import { } from '@elastic/eui'; import { AggFunctionsMapping } from '../../../../../../../../src/plugins/data/public'; import { buildExpressionFunction } from '../../../../../../../../src/plugins/expressions/public'; -import { IndexPatternColumn } from '../../../indexpattern'; import { updateColumnParam, isReferenced } from '../../layer_helpers'; import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesRangeInput } from './values_range_input'; import { getInvalidFieldMessage } from '../helpers'; +import type { IndexPatternLayer } from '../../../types'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { @@ -33,8 +33,15 @@ function ofName(name: string) { }); } -function isSortableByColumn(column: IndexPatternColumn) { - return !column.isBucketed && column.operationType !== 'last_value'; +function isSortableByColumn(layer: IndexPatternLayer, columnId: string) { + const column = layer.columns[columnId]; + return ( + column && + !column.isBucketed && + column.operationType !== 'last_value' && + !('references' in column) && + !isReferenced(layer, columnId) + ); } const DEFAULT_SIZE = 3; @@ -89,9 +96,7 @@ export const termsOperation: OperationDefinition column && !column.isBucketed && !isReferenced(layer, columnId) - ) + .filter(([columnId]) => isSortableByColumn(layer, columnId)) .map(([id]) => id)[0]; const previousBucketsLength = Object.values(layer.columns).filter( @@ -151,11 +156,13 @@ export const termsOperation: OperationDefinition { + onOtherColumnChanged: (layer, thisColumnId, changedColumnId) => { + const columns = layer.columns; + const currentColumn = columns[thisColumnId] as TermsIndexPatternColumn; if (currentColumn.params.orderBy.type === 'column') { // check whether the column is still there and still a metric const columnSortedBy = columns[currentColumn.params.orderBy.columnId]; - if (!columnSortedBy || !isSortableByColumn(columnSortedBy)) { + if (!columnSortedBy || !isSortableByColumn(layer, changedColumnId)) { return { ...currentColumn, params: { @@ -194,7 +201,7 @@ export const termsOperation: OperationDefinition isSortableByColumn(column)) + .filter(([columnId]) => isSortableByColumn(state.layers[layerId], columnId)) .map(([columnId, column]) => { return { value: toValue({ type: 'column', columnId }), 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 dfca730c004dee..d895fa0a03d62b 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 @@ -383,15 +383,25 @@ describe('terms', () => { }, sourceField: 'category', }; - const updatedColumn = termsOperation.onOtherColumnChanged!(initialColumn, { - col1: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', + const updatedColumn = termsOperation.onOtherColumnChanged!( + { + indexPatternId: '', + columnOrder: [], + columns: { + col2: initialColumn, + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, }, - }); + 'col2', + 'col1' + ); + expect(updatedColumn).toBe(initialColumn); }); @@ -410,18 +420,74 @@ describe('terms', () => { }, sourceField: 'category', }; - const updatedColumn = termsOperation.onOtherColumnChanged!(initialColumn, { - col1: { - label: 'Last Value', - dataType: 'number', - isBucketed: false, - sourceField: 'bytes', - operationType: 'last_value', - params: { - sortField: 'time', + const updatedColumn = termsOperation.onOtherColumnChanged!( + { + columns: { + col2: initialColumn, + col1: { + label: 'Last Value', + dataType: 'number', + isBucketed: false, + sourceField: 'bytes', + operationType: 'last_value', + params: { + sortField: 'time', + }, + }, }, + columnOrder: [], + indexPatternId: '', }, - }); + 'col2', + 'col1' + ); + expect(updatedColumn.params).toEqual( + expect.objectContaining({ + orderBy: { type: 'alphabetical' }, + }) + ); + }); + + it('should switch to alphabetical ordering if metric is reference-based', () => { + const initialColumn: TermsIndexPatternColumn = { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'terms', + params: { + orderBy: { type: 'column', columnId: 'col1' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }; + const updatedColumn = termsOperation.onOtherColumnChanged!( + { + columns: { + col2: initialColumn, + col1: { + label: 'Cumulative sum', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['referenced'], + }, + referenced: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'count', + sourceField: 'Records', + }, + }, + columnOrder: [], + indexPatternId: '', + }, + 'col2', + 'col1' + ); expect(updatedColumn.params).toEqual( expect.objectContaining({ orderBy: { type: 'alphabetical' }, @@ -432,20 +498,27 @@ describe('terms', () => { it('should switch to alphabetical ordering if there are no columns to order by', () => { const termsColumn = termsOperation.onOtherColumnChanged!( { - label: 'Top value of category', - dataType: 'string', - isBucketed: true, - - // Private - operationType: 'terms', - params: { - orderBy: { type: 'column', columnId: 'col1' }, - size: 3, - orderDirection: 'asc', + columns: { + col2: { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'terms', + params: { + orderBy: { type: 'column', columnId: 'col1' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }, }, - sourceField: 'category', + columnOrder: [], + indexPatternId: '', }, - {} + 'col2', + 'col1' ); expect(termsColumn.params).toEqual( expect.objectContaining({ @@ -457,33 +530,39 @@ describe('terms', () => { it('should switch to alphabetical ordering if the order column is not a metric anymore', () => { const termsColumn = termsOperation.onOtherColumnChanged!( { - label: 'Top value of category', - dataType: 'string', - isBucketed: true, - - // Private - operationType: 'terms', - params: { - orderBy: { type: 'column', columnId: 'col1' }, - size: 3, - orderDirection: 'asc', - }, - sourceField: 'category', - }, - { - col1: { - label: 'Value of timestamp', - dataType: 'date', - isBucketed: true, + columns: { + col2: { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, - // Private - operationType: 'date_histogram', - params: { - interval: 'w', + // Private + operationType: 'terms', + params: { + orderBy: { type: 'column', columnId: 'col1' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }, + col1: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + params: { + interval: 'w', + }, + sourceField: 'timestamp', }, - sourceField: 'timestamp', }, - } + columnOrder: [], + indexPatternId: '', + }, + 'col2', + 'col1' ); expect(termsColumn.params).toEqual( expect.objectContaining({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts index 7123becf71b4dd..079913347470ad 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts @@ -12,6 +12,7 @@ export { IndexPatternColumn, FieldBasedIndexPatternColumn, IncompleteColumn, + RequiredReference, } from './definitions'; export { createMockedReferenceOperation } from './mocks'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index 8fcf9ec3a06745..c3904487e7c55b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -820,18 +820,83 @@ describe('state_helpers', () => { field: indexPattern.fields[2], // bytes field }); - expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(termsColumn, { - col1: termsColumn, - col2: expect.objectContaining({ - label: 'Average of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'avg', - sourceField: 'bytes', - }), + expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith( + { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: termsColumn, + col2: expect.objectContaining({ + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + sourceField: 'bytes', + operationType: 'avg', + }), + }, + incompleteColumns: {}, + }, + 'col1', + 'col2' + ); + }); + + it('should execute adjustments for other columns when creating a reference', () => { + const termsColumn: TermsIndexPatternColumn = { + label: 'Top values of source', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'terms', + sourceField: 'source', + params: { + orderBy: { type: 'column', columnId: 'willBeReference' }, + orderDirection: 'desc', + size: 5, + }, + }; + + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1', 'willBeReference'], + columns: { + col1: termsColumn, + willBeReference: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + }, + indexPattern, + columnId: 'willBeReference', + op: 'cumulative_sum', }); + + expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith( + { + indexPatternId: '1', + columnOrder: ['col1', 'willBeReference'], + columns: { + col1: { + ...termsColumn, + params: { orderBy: { type: 'alphabetical' }, orderDirection: 'asc', size: 5 }, + }, + willBeReference: expect.objectContaining({ + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + }), + }, + incompleteColumns: {}, + }, + 'col1', + 'willBeReference' + ); }); it('should not wrap the previous operation when switching to reference', () => { @@ -1216,9 +1281,11 @@ describe('state_helpers', () => { columnId: 'col2', }); - expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(termsColumn, { - col1: termsColumn, - }); + expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith( + { indexPatternId: '1', columnOrder: ['col1', 'col2'], columns: { col1: termsColumn } }, + 'col1', + 'col2' + ); }); it('should delete the column and all of its references', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index b45a0bd73818fb..ebd31d5b81b3c2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -268,20 +268,23 @@ export function replaceColumn({ if (operationDefinition.input === 'fullReference') { const referenceIds = operationDefinition.requiredReferences.map(() => generateId()); - const newColumns = { - ...tempLayer.columns, - [columnId]: operationDefinition.buildColumn({ - ...baseOptions, - layer: tempLayer, - referenceIds, - previousColumn, - }), + const newLayer = { + ...tempLayer, + columns: { + ...tempLayer.columns, + [columnId]: operationDefinition.buildColumn({ + ...baseOptions, + layer: tempLayer, + referenceIds, + previousColumn, + }), + }, }; return updateDefaultLabels( { ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: newColumns, + columnOrder: getColumnOrder(newLayer), + columns: adjustColumnReferencesForChangedColumn(newLayer, columnId), }, indexPattern ); @@ -291,12 +294,12 @@ export function replaceColumn({ let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer }); newColumn = adjustLabel(newColumn, previousColumn); - const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; + const newLayer = { ...tempLayer, columns: { ...tempLayer.columns, [columnId]: newColumn } }; return updateDefaultLabels( { ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + columnOrder: getColumnOrder(newLayer), + columns: adjustColumnReferencesForChangedColumn(newLayer, columnId), }, indexPattern ); @@ -315,12 +318,13 @@ export function replaceColumn({ let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, field }); newColumn = adjustLabel(newColumn, previousColumn); - const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; + // const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; + const newLayer = { ...tempLayer, columns: { ...tempLayer.columns, [columnId]: newColumn } }; return updateDefaultLabels( { ...tempLayer, - columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + columnOrder: getColumnOrder(newLayer), + columns: adjustColumnReferencesForChangedColumn(newLayer, columnId), }, indexPattern ); @@ -338,12 +342,12 @@ export function replaceColumn({ newColumn.label = previousColumn.label; } - const newColumns = { ...layer.columns, [columnId]: newColumn }; + const newLayer = { ...layer, columns: { ...layer.columns, [columnId]: newColumn } }; return updateDefaultLabels( { ...resetIncomplete(layer, columnId), - columnOrder: getColumnOrder({ ...layer, columns: newColumns }), - columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + columnOrder: getColumnOrder(newLayer), + columns: adjustColumnReferencesForChangedColumn(newLayer, columnId), }, indexPattern ); @@ -456,17 +460,18 @@ export function updateColumnParam({ }); } -function adjustColumnReferencesForChangedColumn( - columns: Record, - columnId: string -) { - const newColumns = { ...columns }; +function adjustColumnReferencesForChangedColumn(layer: IndexPatternLayer, changedColumnId: string) { + const newColumns = { ...layer.columns }; Object.keys(newColumns).forEach((currentColumnId) => { - if (currentColumnId !== columnId) { + if (currentColumnId !== changedColumnId) { const currentColumn = newColumns[currentColumnId]; const operationDefinition = operationDefinitionMap[currentColumn.operationType]; newColumns[currentColumnId] = operationDefinition.onOtherColumnChanged - ? operationDefinition.onOtherColumnChanged(currentColumn, newColumns) + ? operationDefinition.onOtherColumnChanged( + { ...layer, columns: newColumns }, + currentColumnId, + changedColumnId + ) : currentColumn; } }); @@ -498,7 +503,10 @@ export function deleteColumn({ let newLayer = { ...layer, - columns: adjustColumnReferencesForChangedColumn(hypotheticalColumns, columnId), + columns: adjustColumnReferencesForChangedColumn( + { ...layer, columns: hypotheticalColumns }, + columnId + ), }; extraDeletions.forEach((id) => { 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 index 841011c5884336..09132b142986f9 100644 --- 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 @@ -4,8 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { TimeScaleUnit } from '../time_scale'; -import { IndexPatternColumn } from './definitions'; +import type { IndexPatternLayer } from '../types'; +import type { TimeScaleUnit } from '../time_scale'; +import type { IndexPatternColumn } from './definitions'; import { adjustTimeScaleLabelSuffix, adjustTimeScaleOnOtherColumnChange } from './time_scale_utils'; export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; @@ -48,45 +49,71 @@ describe('time scale utils', () => { isBucketed: false, timeScale: 's', }; + const baseLayer: IndexPatternLayer = { + columns: { col1: baseColumn }, + columnOrder: [], + indexPatternId: '', + }; it('should keep column if there is no time scale', () => { const column = { ...baseColumn, timeScale: undefined }; - expect(adjustTimeScaleOnOtherColumnChange(column, { col1: column })).toBe(column); + expect( + adjustTimeScaleOnOtherColumnChange( + { ...baseLayer, columns: { col1: column } }, + 'col1', + 'col2' + ) + ).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: '', + adjustTimeScaleOnOtherColumnChange( + { + ...baseLayer, + columns: { + col1: baseColumn, + col2: { + operationType: 'date_histogram', + dataType: 'date', + isBucketed: true, + label: '', + sourceField: 'date', + params: { interval: 'auto' }, + }, + }, }, - }) + 'col1', + 'col2' + ) ).toBe(baseColumn); }); it('should remove time scale if there is no date histogram', () => { - expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( + expect(adjustTimeScaleOnOtherColumnChange(baseLayer, 'col1', 'col2')).toHaveProperty( 'timeScale', undefined ); }); it('should remove suffix from label', () => { - expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( - 'label', - 'Count of records' - ); + expect( + adjustTimeScaleOnOtherColumnChange( + { ...baseLayer, columns: { col1: baseColumn } }, + 'col1', + 'col2' + ) + ).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' - ); + expect( + adjustTimeScaleOnOtherColumnChange( + { ...baseLayer, columns: { col1: column } }, + 'col1', + 'col2' + ) + ).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 5d525e573a6177..340cad97e7db09 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 @@ -5,8 +5,9 @@ */ import { unitSuffixesLong } from '../suffix_formatter'; -import { TimeScaleUnit } from '../time_scale'; -import { BaseIndexPatternColumn } from './definitions/column_types'; +import type { TimeScaleUnit } from '../time_scale'; +import type { IndexPatternLayer } from '../types'; +import type { IndexPatternColumn } from './definitions'; export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; @@ -30,10 +31,13 @@ export function adjustTimeScaleLabelSuffix( return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; } -export function adjustTimeScaleOnOtherColumnChange( - column: T, - columns: Partial> -) { +export function adjustTimeScaleOnOtherColumnChange( + layer: IndexPatternLayer, + thisColumnId: string, + changedColumnId: string +): T { + const columns = layer.columns; + const column = columns[thisColumnId] as T; if (!column.timeScale) { return column; } From 9eafe0d13c7cfa36685a83e74c28ca1102a51d3b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Wed, 16 Dec 2020 18:30:03 -0500 Subject: [PATCH 05/10] Update error handling --- .../indexpattern.test.ts | 205 +++++------------- .../indexpattern_datasource/indexpattern.tsx | 119 +++------- .../indexpattern_suggestions.ts | 6 +- .../definitions/calculations/counter_rate.tsx | 4 +- .../calculations/cumulative_sum.tsx | 4 +- .../definitions/calculations/derivative.tsx | 4 +- .../calculations/moving_average.tsx | 1 + .../operations/definitions/helpers.test.ts | 56 +++++ .../operations/layer_helpers.test.ts | 72 +++++- .../operations/layer_helpers.ts | 19 +- .../public/indexpattern_datasource/utils.ts | 4 - 11 files changed, 231 insertions(+), 263 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts 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 a058042f13aed3..2a709d1c76f097 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -855,165 +855,49 @@ describe('IndexPattern Data Source', () => { it('should return null for non-existant columns', () => { expect(publicAPI.getOperationForColumnId('col2')).toBe(null); }); - }); - }); - - describe('#getErrorMessages', () => { - it('should detect a missing reference in a layer', () => { - const state = { - indexPatternRefs: [], - existingFields: {}, - isFirstExistenceFetch: false, - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - dataType: 'number', - isBucketed: false, - label: 'Foo', - operationType: 'count', // <= invalid - sourceField: 'bytes', - }, - }, - }, - }, - currentIndexPatternId: '1', - }; - const messages = indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState); - expect(messages).toHaveLength(1); - expect(messages![0]).toEqual({ - shortMessage: 'Invalid reference.', - longMessage: '"Foo" has an invalid reference.', - }); - }); - it('should detect and batch missing references in a layer', () => { - const state = { - indexPatternRefs: [], - existingFields: {}, - isFirstExistenceFetch: false, - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: { - dataType: 'number', - isBucketed: false, - label: 'Foo', - operationType: 'count', // <= invalid - sourceField: 'bytes', - }, - col2: { - dataType: 'number', - isBucketed: false, - label: 'Foo2', - operationType: 'count', // <= invalid - sourceField: 'memory', - }, - }, - }, - }, - currentIndexPatternId: '1', - }; - const messages = indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState); - expect(messages).toHaveLength(1); - expect(messages![0]).toEqual({ - shortMessage: 'Invalid references.', - longMessage: '"Foo", "Foo2" have invalid reference.', - }); - }); + it('should return null for referenced columns', () => { + publicAPI = indexPatternDatasource.getPublicAPI({ + state: { + ...enrichBaseState(baseState), + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Sum', + dataType: 'number', + isBucketed: false, - it('should detect and batch missing references in multiple layers', () => { - const state = { - indexPatternRefs: [], - existingFields: {}, - isFirstExistenceFetch: false, - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: { - dataType: 'number', - isBucketed: false, - label: 'Foo', - operationType: 'count', // <= invalid - sourceField: 'bytes', - }, - col2: { - dataType: 'number', - isBucketed: false, - label: 'Foo2', - operationType: 'count', // <= invalid - sourceField: 'memory', - }, - }, - }, - second: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - dataType: 'string', - isBucketed: false, - label: 'Foo', - operationType: 'count', // <= invalid - sourceField: 'source', - }, - }, - }, - }, - currentIndexPatternId: '1', - }; - const messages = indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState); - expect(messages).toHaveLength(2); - expect(messages).toEqual([ - { - shortMessage: 'Invalid references on Layer 1.', - longMessage: 'Layer 1 has invalid references in "Foo", "Foo2".', - }, - { - shortMessage: 'Invalid reference on Layer 2.', - longMessage: 'Layer 2 has an invalid reference in "Foo".', - }, - ]); - }); + operationType: 'sum', + sourceField: 'test', + params: {}, + } as IndexPatternColumn, + col2: { + label: 'Cumulative sum', + dataType: 'number', + isBucketed: false, - it('should return no errors if all references are satified', () => { - const state = { - indexPatternRefs: [], - existingFields: {}, - isFirstExistenceFetch: false, - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - dataType: 'number', - isBucketed: false, - label: 'Foo', - operationType: 'avg', - sourceField: 'bytes', + operationType: 'cumulative_sum', + references: ['col1'], + params: {}, + } as IndexPatternColumn, + }, }, }, }, - }, - currentIndexPatternId: '1', - }; - expect( - indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState) - ).toBeUndefined(); + layerId: 'first', + }); + expect(publicAPI.getOperationForColumnId('col1')).toEqual(null); + }); }); + }); - it('should return no errors with layers with no columns', () => { + describe('#getErrorMessages', () => { + it('should use the results of getErrorMessages directly when single layer', () => { + (getErrorMessages as jest.Mock).mockClear(); + (getErrorMessages as jest.Mock).mockReturnValueOnce(['error 1', 'error 2']); const state: IndexPatternPrivateState = { indexPatternRefs: [], existingFields: {}, @@ -1028,10 +912,14 @@ describe('IndexPattern Data Source', () => { }, currentIndexPatternId: '1', }; - expect(indexPatternDatasource.getErrorMessages(state)).toBeUndefined(); + expect(indexPatternDatasource.getErrorMessages(state)).toEqual([ + { longMessage: 'error 1', shortMessage: '' }, + { longMessage: 'error 2', shortMessage: '' }, + ]); + expect(getErrorMessages).toHaveBeenCalledTimes(1); }); - it('should bubble up invalid configuration from operations', () => { + it('should prepend each error with its layer number on multi-layer chart', () => { (getErrorMessages as jest.Mock).mockClear(); (getErrorMessages as jest.Mock).mockReturnValueOnce(['error 1', 'error 2']); const state: IndexPatternPrivateState = { @@ -1045,14 +933,19 @@ describe('IndexPattern Data Source', () => { columnOrder: [], columns: {}, }, + second: { + indexPatternId: '1', + columnOrder: [], + columns: {}, + }, }, currentIndexPatternId: '1', }; expect(indexPatternDatasource.getErrorMessages(state)).toEqual([ - { longMessage: 'error 1', shortMessage: '' }, - { longMessage: 'error 2', shortMessage: '' }, + { longMessage: 'Layer 1 error: error 1', shortMessage: '' }, + { longMessage: 'Layer 1 error: error 2', shortMessage: '' }, ]); - expect(getErrorMessages).toHaveBeenCalledTimes(1); + expect(getErrorMessages).toHaveBeenCalledTimes(2); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 1e68814278c2a9..5ae70395d3ecb4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -39,12 +39,7 @@ import { getDatasourceSuggestionsForVisualizeField, } from './indexpattern_suggestions'; -import { - getInvalidColumnsForLayer, - getInvalidLayers, - isDraggedField, - normalizeOperationDataType, -} from './utils'; +import { isDraggedField, normalizeOperationDataType } from './utils'; import { LayerPanel } from './layerpanel'; import { IndexPatternColumn, getErrorMessages, IncompleteColumn } from './operations'; import { IndexPatternField, IndexPatternPrivateState, IndexPatternPersistedState } from './types'; @@ -55,7 +50,6 @@ import { mergeLayer } from './state_helpers'; import { Datasource, StateSetter } from '../index'; import { ChartsPluginSetup } from '../../../../../src/plugins/charts/public'; import { deleteColumn, isReferenced } from './operations'; -import { FieldBasedIndexPatternColumn } from './operations/definitions/column_types'; import { Dragging } from '../drag_drop/providers'; export { OperationType, IndexPatternColumn, deleteColumn } from './operations'; @@ -351,7 +345,9 @@ export function getIndexPatternDatasource({ const layer = state.layers[layerId]; if (layer && layer.columns[columnId]) { - return columnToOperation(layer.columns[columnId], columnLabelMap[columnId]); + if (!isReferenced(layer, columnId)) { + return columnToOperation(layer.columns[columnId], columnLabelMap[columnId]); + } } return null; }, @@ -369,91 +365,46 @@ export function getIndexPatternDatasource({ if (!state) { return; } - const invalidLayers = getInvalidLayers(state); - const layerErrors = Object.values(state.layers).flatMap((layer) => + const layerErrors = Object.values(state.layers).map((layer) => (getErrorMessages(layer) ?? []).map((message) => ({ shortMessage: '', // Not displayed currently longMessage: message, })) ); - if (invalidLayers.length === 0) { - return layerErrors.length ? layerErrors : undefined; + // Single layer case, no need to explain more + if (layerErrors.length <= 1) { + return layerErrors[0]?.length ? layerErrors[0] : undefined; } - const realIndex = Object.values(state.layers) - .map((layer, i) => { - const filteredIndex = invalidLayers.indexOf(layer); - if (filteredIndex > -1) { - return [filteredIndex, i + 1]; - } - }) - .filter(Boolean) as Array<[number, number]>; - const invalidColumnsForLayer: string[][] = getInvalidColumnsForLayer( - invalidLayers, - state.indexPatterns - ); - const originalLayersList = Object.keys(state.layers); - - if (layerErrors.length || realIndex.length) { - return [ - ...layerErrors, - ...realIndex.map(([filteredIndex, layerIndex]) => { - const columnLabelsWithBrokenReferences: string[] = invalidColumnsForLayer[ - filteredIndex - ].map((columnId) => { - const column = invalidLayers[filteredIndex].columns[ - columnId - ] as FieldBasedIndexPatternColumn; - return column.label; - }); - - if (originalLayersList.length === 1) { - return { - shortMessage: i18n.translate( - 'xpack.lens.indexPattern.dataReferenceFailureShortSingleLayer', - { - defaultMessage: - 'Invalid {columns, plural, one {reference} other {references}}.', - values: { - columns: columnLabelsWithBrokenReferences.length, - }, - } - ), - longMessage: i18n.translate( - 'xpack.lens.indexPattern.dataReferenceFailureLongSingleLayer', - { - defaultMessage: `"{columns}" {columnsLength, plural, one {has an} other {have}} invalid reference.`, - values: { - columns: columnLabelsWithBrokenReferences.join('", "'), - columnsLength: columnLabelsWithBrokenReferences.length, - }, - } - ), - }; - } - return { - shortMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureShort', { - defaultMessage: - 'Invalid {columnsLength, plural, one {reference} other {references}} on Layer {layer}.', - values: { - layer: layerIndex, - columnsLength: columnLabelsWithBrokenReferences.length, - }, - }), - longMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureLong', { - defaultMessage: `Layer {layer} has {columnsLength, plural, one {an invalid} other {invalid}} {columnsLength, plural, one {reference} other {references}} in "{columns}".`, - values: { - layer: layerIndex, - columns: columnLabelsWithBrokenReferences.join('", "'), - columnsLength: columnLabelsWithBrokenReferences.length, - }, - }), - }; - }), - ]; - } + // For multiple layers we will prepend each error with the layer number + const messages = layerErrors.flatMap((errors, index) => { + return errors.map((error) => { + const { shortMessage, longMessage } = error; + return { + shortMessage: shortMessage + ? i18n.translate('xpack.lens.indexPattern.layerErrorWrapper', { + defaultMessage: 'Layer {position} error: {wrappedMessage}', + values: { + position: index + 1, + wrappedMessage: shortMessage, + }, + }) + : '', + longMessage: longMessage + ? i18n.translate('xpack.lens.indexPattern.layerErrorWrapper', { + defaultMessage: 'Layer {position} error: {wrappedMessage}', + values: { + position: index + 1, + wrappedMessage: longMessage, + }, + }) + : '', + }; + }); + }); + return messages.length ? messages : undefined; }, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index a2157ff6bf71ba..f1af19729ca878 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -20,7 +20,7 @@ import { getExistingColumnGroups, isReferenced, } from './operations'; -import { hasField, hasInvalidColumns } from './utils'; +import { hasField, getInvalidLayers } from './utils'; import { IndexPattern, IndexPatternPrivateState, @@ -95,7 +95,7 @@ export function getDatasourceSuggestionsForField( indexPatternId: string, field: IndexPatternField ): IndexPatternSuggestion[] { - if (hasInvalidColumns(state)) return []; + if (getInvalidLayers(state)?.length) return []; const layers = Object.keys(state.layers); const layerIds = layers.filter((id) => state.layers[id].indexPatternId === indexPatternId); @@ -337,7 +337,7 @@ function createNewLayerWithMetricAggregation( export function getDatasourceSuggestionsFromCurrentState( state: IndexPatternPrivateState ): Array> { - if (hasInvalidColumns(state)) return []; + if (getInvalidLayers(state)?.length) return []; const layers = Object.entries(state.layers || {}); if (layers.length > 1) { // Return suggestions that reduce the data to each layer individually diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx index a250eeadef59ac..5d4cfc6623e501 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx @@ -100,8 +100,8 @@ export const counterRateOperation: OperationDefinition< getDisabledStatus(indexPattern, layer) { return checkForDateHistogram( layer, - i18n.translate('xpack.lens.indexPattern.movingAverage', { - defaultMessage: 'Moving average', + i18n.translate('xpack.lens.indexPattern.counterRate', { + defaultMessage: 'Counter rate', }) )?.join(', '); }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx index c223e7f15668ed..fd2ba392e72c48 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx @@ -92,8 +92,8 @@ export const cumulativeSumOperation: OperationDefinition< getDisabledStatus(indexPattern, layer) { return checkForDateHistogram( layer, - i18n.translate('xpack.lens.indexPattern.movingAverage', { - defaultMessage: 'Moving average', + i18n.translate('xpack.lens.indexPattern.cumulativeSum', { + defaultMessage: 'Cumulative sum', }) )?.join(', '); }, 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 f0fc8f0e6332e3..8ffdf6931b54ad 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 @@ -102,8 +102,8 @@ export const derivativeOperation: OperationDefinition< getDisabledStatus(indexPattern, layer) { return checkForDateHistogram( layer, - i18n.translate('xpack.lens.indexPattern.movingAverage', { - defaultMessage: 'Moving average', + i18n.translate('xpack.lens.indexPattern.derivative', { + defaultMessage: 'Differences', }) )?.join(', '); }, 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 4bf32fdf409e70..1b30c805657e80 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 @@ -158,6 +158,7 @@ function MovingAverageParamEditor({ compressed value={inputValue} onChange={(e: React.ChangeEvent) => setInputValue(e.target.value)} + min={1} /> ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts new file mode 100644 index 00000000000000..04e04816d98efd --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.test.ts @@ -0,0 +1,56 @@ +/* + * 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 { createMockedIndexPattern } from '../../mocks'; +import { getInvalidFieldMessage } from './helpers'; + +describe('helpers', () => { + describe('getInvalidFieldMessage', () => { + it('return an error if a field was removed', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'count', // <= invalid + sourceField: 'bytes', + }, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Field bytes was not found'); + }); + + it('returns an error if a field is the wrong type', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'avg', // <= invalid + sourceField: 'timestamp', + }, + createMockedIndexPattern() + ); + expect(messages).toHaveLength(1); + expect(messages![0]).toEqual('Field timestamp was not found'); + }); + + it('returns no message if all fields are matching', () => { + const messages = getInvalidFieldMessage( + { + dataType: 'number', + isBucketed: false, + label: 'Foo', + operationType: 'avg', + sourceField: 'bytes', + }, + createMockedIndexPattern() + ); + expect(messages).toBeUndefined(); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index c3904487e7c55b..95242532774b1a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -1066,7 +1066,7 @@ describe('state_helpers', () => { isTransferable: jest.fn(), toExpression: jest.fn().mockReturnValue([]), getPossibleOperation: jest.fn().mockReturnValue({ dataType: 'number', isBucketed: false }), - getDefaultLabel: () => 'Test reference', + getDefaultLabel: jest.fn().mockReturnValue('Test reference'), }; const layer: IndexPatternLayer = { @@ -1184,6 +1184,7 @@ describe('state_helpers', () => { }, }, columnId: 'col1', + indexPattern, }) ).toEqual({ indexPatternId: '1', @@ -1229,6 +1230,7 @@ describe('state_helpers', () => { }, }, columnId: 'col2', + indexPattern, }) ).toEqual({ indexPatternId: '1', @@ -1279,6 +1281,7 @@ describe('state_helpers', () => { }, }, columnId: 'col2', + indexPattern, }); expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith( @@ -1312,11 +1315,57 @@ describe('state_helpers', () => { }, }, }; - expect(deleteColumn({ layer, columnId: 'col2' })).toEqual( + expect(deleteColumn({ layer, columnId: 'col2', indexPattern })).toEqual( expect.objectContaining({ columnOrder: [], columns: {} }) ); }); + it('should update the labels when deleting columns', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + + operationType: 'count', + sourceField: 'Records', + }, + col2: { + label: 'Changed label', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + deleteColumn({ layer, columnId: 'col1', indexPattern }); + expect(operationDefinitionMap.testReference.getDefaultLabel).toHaveBeenCalledWith( + { + label: 'Changed label', + dataType: 'number', + isBucketed: false, + operationType: 'testReference', + references: ['col1'], + }, + indexPattern, + { + col2: { + label: 'Default label', + dataType: 'number', + isBucketed: false, + operationType: 'testReference', + references: ['col1'], + }, + } + ); + }); + it('should recursively delete references', () => { const layer: IndexPatternLayer = { indexPatternId: '1', @@ -1350,7 +1399,7 @@ describe('state_helpers', () => { }, }, }; - expect(deleteColumn({ layer, columnId: 'col3' })).toEqual( + expect(deleteColumn({ layer, columnId: 'col3', indexPattern })).toEqual( expect.objectContaining({ columnOrder: [], columns: {} }) ); }); @@ -1809,6 +1858,21 @@ describe('state_helpers', () => { }); describe('getErrorMessages', () => { + it('should collect errors from all types of operation definitions', () => { + const mock = jest.fn().mockReturnValue(['error 1']); + operationDefinitionMap.avg.getErrorMessage = mock; + const errors = getErrorMessages({ + indexPatternId: '1', + columnOrder: [], + columns: { + // @ts-expect-error invalid column + col1: { operationType: 'avg' }, + }, + }); + expect(mock).toHaveBeenCalled(); + expect(errors).toHaveLength(1); + }); + it('should collect errors from the operation definitions', () => { const mock = jest.fn().mockReturnValue(['error 1']); operationDefinitionMap.testReference.getErrorMessage = mock; @@ -1825,7 +1889,7 @@ describe('state_helpers', () => { expect(errors).toHaveLength(1); }); - it('should identify missing references', () => { + it('should identify missing references on reference-based calculation', () => { const errors = getErrorMessages({ indexPatternId: '1', columnOrder: [], diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index ebd31d5b81b3c2..711ce2cc845e1a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -259,7 +259,7 @@ export function replaceColumn({ if (previousDefinition.input === 'fullReference') { (previousColumn as ReferenceBasedIndexPatternColumn).references.forEach((id: string) => { - tempLayer = deleteColumn({ layer: tempLayer, columnId: id }); + tempLayer = deleteColumn({ layer: tempLayer, columnId: id, indexPattern }); }); } @@ -481,9 +481,11 @@ function adjustColumnReferencesForChangedColumn(layer: IndexPatternLayer, change export function deleteColumn({ layer, columnId, + indexPattern, }: { layer: IndexPatternLayer; columnId: string; + indexPattern: IndexPattern; }): IndexPatternLayer { const column = layer.columns[columnId]; if (!column) { @@ -510,15 +512,20 @@ export function deleteColumn({ }; extraDeletions.forEach((id) => { - newLayer = deleteColumn({ layer: newLayer, columnId: id }); + newLayer = deleteColumn({ layer: newLayer, columnId: id, indexPattern }); }); const newIncomplete = { ...(newLayer.incompleteColumns || {}) }; delete newIncomplete[columnId]; - // TODO: Deleting should also update labels, but it's not actually required for now because of the - // reference UI not supporting inner deletion - return { ...newLayer, columnOrder: getColumnOrder(newLayer), incompleteColumns: newIncomplete }; + return updateDefaultLabels( + { + ...newLayer, + columnOrder: getColumnOrder(newLayer), + incompleteColumns: newIncomplete, + }, + indexPattern + ); } // Derives column order from column object, respects existing columnOrder @@ -610,7 +617,7 @@ export function getErrorMessages(layer: IndexPatternLayer): string[] | undefined Object.entries(layer.columns).forEach(([columnId, column]) => { const def = operationDefinitionMap[column.operationType]; - if (def.input === 'fullReference' && def.getErrorMessage) { + if (def.getErrorMessage) { errors.push(...(def.getErrorMessage(layer, columnId) ?? [])); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index 702930d02a90ef..e578c08889f6e8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -44,10 +44,6 @@ export function isDraggedField(fieldCandidate: unknown): fieldCandidate is Dragg ); } -export function hasInvalidColumns(state: IndexPatternPrivateState) { - return getInvalidLayers(state).length > 0; -} - export function getInvalidLayers(state: IndexPatternPrivateState) { return Object.values(state.layers).filter((layer) => { return layer.columnOrder.some((columnId) => From e33278d9063ee6105ad948eb5ff758e0b1dbcd4c Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 17 Dec 2020 18:51:55 -0500 Subject: [PATCH 06/10] Update suggestion logic to work with errors and refs --- .../dimension_panel/dimension_editor.tsx | 10 +- .../dimension_panel/reference_editor.tsx | 6 +- .../indexpattern_datasource/indexpattern.tsx | 3 +- .../indexpattern_suggestions.test.tsx | 351 +++++++++++++++--- .../indexpattern_suggestions.ts | 55 ++- .../operations/definitions/cardinality.tsx | 12 +- .../definitions/date_histogram.test.tsx | 28 ++ .../operations/definitions/date_histogram.tsx | 5 +- .../operations/definitions/last_value.tsx | 12 +- .../operations/definitions/metrics.tsx | 11 +- .../operations/definitions/ranges/ranges.tsx | 5 +- .../operations/definitions/terms/index.tsx | 12 +- .../public/indexpattern_datasource/utils.ts | 21 +- 13 files changed, 406 insertions(+), 125 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 76d32692848566..2046681b134f7d 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 @@ -303,7 +303,7 @@ export function DimensionEditor(props: DimensionEditorProps) { key={index} layer={state.layers[layerId]} columnId={referenceId} - updateLayer={(newLayer) => { + updateLayer={(newLayer: IndexPatternLayer) => { setState(mergeLayer({ state, layerId, newLayer })); }} validation={validation} @@ -351,7 +351,13 @@ export function DimensionEditor(props: DimensionEditorProps) { } incompleteOperation={incompleteOperation} onDeleteColumn={() => { - setStateWrapper(deleteColumn({ layer: state.layers[layerId], columnId })); + setStateWrapper( + deleteColumn({ + layer: state.layers[layerId], + columnId, + indexPattern: currentIndexPattern, + }) + ); }} onChoose={(choice) => { setStateWrapper( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index a0a15280ca6e2b..479d909e98c2dc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -196,7 +196,9 @@ export function ReferenceEditor(props: ReferenceEditorProps) { singleSelection={{ asPlainText: true }} onChange={(choices) => { if (choices.length === 0) { - updateLayer(deleteColumn({ layer, columnId })); + updateLayer( + deleteColumn({ layer, columnId, indexPattern: currentIndexPattern }) + ); return; } @@ -236,7 +238,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) { } incompleteOperation={incompleteOperation} onDeleteColumn={() => { - updateLayer(deleteColumn({ layer, columnId })); + updateLayer(deleteColumn({ layer, columnId, indexPattern: currentIndexPattern })); }} onChoose={(choice) => { updateLayer( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 5ae70395d3ecb4..6c6bd2e1bb4391 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -156,10 +156,11 @@ export function getIndexPatternDatasource({ }, removeColumn({ prevState, layerId, columnId }) { + const indexPattern = prevState.indexPatterns[prevState.layers[layerId]?.indexPatternId]; return mergeLayer({ state: prevState, layerId, - newLayer: deleteColumn({ layer: prevState.layers[layerId], columnId }), + newLayer: deleteColumn({ layer: prevState.layers[layerId], columnId, indexPattern }), }); }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx index d70c11a9dc48f4..de768e92efb3d0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx @@ -800,6 +800,7 @@ describe('IndexPattern Data Source suggestions', () => { }); it('puts a date histogram column after the last bucket column on date field', () => { + (generateId as jest.Mock).mockReturnValue('newid'); const initialState = stateWithNonEmptyTables(); const suggestions = getDatasourceSuggestionsForField(initialState, '1', { name: 'timestamp', @@ -808,17 +809,16 @@ describe('IndexPattern Data Source suggestions', () => { aggregatable: true, searchable: true, }); - expect(suggestions).toContainEqual( expect.objectContaining({ state: expect.objectContaining({ layers: { previousLayer: initialState.layers.previousLayer, currentLayer: expect.objectContaining({ - columnOrder: ['cola', 'id1', 'colb'], + columnOrder: ['cola', 'newid', 'colb'], columns: { ...initialState.layers.currentLayer.columns, - id1: expect.objectContaining({ + newid: expect.objectContaining({ operationType: 'date_histogram', sourceField: 'timestamp', }), @@ -835,7 +835,7 @@ describe('IndexPattern Data Source suggestions', () => { columnId: 'cola', }), expect.objectContaining({ - columnId: 'id1', + columnId: 'newid', }), expect.objectContaining({ columnId: 'colb', @@ -863,6 +863,7 @@ describe('IndexPattern Data Source suggestions', () => { }); it('appends a terms column with default size on string field', () => { + (generateId as jest.Mock).mockReturnValue('newid'); const initialState = stateWithNonEmptyTables(); const suggestions = getDatasourceSuggestionsForField(initialState, '1', { name: 'dest', @@ -871,17 +872,16 @@ describe('IndexPattern Data Source suggestions', () => { aggregatable: true, searchable: true, }); - expect(suggestions).toContainEqual( expect.objectContaining({ state: expect.objectContaining({ layers: { previousLayer: initialState.layers.previousLayer, currentLayer: expect.objectContaining({ - columnOrder: ['cola', 'id1', 'colb'], + columnOrder: ['cola', 'newid', 'colb'], columns: { ...initialState.layers.currentLayer.columns, - id1: expect.objectContaining({ + newid: expect.objectContaining({ operationType: 'terms', sourceField: 'dest', params: expect.objectContaining({ size: 3 }), @@ -895,6 +895,7 @@ describe('IndexPattern Data Source suggestions', () => { }); it('suggests both replacing and adding metric if only one other metric is set', () => { + (generateId as jest.Mock).mockReturnValue('newid'); const initialState = stateWithNonEmptyTables(); const suggestions = getDatasourceSuggestionsForField(initialState, '1', { name: 'memory', @@ -903,7 +904,6 @@ describe('IndexPattern Data Source suggestions', () => { aggregatable: true, searchable: true, }); - expect(suggestions).toContainEqual( expect.objectContaining({ state: expect.objectContaining({ @@ -928,11 +928,11 @@ describe('IndexPattern Data Source suggestions', () => { state: expect.objectContaining({ layers: expect.objectContaining({ currentLayer: expect.objectContaining({ - columnOrder: ['cola', 'colb', 'id1'], + columnOrder: ['cola', 'colb', 'newid'], columns: { cola: initialState.layers.currentLayer.columns.cola, colb: initialState.layers.currentLayer.columns.colb, - id1: expect.objectContaining({ + newid: expect.objectContaining({ operationType: 'avg', sourceField: 'memory', }), @@ -945,6 +945,7 @@ describe('IndexPattern Data Source suggestions', () => { }); it('adds a metric column on a number field if no other metrics set', () => { + (generateId as jest.Mock).mockReturnValue('newid'); const initialState = stateWithNonEmptyTables(); const modifiedState: IndexPatternPrivateState = { ...initialState, @@ -973,10 +974,10 @@ describe('IndexPattern Data Source suggestions', () => { layers: { previousLayer: modifiedState.layers.previousLayer, currentLayer: expect.objectContaining({ - columnOrder: ['cola', 'id1'], + columnOrder: ['cola', 'newid'], columns: { ...modifiedState.layers.currentLayer.columns, - id1: expect.objectContaining({ + newid: expect.objectContaining({ operationType: 'avg', sourceField: 'memory', }), @@ -1028,11 +1029,11 @@ describe('IndexPattern Data Source suggestions', () => { }); it('hides any referenced metrics when adding new metrics', () => { + (generateId as jest.Mock).mockReturnValue('newid'); const initialState = stateWithNonEmptyTables(); const modifiedState: IndexPatternPrivateState = { ...initialState, layers: { - // ...initialState.layers, currentLayer: { indexPatternId: '1', columnOrder: ['date', 'metric', 'ref'], @@ -1079,10 +1080,63 @@ describe('IndexPattern Data Source suggestions', () => { columns: [ { columnId: 'date', - operation: { dataType: 'date', isBucketed: true, label: '' }, + operation: expect.objectContaining({ dataType: 'date', isBucketed: true }), }, { - columnId: 'id1', + columnId: 'newid', + operation: expect.objectContaining({ dataType: 'number', isBucketed: false }), + }, + { + columnId: 'ref', + operation: expect.objectContaining({ dataType: 'number', isBucketed: false }), + }, + ], + }), + keptLayerIds: ['currentLayer'], + }) + ); + }); + + it('makes a suggestion to extending from an invalid state with a new metric', () => { + (generateId as jest.Mock).mockReturnValue('newid'); + const initialState = stateWithNonEmptyTables(); + const modifiedState: IndexPatternPrivateState = { + ...initialState, + layers: { + currentLayer: { + indexPatternId: '1', + columnOrder: ['metric', 'ref'], + columns: { + metric: { + label: '', + customLabel: true, + dataType: 'number', + isBucketed: false, + operationType: 'avg', + sourceField: 'bytes', + }, + ref: { + label: '', + customLabel: true, + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + }, + }, + }, + }; + const suggestions = getSuggestionSubset( + getDatasourceSuggestionsForField(modifiedState, '1', documentField) + ); + expect(suggestions).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + changeType: 'extended', + columns: [ + { + columnId: 'newid', operation: { dataType: 'number', isBucketed: false, @@ -1092,11 +1146,15 @@ describe('IndexPattern Data Source suggestions', () => { }, { columnId: 'ref', - operation: { dataType: 'number', isBucketed: false, label: '' }, + operation: { + dataType: 'number', + isBucketed: false, + label: '', + scale: undefined, + }, }, ], }), - keptLayerIds: ['currentLayer'], }) ); }); @@ -1796,31 +1854,26 @@ describe('IndexPattern Data Source suggestions', () => { }; const suggestions = getDatasourceSuggestionsFromCurrentState(state); - // 1 bucket col, 2 metric cols - isTableWithBucketColumns(suggestions[0], ['col1', 'col4', 'col5'], 1); + + // 3 bucket cols, 2 metric cols + isTableWithBucketColumns(suggestions[0], ['col1', 'col2', 'col3', 'col4', 'col5'], 3); // 1 bucket col, 1 metric col isTableWithBucketColumns(suggestions[1], ['col1', 'col4'], 1); // 2 bucket cols, 2 metric cols - isTableWithBucketColumns(suggestions[2], ['col1', 'col2', 'col4', 'col5'], 2); - - // 2 bucket cols, 1 metric col - isTableWithBucketColumns(suggestions[3], ['col1', 'col2', 'col4'], 2); - - // 3 bucket cols, 2 metric cols - isTableWithBucketColumns(suggestions[4], ['col1', 'col2', 'col3', 'col4', 'col5'], 3); + isTableWithBucketColumns(suggestions[2], ['col1', 'col2', 'col4'], 2); // 3 bucket cols, 1 metric col - isTableWithBucketColumns(suggestions[5], ['col1', 'col2', 'col3', 'col4'], 3); + isTableWithBucketColumns(suggestions[3], ['col1', 'col2', 'col3', 'col4'], 3); // first metric col - isTableWithMetricColumns(suggestions[6], ['col4']); + isTableWithMetricColumns(suggestions[4], ['col4']); // second metric col - isTableWithMetricColumns(suggestions[7], ['col5']); + isTableWithMetricColumns(suggestions[5], ['col5']); - expect(suggestions.length).toBe(8); + expect(suggestions.length).toBe(6); }); it('returns an only metric version of a given table', () => { @@ -1899,8 +1952,19 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - const suggestions = getDatasourceSuggestionsFromCurrentState(state); - expect(suggestions[1].table.columns[0].operation.label).toBe('Average of field1'); + const suggestions = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + expect(suggestions).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + changeType: 'reduced', + columns: [ + expect.objectContaining({ + operation: expect.objectContaining({ label: 'Average of field1' }), + }), + ], + }), + }) + ); }); it('returns an alternative metric for an only-metric table', () => { @@ -1953,9 +2017,18 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - const suggestions = getDatasourceSuggestionsFromCurrentState(state); - expect(suggestions[0].table.columns.length).toBe(1); - expect(suggestions[0].table.columns[0].operation.label).toBe('Sum of field1'); + const suggestions = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + expect(suggestions).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + columns: [ + expect.objectContaining({ + operation: expect.objectContaining({ label: 'Sum of field1' }), + }), + ], + }), + }) + ); }); it('contains a reordering suggestion when there are exactly 2 buckets', () => { @@ -2014,7 +2087,7 @@ describe('IndexPattern Data Source suggestions', () => { ); }); - it('does not generate suggestions if invalid fields are referenced', () => { + it('will generate suggestions even if there are errors from missing fields', () => { const initialState = testInitialState(); const state: IndexPatternPrivateState = { indexPatternRefs: [], @@ -2042,12 +2115,198 @@ describe('IndexPattern Data Source suggestions', () => { }, }; - const suggestions = getDatasourceSuggestionsFromCurrentState(state); - expect(suggestions).toEqual([]); + const suggestions = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + expect(suggestions).toContainEqual( + expect.objectContaining({ + table: { + changeType: 'unchanged', + columns: [ + { + columnId: 'col1', + operation: { + dataType: 'string', + isBucketed: true, + label: 'My Op', + scale: undefined, + }, + }, + { + columnId: 'col2', + operation: { + dataType: 'string', + isBucketed: true, + label: 'Top 5', + scale: undefined, + }, + }, + ], + isMultiRow: true, + label: undefined, + layerId: 'first', + }, + }) + ); }); describe('references', () => { - it('will reduce multiple references correctly to a single reference', () => { + it('will extend the table with a date when starting in an invalid state', () => { + const initialState = testInitialState(); + const state: IndexPatternPrivateState = { + ...initialState, + layers: { + ...initialState.layers, + first: { + ...initialState.layers.first, + columnOrder: ['metric', 'ref', 'ref2'], + columns: { + metric: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'count', + sourceField: 'Records', + }, + ref: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + ref2: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric2'], + }, + }, + }, + }, + }; + + const result = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + + expect(result).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + changeType: 'extended', + layerId: 'first', + columns: [ + { + columnId: 'id1', + operation: { + dataType: 'date', + isBucketed: true, + label: 'timestampLabel', + scale: 'interval', + }, + }, + { + columnId: 'ref', + operation: { + dataType: 'number', + isBucketed: false, + label: 'Cumulative sum of Records', + scale: undefined, + }, + }, + { + columnId: 'ref2', + operation: { + dataType: 'number', + isBucketed: false, + label: 'Cumulative sum of (incomplete)', + scale: undefined, + }, + }, + ], + }), + keptLayerIds: ['first'], + }) + ); + }); + + it('will make an unchanged suggestion including incomplete references', () => { + const initialState = testInitialState(); + const state: IndexPatternPrivateState = { + ...initialState, + layers: { + ...initialState.layers, + first: { + ...initialState.layers.first, + columnOrder: ['date', 'ref', 'ref2'], + columns: { + date: { + label: '', + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { interval: 'auto' }, + }, + ref: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + ref2: { + label: '', + dataType: 'number', + isBucketed: false, + operationType: 'cumulative_sum', + references: ['metric'], + }, + }, + }, + }, + }; + + const result = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); + + expect(result).toContainEqual( + expect.objectContaining({ + table: expect.objectContaining({ + changeType: 'unchanged', + layerId: 'first', + columns: [ + { + columnId: 'date', + operation: { + dataType: 'date', + isBucketed: true, + label: '', + scale: undefined, + }, + }, + { + columnId: 'ref', + operation: { + dataType: 'number', + isBucketed: false, + label: '', + scale: undefined, + }, + }, + { + columnId: 'ref2', + operation: { + dataType: 'number', + isBucketed: false, + label: '', + scale: undefined, + }, + }, + ], + }), + keptLayerIds: ['first'], + }) + ); + }); + + it('will skip a reduced suggestion when handling multiple references', () => { const initialState = testInitialState(); const state: IndexPatternPrivateState = { ...initialState, @@ -2101,23 +2360,11 @@ describe('IndexPattern Data Source suggestions', () => { const result = getSuggestionSubset(getDatasourceSuggestionsFromCurrentState(state)); - expect(result).toContainEqual( + expect(result).not.toContainEqual( expect.objectContaining({ table: expect.objectContaining({ changeType: 'reduced', - layerId: 'first', - columns: [ - { - columnId: 'date', - operation: { dataType: 'date', isBucketed: true, label: '' }, - }, - { - columnId: 'ref', - operation: { dataType: 'number', isBucketed: false, label: '' }, - }, - ], }), - keptLayerIds: ['first'], }) ); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index f1af19729ca878..08b731960cc634 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -20,7 +20,7 @@ import { getExistingColumnGroups, isReferenced, } from './operations'; -import { hasField, getInvalidLayers } from './utils'; +import { hasField } from './utils'; import { IndexPattern, IndexPatternPrivateState, @@ -95,7 +95,6 @@ export function getDatasourceSuggestionsForField( indexPatternId: string, field: IndexPatternField ): IndexPatternSuggestion[] { - if (getInvalidLayers(state)?.length) return []; const layers = Object.keys(state.layers); const layerIds = layers.filter((id) => state.layers[id].indexPatternId === indexPatternId); @@ -337,7 +336,6 @@ function createNewLayerWithMetricAggregation( export function getDatasourceSuggestionsFromCurrentState( state: IndexPatternPrivateState ): Array> { - if (getInvalidLayers(state)?.length) return []; const layers = Object.entries(state.layers || {}); if (layers.length > 1) { // Return suggestions that reduce the data to each layer individually @@ -378,7 +376,8 @@ export function getDatasourceSuggestionsFromCurrentState( }), ]); } - return _.flatten( + + const result = _.flatten( Object.entries(state.layers || {}) .filter(([_id, layer]) => layer.columnOrder.length && layer.indexPatternId) .map(([layerId, layer]) => { @@ -396,29 +395,22 @@ export function getDatasourceSuggestionsFromCurrentState( buckets.some((columnId) => layer.columns[columnId].dataType === 'number'); const suggestions: Array> = []; - if (metrics.length === 0) { - // intermediary chart without metric, don't try to suggest reduced versions - suggestions.push( - buildSuggestion({ - state, - layerId, - changeType: 'unchanged', - }) - ); - } else if (buckets.length === 0) { + + // Always suggest an unchanged table, including during invalid states + suggestions.push( + buildSuggestion({ + state, + layerId, + changeType: 'unchanged', + }) + ); + + if (!references.length && metrics.length && buckets.length === 0) { if (timeField) { // suggest current metric over time if there is a default time field suggestions.push(createSuggestionWithDefaultDateHistogram(state, layerId, timeField)); } suggestions.push(...createAlternativeMetricSuggestions(indexPattern, layerId, state)); - // also suggest simple current state - suggestions.push( - buildSuggestion({ - state, - layerId, - changeType: 'unchanged', - }) - ); } else { suggestions.push(...createSimplifiedTableSuggestions(state, layerId)); @@ -437,6 +429,8 @@ export function getDatasourceSuggestionsFromCurrentState( return suggestions; }) ); + // console.log('suggestions', result); + return result; } function createChangedNestingSuggestion(state: IndexPatternPrivateState, layerId: string) { @@ -591,18 +585,11 @@ function createSimplifiedTableSuggestions(state: IndexPatternPrivateState, layer columnOrder: [...bucketedColumns, ...availableMetricColumns], }; - // Assumes that all references have exactly 1 metric, which might not be true - if (availableReferenceColumns.length > 1) { - return [ - allMetricsSuggestion, - { ...layer, columnOrder: [...bucketedColumns, availableReferenceColumns[0]] }, - ]; - } else if (availableMetricColumns.length > 1 && !availableReferenceColumns.length) { - // Only simplify metrics when there are no references - return [ - allMetricsSuggestion, - { ...layer, columnOrder: [...bucketedColumns, availableMetricColumns[0]] }, - ]; + if (availableReferenceColumns.length) { + // Don't remove buckets when dealing with any refs. This can break refs. + return []; + } else if (availableMetricColumns.length > 1) { + return [{ ...layer, columnOrder: [...bucketedColumns, availableMetricColumns[0]] }]; } else { return allMetricsSuggestion; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index 95e905f6021be8..d9dbbe5894c4d4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -18,10 +18,16 @@ const SCALE = 'ratio'; const OPERATION_TYPE = 'cardinality'; const IS_BUCKETED = false; -function ofName(name: string) { +function ofName(name?: string) { return i18n.translate('xpack.lens.indexPattern.cardinalityOf', { defaultMessage: 'Unique count of {name}', - values: { name }, + values: { + name: + name ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }), + }, }); } @@ -59,7 +65,7 @@ export const cardinalityOperation: OperationDefinition - ofName(indexPattern.getFieldByName(column.sourceField)!.displayName), + ofName(indexPattern.getFieldByName(column.sourceField)?.displayName), buildColumn({ field, previousColumn }) { return { label: ofName(field.displayName), 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 e40b9ccf89c1ab..1bd0ea5815ade1 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 @@ -691,4 +691,32 @@ describe('date_histogram', () => { expect(instance.find('[data-test-subj="lensDateHistogramValue"]').exists()).toBeFalsy(); }); }); + + describe('getDefaultLabel', () => { + it('should not throw when the source field is not located', () => { + expect( + dateHistogramOperation.getDefaultLabel( + { + label: '', + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'missing', + params: { interval: 'auto' }, + }, + state.indexPatterns['1'], + { + col1: { + label: '', + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'missing', + params: { interval: 'auto' }, + }, + } + ) + ).toEqual('Missing field'); + }); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index c48b9dad6c0d5b..169850c32f2a42 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -68,7 +68,10 @@ export const dateHistogramOperation: OperationDefinition< } }, getDefaultLabel: (column, indexPattern) => - indexPattern.getFieldByName(column.sourceField)!.displayName, + indexPattern.getFieldByName(column.sourceField)?.displayName ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }), buildColumn({ field }) { let interval = autoInterval; let timeZone: string | undefined; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 4cb2d876c83a13..a3cd19998dc58d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -15,10 +15,16 @@ import { updateColumnParam } from '../layer_helpers'; import { DataType } from '../../../types'; import { getInvalidFieldMessage } from './helpers'; -function ofName(name: string) { +function ofName(name?: string) { return i18n.translate('xpack.lens.indexPattern.lastValueOf', { defaultMessage: 'Last value of {name}', - values: { name }, + values: { + name: + name ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }), + }, }); } @@ -88,7 +94,7 @@ export const lastValueOperation: OperationDefinition - indexPattern.getFieldByName(column.sourceField)!.displayName, + ofName(indexPattern.getFieldByName(column.sourceField)?.displayName), input: 'field', onFieldChange: (oldColumn, field) => { const newParams = { ...oldColumn.params }; 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 ec60d360ad49f8..ffda7e16ff3f39 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 @@ -44,8 +44,13 @@ function buildMetricOperation>({ priority?: number; optionalTimeScaling?: boolean; }) { - const labelLookup = (name: string, column?: BaseIndexPatternColumn) => { - const rawLabel = ofName(name); + const labelLookup = (name?: string, column?: BaseIndexPatternColumn) => { + const rawLabel = ofName( + name ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }) + ); if (!optionalTimeScaling) { return rawLabel; } @@ -86,7 +91,7 @@ function buildMetricOperation>({ ? adjustTimeScaleOnOtherColumnChange(layer, thisColumnId, changedColumnId) : layer.columns[thisColumnId], getDefaultLabel: (column, indexPattern, columns) => - labelLookup(indexPattern.getFieldByName(column.sourceField)!.displayName, column), + labelLookup(indexPattern.getFieldByName(column.sourceField)?.displayName, column), buildColumn: ({ field, previousColumn }) => ({ label: labelLookup(field.displayName, previousColumn), dataType: 'number', 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 062e2afb8a5bf0..38712e2f362a39 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 @@ -99,7 +99,10 @@ export const rangeOperation: OperationDefinition - indexPattern.getFieldByName(column.sourceField)!.displayName, + indexPattern.getFieldByName(column.sourceField)?.displayName ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }), buildColumn({ field }) { return { label: field.displayName, 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 154e1086d744ef..72967dca3cdf37 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 @@ -26,10 +26,16 @@ import { ValuesRangeInput } from './values_range_input'; import { getInvalidFieldMessage } from '../helpers'; import type { IndexPatternLayer } from '../../../types'; -function ofName(name: string) { +function ofName(name?: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { defaultMessage: 'Top values of {name}', - values: { name }, + values: { + name: + name ?? + i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }), + }, }); } @@ -142,7 +148,7 @@ export const termsOperation: OperationDefinition - ofName(indexPattern.getFieldByName(column.sourceField)!.displayName), + ofName(indexPattern.getFieldByName(column.sourceField)?.displayName), onFieldChange: (oldColumn, field) => { const newParams = { ...oldColumn.params }; if ('format' in newParams && field.type !== 'number') { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index e578c08889f6e8..57cc4abeb723a2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -5,7 +5,7 @@ */ import { DataType } from '../types'; -import { IndexPatternPrivateState, IndexPattern, IndexPatternLayer } from './types'; +import { IndexPattern, IndexPatternLayer } from './types'; import { DraggedField } from './indexpattern'; import { BaseIndexPatternColumn, @@ -44,25 +44,6 @@ export function isDraggedField(fieldCandidate: unknown): fieldCandidate is Dragg ); } -export function getInvalidLayers(state: IndexPatternPrivateState) { - return Object.values(state.layers).filter((layer) => { - return layer.columnOrder.some((columnId) => - isColumnInvalid(layer, columnId, state.indexPatterns[layer.indexPatternId]) - ); - }); -} - -export function getInvalidColumnsForLayer( - layers: IndexPatternLayer[], - indexPatternMap: Record -) { - return layers.map((layer) => { - return layer.columnOrder.filter((columnId) => - isColumnInvalid(layer, columnId, indexPatternMap[layer.indexPatternId]) - ); - }); -} - export function isColumnInvalid( layer: IndexPatternLayer, columnId: string, From a6cfe67cb52a9a9fbf807bc5dd90b92bc261cfe5 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Thu, 17 Dec 2020 21:04:50 -0500 Subject: [PATCH 07/10] Support ParamEditor in references to fix Last Value: refactoring as needed --- .../dimension_panel/dimension_editor.tsx | 33 +- .../dimension_panel/reference_editor.test.tsx | 11 +- .../dimension_panel/reference_editor.tsx | 49 +- .../indexpattern_suggestions.ts | 4 +- .../calculations/moving_average.tsx | 13 +- .../definitions/date_histogram.test.tsx | 460 +++++++++--------- .../operations/definitions/date_histogram.tsx | 12 +- .../definitions/filters/filters.test.tsx | 190 ++++---- .../definitions/filters/filters.tsx | 10 +- .../operations/definitions/index.ts | 13 +- .../definitions/last_value.test.tsx | 164 +++---- .../operations/definitions/last_value.tsx | 18 +- .../definitions/ranges/ranges.test.tsx | 434 +++++++---------- .../operations/definitions/ranges/ranges.tsx | 56 +-- .../operations/definitions/terms/index.tsx | 46 +- .../definitions/terms/terms.test.tsx | 274 +++++------ .../operations/layer_helpers.test.ts | 46 +- .../operations/layer_helpers.ts | 51 +- 18 files changed, 865 insertions(+), 1019 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 2046681b134f7d..b8d409c27af74a 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 @@ -108,7 +108,15 @@ export function DimensionEditor(props: DimensionEditorProps) { layerId, currentIndexPattern, hideGrouping, + dateRange, } = props; + const services = { + data: props.data, + uiSettings: props.uiSettings, + savedObjectsClient: props.savedObjectsClient, + http: props.http, + storage: props.storage, + }; const { fieldByOperation, operationWithoutField } = operationSupportMatrix; const setStateWrapper = (layer: IndexPatternLayer) => { @@ -310,6 +318,8 @@ export function DimensionEditor(props: DimensionEditorProps) { currentIndexPattern={currentIndexPattern} existingFields={state.existingFields} selectionStyle={selectedOperationDefinition.selectionStyle} + dateRange={dateRange} + {...services} /> ); })} @@ -377,17 +387,13 @@ export function DimensionEditor(props: DimensionEditorProps) { {!currentFieldIsInvalid && !incompleteInfo && selectedColumn && ParamEditor && ( <> )} @@ -447,12 +453,15 @@ export function DimensionEditor(props: DimensionEditorProps) { selectedColumn={selectedColumn} onChange={(newFormat) => { setState( - updateColumnParam({ + mergeLayer({ state, layerId, - currentColumn: selectedColumn, - paramName: 'format', - value: newFormat, + newLayer: updateColumnParam({ + layer: state.layers[layerId], + columnId, + paramName: 'format', + value: newFormat, + }), }) ); }} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx index 3f4c5725654363..a89236d30d8c18 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx @@ -9,6 +9,9 @@ import { ReactWrapper, ShallowWrapper } from 'enzyme'; import { act } from 'react-dom/test-utils'; import { EuiComboBox } from '@elastic/eui'; import { mountWithIntl as mount } from '@kbn/test/jest'; +import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import type { DataPublicPluginStart } from 'src/plugins/data/public'; import { OperationMetadata } from '../../types'; import { createMockedIndexPattern } from '../mocks'; import { ReferenceEditor, ReferenceEditorProps } from './reference_editor'; @@ -40,6 +43,12 @@ describe('reference editor', () => { source: true, }, }, + dateRange: { fromDate: 'now-1d', toDate: 'now' }, + storage: {} as IStorageWrapper, + uiSettings: {} as IUiSettingsClient, + savedObjectsClient: {} as SavedObjectsClientContract, + http: {} as HttpSetup, + data: {} as DataPublicPluginStart, }; } @@ -297,7 +306,7 @@ describe('reference editor', () => { ); const fieldSelect = wrapper.find(FieldSelect); - expect(fieldSelect.prop('fieldIsInvalid')).toEqual(false); + expect(fieldSelect.prop('fieldIsInvalid')).toEqual(true); expect(fieldSelect.prop('selectedField')).toEqual('bytes'); expect(fieldSelect.prop('selectedOperationType')).toEqual('avg'); expect(fieldSelect.prop('incompleteOperation')).toEqual('max'); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index 479d909e98c2dc..8f45b45a4a3c7e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -9,7 +9,11 @@ import _ from 'lodash'; import React, { useMemo } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, EuiSpacer, EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; -import { OperationSupportMatrix } from './operation_support'; +import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import type { DataPublicPluginStart } from 'src/plugins/data/public'; +import type { DateRange } from '../../../common'; +import type { OperationSupportMatrix } from './operation_support'; import type { OperationType } from '../indexpattern'; import { operationDefinitionMap, @@ -35,6 +39,14 @@ export interface ReferenceEditorProps { updateLayer: (newLayer: IndexPatternLayer) => void; currentIndexPattern: IndexPattern; existingFields: IndexPatternPrivateState['existingFields']; + dateRange: DateRange; + + // Services + uiSettings: IUiSettingsClient; + storage: IStorageWrapper; + savedObjectsClient: SavedObjectsClientContract; + http: HttpSetup; + data: DataPublicPluginStart; } export function ReferenceEditor(props: ReferenceEditorProps) { @@ -46,11 +58,15 @@ export function ReferenceEditor(props: ReferenceEditorProps) { existingFields, validation, selectionStyle, + dateRange, + ...services } = props; const column = layer.columns[columnId]; const selectedOperationDefinition = column && operationDefinitionMap[column.operationType]; + const ParamEditor = selectedOperationDefinition?.paramEditor; + const incompleteInfo = layer.incompleteColumns ? layer.incompleteColumns[columnId] : undefined; const incompleteOperation = incompleteInfo?.operationType; const incompleteField = incompleteInfo?.sourceField ?? null; @@ -161,11 +177,6 @@ export function ReferenceEditor(props: ReferenceEditorProps) { ? [functionOptions.find(({ value }) => value === column.operationType)!] : []; - const invalidField = - column && - selectedOperationDefinition.input === 'field' && - !operationSupportMatrix.fieldByOperation[column.operationType]?.size; - return (
@@ -177,7 +188,11 @@ export function ReferenceEditor(props: ReferenceEditorProps) { defaultMessage: 'Choose a sub-function', })} fullWidth - isInvalid={!column || Boolean(incompleteInfo?.operationType)} + isInvalid={ + // If the operationType is incomplete, the user needs to select a field- so + // the function is marked as valid. + !column && !Boolean(incompleteInfo?.operationType) + } > { @@ -219,10 +234,10 @@ export function ReferenceEditor(props: ReferenceEditorProps) { defaultMessage: 'Select a field', })} fullWidth - isInvalid={invalidField} + isInvalid={Boolean(incompleteInfo?.operationType)} > ) : null} + + {column && !incompleteInfo && ParamEditor && ( + <> + + + )}
); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index 08b731960cc634..969324c67e9094 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -377,7 +377,7 @@ export function getDatasourceSuggestionsFromCurrentState( ]); } - const result = _.flatten( + return _.flatten( Object.entries(state.layers || {}) .filter(([_id, layer]) => layer.columnOrder.length && layer.indexPatternId) .map(([layerId, layer]) => { @@ -429,8 +429,6 @@ export function getDatasourceSuggestionsFromCurrentState( return suggestions; }) ); - // console.log('suggestions', result); - return result; } function createChangedNestingSuggestion(state: IndexPatternPrivateState, layerId: string) { 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 1b30c805657e80..13507416a38c5c 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 @@ -119,10 +119,10 @@ export const movingAverageOperation: OperationDefinition< }; function MovingAverageParamEditor({ - state, - setState, + layer, + updateLayer, currentColumn, - layerId, + columnId, }: ParamEditorProps) { const [inputValue, setInputValue] = useState(String(currentColumn.params.window)); @@ -132,11 +132,10 @@ function MovingAverageParamEditor({ return; } const inputNumber = Number(inputValue); - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'window', value: inputNumber, }) 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 1bd0ea5815ade1..8c2fa43b541d4f 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 @@ -5,19 +5,19 @@ */ import React from 'react'; -import { DateHistogramIndexPatternColumn } from './date_histogram'; +import type { DateHistogramIndexPatternColumn } from './date_histogram'; import { dateHistogramOperation } from './index'; import { shallow } from 'enzyme'; import { EuiSwitch, EuiSwitchEvent } from '@elastic/eui'; -import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; -import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import type { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { UI_SETTINGS } from '../../../../../../../src/plugins/data/public'; import { dataPluginMock, getCalculateAutoTimeExpression, } from '../../../../../../../src/plugins/data/public/mocks'; import { createMockedIndexPattern } from '../../mocks'; -import { IndexPatternPrivateState } from '../../types'; +import type { IndexPatternLayer, IndexPattern } from '../../types'; import { getFieldByNameFactory } from '../../pure_helpers'; const dataStart = dataPluginMock.createStartContract(); @@ -29,6 +29,60 @@ dataStart.search.aggs.calculateAutoTimeExpression = getCalculateAutoTimeExpressi } ); +const indexPattern1: IndexPattern = { + id: '1', + title: 'Mock Indexpattern', + timeFieldName: 'timestamp', + hasRestrictions: false, + fields: [ + { + name: 'timestamp', + displayName: 'timestampLabel', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + }, + ], + + getFieldByName: getFieldByNameFactory([ + { + name: 'timestamp', + displayName: 'timestampLabel', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + }, + ]), +}; + +const indexPattern2: IndexPattern = { + id: '2', + title: 'Mock Indexpattern 2', + hasRestrictions: false, + fields: [ + { + name: 'other_timestamp', + displayName: 'other_timestamp', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + }, + ], + getFieldByName: getFieldByNameFactory([ + { + name: 'other_timestamp', + displayName: 'other_timestamp', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + }, + ]), +}; + const defaultOptions = { storage: {} as IStorageWrapper, uiSettings: {} as IUiSettingsClient, @@ -39,150 +93,47 @@ const defaultOptions = { }, data: dataStart, http: {} as HttpSetup, + indexPattern: indexPattern1, }; describe('date_histogram', () => { - let state: IndexPatternPrivateState; + let layer: IndexPatternLayer; const InlineOptions = dateHistogramOperation.paramEditor!; beforeEach(() => { - state = { - indexPatternRefs: [], - existingFields: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - indexPatterns: { - 1: { - id: '1', - title: 'Mock Indexpattern', - timeFieldName: 'timestamp', - hasRestrictions: false, - fields: [ - { - name: 'timestamp', - displayName: 'timestampLabel', - type: 'date', - esTypes: ['date'], - aggregatable: true, - searchable: true, - }, - ], - - getFieldByName: getFieldByNameFactory([ - { - name: 'timestamp', - displayName: 'timestampLabel', - type: 'date', - esTypes: ['date'], - aggregatable: true, - searchable: true, - }, - ]), - }, - 2: { - id: '2', - title: 'Mock Indexpattern 2', - hasRestrictions: false, - fields: [ - { - name: 'other_timestamp', - displayName: 'other_timestamp', - type: 'date', - esTypes: ['date'], - aggregatable: true, - searchable: true, - }, - ], - getFieldByName: getFieldByNameFactory([ - { - name: 'other_timestamp', - displayName: 'other_timestamp', - type: 'date', - esTypes: ['date'], - aggregatable: true, - searchable: true, - }, - ]), - }, - }, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Value of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - params: { - interval: '42w', - }, - sourceField: 'timestamp', - }, - }, - }, - second: { - indexPatternId: '2', - columnOrder: ['col2'], - columns: { - col2: { - label: 'Value of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - params: { - interval: 'd', - }, - sourceField: 'other_timestamp', - }, - }, - }, - third: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Value of timestamp', - dataType: 'date', - isBucketed: true, + layer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, - // Private - operationType: 'date_histogram', - params: { - interval: 'auto', - }, - sourceField: 'timestamp', - }, + // Private + operationType: 'date_histogram', + params: { + interval: '42w', }, + sourceField: 'timestamp', }, }, }; }); - function stateWithInterval(interval: string) { + function layerWithInterval(interval: string) { return ({ - ...state, - layers: { - ...state.layers, - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - interval, - }, - }, + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + interval, }, }, }, - } as unknown) as IndexPatternPrivateState; + } as unknown) as IndexPatternLayer; } describe('buildColumn', () => { @@ -246,9 +197,9 @@ describe('date_histogram', () => { describe('toEsAggsFn', () => { it('should reflect params correctly', () => { const esAggsFn = dateHistogramOperation.toEsAggsFn( - state.layers.first.columns.col1 as DateHistogramIndexPatternColumn, + layer.columns.col1 as DateHistogramIndexPatternColumn, 'col1', - state.indexPatterns['1'] + indexPattern1 ); expect(esAggsFn).toEqual( expect.objectContaining({ @@ -263,10 +214,10 @@ describe('date_histogram', () => { it('should not use normalized es interval for rollups', () => { const esAggsFn = dateHistogramOperation.toEsAggsFn( - state.layers.first.columns.col1 as DateHistogramIndexPatternColumn, + layer.columns.col1 as DateHistogramIndexPatternColumn, 'col1', { - ...state.indexPatterns['1'], + ...indexPattern1, fields: [ { name: 'timestamp', @@ -465,15 +416,14 @@ describe('date_histogram', () => { describe('param editor', () => { it('should render current value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -482,15 +432,34 @@ describe('date_histogram', () => { }); it('should render current value for other index pattern', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); + + const secondLayer: IndexPatternLayer = { + indexPatternId: '2', + columnOrder: ['col2'], + columns: { + col2: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + params: { + interval: 'd', + }, + sourceField: 'other_timestamp', + }, + }, + }; const instance = shallow( ); @@ -499,14 +468,33 @@ describe('date_histogram', () => { }); it('should render disabled switch and no time interval control for auto interval', () => { + const thirdLayer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + params: { + interval: 'auto', + }, + sourceField: 'timestamp', + }, + }, + }; + const instance = shallow( ); expect(instance.find('[data-test-subj="lensDateHistogramValue"]').exists()).toBeFalsy(); @@ -515,35 +503,52 @@ describe('date_histogram', () => { }); it('should allow switching to manual interval', () => { - const setStateSpy = jest.fn(); + const thirdLayer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Value of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + params: { + interval: 'auto', + }, + sourceField: 'timestamp', + }, + }, + }; + + const updateLayerSpy = jest.fn(); const instance = shallow( ); instance.find(EuiSwitch).prop('onChange')!({ target: { checked: true }, } as EuiSwitchEvent); - expect(setStateSpy).toHaveBeenCalled(); - const newState = setStateSpy.mock.calls[0][0]; - expect(newState).toHaveProperty('layers.third.columns.col1.params.interval', '30d'); + expect(updateLayerSpy).toHaveBeenCalled(); + const newLayer = updateLayerSpy.mock.calls[0][0]; + expect(newLayer).toHaveProperty('columns.col1.params.interval', '30d'); }); it('should force calendar values to 1', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); instance.find('[data-test-subj="lensDateHistogramValue"]').prop('onChange')!({ @@ -551,67 +556,63 @@ describe('date_histogram', () => { value: '2', }, } as React.ChangeEvent); - expect(setStateSpy).toHaveBeenCalledWith(stateWithInterval('1w')); + expect(updateLayerSpy).toHaveBeenCalledWith(layerWithInterval('1w')); }); it('should display error if an invalid interval is specified', () => { - const setStateSpy = jest.fn(); - const testState = stateWithInterval('4quid'); + const updateLayerSpy = jest.fn(); + const testLayer = layerWithInterval('4quid'); const instance = shallow( ); expect(instance.find('[data-test-subj="lensDateHistogramError"]').exists()).toBeTruthy(); }); it('should not display error if interval value is blank', () => { - const setStateSpy = jest.fn(); - const testState = stateWithInterval('d'); + const updateLayerSpy = jest.fn(); + const testLayer = layerWithInterval('d'); const instance = shallow( ); expect(instance.find('[data-test-subj="lensDateHistogramError"]').exists()).toBeFalsy(); }); it('should display error if interval value is 0', () => { - const setStateSpy = jest.fn(); - const testState = stateWithInterval('0d'); + const updateLayerSpy = jest.fn(); + const testLayer = layerWithInterval('0d'); const instance = shallow( ); expect(instance.find('[data-test-subj="lensDateHistogramError"]').exists()).toBeTruthy(); }); it('should update the unit', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); instance.find('[data-test-subj="lensDateHistogramUnit"]').prop('onChange')!({ @@ -619,21 +620,20 @@ describe('date_histogram', () => { value: 'd', }, } as React.ChangeEvent); - expect(setStateSpy).toHaveBeenCalledWith(stateWithInterval('42d')); + expect(updateLayerSpy).toHaveBeenCalledWith(layerWithInterval('42d')); }); it('should update the value', () => { - const setStateSpy = jest.fn(); - const testState = stateWithInterval('42d'); + const updateLayerSpy = jest.fn(); + const testLayer = layerWithInterval('42d'); const instance = shallow( ); instance.find('[data-test-subj="lensDateHistogramValue"]').prop('onChange')!({ @@ -641,50 +641,48 @@ describe('date_histogram', () => { value: '9', }, } as React.ChangeEvent); - expect(setStateSpy).toHaveBeenCalledWith(stateWithInterval('9d')); + expect(updateLayerSpy).toHaveBeenCalledWith(layerWithInterval('9d')); }); it('should not render options if they are restricted', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); + + const indexPattern = { + ...indexPattern1, + fields: [ + { + ...indexPattern1.fields[0], + aggregationRestrictions: { + date_histogram: { + agg: 'date_histogram', + time_zone: 'UTC', + calendar_interval: '1h', + }, + }, + }, + ], + getFieldByName: getFieldByNameFactory([ + { + ...indexPattern1.fields[0], + aggregationRestrictions: { + date_histogram: { + agg: 'date_histogram', + time_zone: 'UTC', + calendar_interval: '1h', + }, + }, + }, + ]), + }; + const instance = shallow( ); @@ -704,7 +702,7 @@ describe('date_histogram', () => { sourceField: 'missing', params: { interval: 'auto' }, }, - state.indexPatterns['1'], + indexPattern1, { col1: { label: '', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index 169850c32f2a42..c904eefba9bd03 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -146,12 +146,8 @@ export const dateHistogramOperation: OperationDefinition< extended_bounds: JSON.stringify({}), }).toAst(); }, - paramEditor: ({ state, setState, currentColumn, layerId, dateRange, data }) => { - const field = - currentColumn && - state.indexPatterns[state.layers[layerId].indexPatternId].getFieldByName( - currentColumn.sourceField - ); + paramEditor: ({ layer, columnId, currentColumn, updateLayer, dateRange, data, indexPattern }) => { + const field = currentColumn && indexPattern.getFieldByName(currentColumn.sourceField); const intervalIsRestricted = field!.aggregationRestrictions && field!.aggregationRestrictions.date_histogram; @@ -170,14 +166,14 @@ export const dateHistogramOperation: OperationDefinition< const value = ev.target.checked ? data.search.aggs.calculateAutoTimeExpression({ from: fromDate, to: toDate }) || '1h' : autoInterval; - setState(updateColumnParam({ state, layerId, currentColumn, paramName: 'interval', value })); + updateLayer(updateColumnParam({ layer, columnId, paramName: 'interval', value })); } const setInterval = (newInterval: typeof interval) => { const isCalendarInterval = calendarOnlyIntervals.has(newInterval.unit); const value = `${isCalendarInterval ? '1' : newInterval.value}${newInterval.unit || 'd'}`; - setState(updateColumnParam({ state, layerId, currentColumn, paramName: 'interval', value })); + updateLayer(updateColumnParam({ layer, columnId, paramName: 'interval', value })); }; return ( 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 bb8e52ba443a2f..cf57c35f6f68bf 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 @@ -7,12 +7,13 @@ import React from 'react'; import { mount } from 'enzyme'; import { act } from 'react-dom/test-utils'; -import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; -import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import type { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { dataPluginMock } from '../../../../../../../../src/plugins/data/public/mocks'; -import { FiltersIndexPatternColumn } from '.'; +import type { FiltersIndexPatternColumn } from '.'; import { filtersOperation } from '../index'; -import { IndexPatternPrivateState } from '../../../types'; +import type { IndexPatternLayer } from '../../../types'; +import { createMockedIndexPattern } from '../../../mocks'; import { FilterPopover } from './filter_popover'; const defaultProps = { @@ -22,6 +23,7 @@ const defaultProps = { dateRange: { fromDate: 'now-1d', toDate: 'now' }, data: dataPluginMock.createStartContract(), http: {} as HttpSetup, + indexPattern: createMockedIndexPattern(), }; // mocking random id generator function @@ -38,49 +40,40 @@ jest.mock('@elastic/eui', () => { }); describe('filters', () => { - let state: IndexPatternPrivateState; + let layer: IndexPatternLayer; const InlineOptions = filtersOperation.paramEditor!; beforeEach(() => { - state = { - indexPatternRefs: [], - indexPatterns: {}, - existingFields: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: { - label: 'filters', - dataType: 'document', - operationType: 'filters', - scale: 'ordinal', - isBucketed: true, - params: { - filters: [ - { - input: { query: 'bytes >= 1', language: 'kuery' }, - label: 'More than one', - }, - { - input: { query: 'src : 2', language: 'kuery' }, - label: '', - }, - ], + layer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'filters', + dataType: 'document', + operationType: 'filters', + scale: 'ordinal', + isBucketed: true, + params: { + filters: [ + { + input: { query: 'bytes >= 1', language: 'kuery' }, + label: 'More than one', }, - }, - col2: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, + { + input: { query: 'src : 2', language: 'kuery' }, + label: '', + }, + ], }, }, + col2: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, }, }; }); @@ -88,9 +81,9 @@ describe('filters', () => { describe('toEsAggsFn', () => { it('should reflect params correctly', () => { const esAggsFn = filtersOperation.toEsAggsFn( - state.layers.first.columns.col1 as FiltersIndexPatternColumn, + layer.columns.col1 as FiltersIndexPatternColumn, 'col1', - state.indexPatterns['1'] + createMockedIndexPattern() ); expect(esAggsFn).toEqual( expect.objectContaining({ @@ -133,15 +126,14 @@ describe('filters', () => { })); it('should update state when changing a filter', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -156,34 +148,29 @@ describe('filters', () => { }); }); instance.update(); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - filters: [ - { - input: { - query: 'dest : 5', - language: 'lucene', - }, - label: 'Dest5', - }, - { - input: { - language: 'kuery', - query: 'src : 2', - }, - label: '', - }, - ], + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + filters: [ + { + input: { + query: 'dest : 5', + language: 'lucene', + }, + label: 'Dest5', }, - }, + { + input: { + language: 'kuery', + query: 'src : 2', + }, + label: '', + }, + ], }, }, }, @@ -192,15 +179,14 @@ describe('filters', () => { describe('Modify filters', () => { it('should correctly show existing filters ', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); expect( @@ -218,15 +204,14 @@ describe('filters', () => { }); it('should remove filter', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -234,27 +219,22 @@ describe('filters', () => { .find('[data-test-subj="lns-customBucketContainer-remove"]') .at(2) .simulate('click'); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - filters: [ - { - input: { - language: 'kuery', - query: 'bytes >= 1', - }, - label: 'More than one', - }, - ], + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + filters: [ + { + input: { + language: 'kuery', + query: 'bytes >= 1', + }, + label: 'More than one', }, - }, + ], }, }, }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx index b6c0b565f9565f..8382aef7cc34ff 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx @@ -128,16 +128,14 @@ export const filtersOperation: OperationDefinition { - const indexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; + paramEditor: ({ layer, columnId, currentColumn, indexPattern, updateLayer, data }) => { const filters = currentColumn.params.filters; const setFilters = (newFilters: Filter[]) => - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'filters', value: newFilters, }) 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 ce2b412aceb39f..ade89d49b6603b 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 @@ -36,12 +36,7 @@ import { countOperation, CountIndexPatternColumn } from './count'; import { lastValueOperation, LastValueIndexPatternColumn } from './last_value'; import { OperationMetadata } from '../../../types'; import type { BaseIndexPatternColumn, ReferenceBasedIndexPatternColumn } from './column_types'; -import { - IndexPatternPrivateState, - IndexPattern, - IndexPatternField, - IndexPatternLayer, -} from '../../types'; +import { IndexPattern, IndexPatternField, IndexPatternLayer } from '../../types'; import { DateRange } from '../../../../common'; import { ExpressionAstFunction } from '../../../../../../../src/plugins/expressions/public'; import { DataPublicPluginStart } from '../../../../../../../src/plugins/data/public'; @@ -115,10 +110,10 @@ export { */ export interface ParamEditorProps { currentColumn: C; - state: IndexPatternPrivateState; - setState: (newState: IndexPatternPrivateState) => void; + layer: IndexPatternLayer; + updateLayer: (newLayer: IndexPatternLayer) => void; columnId: string; - layerId: string; + indexPattern: IndexPattern; uiSettings: IUiSettingsClient; storage: IStorageWrapper; savedObjectsClient: SavedObjectsClientContract; 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 a78e9c5b801182..bde8cd0e424277 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 @@ -13,7 +13,7 @@ import { dataPluginMock } from '../../../../../../../src/plugins/data/public/moc import { createMockedIndexPattern } from '../../mocks'; import { LastValueIndexPatternColumn } from './last_value'; import { lastValueOperation } from './index'; -import { IndexPatternPrivateState, IndexPattern, IndexPatternLayer } from '../../types'; +import type { IndexPattern, IndexPatternLayer } from '../../types'; const defaultProps = { storage: {} as IStorageWrapper, @@ -22,52 +22,41 @@ const defaultProps = { dateRange: { fromDate: 'now-1d', toDate: 'now' }, data: dataPluginMock.createStartContract(), http: {} as HttpSetup, + indexPattern: { + ...createMockedIndexPattern(), + hasRestrictions: false, + } as IndexPattern, }; describe('last_value', () => { - let state: IndexPatternPrivateState; + let layer: IndexPatternLayer; const InlineOptions = lastValueOperation.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: 'Last value of a', - dataType: 'number', - isBucketed: false, - sourceField: 'a', - operationType: 'last_value', - params: { - sortField: 'datefield', - }, - }, + layer = { + 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: 'Last value of a', + dataType: 'number', + isBucketed: false, + sourceField: 'a', + operationType: 'last_value', + params: { + sortField: 'datefield', }, }, }, @@ -76,7 +65,7 @@ describe('last_value', () => { describe('toEsAggsFn', () => { it('should reflect params correctly', () => { - const lastValueColumn = state.layers.first.columns.col2 as LastValueIndexPatternColumn; + const lastValueColumn = layer.columns.col2 as LastValueIndexPatternColumn; const esAggsFn = lastValueOperation.toEsAggsFn( { ...lastValueColumn, params: { ...lastValueColumn.params } }, 'col1', @@ -322,17 +311,15 @@ describe('last_value', () => { it('should return disabledStatus if indexPattern does contain date field', () => { const indexPattern = createMockedIndexPattern(); - expect(lastValueOperation.getDisabledStatus!(indexPattern, state.layers.first)).toEqual( - undefined - ); + expect(lastValueOperation.getDisabledStatus!(indexPattern, layer)).toEqual(undefined); const indexPatternWithoutTimeFieldName = { ...indexPattern, timeFieldName: undefined, }; - expect( - lastValueOperation.getDisabledStatus!(indexPatternWithoutTimeFieldName, state.layers.first) - ).toEqual(undefined); + expect(lastValueOperation.getDisabledStatus!(indexPatternWithoutTimeFieldName, layer)).toEqual( + undefined + ); const indexPatternWithoutTimefields = { ...indexPatternWithoutTimeFieldName, @@ -341,7 +328,7 @@ describe('last_value', () => { const disabledStatus = lastValueOperation.getDisabledStatus!( indexPatternWithoutTimefields, - state.layers.first + layer ); expect(disabledStatus).toEqual( 'This function requires the presence of a date field in your index' @@ -350,15 +337,14 @@ describe('last_value', () => { describe('param editor', () => { it('should render current sortField', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -368,15 +354,14 @@ describe('last_value', () => { }); it('should update state when changing sortField', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -385,20 +370,15 @@ describe('last_value', () => { .find(EuiComboBox) .prop('onChange')!([{ label: 'datefield2', value: 'datefield2' }]); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col2: { - ...state.layers.first.columns.col2, - params: { - ...(state.layers.first.columns.col2 as LastValueIndexPatternColumn).params, - sortField: 'datefield2', - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col2: { + ...layer.columns.col2, + params: { + ...(layer.columns.col2 as LastValueIndexPatternColumn).params, + sortField: 'datefield2', }, }, }, @@ -408,10 +388,10 @@ describe('last_value', () => { describe('getErrorMessage', () => { let indexPattern: IndexPattern; - let layer: IndexPatternLayer; + let errorLayer: IndexPatternLayer; beforeEach(() => { indexPattern = createMockedIndexPattern(); - layer = { + errorLayer = { columns: { col1: { dataType: 'boolean', @@ -428,53 +408,55 @@ describe('last_value', () => { }; }); it('returns undefined if sourceField exists and sortField is of type date ', () => { - expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual(undefined); + expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual( + undefined + ); }); it('shows error message if the sourceField does not exist in index pattern', () => { - layer = { - ...layer, + errorLayer = { + ...errorLayer, columns: { col1: { - ...layer.columns.col1, + ...errorLayer.columns.col1, sourceField: 'notExisting', } as LastValueIndexPatternColumn, }, }; - expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual([ 'Field notExisting was not found', ]); }); it('shows error message if the sortField does not exist in index pattern', () => { - layer = { - ...layer, + errorLayer = { + ...errorLayer, columns: { col1: { - ...layer.columns.col1, + ...errorLayer.columns.col1, params: { - ...layer.columns.col1.params, + ...errorLayer.columns.col1.params, sortField: 'notExisting', }, } as LastValueIndexPatternColumn, }, }; - expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual([ 'Field notExisting was not found', ]); }); it('shows error message if the sortField is not date', () => { - layer = { - ...layer, + errorLayer = { + ...errorLayer, columns: { col1: { - ...layer.columns.col1, + ...errorLayer.columns.col1, params: { - ...layer.columns.col1.params, + ...errorLayer.columns.col1.params, sortField: 'bytes', }, } as LastValueIndexPatternColumn, }, }; - expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + expect(lastValueOperation.getErrorMessage!(errorLayer, 'col1', indexPattern)).toEqual([ 'Field bytes is not a date field and cannot be used for sorting', ]); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index a3cd19998dc58d..553ee8f4f897d9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -192,12 +192,11 @@ export const lastValueOperation: OperationDefinition { - const currentIndexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; - const dateFields = getDateFields(currentIndexPattern); + paramEditor: ({ layer, updateLayer, columnId, currentColumn, indexPattern }) => { + const dateFields = getDateFields(indexPattern); const isSortFieldInvalid = !!getInvalidSortFieldMessage( currentColumn.params.sortField, - currentIndexPattern + indexPattern ); return ( <> @@ -234,11 +233,10 @@ export const lastValueOperation: OperationDefinition & React.MouseEvent; +const sourceField = 'MyField'; const defaultOptions = { storage: {} as IStorageWrapper, // need this for MAX_HISTOGRAM value @@ -64,75 +65,64 @@ const defaultOptions = { }, data: dataPluginMockValue, http: {} as HttpSetup, + indexPattern: { + id: '1', + title: 'my_index_pattern', + hasRestrictions: false, + fields: [{ name: sourceField, type: 'number', displayName: sourceField }], + getFieldByName: getFieldByNameFactory([ + { name: sourceField, type: 'number', displayName: sourceField }, + ]), + }, }; describe('ranges', () => { - let state: IndexPatternPrivateState; + let layer: IndexPatternLayer; const InlineOptions = rangeOperation.paramEditor!; - const sourceField = 'MyField'; const MAX_HISTOGRAM_VALUE = 100; const GRANULARITY_DEFAULT_VALUE = (MAX_HISTOGRAM_VALUE - MIN_HISTOGRAM_BARS) / 2; const GRANULARITY_STEP = (MAX_HISTOGRAM_VALUE - MIN_HISTOGRAM_BARS) / SLICES; function setToHistogramMode() { - const column = state.layers.first.columns.col1 as RangeIndexPatternColumn; + const column = layer.columns.col1 as RangeIndexPatternColumn; column.dataType = 'number'; column.scale = 'interval'; column.params.type = MODES.Histogram; } function setToRangeMode() { - const column = state.layers.first.columns.col1 as RangeIndexPatternColumn; + const column = layer.columns.col1 as RangeIndexPatternColumn; column.dataType = 'string'; column.scale = 'ordinal'; column.params.type = MODES.Range; } - function getDefaultState(): IndexPatternPrivateState { + function getDefaultLayer(): IndexPatternLayer { return { - indexPatternRefs: [], - indexPatterns: { - '1': { - id: '1', - title: 'my_index_pattern', - hasRestrictions: false, - fields: [{ name: sourceField, type: 'number', displayName: sourceField }], - getFieldByName: getFieldByNameFactory([ - { name: sourceField, type: 'number', displayName: sourceField }, - ]), - }, - }, - existingFields: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - // Start with the histogram type - col1: { - label: sourceField, - dataType: 'number', - operationType: 'range', - scale: 'interval', - isBucketed: true, - sourceField, - params: { - type: MODES.Histogram, - ranges: [{ from: 0, to: DEFAULT_INTERVAL, label: '' }], - maxBars: 'auto', - }, - }, - col2: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + // Start with the histogram type + col1: { + label: sourceField, + dataType: 'number', + operationType: 'range', + scale: 'interval', + isBucketed: true, + sourceField, + params: { + type: MODES.Histogram, + ranges: [{ from: 0, to: DEFAULT_INTERVAL, label: '' }], + maxBars: 'auto', }, }, + col2: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, }, }; } @@ -142,7 +132,7 @@ describe('ranges', () => { }); beforeEach(() => { - state = getDefaultState(); + layer = getDefaultLayer(); }); describe('toEsAggsFn', () => { @@ -150,7 +140,7 @@ describe('ranges', () => { it('should reflect params correctly', () => { const esAggsFn = rangeOperation.toEsAggsFn( - state.layers.first.columns.col1 as RangeIndexPatternColumn, + layer.columns.col1 as RangeIndexPatternColumn, 'col1', {} as IndexPattern ); @@ -189,10 +179,10 @@ describe('ranges', () => { }); it('should set maxBars param if provided', () => { - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.maxBars = 10; + (layer.columns.col1 as RangeIndexPatternColumn).params.maxBars = 10; const esAggsFn = rangeOperation.toEsAggsFn( - state.layers.first.columns.col1 as RangeIndexPatternColumn, + layer.columns.col1 as RangeIndexPatternColumn, 'col1', {} as IndexPattern ); @@ -211,7 +201,7 @@ describe('ranges', () => { setToRangeMode(); const esAggsFn = rangeOperation.toEsAggsFn( - state.layers.first.columns.col1 as RangeIndexPatternColumn, + layer.columns.col1 as RangeIndexPatternColumn, 'col1', {} as IndexPattern ); @@ -225,12 +215,12 @@ describe('ranges', () => { it('should include custom labels', () => { setToRangeMode(); - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.ranges = [ + (layer.columns.col1 as RangeIndexPatternColumn).params.ranges = [ { from: 0, to: 100, label: 'customlabel' }, ]; const esAggsFn = rangeOperation.toEsAggsFn( - state.layers.first.columns.col1 as RangeIndexPatternColumn, + layer.columns.col1 as RangeIndexPatternColumn, 'col1', {} as IndexPattern ); @@ -276,36 +266,30 @@ describe('ranges', () => { describe('paramEditor', () => { describe('Modify intervals in basic mode', () => { beforeEach(() => { - state = getDefaultState(); + layer = getDefaultLayer(); }); it('should start update the state with the default maxBars value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); mount( ); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - maxBars: GRANULARITY_DEFAULT_VALUE, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + maxBars: GRANULARITY_DEFAULT_VALUE, }, }, }, @@ -313,16 +297,15 @@ describe('ranges', () => { }); it('should update state when changing Max bars number', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -343,20 +326,15 @@ describe('ranges', () => { jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); }); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - maxBars: MAX_HISTOGRAM_VALUE, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + maxBars: MAX_HISTOGRAM_VALUE, }, }, }, @@ -364,16 +342,15 @@ describe('ranges', () => { }); it('should update the state using the plus or minus buttons by the step amount', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -389,20 +366,15 @@ describe('ranges', () => { jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); }); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + maxBars: GRANULARITY_DEFAULT_VALUE - GRANULARITY_STEP, }, }, }, @@ -417,25 +389,19 @@ describe('ranges', () => { jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); }); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - maxBars: GRANULARITY_DEFAULT_VALUE, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + maxBars: GRANULARITY_DEFAULT_VALUE, }, }, }, }); - // }); }); }); @@ -446,16 +412,15 @@ describe('ranges', () => { beforeEach(() => setToRangeMode()); it('should show one range interval to start with', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -463,16 +428,15 @@ describe('ranges', () => { }); it('should add a new range', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -495,23 +459,18 @@ describe('ranges', () => { } as React.ChangeEvent); jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - ranges: [ - { from: 0, to: DEFAULT_INTERVAL, label: '' }, - { from: 50, to: Infinity, label: '' }, - ], - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + ranges: [ + { from: 0, to: DEFAULT_INTERVAL, label: '' }, + { from: 50, to: Infinity, label: '' }, + ], }, }, }, @@ -520,16 +479,15 @@ describe('ranges', () => { }); it('should add a new range with custom label', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -552,23 +510,18 @@ describe('ranges', () => { } as React.ChangeEvent); jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - ranges: [ - { from: 0, to: DEFAULT_INTERVAL, label: '' }, - { from: DEFAULT_INTERVAL, to: Infinity, label: 'customlabel' }, - ], - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + ranges: [ + { from: 0, to: DEFAULT_INTERVAL, label: '' }, + { from: DEFAULT_INTERVAL, to: Infinity, label: 'customlabel' }, + ], }, }, }, @@ -577,16 +530,15 @@ describe('ranges', () => { }); it('should open a popover to edit an existing range', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -607,20 +559,15 @@ describe('ranges', () => { } as React.ChangeEvent); jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...state.layers.first.columns.col1.params, - ranges: [{ from: 0, to: 50, label: '' }], - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + ranges: [{ from: 0, to: 50, label: '' }], }, }, }, @@ -629,16 +576,15 @@ describe('ranges', () => { }); it('should not accept invalid ranges', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -670,10 +616,10 @@ describe('ranges', () => { }); it('should be possible to remove a range if multiple', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); // Add an extra range - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.ranges.push({ + (layer.columns.col1 as RangeIndexPatternColumn).params.ranges.push({ from: DEFAULT_INTERVAL, to: 2 * DEFAULT_INTERVAL, label: '', @@ -682,11 +628,10 @@ describe('ranges', () => { const instance = mount( ); @@ -709,10 +654,10 @@ describe('ranges', () => { }); it('should handle correctly open ranges when saved', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); // Add an extra open range: - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.ranges.push({ + (layer.columns.col1 as RangeIndexPatternColumn).params.ranges.push({ from: null, to: null, label: '', @@ -721,11 +666,10 @@ describe('ranges', () => { const instance = mount( ); @@ -749,21 +693,21 @@ describe('ranges', () => { }); it('should correctly handle the default formatter for the field', () => { - const setStateSpy = jest.fn(); - - // set a default formatter for the sourceField used - state.indexPatterns['1'].fieldFormatMap = { - MyField: { id: 'custom', params: {} }, - }; + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -773,15 +717,10 @@ describe('ranges', () => { }); it('should correctly pick the dimension formatter for the field', () => { - const setStateSpy = jest.fn(); - - // set a default formatter for the sourceField used - state.indexPatterns['1'].fieldFormatMap = { - MyField: { id: 'custom', params: {} }, - }; + const updateLayerSpy = jest.fn(); // now set a format on the range operation - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.format = { + (layer.columns.col1 as RangeIndexPatternColumn).params.format = { id: 'bytes', params: { decimals: 0 }, }; @@ -789,11 +728,16 @@ describe('ranges', () => { const instance = mount( ); @@ -803,25 +747,24 @@ describe('ranges', () => { }); it('should not update the state on mount', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); mount( ); - expect(setStateSpy.mock.calls.length).toBe(0); + expect(updateLayerSpy.mock.calls.length).toBe(0); }); it('should not reset formatters when switching between custom ranges and auto histogram', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); // now set a format on the range operation - (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.format = { + (layer.columns.col1 as RangeIndexPatternColumn).params.format = { id: 'custom', params: { decimals: 3 }, }; @@ -829,11 +772,10 @@ describe('ranges', () => { const instance = mount( ); @@ -842,7 +784,7 @@ describe('ranges', () => { instance.find(EuiLink).first().prop('onClick')!({} as ReactMouseEvent); }); - expect(setStateSpy.mock.calls[1][0].layers.first.columns.col1.params.format).toEqual({ + expect(updateLayerSpy.mock.calls[1][0].columns.col1.params.format).toEqual({ id: 'custom', params: { decimals: 3 }, }); 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 38712e2f362a39..b687e6fe3da504 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 @@ -16,7 +16,6 @@ import { RangeEditor } from './range_editor'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { updateColumnParam } from '../../layer_helpers'; -import { mergeLayer } from '../../../state_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; @@ -176,8 +175,15 @@ export const rangeOperation: OperationDefinition { - const indexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; + paramEditor: ({ + layer, + columnId, + currentColumn, + updateLayer, + indexPattern, + uiSettings, + data, + }) => { const currentField = indexPattern.getFieldByName(currentColumn.sourceField); const numberFormat = currentColumn.params.format; const numberFormatterPattern = @@ -201,11 +207,10 @@ export const rangeOperation: OperationDefinition { - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName, value, }) @@ -220,29 +225,24 @@ export const rangeOperation: OperationDefinition isSortableByColumn(state.layers[layerId], columnId)) - .map(([columnId, column]) => { + const orderOptions = Object.entries(layer.columns) + .filter(([sortId]) => isSortableByColumn(layer, sortId)) + .map(([sortId, column]) => { return { - value: toValue({ type: 'column', columnId }), + value: toValue({ type: 'column', columnId: sortId }), text: column.label, }; }); @@ -232,11 +231,10 @@ export const termsOperation: OperationDefinition { - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'size', value, }) @@ -275,11 +273,10 @@ export const termsOperation: OperationDefinition - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'otherBucket', value: e.target.checked, }) @@ -296,11 +293,10 @@ export const termsOperation: OperationDefinition - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'missingBucket', value: e.target.checked, }) @@ -324,11 +320,10 @@ export const termsOperation: OperationDefinition) => - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'orderBy', value: fromValue(e.target.value), }) @@ -365,11 +360,10 @@ export const termsOperation: OperationDefinition) => - setState( + updateLayer( updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName: 'orderDirection', value: e.target.value as 'asc' | 'desc', }) 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 d895fa0a03d62b..e0cdfc44c9d6f1 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 @@ -8,14 +8,14 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { shallow, mount } from 'enzyme'; import { EuiRange, EuiSelect, EuiSwitch, EuiSwitchEvent } from '@elastic/eui'; -import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; -import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import type { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { dataPluginMock } from '../../../../../../../../src/plugins/data/public/mocks'; import { createMockedIndexPattern } from '../../../mocks'; import { ValuesRangeInput } from './values_range_input'; -import { TermsIndexPatternColumn } from '.'; +import type { TermsIndexPatternColumn } from '.'; import { termsOperation } from '../index'; -import { IndexPatternPrivateState, IndexPattern, IndexPatternLayer } from '../../../types'; +import { IndexPattern, IndexPatternLayer } from '../../../types'; const defaultProps = { storage: {} as IStorageWrapper, @@ -24,48 +24,36 @@ const defaultProps = { dateRange: { fromDate: 'now-1d', toDate: 'now' }, data: dataPluginMock.createStartContract(), http: {} as HttpSetup, + indexPattern: createMockedIndexPattern(), }; describe('terms', () => { - let state: IndexPatternPrivateState; + let layer: IndexPatternLayer; const InlineOptions = termsOperation.paramEditor!; beforeEach(() => { - state = { - indexPatternRefs: [], - indexPatterns: { - '1': { - 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: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, + layer = { + 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: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', }, }, }; @@ -73,7 +61,7 @@ describe('terms', () => { describe('toEsAggsFn', () => { it('should reflect params correctly', () => { - const termsColumn = state.layers.first.columns.col1 as TermsIndexPatternColumn; + const termsColumn = layer.columns.col1 as TermsIndexPatternColumn; const esAggsFn = termsOperation.toEsAggsFn( { ...termsColumn, params: { ...termsColumn.params, otherBucket: true } }, 'col1', @@ -92,7 +80,7 @@ describe('terms', () => { }); it('should not enable missing bucket if other bucket is not set', () => { - const termsColumn = state.layers.first.columns.col1 as TermsIndexPatternColumn; + const termsColumn = layer.columns.col1 as TermsIndexPatternColumn; const esAggsFn = termsOperation.toEsAggsFn( { ...termsColumn, @@ -339,7 +327,7 @@ describe('terms', () => { it('should use the default size when there is an existing bucket', () => { const termsColumn = termsOperation.buildColumn({ indexPattern: createMockedIndexPattern(), - layer: state.layers.first, + layer, field: { aggregatable: true, searchable: true, @@ -574,15 +562,14 @@ describe('terms', () => { describe('param editor', () => { it('should render current other bucket value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -594,15 +581,18 @@ describe('terms', () => { }); it('should hide other bucket setting for rollups', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -610,15 +600,14 @@ describe('terms', () => { }); it('should disable missing bucket setting as long as other bucket is not set', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -630,23 +619,22 @@ describe('terms', () => { }); it('should enable missing bucket setting as long as other bucket is set', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -658,15 +646,14 @@ describe('terms', () => { }); it('should update state when clicking other bucket toggle', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -679,20 +666,15 @@ describe('terms', () => { }, } as EuiSwitchEvent); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...(state.layers.first.columns.col1 as TermsIndexPatternColumn).params, - otherBucket: true, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as TermsIndexPatternColumn).params, + otherBucket: true, }, }, }, @@ -700,15 +682,14 @@ describe('terms', () => { }); it('should render current order by value and options', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -723,15 +704,14 @@ describe('terms', () => { }); it('should update state with the order by value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -744,22 +724,17 @@ describe('terms', () => { }, } as React.ChangeEvent); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...(state.layers.first.columns.col1 as TermsIndexPatternColumn).params, - orderBy: { - type: 'column', - columnId: 'col2', - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as TermsIndexPatternColumn).params, + orderBy: { + type: 'column', + columnId: 'col2', }, }, }, @@ -768,15 +743,14 @@ describe('terms', () => { }); it('should render current order direction value and options', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -789,15 +763,14 @@ describe('terms', () => { }); it('should update state with the order direction value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = shallow( ); @@ -810,20 +783,15 @@ describe('terms', () => { }, } as React.ChangeEvent); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...(state.layers.first.columns.col1 as TermsIndexPatternColumn).params, - orderDirection: 'desc', - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as TermsIndexPatternColumn).params, + orderDirection: 'desc', }, }, }, @@ -831,15 +799,14 @@ describe('terms', () => { }); it('should render current size value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -847,15 +814,14 @@ describe('terms', () => { }); it('should update state with the size value', () => { - const setStateSpy = jest.fn(); + const updateLayerSpy = jest.fn(); const instance = mount( ); @@ -863,20 +829,15 @@ describe('terms', () => { instance.find(ValuesRangeInput).prop('onChange')!(7); }); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col1: { - ...state.layers.first.columns.col1, - params: { - ...(state.layers.first.columns.col1 as TermsIndexPatternColumn).params, - size: 7, - }, - }, + expect(updateLayerSpy).toHaveBeenCalledWith({ + ...layer, + columns: { + ...layer.columns, + col1: { + ...layer.columns.col1, + params: { + ...(layer.columns.col1 as TermsIndexPatternColumn).params, + size: 7, }, }, }, @@ -885,7 +846,6 @@ describe('terms', () => { }); describe('getErrorMessage', () => { let indexPattern: IndexPattern; - let layer: IndexPatternLayer; beforeEach(() => { indexPattern = createMockedIndexPattern(); layer = { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index 95242532774b1a..79a71b386318c3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -18,7 +18,7 @@ import { operationDefinitionMap, OperationType } from '../operations'; import { TermsIndexPatternColumn } from './definitions/terms'; import { DateHistogramIndexPatternColumn } from './definitions/date_histogram'; import { AvgIndexPatternColumn } from './definitions/metrics'; -import type { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from '../types'; +import type { IndexPattern, IndexPatternLayer } from '../types'; import { documentField } from '../document_field'; import { getFieldByNameFactory } from '../pure_helpers'; import { generateId } from '../../id_generator'; @@ -1420,31 +1420,19 @@ describe('state_helpers', () => { sourceField: 'timestamp', }; - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { + expect( + updateColumnParam({ + layer: { indexPatternId: '1', columnOrder: ['col1'], columns: { col1: currentColumn, }, }, - }, - }; - - expect( - updateColumnParam({ - state, - layerId: 'first', - currentColumn, + columnId: 'col1', paramName: 'interval', value: 'M', - }).layers.first.columns.col1 + }).columns.col1 ).toEqual({ ...currentColumn, params: { interval: 'M' }, @@ -1461,31 +1449,19 @@ describe('state_helpers', () => { sourceField: 'bytes', }; - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { + expect( + updateColumnParam({ + layer: { indexPatternId: '1', columnOrder: ['col1'], columns: { col1: currentColumn, }, }, - }, - }; - - expect( - updateColumnParam({ - state, - layerId: 'first', - currentColumn, + columnId: 'col1', paramName: 'format', value: { id: 'bytes' }, - }).layers.first.columns.col1 + }).columns.col1 ).toEqual({ ...currentColumn, params: { format: { id: 'bytes' } }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index 711ce2cc845e1a..177f828ff1f0f9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -13,14 +13,8 @@ import { IndexPatternColumn, RequiredReference, } from './definitions'; -import type { - IndexPattern, - IndexPatternField, - IndexPatternLayer, - IndexPatternPrivateState, -} from '../types'; +import type { IndexPattern, IndexPatternField, IndexPatternLayer } from '../types'; import { getSortScoreByPriority } from './operations'; -import { mergeLayer } from '../state_helpers'; import { generateId } from '../../id_generator'; import { ReferenceBasedIndexPatternColumn } from './definitions/column_types'; @@ -424,40 +418,29 @@ export function getMetricOperationTypes(field: IndexPatternField) { } export function updateColumnParam({ - state, - layerId, - currentColumn, + layer, + columnId, paramName, value, }: { - state: IndexPatternPrivateState; - layerId: string; - currentColumn: C; + layer: IndexPatternLayer; + columnId: string; paramName: string; value: unknown; -}): IndexPatternPrivateState { - const columnId = Object.entries(state.layers[layerId].columns).find( - ([_columnId, column]) => column === currentColumn - )![0]; - - const layer = state.layers[layerId]; - - return mergeLayer({ - state, - layerId, - newLayer: { - columns: { - ...layer.columns, - [columnId]: { - ...currentColumn, - params: { - ...currentColumn.params, - [paramName]: value, - }, +}): IndexPatternLayer { + return { + ...layer, + columns: { + ...layer.columns, + [columnId]: { + ...layer.columns[columnId], + params: { + ...layer.columns[columnId].params, + [paramName]: value, }, }, - }, - }); + } as Record, + }; } function adjustColumnReferencesForChangedColumn(layer: IndexPatternLayer, changedColumnId: string) { From 8820866d9c9e77cb0fda28112151882eb96865e0 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Fri, 18 Dec 2020 17:33:46 -0500 Subject: [PATCH 08/10] Fix error states --- .../dimension_panel/reference_editor.test.tsx | 86 +++++++++++++++++++ .../dimension_panel/reference_editor.tsx | 20 +++-- .../operations/__mocks__/index.ts | 1 - .../definitions/calculations/counter_rate.tsx | 6 +- .../calculations/cumulative_sum.tsx | 11 ++- .../definitions/calculations/derivative.tsx | 6 +- .../calculations/moving_average.tsx | 6 +- .../definitions/calculations/utils.test.ts | 73 ++++++++++++++++ .../definitions/calculations/utils.ts | 80 ++++++++++++++++- .../operations/layer_helpers.test.ts | 48 +---------- .../operations/layer_helpers.ts | 53 ------------ 11 files changed, 269 insertions(+), 121 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.test.ts diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx index a89236d30d8c18..d6c059ae8c3945 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx @@ -345,4 +345,90 @@ describe('reference editor', () => { expect(fieldSelect.prop('selectedOperationType')).toEqual('avg'); expect(fieldSelect.prop('incompleteOperation')).toBeUndefined(); }); + + it('should show the FieldSelect as invalid in the empty state for field-only forms', () => { + wrapper = mount( + true, + }} + /> + ); + + const fieldSelect = wrapper.find(FieldSelect); + expect(fieldSelect.prop('fieldIsInvalid')).toEqual(true); + expect(fieldSelect.prop('selectedField')).toBeUndefined(); + expect(fieldSelect.prop('selectedOperationType')).toBeUndefined(); + expect(fieldSelect.prop('incompleteOperation')).toBeUndefined(); + }); + + it('should show the ParamEditor for functions that offer one', () => { + wrapper = mount( + true, + }} + /> + ); + + expect(wrapper.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').exists()).toBe( + true + ); + }); + + it('should hide the ParamEditor for incomplete functions', () => { + wrapper = mount( + true, + }} + /> + ); + + expect(wrapper.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').exists()).toBe( + false + ); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index 8f45b45a4a3c7e..d567533483f3b8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -177,6 +177,14 @@ export function ReferenceEditor(props: ReferenceEditorProps) { ? [functionOptions.find(({ value }) => value === column.operationType)!] : []; + // If the operationType is incomplete, the user needs to select a field- so + // the function is marked as valid. + const showOperationInvalid = !column && !Boolean(incompleteInfo?.operationType); + // The field is invalid if the operation has been updated without a field, + // or if we are in a field-only mode but empty state + const showFieldInvalid = + Boolean(incompleteInfo?.operationType) || (selectionStyle === 'field' && !column); + return (
@@ -188,11 +196,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) { defaultMessage: 'Choose a sub-function', })} fullWidth - isInvalid={ - // If the operationType is incomplete, the user needs to select a field- so - // the function is marked as valid. - !column && !Boolean(incompleteInfo?.operationType) - } + isInvalid={showOperationInvalid} > { @@ -234,10 +238,10 @@ export function ReferenceEditor(props: ReferenceEditorProps) { defaultMessage: 'Select a field', })} fullWidth - isInvalid={Boolean(incompleteInfo?.operationType)} + isInvalid={showFieldInvalid} > { return hasDateField(newIndexPattern); }, - getErrorMessage: (layer: IndexPatternLayer) => { - return checkForDateHistogram( + getErrorMessage: (layer: IndexPatternLayer, columnId: string) => { + return getErrorsForDateReference( layer, + columnId, i18n.translate('xpack.lens.indexPattern.counterRate', { defaultMessage: 'Counter rate', }) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx index fd2ba392e72c48..33314bb9073827 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx @@ -7,7 +7,11 @@ import { i18n } from '@kbn/i18n'; import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types'; import { IndexPatternLayer } from '../../../types'; -import { checkForDateHistogram, dateBasedOperationToExpression } from './utils'; +import { + checkForDateHistogram, + getErrorsForDateReference, + dateBasedOperationToExpression, +} from './utils'; import { OperationDefinition } from '..'; const ofName = (name?: string) => { @@ -81,9 +85,10 @@ export const cumulativeSumOperation: OperationDefinition< isTransferable: () => { return true; }, - getErrorMessage: (layer: IndexPatternLayer) => { - return checkForDateHistogram( + getErrorMessage: (layer: IndexPatternLayer, columnId: string) => { + return getErrorsForDateReference( layer, + columnId, i18n.translate('xpack.lens.indexPattern.cumulativeSum', { defaultMessage: 'Cumulative sum', }) 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 8ffdf6931b54ad..3540fef1307634 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 @@ -10,6 +10,7 @@ import { IndexPatternLayer } from '../../../types'; import { buildLabelFunction, checkForDateHistogram, + getErrorsForDateReference, dateBasedOperationToExpression, hasDateField, } from './utils'; @@ -91,9 +92,10 @@ export const derivativeOperation: OperationDefinition< return hasDateField(newIndexPattern); }, onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, - getErrorMessage: (layer: IndexPatternLayer) => { - return checkForDateHistogram( + getErrorMessage: (layer: IndexPatternLayer, columnId: string) => { + return getErrorsForDateReference( layer, + columnId, i18n.translate('xpack.lens.indexPattern.derivative', { defaultMessage: 'Differences', }) 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 13507416a38c5c..837620e80aaa63 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 @@ -14,6 +14,7 @@ import { IndexPatternLayer } from '../../../types'; import { buildLabelFunction, checkForDateHistogram, + getErrorsForDateReference, dateBasedOperationToExpression, hasDateField, } from './utils'; @@ -99,9 +100,10 @@ export const movingAverageOperation: OperationDefinition< return hasDateField(newIndexPattern); }, onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, - getErrorMessage: (layer: IndexPatternLayer) => { - return checkForDateHistogram( + getErrorMessage: (layer: IndexPatternLayer, columnId: string) => { + return getErrorsForDateReference( layer, + columnId, i18n.translate('xpack.lens.indexPattern.movingAverage', { defaultMessage: 'Moving average', }) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.test.ts new file mode 100644 index 00000000000000..403f2b87ac86e9 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.test.ts @@ -0,0 +1,73 @@ +/* + * 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 { checkReferences } from './utils'; +import { operationDefinitionMap } from '..'; +import { createMockedReferenceOperation } from '../../mocks'; + +// Mock prevents issue with circular loading +jest.mock('..'); + +describe('utils', () => { + beforeEach(() => { + // @ts-expect-error test-only operation type + operationDefinitionMap.testReference = createMockedReferenceOperation(); + }); + + describe('checkReferences', () => { + it('should show an error if the reference is missing', () => { + expect( + checkReferences( + { + columns: { + ref: { + label: 'Label', + // @ts-expect-error test-only operation type + operationType: 'testReference', + isBucketed: false, + dataType: 'number', + references: ['missing'], + }, + }, + columnOrder: ['ref'], + indexPatternId: '', + }, + 'ref' + ) + ).toEqual(['"Label" is not fully configured']); + }); + + it('should show an error if the reference is not allowed per the requirements', () => { + expect( + checkReferences( + { + columns: { + ref: { + label: 'Label', + // @ts-expect-error test-only operation type + operationType: 'testReference', + isBucketed: false, + dataType: 'number', + references: ['invalid'], + }, + invalid: { + label: 'Date', + operationType: 'date_histogram', + isBucketed: true, + dataType: 'date', + sourceField: 'timestamp', + params: { interval: 'auto' }, + }, + }, + columnOrder: ['invalid', 'ref'], + indexPatternId: '', + }, + 'ref' + ) + ).toEqual(['Dimension "Label" is configured incorrectly']); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts index bac45f683e4449..6cea174829683d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts @@ -5,11 +5,13 @@ */ import { i18n } from '@kbn/i18n'; -import { ExpressionFunctionAST } from '@kbn/interpreter/common'; -import { TimeScaleUnit } from '../../../time_scale'; -import { IndexPattern, IndexPatternLayer } from '../../../types'; +import type { ExpressionFunctionAST } from '@kbn/interpreter/common'; +import type { TimeScaleUnit } from '../../../time_scale'; +import type { IndexPattern, IndexPatternLayer } from '../../../types'; import { adjustTimeScaleLabelSuffix } from '../../time_scale_utils'; -import { ReferenceBasedIndexPatternColumn } from '../column_types'; +import type { ReferenceBasedIndexPatternColumn } from '../column_types'; +import { operationDefinitionMap } from '..'; +import type { IndexPatternColumn, RequiredReference } from '..'; export const buildLabelFunction = (ofName: (name?: string) => string) => ( name?: string, @@ -41,6 +43,76 @@ export function checkForDateHistogram(layer: IndexPatternLayer, name: string) { ]; } +export function checkReferences(layer: IndexPatternLayer, columnId: string) { + const column = layer.columns[columnId] as ReferenceBasedIndexPatternColumn; + + const errors: string[] = []; + + column.references.forEach((referenceId, index) => { + if (!layer.columns[referenceId]) { + errors.push( + i18n.translate('xpack.lens.indexPattern.missingReferenceError', { + defaultMessage: '"{dimensionLabel}" is not fully configured', + values: { + dimensionLabel: column.label, + }, + }) + ); + } else { + const referenceColumn = layer.columns[referenceId]!; + const requirements = + // @ts-expect-error not statically analyzed + operationDefinitionMap[column.operationType].requiredReferences[index]; + const isValid = isColumnValidAsReference({ + validation: requirements, + column: referenceColumn, + }); + + if (!isValid) { + errors.push( + i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', { + defaultMessage: 'Dimension "{dimensionLabel}" is configured incorrectly', + values: { + dimensionLabel: column.label, + }, + }) + ); + } + } + }); + return errors.length ? errors : undefined; +} + +export function isColumnValidAsReference({ + column, + validation, +}: { + column: IndexPatternColumn; + validation: RequiredReference; +}): boolean { + if (!column) return false; + const operationType = column.operationType; + const operationDefinition = operationDefinitionMap[operationType]; + return ( + validation.input.includes(operationDefinition.input) && + (!validation.specificOperations || validation.specificOperations.includes(operationType)) && + validation.validateMetadata(column) + ); +} + +export function getErrorsForDateReference( + layer: IndexPatternLayer, + columnId: string, + name: string +) { + const dateErrors = checkForDateHistogram(layer, name) ?? []; + const referenceErrors = checkReferences(layer, columnId) ?? []; + if (dateErrors.length || referenceErrors.length) { + return [...dateErrors, ...referenceErrors]; + } + return; +} + export function hasDateField(indexPattern: IndexPattern) { return indexPattern.fields.some((field) => field.type === 'date'); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index 79a71b386318c3..9496f95f74dec2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -1834,7 +1834,7 @@ describe('state_helpers', () => { }); describe('getErrorMessages', () => { - it('should collect errors from all types of operation definitions', () => { + it('should collect errors from metric-type operation definitions', () => { const mock = jest.fn().mockReturnValue(['error 1']); operationDefinitionMap.avg.getErrorMessage = mock; const errors = getErrorMessages({ @@ -1849,7 +1849,7 @@ describe('state_helpers', () => { expect(errors).toHaveLength(1); }); - it('should collect errors from the operation definitions', () => { + it('should collect errors from reference-type operation definitions', () => { const mock = jest.fn().mockReturnValue(['error 1']); operationDefinitionMap.testReference.getErrorMessage = mock; const errors = getErrorMessages({ @@ -1864,49 +1864,5 @@ describe('state_helpers', () => { expect(mock).toHaveBeenCalled(); expect(errors).toHaveLength(1); }); - - it('should identify missing references on reference-based calculation', () => { - const errors = getErrorMessages({ - indexPatternId: '1', - columnOrder: [], - columns: { - col1: - // @ts-expect-error not statically analyzed yet - { operationType: 'testReference', references: ['ref1', 'ref2'] }, - }, - }); - expect(errors).toHaveLength(2); - }); - - it('should identify references that are no longer valid', () => { - // There is only one operation with `none` as the input type - // @ts-expect-error this function is not valid - operationDefinitionMap.testReference.requiredReferences = [ - { - input: ['none'], - validateMetadata: () => true, - }, - ]; - - const errors = getErrorMessages({ - indexPatternId: '1', - columnOrder: [], - columns: { - // @ts-expect-error incomplete operation - ref1: { - dataType: 'string', - isBucketed: true, - operationType: 'terms', - }, - col1: { - label: '', - references: ['ref1'], - // @ts-expect-error tests only - operationType: 'testReference', - }, - }, - }); - expect(errors).toHaveLength(1); - }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index 177f828ff1f0f9..3ee0e86b0d6889 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -5,7 +5,6 @@ */ import _, { partition } from 'lodash'; -import { i18n } from '@kbn/i18n'; import { operationDefinitionMap, operationDefinitions, @@ -603,41 +602,6 @@ export function getErrorMessages(layer: IndexPatternLayer): string[] | undefined if (def.getErrorMessage) { errors.push(...(def.getErrorMessage(layer, columnId) ?? [])); } - - if ('references' in column) { - column.references.forEach((referenceId, index) => { - if (!layer.columns[referenceId]) { - errors.push( - i18n.translate('xpack.lens.indexPattern.missingReferenceError', { - defaultMessage: '"{dimensionLabel}" is missing required configuration', - values: { - dimensionLabel: column.label, - }, - }) - ); - } else { - const referenceColumn = layer.columns[referenceId]!; - const requirements = - // @ts-expect-error not statically analyzed - operationDefinitionMap[column.operationType].requiredReferences[index]; - const isValid = isColumnValidAsReference({ - validation: requirements, - column: referenceColumn, - }); - - if (!isValid) { - errors.push( - i18n.translate('xpack.lens.indexPattern.invalidReferenceConfiguration', { - defaultMessage: 'Dimension {dimensionLabel} does not have a valid configuration', - values: { - dimensionLabel: column.label, - }, - }) - ); - } - } - }); - } }); return errors.length ? errors : undefined; @@ -650,23 +614,6 @@ export function isReferenced(layer: IndexPatternLayer, columnId: string): boolea return allReferences.includes(columnId); } -export function isColumnValidAsReference({ - column, - validation, -}: { - column: IndexPatternColumn; - validation: RequiredReference; -}): boolean { - if (!column) return false; - const operationType = column.operationType; - const operationDefinition = operationDefinitionMap[operationType]; - return ( - validation.input.includes(operationDefinition.input) && - (!validation.specificOperations || validation.specificOperations.includes(operationType)) && - validation.validateMetadata(column) - ); -} - export function isOperationAllowedAsReference({ operationType, validation, From 1cf5cd8fa9843ced8c2d8e089b5cfdfd4965aaf8 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 21 Dec 2020 19:20:35 -0500 Subject: [PATCH 09/10] Update logic for showing references in dimension editor, add tests --- .../workspace_panel/workspace_panel.tsx | 11 ++- .../dimension_panel/dimension_editor.tsx | 11 ++- .../dimension_panel/dimension_panel.test.tsx | 60 ++++++++++++++ .../dimension_panel/field_select.tsx | 5 +- .../dimension_panel/reference_editor.test.tsx | 2 + .../dimension_panel/reference_editor.tsx | 25 ++++-- .../definitions/calculations/counter_rate.tsx | 14 ++-- .../calculations/cumulative_sum.tsx | 15 ++-- .../definitions/calculations/derivative.tsx | 14 ++-- .../calculations/moving_average.tsx | 37 ++++++--- .../definitions/calculations/utils.ts | 8 +- .../operations/definitions/cardinality.tsx | 13 +--- .../operations/definitions/date_histogram.tsx | 8 +- .../operations/definitions/helpers.tsx | 9 +++ .../operations/definitions/index.ts | 4 +- .../operations/definitions/last_value.tsx | 13 +--- .../operations/definitions/metrics.tsx | 17 ++-- .../operations/layer_helpers.ts | 66 +++++++--------- .../operations/operations.ts | 11 ++- .../test/functional/apps/lens/smokescreen.ts | 78 +++++++++++++++++++ .../test/functional/page_objects/lens_page.ts | 39 ++++++++++ 21 files changed, 341 insertions(+), 119 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index 95f87a8296fc8b..99a5869a60872b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -400,13 +400,17 @@ export const InnerVisualizationWrapper = ({ showExtraErrors = localState.configurationValidationError .slice(1) .map(({ longMessage }) => ( - + {longMessage} )); } else { showExtraErrors = ( - + { setLocalState((prevState: WorkspaceState) => ({ @@ -414,6 +418,7 @@ export const InnerVisualizationWrapper = ({ expandError: !prevState.expandError, })); }} + data-test-subj="configuration-failure-more-errors" > {i18n.translate('xpack.lens.editorFrame.configurationFailureMoreErrors', { defaultMessage: ` +{errors} {errors, plural, one {error} other {errors}}`, @@ -445,7 +450,7 @@ export const InnerVisualizationWrapper = ({ - + {localState.configurationValidationError[0].longMessage} {showExtraErrors} 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 626bc50f360935..cc22cbbf57883d 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 @@ -16,6 +16,7 @@ import { EuiListGroupItemProps, EuiFormLabel, EuiToolTip, + EuiText, } from '@elastic/eui'; import { IndexPatternDimensionEditorProps } from './dimension_panel'; import { OperationSupportMatrix } from './operation_support'; @@ -184,7 +185,15 @@ export function DimensionEditor(props: DimensionEditorProps) { } let label: EuiListGroupItemProps['label'] = operationPanels[operationType].displayName; - if (disabledStatus) { + if (isActive && disabledStatus) { + label = ( + + + {operationPanels[operationType].displayName} + + + ); + } else if (disabledStatus) { label = ( {operationPanels[operationType].displayName} 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 65b7bd9a759b8b..95a6c351e1fc23 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 @@ -1581,4 +1581,64 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(wrapper.find('ReferenceEditor')).toHaveLength(0); }); + + it('should show a warning when the current dimension is no longer configurable', () => { + const stateWithInvalidCol: IndexPatternPrivateState = getStateWithColumns({ + col1: { + label: 'Invalid derivative', + dataType: 'number', + isBucketed: false, + operationType: 'derivative', + references: ['ref1'], + }, + }); + + wrapper = mount( + + ); + + expect( + wrapper + .find('[data-test-subj="lns-indexPatternDimension-derivative incompatible"]') + .find('EuiText[color="danger"]') + .first() + ).toBeTruthy(); + }); + + it('should remove options to select references when there are no time fields', () => { + const stateWithoutTime: IndexPatternPrivateState = { + ...getStateWithColumns({ + col1: { + label: 'Avg', + dataType: 'number', + isBucketed: false, + operationType: 'avg', + sourceField: 'bytes', + }, + }), + indexPatterns: { + 1: { + id: '1', + title: 'my-fake-index-pattern', + hasRestrictions: false, + fields, + getFieldByName: getFieldByNameFactory([ + { + name: 'bytes', + displayName: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + ]), + }, + }, + }; + + wrapper = mount( + + ); + + expect(wrapper.find('[data-test-subj="lns-indexPatternDimension-derivative"]')).toHaveLength(0); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx index 406a32f62b2c7e..fbdf90e6cc4c79 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/field_select.tsx @@ -41,6 +41,7 @@ export interface FieldSelectProps extends EuiComboBoxProps<{}> { onDeleteColumn: () => void; existingFields: IndexPatternPrivateState['existingFields']; fieldIsInvalid: boolean; + markAllFieldsCompatible?: boolean; } export function FieldSelect({ @@ -53,6 +54,7 @@ export function FieldSelect({ onDeleteColumn, existingFields, fieldIsInvalid, + markAllFieldsCompatible, ...rest }: FieldSelectProps) { const { operationByField } = operationSupportMatrix; @@ -93,7 +95,7 @@ export function FieldSelect({ : operationByField[field]!.values().next().value, }, exists: containsData(field), - compatible: isCompatibleWithCurrentOperation(field), + compatible: markAllFieldsCompatible || isCompatibleWithCurrentOperation(field), }; }) .sort((a, b) => { @@ -163,6 +165,7 @@ export function FieldSelect({ currentIndexPattern, operationByField, existingFields, + markAllFieldsCompatible, ]); return ( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx index d6c059ae8c3945..0891dd27fcf172 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.test.tsx @@ -310,6 +310,7 @@ describe('reference editor', () => { expect(fieldSelect.prop('selectedField')).toEqual('bytes'); expect(fieldSelect.prop('selectedOperationType')).toEqual('avg'); expect(fieldSelect.prop('incompleteOperation')).toEqual('max'); + expect(fieldSelect.prop('markAllFieldsCompatible')).toEqual(false); }); it('should pass the incomplete field info to FieldSelect', () => { @@ -363,6 +364,7 @@ describe('reference editor', () => { expect(fieldSelect.prop('selectedField')).toBeUndefined(); expect(fieldSelect.prop('selectedOperationType')).toBeUndefined(); expect(fieldSelect.prop('incompleteOperation')).toBeUndefined(); + expect(fieldSelect.prop('markAllFieldsCompatible')).toEqual(true); }); it('should show the ParamEditor for functions that offer one', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx index d567533483f3b8..d73530ec8a9204 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/reference_editor.tsx @@ -86,7 +86,12 @@ export function ReferenceEditor(props: ReferenceEditorProps) { .forEach((op) => { if (op.input === 'field') { const allFields = currentIndexPattern.fields.filter((field) => - isOperationAllowedAsReference({ operationType: op.type, validation, field }) + isOperationAllowedAsReference({ + operationType: op.type, + validation, + field, + indexPattern: currentIndexPattern, + }) ); if (allFields.length) { operationTypes.add(op.type); @@ -98,7 +103,13 @@ export function ReferenceEditor(props: ReferenceEditorProps) { operationByField[field.name]?.add(op.type); }); } - } else if (isOperationAllowedAsReference({ operationType: op.type, validation })) { + } else if ( + isOperationAllowedAsReference({ + operationType: op.type, + validation, + indexPattern: currentIndexPattern, + }) + ) { operationTypes.add(op.type); operationWithoutField.add(op.type); } @@ -111,8 +122,9 @@ export function ReferenceEditor(props: ReferenceEditorProps) { }; }, [currentIndexPattern, validation]); - const functionOptions: Array> = []; - operationSupportMatrix.operationTypes.forEach((operationType) => { + const functionOptions: Array> = Array.from( + operationSupportMatrix.operationTypes + ).map((operationType) => { const def = operationDefinitionMap[operationType]; const label = operationPanels[operationType].displayName; const isCompatible = @@ -123,14 +135,14 @@ export function ReferenceEditor(props: ReferenceEditorProps) { operationSupportMatrix.fieldByOperation[operationType]?.has(column.sourceField)) || (column && !hasField(column) && def.input !== 'field'); - functionOptions.push({ + return { label, value: operationType, className: 'lnsIndexPatternDimensionEditor__operation', 'data-test-subj': `lns-indexPatternDimension-${operationType}${ isCompatible ? '' : ' incompatible' }`, - }); + }; }); function onChooseFunction(operationType: OperationType) { @@ -256,6 +268,7 @@ export function ReferenceEditor(props: ReferenceEditorProps) { : (column as FieldBasedIndexPatternColumn)?.sourceField } incompleteOperation={incompleteOperation} + markAllFieldsCompatible={selectionStyle === 'field'} onDeleteColumn={() => { updateLayer(deleteColumn({ layer, columnId, indexPattern: currentIndexPattern })); }} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx index 821b80a27873ab..4fd045c17740d3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx @@ -53,12 +53,14 @@ export const counterRateOperation: OperationDefinition< validateMetadata: (meta) => meta.dataType === 'number' && !meta.isBucketed, }, ], - getPossibleOperation: () => { - return { - dataType: 'number', - isBucketed: false, - scale: 'ratio', - }; + getPossibleOperation: (indexPattern) => { + if (hasDateField(indexPattern)) { + return { + dataType: 'number', + isBucketed: false, + scale: 'ratio', + }; + } }, getDefaultLabel: (column, indexPattern, columns) => { const ref = columns[column.references[0]]; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx index 33314bb9073827..7067b6470bec73 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx @@ -11,6 +11,7 @@ import { checkForDateHistogram, getErrorsForDateReference, dateBasedOperationToExpression, + hasDateField, } from './utils'; import { OperationDefinition } from '..'; @@ -50,12 +51,14 @@ export const cumulativeSumOperation: OperationDefinition< validateMetadata: (meta) => meta.dataType === 'number' && !meta.isBucketed, }, ], - getPossibleOperation: () => { - return { - dataType: 'number', - isBucketed: false, - scale: 'ratio', - }; + getPossibleOperation: (indexPattern) => { + if (hasDateField(indexPattern)) { + return { + dataType: 'number', + isBucketed: false, + scale: 'ratio', + }; + } }, getDefaultLabel: (column, indexPattern, columns) => { const ref = columns[column.references[0]]; 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 3540fef1307634..358046ad5bfb9c 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 @@ -52,12 +52,14 @@ export const derivativeOperation: OperationDefinition< validateMetadata: (meta) => meta.dataType === 'number' && !meta.isBucketed, }, ], - getPossibleOperation: () => { - return { - dataType: 'number', - isBucketed: false, - scale: 'ratio', - }; + getPossibleOperation: (indexPattern) => { + if (hasDateField(indexPattern)) { + return { + dataType: 'number', + isBucketed: false, + scale: 'ratio', + }; + } }, getDefaultLabel: (column, indexPattern, columns) => { const ref = columns[column.references[0]]; 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 837620e80aaa63..e1bc378635f1d6 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 @@ -61,12 +61,14 @@ export const movingAverageOperation: OperationDefinition< validateMetadata: (meta) => meta.dataType === 'number' && !meta.isBucketed, }, ], - getPossibleOperation: () => { - return { - dataType: 'number', - isBucketed: false, - scale: 'ratio', - }; + getPossibleOperation: (indexPattern) => { + if (hasDateField(indexPattern)) { + return { + dataType: 'number', + isBucketed: false, + scale: 'ratio', + }; + } }, getDefaultLabel: (column, indexPattern, columns) => { return ofName(columns[column.references[0]]?.label, column.timeScale); @@ -120,6 +122,19 @@ 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, @@ -130,10 +145,8 @@ function MovingAverageParamEditor({ useDebounceWithOptions( () => { - if (inputValue === '') { - return; - } - const inputNumber = Number(inputValue); + if (!isValidNumber(inputValue)) return; + const inputNumber = parseInt(inputValue, 10); updateLayer( updateColumnParam({ layer, @@ -147,6 +160,7 @@ function MovingAverageParamEditor({ 256, [inputValue] ); + return ( ) => setInputValue(e.target.value)} min={1} + step={1} + isInvalid={!isValidNumber(inputValue)} /> ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts index 6cea174829683d..ca4b7c53b7ec74 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts @@ -60,9 +60,11 @@ export function checkReferences(layer: IndexPatternLayer, columnId: string) { ); } else { const referenceColumn = layer.columns[referenceId]!; - const requirements = - // @ts-expect-error not statically analyzed - operationDefinitionMap[column.operationType].requiredReferences[index]; + const definition = operationDefinitionMap[column.operationType]; + if (definition.input !== 'fullReference') { + throw new Error('inconsistent state - column is not a reference operation'); + } + const requirements = definition.requiredReferences[index]; const isValid = isColumnValidAsReference({ validation: requirements, column: referenceColumn, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index d9dbbe5894c4d4..970f56020c7cdc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -10,7 +10,7 @@ import { buildExpressionFunction } from '../../../../../../../src/plugins/expres import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; -import { getInvalidFieldMessage } from './helpers'; +import { getInvalidFieldMessage, getSafeName } from './helpers'; const supportedTypes = new Set(['string', 'boolean', 'number', 'ip', 'date']); @@ -18,15 +18,11 @@ const SCALE = 'ratio'; const OPERATION_TYPE = 'cardinality'; const IS_BUCKETED = false; -function ofName(name?: string) { +function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.cardinalityOf', { defaultMessage: 'Unique count of {name}', values: { - name: - name ?? - i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { - defaultMessage: 'Missing field', - }), + name, }, }); } @@ -64,8 +60,7 @@ export const cardinalityOperation: OperationDefinition - ofName(indexPattern.getFieldByName(column.sourceField)?.displayName), + getDefaultLabel: (column, indexPattern) => ofName(getSafeName(column.sourceField, indexPattern)), buildColumn({ field, previousColumn }) { return { label: ofName(field.displayName), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index c904eefba9bd03..a41cc88c4f2927 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -28,7 +28,7 @@ import { search, } from '../../../../../../../src/plugins/data/public'; import { buildExpressionFunction } from '../../../../../../../src/plugins/expressions/public'; -import { getInvalidFieldMessage } from './helpers'; +import { getInvalidFieldMessage, getSafeName } from './helpers'; const { isValidInterval } = search.aggs; const autoInterval = 'auto'; @@ -67,11 +67,7 @@ export const dateHistogramOperation: OperationDefinition< }; } }, - getDefaultLabel: (column, indexPattern) => - indexPattern.getFieldByName(column.sourceField)?.displayName ?? - i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { - defaultMessage: 'Missing field', - }), + getDefaultLabel: (column, indexPattern) => getSafeName(column.sourceField, indexPattern), buildColumn({ field }) { let interval = autoInterval; let timeZone: string | undefined; 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 640a357d9a7a45..7b96bcf4f20698 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 @@ -62,3 +62,12 @@ export function getInvalidFieldMessage( ] : undefined; } + +export function getSafeName(name: string, indexPattern: IndexPattern): string { + const field = indexPattern.getFieldByName(name); + return field + ? field.displayName + : i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { + defaultMessage: 'Missing field', + }); +} 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 ade89d49b6603b..6231460347de26 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 @@ -315,9 +315,9 @@ interface FullReferenceOperationDefinition { ) => ReferenceBasedIndexPatternColumn & C; /** * Returns the meta data of the operation if applied. Undefined - * if the field is not applicable. + * if the operation can't be added with these fields. */ - getPossibleOperation: () => OperationMetadata; + getPossibleOperation: (indexPattern: IndexPattern) => OperationMetadata | undefined; /** * A chain of expression functions which will transform the table */ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 553ee8f4f897d9..256ef7f75676d2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -13,17 +13,13 @@ import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; import { updateColumnParam } from '../layer_helpers'; import { DataType } from '../../../types'; -import { getInvalidFieldMessage } from './helpers'; +import { getInvalidFieldMessage, getSafeName } from './helpers'; -function ofName(name?: string) { +function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.lastValueOf', { defaultMessage: 'Last value of {name}', values: { - name: - name ?? - i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { - defaultMessage: 'Missing field', - }), + name, }, }); } @@ -93,8 +89,7 @@ export const lastValueOperation: OperationDefinition - ofName(indexPattern.getFieldByName(column.sourceField)?.displayName), + getDefaultLabel: (column, indexPattern) => ofName(getSafeName(column.sourceField, indexPattern)), input: 'field', onFieldChange: (oldColumn, field) => { const newParams = { ...oldColumn.params }; 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 ffda7e16ff3f39..eb25b5d932b1f8 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 @@ -7,7 +7,7 @@ import { i18n } from '@kbn/i18n'; import { buildExpressionFunction } from '../../../../../../../src/plugins/expressions/public'; import { OperationDefinition } from './index'; -import { getInvalidFieldMessage } from './helpers'; +import { getInvalidFieldMessage, getSafeName } from './helpers'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn, @@ -44,17 +44,12 @@ function buildMetricOperation>({ priority?: number; optionalTimeScaling?: boolean; }) { - const labelLookup = (name?: string, column?: BaseIndexPatternColumn) => { - const rawLabel = ofName( - name ?? - i18n.translate('xpack.lens.indexPattern.missingFieldLabel', { - defaultMessage: 'Missing field', - }) - ); + const labelLookup = (name: string, column?: BaseIndexPatternColumn) => { + const label = ofName(name); if (!optionalTimeScaling) { - return rawLabel; + return label; } - return adjustTimeScaleLabelSuffix(rawLabel, undefined, column?.timeScale); + return adjustTimeScaleLabelSuffix(label, undefined, column?.timeScale); }; return { @@ -91,7 +86,7 @@ function buildMetricOperation>({ ? adjustTimeScaleOnOtherColumnChange(layer, thisColumnId, changedColumnId) : layer.columns[thisColumnId], getDefaultLabel: (column, indexPattern, columns) => - labelLookup(indexPattern.getFieldByName(column.sourceField)?.displayName, column), + labelLookup(getSafeName(column.sourceField, indexPattern), column), buildColumn: ({ field, previousColumn }) => ({ label: labelLookup(field.displayName, previousColumn), dataType: 'number', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts index 3ee0e86b0d6889..2d8078b9a61545 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -82,7 +82,7 @@ export function insertNewColumn({ // access to the operationSupportMatrix, we should validate the metadata against // the possible fields const validOperations = Object.values(operationDefinitionMap).filter(({ type }) => - isOperationAllowedAsReference({ validation, operationType: type }) + isOperationAllowedAsReference({ validation, operationType: type, indexPattern }) ); if (!validOperations.length) { @@ -127,31 +127,23 @@ export function insertNewColumn({ return newId; }); - const possibleOperation = operationDefinition.getPossibleOperation(); - const isBucketed = Boolean(possibleOperation.isBucketed); - if (isBucketed) { - tempLayer = addBucket( - tempLayer, - operationDefinition.buildColumn({ - ...baseOptions, - layer: tempLayer, - referenceIds, - }), - columnId - ); - } else { - tempLayer = addMetric( - tempLayer, - operationDefinition.buildColumn({ - ...baseOptions, - layer: tempLayer, - referenceIds, - }), - columnId + const possibleOperation = operationDefinition.getPossibleOperation(indexPattern); + if (!possibleOperation) { + throw new Error( + `Can't create operation ${op} because it's incompatible with the index pattern` ); } + const isBucketed = Boolean(possibleOperation.isBucketed); - return updateDefaultLabels(tempLayer, indexPattern); + const addOperationFn = isBucketed ? addBucket : addMetric; + return updateDefaultLabels( + addOperationFn( + tempLayer, + operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, referenceIds }), + columnId + ), + indexPattern + ); } const invalidFieldName = (layer.incompleteColumns ?? {})[columnId]?.sourceField; @@ -206,17 +198,15 @@ export function insertNewColumn({ }; } const isBucketed = Boolean(possibleOperation.isBucketed); - if (isBucketed) { - return updateDefaultLabels( - addBucket(layer, operationDefinition.buildColumn({ ...baseOptions, layer, field }), columnId), - indexPattern - ); - } else { - return updateDefaultLabels( - addMetric(layer, operationDefinition.buildColumn({ ...baseOptions, layer, field }), columnId), - indexPattern - ); - } + const addOperationFn = isBucketed ? addBucket : addMetric; + return updateDefaultLabels( + addOperationFn( + layer, + operationDefinition.buildColumn({ ...baseOptions, layer, field }), + columnId + ), + indexPattern + ); } export function replaceColumn({ @@ -311,7 +301,6 @@ export function replaceColumn({ let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, field }); newColumn = adjustLabel(newColumn, previousColumn); - // const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; const newLayer = { ...tempLayer, columns: { ...tempLayer.columns, [columnId]: newColumn } }; return updateDefaultLabels( { @@ -618,9 +607,11 @@ export function isOperationAllowedAsReference({ operationType, validation, field, + indexPattern, }: { operationType: OperationType; validation: RequiredReference; + indexPattern: IndexPattern; field?: IndexPatternField; }): boolean { const operationDefinition = operationDefinitionMap[operationType]; @@ -629,9 +620,12 @@ export function isOperationAllowedAsReference({ if (field && operationDefinition.input === 'field') { const metadata = operationDefinition.getPossibleOperationForField(field); hasValidMetadata = Boolean(metadata) && validation.validateMetadata(metadata!); - } else if (operationDefinition.input !== 'field') { + } else if (operationDefinition.input === 'none') { const metadata = operationDefinition.getPossibleOperation(); hasValidMetadata = Boolean(metadata) && validation.validateMetadata(metadata!); + } else if (operationDefinition.input === 'fullReference') { + const metadata = operationDefinition.getPossibleOperation(indexPattern); + hasValidMetadata = Boolean(metadata) && validation.validateMetadata(metadata!); } else { // TODO: How can we validate the metadata without a specific field? } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts index 58685fa494a046..c111983ea2cd6c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts @@ -167,10 +167,13 @@ export function getAvailableOperationsByMetadata(indexPattern: IndexPattern) { operationDefinition.getPossibleOperation() ); } else if (operationDefinition.input === 'fullReference') { - addToMap( - { type: 'fullReference', operationType: operationDefinition.type }, - operationDefinition.getPossibleOperation() - ); + const validOperation = operationDefinition.getPossibleOperation(indexPattern); + if (validOperation) { + addToMap( + { type: 'fullReference', operationType: operationDefinition.type }, + validOperation + ); + } } }); diff --git a/x-pack/test/functional/apps/lens/smokescreen.ts b/x-pack/test/functional/apps/lens/smokescreen.ts index 92ea9508cf8371..823f1a066b51eb 100644 --- a/x-pack/test/functional/apps/lens/smokescreen.ts +++ b/x-pack/test/functional/apps/lens/smokescreen.ts @@ -326,6 +326,84 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await PageObjects.lens.getDatatableCellText(0, 1)).to.eql('6,011.351'); }); + it('should create a valid XY chart with references', async () => { + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickVisType('lens'); + await PageObjects.lens.goToTimeRange(); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'moving_average', + keepOpen: true, + }); + + await PageObjects.lens.configureReference({ + operation: 'sum', + field: 'bytes', + }); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'cumulative_sum', + keepOpen: true, + }); + + await PageObjects.lens.configureReference({ + field: 'Records', + }); + + await PageObjects.lens.closeDimensionEditor(); + + // Two Y axes that are both valid + expect(await find.allByCssSelector('.echLegendItem')).to.have.length(2); + }); + + /** + * The edge cases are: + * + * 1. Showing errors when creating a partial configuration + * 2. Being able to drag in a new field while in partial config + * 3. Being able to switch charts while in partial config + */ + it('should handle edge cases in reference-based operations', async () => { + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickVisType('lens'); + await PageObjects.lens.goToTimeRange(); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'cumulative_sum', + }); + expect(await PageObjects.lens.getErrorCount()).to.eql(1); + + await PageObjects.lens.removeDimension('lnsXY_xDimensionPanel'); + expect(await PageObjects.lens.getErrorCount()).to.eql(2); + + await PageObjects.lens.dragFieldToDimensionTrigger( + '@timestamp', + 'lnsXY_xDimensionPanel > lns-empty-dimension' + ); + expect(await PageObjects.lens.getErrorCount()).to.eql(1); + + expect(await PageObjects.lens.hasChartSwitchWarning('lnsDatatable')).to.eql(false); + await PageObjects.lens.switchToVisualization('lnsDatatable'); + + expect(await PageObjects.lens.getDatatableHeaderText(1)).to.eql( + 'Cumulative sum of (incomplete)' + ); + }); + it('should allow to change index pattern', async () => { await PageObjects.lens.switchFirstLayerIndexPattern('log*'); expect(await PageObjects.lens.getFirstLayerIndexPattern()).to.equal('log*'); diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index 2159f939a56f7d..7e1fb4ab10a4a0 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -122,6 +122,32 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont } }, + /** + * Changes the specified dimension to the specified operation and (optinally) field. + * + * @param opts.dimension - the selector of the dimension being changed + * @param opts.operation - the desired operation ID for the dimension + * @param opts.field - the desired field for the dimension + * @param layerIndex - the index of the layer + */ + async configureReference(opts: { + operation?: string; + field?: string; + isPreviousIncompatible?: boolean; + }) { + if (opts.operation) { + const target = await testSubjects.find('indexPattern-subFunction-selection-row'); + await comboBox.openOptionsList(target); + await comboBox.setElement(target, opts.operation); + } + + if (opts.field) { + const target = await testSubjects.find('indexPattern-reference-field-selection-row'); + await comboBox.openOptionsList(target); + await comboBox.setElement(target, opts.field); + } + }, + /** * Drags field to workspace * @@ -327,6 +353,19 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont }); }, + /** Counts the visible warnings in the config panel */ + async getErrorCount() { + const moreButton = await testSubjects.exists('configuration-failure-more-errors'); + if (moreButton) { + await retry.try(async () => { + await testSubjects.click('configuration-failure-more-errors'); + await testSubjects.missingOrFail('configuration-failure-more-errors'); + }); + } + const errors = await testSubjects.findAll('configuration-failure-error'); + return errors?.length ?? 0; + }, + /** * Checks a specific subvisualization in the chart switcher for a "data loss" indicator * From bc2087a11933bde2a16c86e7e7af11699f4e3fd8 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 21 Dec 2020 19:38:29 -0500 Subject: [PATCH 10/10] Fix tests --- x-pack/test/functional/apps/lens/smokescreen.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/x-pack/test/functional/apps/lens/smokescreen.ts b/x-pack/test/functional/apps/lens/smokescreen.ts index 823f1a066b51eb..c212a371401d90 100644 --- a/x-pack/test/functional/apps/lens/smokescreen.ts +++ b/x-pack/test/functional/apps/lens/smokescreen.ts @@ -336,28 +336,25 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { operation: 'date_histogram', field: '@timestamp', }); - await PageObjects.lens.configureDimension({ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', operation: 'moving_average', keepOpen: true, }); - await PageObjects.lens.configureReference({ operation: 'sum', field: 'bytes', }); + await PageObjects.lens.closeDimensionEditor(); await PageObjects.lens.configureDimension({ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', operation: 'cumulative_sum', keepOpen: true, }); - await PageObjects.lens.configureReference({ field: 'Records', }); - await PageObjects.lens.closeDimensionEditor(); // Two Y axes that are both valid @@ -399,7 +396,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await PageObjects.lens.hasChartSwitchWarning('lnsDatatable')).to.eql(false); await PageObjects.lens.switchToVisualization('lnsDatatable'); - expect(await PageObjects.lens.getDatatableHeaderText(1)).to.eql( + expect(await PageObjects.lens.getDimensionTriggerText('lnsDatatable_metrics')).to.eql( 'Cumulative sum of (incomplete)' ); });