From eb4b573994285a61f6284ef363aa3d8b4f9beb7d Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Tue, 17 Nov 2020 18:55:28 -0500 Subject: [PATCH 01/10] [Lens] Implement types for reference-based operations --- .../visualization.test.tsx | 4 +- .../editor_frame/state_helpers.ts | 2 +- .../workspace_panel/workspace_panel.tsx | 4 +- .../dimension_panel/dimension_editor.tsx | 2 +- .../dimension_panel/dimension_panel.test.tsx | 1 + .../dimension_panel/operation_support.ts | 2 + .../indexpattern.test.ts | 116 +++- .../indexpattern_datasource/indexpattern.tsx | 120 ++-- .../indexpattern_suggestions.ts | 6 +- .../public/indexpattern_datasource/mocks.ts | 2 +- .../operations/__mocks__/index.ts | 6 + .../operations/definitions/cardinality.tsx | 2 + .../operations/definitions/column_types.ts | 24 +- .../operations/definitions/count.tsx | 1 + .../operations/definitions/date_histogram.tsx | 2 + .../definitions/filters/filters.tsx | 1 + .../operations/definitions/index.ts | 105 +++- .../operations/definitions/metrics.tsx | 2 + .../operations/definitions/ranges/ranges.tsx | 4 +- .../operations/definitions/terms/index.tsx | 19 +- .../operations/index.ts | 9 +- .../operations/layer_helpers.test.ts | 572 +++++++++++++++++- .../operations/layer_helpers.ts | 274 ++++++++- .../operations/mocks.ts | 39 ++ .../operations/operations.ts | 9 + .../indexpattern_datasource/to_expression.ts | 35 +- .../public/indexpattern_datasource/types.ts | 4 +- .../public/indexpattern_datasource/utils.ts | 8 +- 28 files changed, 1227 insertions(+), 148 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx index 0af8e01d7290d..cf3752e649600 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx @@ -410,7 +410,7 @@ describe('Datatable Visualization', () => { const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame); - expect(error).not.toBeDefined(); + expect(error).toBeUndefined(); }); it('returns undefined if the metric dimension is defined', () => { @@ -427,7 +427,7 @@ describe('Datatable Visualization', () => { const error = datatableVisualization.getErrorMessages({ layers: [layer] }, frame); - expect(error).not.toBeDefined(); + expect(error).toBeUndefined(); }); }); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts index 647c0f3ac9cca..0c96fc45de128 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts @@ -134,7 +134,7 @@ export const validateDatasourceAndVisualization = ( ? currentVisualization?.getErrorMessages(currentVisualizationState, frameAPI) : undefined; - if (datasourceValidationErrors || visualizationValidationErrors) { + if (datasourceValidationErrors?.length || visualizationValidationErrors?.length) { return [...(datasourceValidationErrors || []), ...(visualizationValidationErrors || [])]; } return undefined; 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 00cb932a6d4e2..95aeedbd857ca 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 @@ -385,7 +385,7 @@ export const InnerVisualizationWrapper = ({ [dispatch] ); - if (localState.configurationValidationError) { + if (localState.configurationValidationError?.length) { let showExtraErrors = null; if (localState.configurationValidationError.length > 1) { if (localState.expandError) { @@ -445,7 +445,7 @@ export const InnerVisualizationWrapper = ({ ); } - if (localState.expressionBuildError) { + if (localState.expressionBuildError?.length) { return ( 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 cd196745f3315..e5c05a1cf8c7a 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 @@ -419,7 +419,7 @@ export function DimensionEditor(props: DimensionEditorProps) { function getErrorMessage( selectedColumn: IndexPatternColumn | undefined, incompatibleSelectedOperationType: boolean, - input: 'none' | 'field' | undefined, + input: 'none' | 'field' | 'fullReference' | undefined, fieldInvalid: boolean ) { if (selectedColumn && incompatibleSelectedOperationType) { 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 b2edc61a56736..2e57ecee86033 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 @@ -1054,6 +1054,7 @@ describe('IndexPatternDimensionEditorPanel', () => { indexPatternId: '1', columns: {}, columnOrder: [], + incompleteColumns: {}, }, }, }); 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 31fb5277d53ec..817fdf637f001 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 @@ -21,6 +21,8 @@ type Props = Pick< 'layerId' | 'columnId' | 'state' | 'filterOperations' >; +// TODO: the support matrix should be available outside of the dimension panel + // TODO: This code has historically been memoized, as a potentially performance // sensitive task. If we can add memoization without breaking the behavior, we should. export const getOperationSupportMatrix = (props: Props): OperationSupportMatrix => { 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 51d95245adb25..3cf9bdc3a92f1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -13,9 +13,15 @@ import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; import { Ast } from '@kbn/interpreter/common'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; import { getFieldByNameFactory } from './pure_helpers'; +import { + operationDefinitionMap, + getErrorMessages, + createMockedReferenceOperation, +} from './operations'; jest.mock('./loader'); jest.mock('../id_generator'); +jest.mock('./operations'); const fieldsOne = [ { @@ -489,6 +495,56 @@ describe('IndexPattern Data Source', () => { expect(ast.chain[0].arguments.timeFields).toEqual(['timestamp']); expect(ast.chain[0].arguments.timeFields).not.toContain('timefield'); }); + + describe('references', () => { + beforeEach(() => { + // @ts-expect-error we are inserting an invalid type + operationDefinitionMap.testReference = createMockedReferenceOperation(); + + // @ts-expect-error we are inserting an invalid type + operationDefinitionMap.testReference.toExpression.mockReturnValue(['mock']); + }); + + afterEach(() => { + delete operationDefinitionMap.testReference; + }); + + it('should collect expression references and append them', async () => { + const queryBaseState: IndexPatternBaseState = { + currentIndexPatternId: '1', + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Count of records', + dataType: 'date', + isBucketed: false, + sourceField: 'timefield', + operationType: 'cardinality', + }, + col2: { + label: 'Reference', + dataType: 'number', + isBucketed: false, + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }, + }, + }; + + const state = enrichBaseState(queryBaseState); + + const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; + // @ts-expect-error we can't isolate just the reference type + expect(operationDefinitionMap.testReference.toExpression).toHaveBeenCalled(); + expect(ast.chain[2]).toEqual('mock'); + }); + }); }); describe('#insertLayer', () => { @@ -599,11 +655,33 @@ describe('IndexPattern Data Source', () => { describe('getTableSpec', () => { it('should include col1', () => { - expect(publicAPI.getTableSpec()).toEqual([ - { - columnId: 'col1', + expect(publicAPI.getTableSpec()).toEqual([{ columnId: 'col1' }]); + }); + + it('should skip columns that are being referenced', () => { + publicAPI = indexPatternDatasource.getPublicAPI({ + state: { + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + // @ts-ignore this is too little information for a real column + col1: { + dataType: 'number', + }, + col2: { + // @ts-expect-error update once we have a reference operation outside tests + references: ['col1'], + }, + }, + }, + }, }, - ]); + layerId: 'first', + }); + + expect(publicAPI.getTableSpec()).toEqual([{ columnId: 'col2' }]); }); }); @@ -764,7 +842,7 @@ describe('IndexPattern Data Source', () => { dataType: 'number', isBucketed: false, label: 'Foo', - operationType: 'document', + operationType: 'avg', sourceField: 'bytes', }, }, @@ -774,7 +852,7 @@ describe('IndexPattern Data Source', () => { }; expect( indexPatternDatasource.getErrorMessages(state as IndexPatternPrivateState) - ).not.toBeDefined(); + ).toBeUndefined(); }); it('should return no errors with layers with no columns', () => { @@ -792,7 +870,31 @@ describe('IndexPattern Data Source', () => { }, currentIndexPatternId: '1', }; - expect(indexPatternDatasource.getErrorMessages(state)).not.toBeDefined(); + expect(indexPatternDatasource.getErrorMessages(state)).toBeUndefined(); + }); + + it('should bubble up invalid configuration from operations', () => { + (getErrorMessages as jest.Mock).mockClear(); + (getErrorMessages as jest.Mock).mockReturnValueOnce(['error 1', 'error 2']); + const state: IndexPatternPrivateState = { + indexPatternRefs: [], + existingFields: {}, + isFirstExistenceFetch: false, + indexPatterns: expectedIndexPatterns, + layers: { + first: { + indexPatternId: '1', + columnOrder: [], + columns: {}, + }, + }, + currentIndexPatternId: '1', + }; + expect(indexPatternDatasource.getErrorMessages(state)).toEqual([ + { shortMessage: 'error 1', longMessage: '' }, + { shortMessage: 'error 2', longMessage: '' }, + ]); + 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 94f240058d618..2c64431867df0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -40,13 +40,13 @@ import { } from './indexpattern_suggestions'; import { - getInvalidFieldReferencesForLayer, - getInvalidReferences, + getInvalidFieldsForLayer, + getInvalidLayers, isDraggedField, normalizeOperationDataType, } from './utils'; import { LayerPanel } from './layerpanel'; -import { IndexPatternColumn } from './operations'; +import { IndexPatternColumn, getErrorMessages } from './operations'; import { IndexPatternField, IndexPatternPrivateState, IndexPatternPersistedState } from './types'; import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public'; import { DataPublicPluginStart } from '../../../../../src/plugins/data/public'; @@ -54,7 +54,7 @@ import { VisualizeFieldContext } from '../../../../../src/plugins/ui_actions/pub import { mergeLayer } from './state_helpers'; import { Datasource, StateSetter } from '../index'; import { ChartsPluginSetup } from '../../../../../src/plugins/charts/public'; -import { deleteColumn } from './operations'; +import { deleteColumn, isReferenced } from './operations'; import { FieldBasedIndexPatternColumn } from './operations/definitions/column_types'; import { Dragging } from '../drag_drop/providers'; @@ -325,7 +325,9 @@ export function getIndexPatternDatasource({ datasourceId: 'indexpattern', getTableSpec: () => { - return state.layers[layerId].columnOrder.map((colId) => ({ columnId: colId })); + return state.layers[layerId].columnOrder + .filter((colId) => !isReferenced(state.layers[layerId], colId)) + .map((colId) => ({ columnId: colId })); }, getOperationForColumnId: (columnId: string) => { const layer = state.layers[layerId]; @@ -349,10 +351,17 @@ export function getIndexPatternDatasource({ if (!state) { return; } - const invalidLayers = getInvalidReferences(state); + const invalidLayers = getInvalidLayers(state); + + const layerErrors = Object.values(state.layers).flatMap((layer) => + (getErrorMessages(layer) ?? []).map((message) => ({ + shortMessage: message, + longMessage: '', + })) + ); if (invalidLayers.length === 0) { - return; + return layerErrors.length ? layerErrors : undefined; } const realIndex = Object.values(state.layers) @@ -363,64 +372,69 @@ export function getIndexPatternDatasource({ } }) .filter(Boolean) as Array<[number, number]>; - const invalidFieldsPerLayer: string[][] = getInvalidFieldReferencesForLayer( + const invalidFieldsPerLayer: string[][] = getInvalidFieldsForLayer( invalidLayers, state.indexPatterns ); const originalLayersList = Object.keys(state.layers); - return realIndex.map(([filteredIndex, layerIndex]) => { - const fieldsWithBrokenReferences: string[] = invalidFieldsPerLayer[filteredIndex].map( - (columnId) => { - const column = invalidLayers[filteredIndex].columns[ - columnId - ] as FieldBasedIndexPatternColumn; - return column.sourceField; - } - ); - - if (originalLayersList.length === 1) { - return { - shortMessage: i18n.translate( - 'xpack.lens.indexPattern.dataReferenceFailureShortSingleLayer', - { - defaultMessage: 'Invalid {fields, plural, one {reference} other {references}}.', + if (layerErrors.length || realIndex.length) { + return [ + ...layerErrors, + ...realIndex.map(([filteredIndex, layerIndex]) => { + const fieldsWithBrokenReferences: string[] = invalidFieldsPerLayer[filteredIndex].map( + (columnId) => { + const column = invalidLayers[filteredIndex].columns[ + columnId + ] as FieldBasedIndexPatternColumn; + return column.sourceField; + } + ); + + if (originalLayersList.length === 1) { + return { + shortMessage: i18n.translate( + 'xpack.lens.indexPattern.dataReferenceFailureShortSingleLayer', + { + defaultMessage: 'Invalid {fields, plural, one {reference} other {references}}.', + values: { + fields: fieldsWithBrokenReferences.length, + }, + } + ), + longMessage: i18n.translate( + 'xpack.lens.indexPattern.dataReferenceFailureLongSingleLayer', + { + defaultMessage: `{fieldsLength, plural, one {Field} other {Fields}} "{fields}" {fieldsLength, plural, one {has an} other {have}} invalid reference.`, + values: { + fields: fieldsWithBrokenReferences.join('", "'), + fieldsLength: fieldsWithBrokenReferences.length, + }, + } + ), + }; + } + return { + shortMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureShort', { + defaultMessage: + 'Invalid {fieldsLength, plural, one {reference} other {references}} on Layer {layer}.', values: { - fields: fieldsWithBrokenReferences.length, + layer: layerIndex, + fieldsLength: fieldsWithBrokenReferences.length, }, - } - ), - longMessage: i18n.translate( - 'xpack.lens.indexPattern.dataReferenceFailureLongSingleLayer', - { - defaultMessage: `{fieldsLength, plural, one {Field} other {Fields}} "{fields}" {fieldsLength, plural, one {has an} other {have}} invalid reference.`, + }), + longMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureLong', { + defaultMessage: `Layer {layer} has {fieldsLength, plural, one {an invalid} other {invalid}} {fieldsLength, plural, one {reference} other {references}} in {fieldsLength, plural, one {field} other {fields}} "{fields}".`, values: { + layer: layerIndex, fields: fieldsWithBrokenReferences.join('", "'), fieldsLength: fieldsWithBrokenReferences.length, }, - } - ), - }; - } - return { - shortMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureShort', { - defaultMessage: - 'Invalid {fieldsLength, plural, one {reference} other {references}} on Layer {layer}.', - values: { - layer: layerIndex, - fieldsLength: fieldsWithBrokenReferences.length, - }, + }), + }; }), - longMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureLong', { - defaultMessage: `Layer {layer} has {fieldsLength, plural, one {an invalid} other {invalid}} {fieldsLength, plural, one {reference} other {references}} in {fieldsLength, plural, one {field} other {fields}} "{fields}".`, - values: { - layer: layerIndex, - fields: fieldsWithBrokenReferences.join('", "'), - fieldsLength: fieldsWithBrokenReferences.length, - }, - }), - }; - }); + ]; + } }, }; 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 ccdefee62ad5c..263b4646c9feb 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -18,7 +18,7 @@ import { IndexPatternColumn, OperationType, } from './operations'; -import { hasField, hasInvalidReference } from './utils'; +import { hasField, hasInvalidFields } from './utils'; import { IndexPattern, IndexPatternPrivateState, @@ -90,7 +90,7 @@ export function getDatasourceSuggestionsForField( indexPatternId: string, field: IndexPatternField ): IndexPatternSugestion[] { - if (hasInvalidReference(state)) return []; + if (hasInvalidFields(state)) return []; const layers = Object.keys(state.layers); const layerIds = layers.filter((id) => state.layers[id].indexPatternId === indexPatternId); @@ -331,7 +331,7 @@ function createNewLayerWithMetricAggregation( export function getDatasourceSuggestionsFromCurrentState( state: IndexPatternPrivateState ): Array> { - if (hasInvalidReference(state)) return []; + if (hasInvalidFields(state)) 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/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts index 2c6f42668d863..d0cbcee61db6f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts @@ -6,7 +6,7 @@ import { DragContextState } from '../drag_drop'; import { getFieldByNameFactory } from './pure_helpers'; -import { IndexPattern } from './types'; +import type { IndexPattern } from './types'; export const createMockedIndexPattern = (): IndexPattern => { const fields = [ 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 72dfe85dfc0e9..f27fb8d4642f6 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 @@ -6,12 +6,14 @@ const actualOperations = jest.requireActual('../operations'); const actualHelpers = jest.requireActual('../layer_helpers'); +const actualMocks = jest.requireActual('../mocks'); jest.spyOn(actualOperations.operationDefinitionMap.date_histogram, 'paramEditor'); jest.spyOn(actualOperations.operationDefinitionMap.terms, 'onOtherColumnChanged'); jest.spyOn(actualHelpers, 'insertOrReplaceColumn'); jest.spyOn(actualHelpers, 'insertNewColumn'); jest.spyOn(actualHelpers, 'replaceColumn'); +jest.spyOn(actualHelpers, 'getErrorMessages'); export const { getAvailableOperationsByMetadata, @@ -35,4 +37,8 @@ export const { updateLayerIndexPattern, mergeLayer, isColumnTransferable, + getErrorMessages, + isReferenced, } = actualHelpers; + +export const { createMockedReferenceOperation } = actualMocks; 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 bd8c4b4683396..fd3ca4319669e 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 @@ -52,6 +52,8 @@ export const cardinalityOperation: OperationDefinition + 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/column_types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts index bd4b452a49e1d..13bddc0c2ec26 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts @@ -4,13 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Operation } from '../../../types'; +import type { Operation } from '../../../types'; -/** - * This is the root type of a column. If you are implementing a new - * operation, extend your column type on `BaseIndexPatternColumn` to make - * sure it's matching all the basic requirements. - */ export interface BaseIndexPatternColumn extends Operation { // Private operationType: string; @@ -18,7 +13,8 @@ export interface BaseIndexPatternColumn extends Operation { } // Formatting can optionally be added to any column -export interface FormattedIndexPatternColumn extends BaseIndexPatternColumn { +// export interface FormattedIndexPatternColumn extends BaseIndexPatternColumn { +export type FormattedIndexPatternColumn = BaseIndexPatternColumn & { params?: { format: { id: string; @@ -27,8 +23,20 @@ export interface FormattedIndexPatternColumn extends BaseIndexPatternColumn { }; }; }; -} +}; export interface FieldBasedIndexPatternColumn extends BaseIndexPatternColumn { sourceField: string; } + +export interface ReferenceBasedIndexPatternColumn + extends BaseIndexPatternColumn, + FormattedIndexPatternColumn { + references: string[]; +} + +// Used to store the temporary invalid state +export interface IncompleteColumn { + operationType?: string; + sourceField?: string; +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index e33fc681b2579..30f64929fc1af 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -41,6 +41,7 @@ export const countOperation: OperationDefinition countLabel, buildColumn({ field, previousColumn }) { return { label: countLabel, 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 659390a42f261..efac9c151a435 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 @@ -59,6 +59,8 @@ export const dateHistogramOperation: OperationDefinition< }; } }, + getDefaultLabel: (column, indexPattern) => + indexPattern.getFieldByName(column.sourceField)!.displayName, buildColumn({ field }) { let interval = autoInterval; let timeZone: string | undefined; 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 522e951bfba34..1b0452d18a79c 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 @@ -75,6 +75,7 @@ export const filtersOperation: OperationDefinition true, + getDefaultLabel: () => filtersLabel, buildColumn({ previousColumn }) { let params = { filters: [defaultFilter] }; if (previousColumn?.operationType === 'terms') { 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 5c067ebaf21e9..564a4ca340b40 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 @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { ExpressionFunctionAST } from '@kbn/interpreter/common'; import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { termsOperation, TermsIndexPatternColumn } from './terms'; @@ -24,8 +25,13 @@ import { import { dateHistogramOperation, DateHistogramIndexPatternColumn } from './date_histogram'; import { countOperation, CountIndexPatternColumn } from './count'; import { StateSetter, OperationMetadata } from '../../../types'; -import { BaseIndexPatternColumn } from './column_types'; -import { IndexPatternPrivateState, IndexPattern, IndexPatternField } from '../../types'; +import type { BaseIndexPatternColumn, ReferenceBasedIndexPatternColumn } from './column_types'; +import { + IndexPatternPrivateState, + IndexPattern, + IndexPatternField, + IndexPatternLayer, +} from '../../types'; import { DateRange } from '../../../../common'; import { DataPublicPluginStart } from '../../../../../../../src/plugins/data/public'; import { RangeIndexPatternColumn, rangeOperation } from './ranges'; @@ -50,6 +56,8 @@ export type IndexPatternColumn = export type FieldBasedIndexPatternColumn = Extract; +export { IncompleteColumn } from './column_types'; + // List of all operation definitions registered to this data source. // If you want to implement a new operation, add the definition to this array and // the column type to the `IndexPatternColumn` union type below. @@ -104,6 +112,14 @@ interface BaseOperationDefinitionProps { * Should be i18n-ified. */ displayName: string; + /** + * The default label is assigned by the editor + */ + getDefaultLabel: ( + column: C, + indexPattern: IndexPattern, + columns: Record + ) => string; /** * This function is called if another column in the same layer changed or got removed. * Can be used to update references to other columns (e.g. for sorting). @@ -118,11 +134,6 @@ interface BaseOperationDefinitionProps { * React component for operation specific settings shown in the popover editor */ paramEditor?: React.ComponentType>; - /** - * Function turning a column into an agg config passed to the `esaggs` function - * together with the agg configs returned from other columns. - */ - toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; /** * Returns true if the `column` can also be used on `newIndexPattern`. * If this function returns false, the column is removed when switching index pattern @@ -138,7 +149,7 @@ interface BaseOperationDefinitionProps { } interface BaseBuildColumnArgs { - columns: Partial>; + columns: Record; indexPattern: IndexPattern; } @@ -156,7 +167,12 @@ interface FieldlessOperationDefinition { * Returns the meta data of the operation if applied. Undefined * if the field is not applicable. */ - getPossibleOperation: () => OperationMetadata | undefined; + getPossibleOperation: () => OperationMetadata; + /** + * Function turning a column into an agg config passed to the `esaggs` function + * together with the agg configs returned from other columns. + */ + toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; } interface FieldBasedOperationDefinition { @@ -167,7 +183,7 @@ interface FieldBasedOperationDefinition { */ getPossibleOperationForField: (field: IndexPatternField) => OperationMetadata | undefined; /** - * Builds the column object for the given parameters. Should include default p + * Builds the column object for the given parameters. */ buildColumn: ( arg: BaseBuildColumnArgs & { @@ -191,11 +207,77 @@ interface FieldBasedOperationDefinition { * @param field The field that the user changed to. */ onFieldChange: (oldColumn: C, field: IndexPatternField) => C; + /** + * Function turning a column into an agg config passed to the `esaggs` function + * together with the agg configs returned from other columns. + */ + toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; +} + +export interface RequiredReference { + // Limit the input types, usually used to prevent other references from being used + input: Array; + // Function which is used to determine if the reference is bucketed, or if it's a number + validateMetadata: (metadata: OperationMetadata) => boolean; + // Do not use specificOperations unless you need to limit to only one or two exact + // operation types. The main use case is Cumulative Sum, where we need to only take the + // sum of Count or sum of Sum. + specificOperations?: OperationType[]; +} + +// Full reference uses one or more reference operations which are visible to the user +// Partial reference is similar except that it uses the field selector +interface FullReferenceOperationDefinition { + input: 'fullReference'; + /** + * The filters provided here are used to construct the UI, transition correctly + * between operations, and validate the configuration. + */ + requiredReferences: RequiredReference[]; + + /** + * The type of UI that is shown in the editor for this function: + * - full: List of sub-functions and fields + * - field: List of fields, selects first operation per field + */ + selectionStyle: 'full' | 'field'; + + /** + * Builds the column object for the given parameters. Should include default p + */ + buildColumn: ( + arg: BaseBuildColumnArgs & { + columnOrder: string[]; + referenceIds: string[]; + previousColumn?: IndexPatternColumn; + } + ) => ReferenceBasedIndexPatternColumn & C; + /** + * Returns the meta data of the operation if applied. Undefined + * if the field is not applicable. + */ + getPossibleOperation: () => OperationMetadata; + /** + * A chain of expression functions which will transform the table + */ + toExpression: ( + layer: IndexPatternLayer, + columnId: string, + indexPattern: IndexPattern + ) => ExpressionFunctionAST[]; + /** + * Validate that the operation has the right preconditions in the state. For example: + * + * - Requires a date histogram operation somewhere before it in order + * - Missing references + */ + getErrorMessage?: (layer: IndexPatternLayer, columnId: string) => string[] | undefined; } interface OperationDefinitionMap { field: FieldBasedOperationDefinition; none: FieldlessOperationDefinition; + fullReference: FullReferenceOperationDefinition; } /** @@ -220,7 +302,8 @@ export type OperationType = typeof internalOperationDefinitions[number]['type']; */ export type GenericOperationDefinition = | OperationDefinition - | OperationDefinition; + | OperationDefinition + | OperationDefinition; /** * List of all available operation definitions 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 37a7ef8ee2563..96df72ba8b7c1 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 @@ -52,6 +52,8 @@ function buildMetricOperation>({ (!newField.aggregationRestrictions || newField.aggregationRestrictions![type]) ); }, + getDefaultLabel: (column, indexPattern, columns) => + ofName(indexPattern.getFieldByName(column.sourceField)!.displayName), buildColumn: ({ field, previousColumn }) => ({ label: ofName(field.displayName), 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 b1cb2312d5bb8..d2456e1c8d375 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 @@ -122,9 +122,11 @@ export const rangeOperation: OperationDefinition + indexPattern.getFieldByName(column.sourceField)!.displayName, buildColumn({ field }) { return { - label: field.name, + label: field.displayName, dataType: 'number', // string for Range operationType: 'range', sourceField: field.name, 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 ddc473a5c588d..e11329b8e50a9 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 @@ -17,7 +17,7 @@ import { EuiText, } from '@elastic/eui'; import { IndexPatternColumn } from '../../../indexpattern'; -import { updateColumnParam } from '../../layer_helpers'; +import { updateColumnParam, isReferenced } from '../../layer_helpers'; import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; @@ -84,7 +84,20 @@ export const termsOperation: OperationDefinition column && isSortableByColumn(column)) + .filter( + ([columnId, column]) => + column && + !column.isBucketed && + !isReferenced( + // Fake layer, but works with real columns + { + columns, + columnOrder: [], + indexPatternId: '', + }, + columnId + ) + ) .map(([id]) => id)[0]; const previousBucketsLength = Object.values(columns).filter((col) => col && col.isBucketed) @@ -131,6 +144,8 @@ export const termsOperation: OperationDefinition + 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/operations/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts index f0e02c7ff0faf..3ad9a1e5b3674 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts @@ -6,4 +6,11 @@ export * from './operations'; export * from './layer_helpers'; -export { OperationType, IndexPatternColumn, FieldBasedIndexPatternColumn } from './definitions'; +export { + OperationType, + IndexPatternColumn, + FieldBasedIndexPatternColumn, + IncompleteColumn, +} 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 e1a31dc274837..f983a6a334d53 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 @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import type { OperationMetadata } from '../../types'; import { insertNewColumn, replaceColumn, @@ -11,16 +12,20 @@ import { getColumnOrder, deleteColumn, updateLayerIndexPattern, + getErrorMessages, } from './layer_helpers'; import { operationDefinitionMap, OperationType } from '../operations'; import { TermsIndexPatternColumn } from './definitions/terms'; import { DateHistogramIndexPatternColumn } from './definitions/date_histogram'; import { AvgIndexPatternColumn } from './definitions/metrics'; -import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from '../types'; +import type { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from '../types'; import { documentField } from '../document_field'; import { getFieldByNameFactory } from '../pure_helpers'; +import { generateId } from '../../id_generator'; +import { createMockedReferenceOperation } from './mocks'; jest.mock('../operations'); +jest.mock('../../id_generator'); const indexPatternFields = [ { @@ -74,10 +79,22 @@ const indexPattern = { timeFieldName: 'timestamp', hasRestrictions: false, fields: indexPatternFields, - getFieldByName: getFieldByNameFactory(indexPatternFields), + getFieldByName: getFieldByNameFactory([...indexPatternFields, documentField]), }; describe('state_helpers', () => { + beforeEach(() => { + let count = 0; + (generateId as jest.Mock).mockImplementation(() => `id${++count}`); + + // @ts-expect-error we are inserting an invalid type + operationDefinitionMap.testReference = createMockedReferenceOperation(); + }); + + afterEach(() => { + delete operationDefinitionMap.testReference; + }); + describe('insertNewColumn', () => { it('should throw for invalid operations', () => { expect(() => { @@ -315,6 +332,110 @@ describe('state_helpers', () => { }) ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2', 'col3'] })); }); + + describe('inserting a new reference', () => { + it('should throw if the required references are impossible to match', () => { + // @ts-expect-error this function is not valid + operationDefinitionMap.testReference.requiredReferences = [ + { + input: ['none', 'field'], + validateMetadata: () => false, + specificOperations: [], + }, + ]; + const layer: IndexPatternLayer = { indexPatternId: '1', columnOrder: [], columns: {} }; + expect(() => { + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'testReference' as OperationType, + }); + }).toThrow(); + }); + + it('should leave the references empty if too ambiguous', () => { + const layer: IndexPatternLayer = { indexPatternId: '1', columnOrder: [], columns: {} }; + const result = insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'testReference' as OperationType, + }); + + expect(operationDefinitionMap.testReference.buildColumn).toHaveBeenCalledWith( + expect.objectContaining({ + referenceIds: ['id1'], + }) + ); + expect(result).toEqual( + expect.objectContaining({ + columns: { + col2: expect.objectContaining({ references: ['id1'] }), + }, + }) + ); + }); + + it('should create an operation if there is exactly one possible match', () => { + // 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 layer: IndexPatternLayer = { indexPatternId: '1', columnOrder: [], columns: {} }; + const result = insertNewColumn({ + layer, + indexPattern, + columnId: 'col1', + // @ts-expect-error invalid type + op: 'testReference', + }); + expect(result.columnOrder).toEqual(['id1', 'col1']); + expect(result.columns).toEqual( + expect.objectContaining({ + id1: expect.objectContaining({ operationType: 'filters' }), + col1: expect.objectContaining({ references: ['id1'] }), + }) + ); + }); + + it('should create a referenced column if the ID is being used as a reference', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + dataType: 'number', + isBucketed: false, + + // @ts-expect-error only in test + operationType: 'testReference', + references: ['ref1'], + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'ref1', + op: 'count', + field: documentField, + }) + ).toEqual( + expect.objectContaining({ + columns: { + col1: expect.objectContaining({ references: ['ref1'] }), + ref1: expect.objectContaining({}), + }, + }) + ); + }); + }); }); describe('replaceColumn', () => { @@ -655,10 +776,301 @@ describe('state_helpers', () => { }), }); }); + + it('should not wrap the previous operation when switching to reference', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Count', + customLabel: true, + dataType: 'number' as const, + isBucketed: false, + sourceField: 'Records', + operationType: 'count' as const, + }, + }, + }; + const result = replaceColumn({ + layer, + indexPattern, + columnId: 'col1', + op: 'testReference' as OperationType, + }); + + expect(operationDefinitionMap.testReference.buildColumn).toHaveBeenCalledWith( + expect.objectContaining({ + referenceIds: ['id1'], + }) + ); + expect(result.columns).toEqual( + expect.objectContaining({ + col1: expect.objectContaining({ operationType: 'testReference' }), + }) + ); + }); + + it('should delete the previous references and reset to default values when going from reference to no-input', () => { + // @ts-expect-error this function is not valid + operationDefinitionMap.testReference.requiredReferences = [ + { + input: ['none'], + validateMetadata: () => true, + }, + ]; + const expectedCol = { + dataType: 'string' as const, + isBucketed: true, + + operationType: 'filters' as const, + params: { + // These filters are reset + filters: [{ input: { query: 'field: true', language: 'kuery' }, label: 'Custom label' }], + }, + }; + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + ...expectedCol, + label: 'Custom label', + customLabel: true, + }, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + expect( + replaceColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'filters', + }) + ).toEqual( + expect.objectContaining({ + columnOrder: ['col2'], + columns: { + col2: { + ...expectedCol, + label: 'Filters', + scale: 'ordinal', // added in buildColumn + params: { + filters: [{ input: { query: '', language: 'kuery' }, label: '' }], + }, + }, + }, + }) + ); + }); + + it('should delete the inner references when switching away from reference to field-based operation', () => { + const expectedCol = { + label: 'Count of records', + dataType: 'number' as const, + isBucketed: false, + + operationType: 'count' as const, + sourceField: 'Records', + }; + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: expectedCol, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + expect( + replaceColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'count', + field: documentField, + }) + ).toEqual( + expect.objectContaining({ + columnOrder: ['col2'], + columns: { + col2: expect.objectContaining(expectedCol), + }, + }) + ); + }); + + it('should reset when switching from one reference to another', () => { + operationDefinitionMap.secondTest = { + input: 'fullReference', + displayName: 'Reference test 2', + // @ts-expect-error this type is not statically available + type: 'secondTest', + requiredReferences: [ + { + // Any numeric metric that isn't also a reference + input: ['none', 'field'], + validateMetadata: (meta: OperationMetadata) => + meta.dataType === 'number' && !meta.isBucketed, + }, + ], + // @ts-expect-error don't want to define valid arguments + buildColumn: jest.fn((args) => { + return { + label: 'Test reference', + isBucketed: false, + dataType: 'number', + + operationType: 'secondTest', + references: args.referenceIds, + }; + }), + isTransferable: jest.fn(), + toExpression: jest.fn().mockReturnValue([]), + getPossibleOperation: jest.fn().mockReturnValue({ dataType: 'number', isBucketed: false }), + }; + + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Count', + customLabel: true, + dataType: 'number' as const, + isBucketed: false, + + operationType: 'count' as const, + sourceField: 'Records', + }, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + expect( + replaceColumn({ + layer, + indexPattern, + columnId: 'col2', + // @ts-expect-error not statically available + op: 'secondTest', + }) + ).toEqual( + expect.objectContaining({ + columnOrder: ['col2'], + columns: { + col2: expect.objectContaining({ references: ['id1'] }), + }, + incompleteColumns: {}, + }) + ); + + delete operationDefinitionMap.secondTest; + }); + + it('should allow making a replacement on an operation that is being referenced, even if it ends up invalid', () => { + // @ts-expect-error this function is not valid + operationDefinitionMap.testReference.requiredReferences = [ + { + input: ['field'], + validateMetadata: (meta: OperationMetadata) => meta.dataType === 'number', + specificOperations: ['sum'], + }, + ]; + + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Asdf', + customLabel: true, + dataType: 'number' as const, + isBucketed: false, + + operationType: 'sum' as const, + sourceField: 'bytes', + }, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + expect( + replaceColumn({ + layer, + indexPattern, + columnId: 'col1', + op: 'count', + field: documentField, + }) + ).toEqual( + expect.objectContaining({ + columnOrder: ['col1', 'col2'], + columns: { + col1: expect.objectContaining({ + sourceField: 'Records', + operationType: 'count', + }), + col2: expect.objectContaining({ references: ['col1'] }), + }, + }) + ); + }); }); describe('deleteColumn', () => { - it('should remove column', () => { + it('should clear incomplete columns when column is already empty', () => { + expect( + deleteColumn({ + layer: { + indexPatternId: '1', + columnOrder: [], + columns: {}, + incompleteColumns: { + col1: { sourceField: 'test' }, + }, + }, + columnId: 'col1', + }) + ).toEqual({ + indexPatternId: '1', + columnOrder: [], + columns: {}, + incompleteColumns: {}, + }); + }); + + it('should remove column and any incomplete state', () => { const termsColumn: TermsIndexPatternColumn = { label: 'Top values of source', dataType: 'string', @@ -682,25 +1094,33 @@ describe('state_helpers', () => { columns: { col1: termsColumn, col2: { - label: 'Count', + label: 'Count of records', dataType: 'number', isBucketed: false, sourceField: 'Records', operationType: 'count', }, }, + incompleteColumns: { + col2: { sourceField: 'other' }, + }, }, columnId: 'col2', - }).columns + }) ).toEqual({ - col1: { - ...termsColumn, - params: { - ...termsColumn.params, - orderBy: { type: 'alphabetical' }, - orderDirection: 'asc', + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + ...termsColumn, + params: { + ...termsColumn.params, + orderBy: { type: 'alphabetical' }, + orderDirection: 'asc', + }, }, }, + incompleteColumns: {}, }); }); @@ -742,6 +1162,73 @@ describe('state_helpers', () => { col1: termsColumn, }); }); + + it('should delete the column and all of its references', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + + operationType: 'count', + sourceField: 'Records', + }, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + }, + }; + expect(deleteColumn({ layer, columnId: 'col2' })).toEqual( + expect.objectContaining({ columnOrder: [], columns: {} }) + ); + }); + + it('should recursively delete references', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2', 'col3'], + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + + operationType: 'count', + sourceField: 'Records', + }, + col2: { + label: 'Test reference', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col1'], + }, + col3: { + label: 'Test reference 2', + dataType: 'number', + isBucketed: false, + + // @ts-expect-error not a valid type + operationType: 'testReference', + references: ['col2'], + }, + }, + }; + expect(deleteColumn({ layer, columnId: 'col3' })).toEqual( + expect.objectContaining({ columnOrder: [], columns: {} }) + ); + }); }); describe('updateColumnParam', () => { @@ -1141,4 +1628,67 @@ describe('state_helpers', () => { }); }); }); + + describe('getErrorMessages', () => { + it('should collect errors from the operation definitions', () => { + const mock = jest.fn().mockReturnValue(['error 1']); + // @ts-expect-error not statically analyzed + operationDefinitionMap.testReference.getErrorMessage = mock; + const errors = getErrorMessages({ + indexPatternId: '1', + columnOrder: [], + columns: { + col1: + // @ts-expect-error not statically analyzed + { operationType: 'testReference', references: [] }, + }, + }); + expect(mock).toHaveBeenCalled(); + expect(errors).toHaveLength(1); + }); + + it('should identify missing references', () => { + 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 f071df1542147..2d532b36d0625 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,13 +5,15 @@ */ import _, { partition } from 'lodash'; +import { i18n } from '@kbn/i18n'; import { operationDefinitionMap, operationDefinitions, OperationType, IndexPatternColumn, + RequiredReference, } from './definitions'; -import { +import type { IndexPattern, IndexPatternField, IndexPatternLayer, @@ -19,6 +21,7 @@ import { } from '../types'; import { getSortScoreByPriority } from './operations'; import { mergeLayer } from '../state_helpers'; +import { generateId } from '../../id_generator'; interface ColumnChange { op: OperationType; @@ -54,13 +57,8 @@ export function insertNewColumn({ previousColumn: layer.columns[columnId], }; - // TODO: Reference based operations require more setup to create the references - if (operationDefinition.input === 'none') { const possibleOperation = operationDefinition.getPossibleOperation(); - if (!possibleOperation) { - throw new Error('Tried to create an invalid operation'); - } const isBucketed = Boolean(possibleOperation.isBucketed); if (isBucketed) { return addBucket(layer, operationDefinition.buildColumn(baseOptions), columnId); @@ -69,6 +67,80 @@ export function insertNewColumn({ } } + if (operationDefinition.input === 'fullReference') { + let tempLayer = { ...layer }; + const referenceIds = operationDefinition.requiredReferences.map((validation) => { + const validOperations = Object.values(operationDefinitionMap).filter(({ type }) => + isOperationAllowedAsReference({ validation, operationType: type }) + ); + + if (!validOperations.length) { + throw new Error( + `Can't create reference, ${op} has a validation function which doesn't allow any operations` + ); + } + + const newId = generateId(); + if (validOperations.length === 1) { + const def = validOperations[0]; + + const validFields = + def.input === 'field' ? indexPattern.fields.filter(def.getPossibleOperationForField) : []; + + if (def.input === 'none') { + tempLayer = insertNewColumn({ + layer: tempLayer, + columnId: newId, + op: def.type, + indexPattern, + }); + } else if (validFields.length === 1) { + // Recursively update the layer for each new reference + tempLayer = insertNewColumn({ + layer: tempLayer, + columnId: newId, + op: def.type, + indexPattern, + field: validFields[0], + }); + } else { + tempLayer = { + ...tempLayer, + incompleteColumns: { + ...tempLayer.incompleteColumns, + [newId]: { operationType: def.type }, + }, + }; + } + } + return newId; + }); + + const possibleOperation = operationDefinition.getPossibleOperation(); + const isBucketed = Boolean(possibleOperation.isBucketed); + if (isBucketed) { + return addBucket( + tempLayer, + operationDefinition.buildColumn({ + ...baseOptions, + columnOrder: layer.columnOrder, + referenceIds, + }), + columnId + ); + } else { + return addMetric( + tempLayer, + operationDefinition.buildColumn({ + ...baseOptions, + columnOrder: layer.columnOrder, + referenceIds, + }), + columnId + ); + } + } + if (!field) { throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); } @@ -99,8 +171,9 @@ export function replaceColumn({ throw new Error(`Can't replace column because there is no prior column`); } - const isNewOperation = Boolean(op) && op !== previousColumn.operationType; - const operationDefinition = operationDefinitionMap[op || previousColumn.operationType]; + const isNewOperation = op !== previousColumn.operationType; + const operationDefinition = operationDefinitionMap[op]; + const previousDefinition = operationDefinitionMap[previousColumn.operationType]; if (!operationDefinition) { throw new Error('No suitable operation found for given parameters'); @@ -113,22 +186,48 @@ export function replaceColumn({ }; if (isNewOperation) { - // TODO: Reference based operations require more setup to create the references + let tempLayer = { ...layer }; + + if (previousDefinition.input === 'fullReference') { + // @ts-expect-error references are not statically analyzed + previousColumn.references.forEach((id: string) => { + tempLayer = deleteColumn({ layer: tempLayer, columnId: id }); + }); + } + + if (operationDefinition.input === 'fullReference') { + const referenceIds = operationDefinition.requiredReferences.map(() => generateId()); + + const incompleteColumns = { ...(tempLayer.incompleteColumns || {}) }; + delete incompleteColumns[columnId]; + return { + ...tempLayer, + columns: { + ...tempLayer.columns, + [columnId]: operationDefinition.buildColumn({ + ...baseOptions, + columns: tempLayer.columns, + columnOrder: tempLayer.columnOrder, + referenceIds, + previousColumn, + }), + }, + incompleteColumns, + }; + } if (operationDefinition.input === 'none') { const newColumn = operationDefinition.buildColumn(baseOptions); - if (previousColumn.customLabel) { newColumn.customLabel = true; newColumn.label = previousColumn.label; } + const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { - ...layer, - columns: adjustColumnReferencesForChangedColumn( - { ...layer.columns, [columnId]: newColumn }, - columnId - ), + ...tempLayer, + columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), }; } @@ -143,10 +242,10 @@ export function replaceColumn({ newColumn.label = previousColumn.label; } - const newColumns = { ...layer.columns, [columnId]: newColumn }; + const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { - ...layer, - columnOrder: getColumnOrder({ ...layer, columns: newColumns }), + ...tempLayer, + columnOrder: getColumnOrder({ ...tempLayer, columns: newColumns }), columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), }; } else if ( @@ -294,14 +393,36 @@ export function deleteColumn({ layer: IndexPatternLayer; columnId: string; }): IndexPatternLayer { + const column = layer.columns[columnId]; + if (!column) { + const newIncomplete = { ...(layer.incompleteColumns || {}) }; + delete newIncomplete[columnId]; + return { + ...layer, + columnOrder: layer.columnOrder.filter((id) => id !== columnId), + incompleteColumns: newIncomplete, + }; + } + + // @ts-expect-error this fails statically because there are no references added + const extraDeletions: string[] = 'references' in column ? column.references : []; + const hypotheticalColumns = { ...layer.columns }; delete hypotheticalColumns[columnId]; - const newLayer = { + let newLayer = { ...layer, columns: adjustColumnReferencesForChangedColumn(hypotheticalColumns, columnId), }; - return { ...newLayer, columnOrder: getColumnOrder(newLayer) }; + + extraDeletions.forEach((id) => { + newLayer = deleteColumn({ layer: newLayer, columnId: id }); + }); + + const newIncomplete = { ...(newLayer.incompleteColumns || {}) }; + delete newIncomplete[columnId]; + + return { ...newLayer, columnOrder: getColumnOrder(newLayer), incompleteColumns: newIncomplete }; } export function getColumnOrder(layer: IndexPatternLayer): string[] { @@ -342,3 +463,116 @@ export function updateLayerIndexPattern( columnOrder: newColumnOrder, }; } + +/** + * Collects all errors from the columns in the layer, for display in the workspace. This includes: + * + * - All columns have complete references + * - All column references are valid + * - All prerequisites are met + */ +export function getErrorMessages(layer: IndexPatternLayer): string[] | undefined { + const errors: string[] = []; + + Object.entries(layer.columns).forEach(([columnId, column]) => { + const def = operationDefinitionMap[column.operationType]; + if (def.input === 'fullReference' && def.getErrorMessage) { + errors.push(...(def.getErrorMessage(layer, columnId) ?? [])); + } + + if ('references' in column) { + // @ts-expect-error references are not statically analyzed yet + column.references.forEach((referenceId, index) => { + if (!layer.columns[referenceId]) { + errors.push( + i18n.translate('xpack.lens.indexPattern.missingReferenceError', { + defaultMessage: 'Dimension {dimensionLabel} is incomplete', + values: { + // @ts-expect-error references are not statically analyzed yet + 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: { + // @ts-expect-error references are not statically analyzed yet + dimensionLabel: column.label, + }, + }) + ); + } + } + }); + } + }); + + return errors.length ? errors : undefined; +} + +export function isReferenced(layer: IndexPatternLayer, columnId: string): boolean { + const allReferences = Object.values(layer.columns).flatMap((col) => + 'references' in col + ? // @ts-expect-error not statically analyzed + col.references + : [] + ); + return allReferences.includes(columnId); +} + +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) + ); +} + +function isOperationAllowedAsReference({ + operationType, + validation, + field, +}: { + operationType: OperationType; + validation: RequiredReference; + field?: IndexPatternField; +}): boolean { + const operationDefinition = operationDefinitionMap[operationType]; + + let hasValidMetadata = true; + if (field && operationDefinition.input === 'field') { + const metadata = operationDefinition.getPossibleOperationForField(field); + hasValidMetadata = Boolean(metadata) && validation.validateMetadata(metadata!); + } else if (operationDefinition.input !== 'field') { + const metadata = operationDefinition.getPossibleOperation(); + hasValidMetadata = Boolean(metadata) && validation.validateMetadata(metadata!); + } else { + // TODO: How can we validate the metadata without a specific field? + } + return ( + validation.input.includes(operationDefinition.input) && + (!validation.specificOperations || validation.specificOperations.includes(operationType)) && + hasValidMetadata + ); +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts new file mode 100644 index 0000000000000..c3f7dac03ada3 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts @@ -0,0 +1,39 @@ +/* + * 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 type { OperationMetadata } from '../../types'; +import type { OperationType } from './definitions'; + +export const createMockedReferenceOperation = () => { + return { + input: 'fullReference', + displayName: 'Reference test', + type: 'testReference' as OperationType, + selectionStyle: 'full', + requiredReferences: [ + { + // Any numeric metric that isn't also a reference + input: ['none', 'field'], + validateMetadata: (meta: OperationMetadata) => + meta.dataType === 'number' && !meta.isBucketed, + }, + ], + buildColumn: jest.fn((args) => { + return { + label: 'Test reference', + isBucketed: false, + dataType: 'number', + + operationType: 'testReference', + references: args.referenceIds, + }; + }), + isTransferable: jest.fn(), + toExpression: jest.fn().mockReturnValue([]), + getPossibleOperation: jest.fn().mockReturnValue({ dataType: 'number', isBucketed: false }), + getDefaultLabel: jest.fn().mockReturnValue('Default label'), + }; +}; 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 8d489df366088..58685fa494a04 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts @@ -87,6 +87,10 @@ type OperationFieldTuple = | { type: 'none'; operationType: OperationType; + } + | { + type: 'fullReference'; + operationType: OperationType; }; /** @@ -162,6 +166,11 @@ export function getAvailableOperationsByMetadata(indexPattern: IndexPattern) { }, operationDefinition.getPossibleOperation() ); + } else if (operationDefinition.input === 'fullReference') { + addToMap( + { type: 'fullReference', operationType: operationDefinition.type }, + operationDefinition.getPossibleOperation() + ); } }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index ea7aa62054e5c..5b66d4aae77ab 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -7,32 +7,29 @@ import { Ast, ExpressionFunctionAST } from '@kbn/interpreter/common'; import { IndexPatternColumn } from './indexpattern'; import { operationDefinitionMap } from './operations'; -import { IndexPattern, IndexPatternPrivateState } from './types'; +import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types'; import { OriginalColumn } from './rename_columns'; import { dateHistogramOperation } from './operations/definitions'; -function getExpressionForLayer( - indexPattern: IndexPattern, - columns: Record, - columnOrder: string[] -): Ast | null { +function getExpressionForLayer(layer: IndexPatternLayer, indexPattern: IndexPattern): Ast | null { + const { columns, columnOrder } = layer; + if (columnOrder.length === 0) { return null; } - function getEsAggsConfig(column: C, columnId: string) { - return operationDefinitionMap[column.operationType].toEsAggsConfig( - column, - columnId, - indexPattern - ); - } - const columnEntries = columnOrder.map((colId) => [colId, columns[colId]] as const); if (columnEntries.length) { - const aggs = columnEntries.map(([colId, col]) => { - return getEsAggsConfig(col, colId); + const aggs: unknown[] = []; + const expressions: ExpressionFunctionAST[] = []; + columnEntries.forEach(([colId, col]) => { + const def = operationDefinitionMap[col.operationType]; + if (def.input === 'fullReference') { + expressions.push(...def.toExpression(layer, colId, indexPattern)); + } else { + aggs.push(def.toEsAggsConfig(col, colId, indexPattern)); + } }); const idMap = columnEntries.reduce((currentIdMap, [colId, column], index) => { @@ -119,6 +116,7 @@ function getExpressionForLayer( }, }, ...formatterOverrides, + ...expressions, ], }; } @@ -129,9 +127,8 @@ function getExpressionForLayer( export function toExpression(state: IndexPatternPrivateState, layerId: string) { if (state.layers[layerId]) { return getExpressionForLayer( - state.indexPatterns[state.layers[layerId].indexPatternId], - state.layers[layerId].columns, - state.layers[layerId].columnOrder + state.layers[layerId], + state.indexPatterns[state.layers[layerId].indexPatternId] ); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts index 1e6fc5a5806b5..e4958da471417 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts @@ -5,7 +5,7 @@ */ import { IFieldType } from 'src/plugins/data/common'; -import { IndexPatternColumn } from './operations'; +import { IndexPatternColumn, IncompleteColumn } from './operations'; import { IndexPatternAggRestrictions } from '../../../../../src/plugins/data/public'; export interface IndexPattern { @@ -35,6 +35,8 @@ export interface IndexPatternLayer { columns: Record; // Each layer is tied to the index pattern that created it indexPatternId: string; + // Partial columns represent the temporary invalid states + incompleteColumns?: Record; } export interface IndexPatternPersistedState { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index d0ea81d135156..01b834610eb1a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -42,11 +42,11 @@ export function isDraggedField(fieldCandidate: unknown): fieldCandidate is Dragg ); } -export function hasInvalidReference(state: IndexPatternPrivateState) { - return getInvalidReferences(state).length > 0; +export function hasInvalidFields(state: IndexPatternPrivateState) { + return getInvalidLayers(state).length > 0; } -export function getInvalidReferences(state: IndexPatternPrivateState) { +export function getInvalidLayers(state: IndexPatternPrivateState) { return Object.values(state.layers).filter((layer) => { return layer.columnOrder.some((columnId) => { const column = layer.columns[columnId]; @@ -62,7 +62,7 @@ export function getInvalidReferences(state: IndexPatternPrivateState) { }); } -export function getInvalidFieldReferencesForLayer( +export function getInvalidFieldsForLayer( layers: IndexPatternLayer[], indexPatternMap: Record ) { From a83c7276bf15be9f3121cd8dcebb03f3d0d96092 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 18 Nov 2020 15:28:05 +0100 Subject: [PATCH 02/10] start working on time scaling UI --- .../series_calculation_helpers.ts | 8 +- .../dimension_panel/dimension_editor.tsx | 11 ++ .../dimension_panel/time_scaling.tsx | 143 ++++++++++++++++++ .../operations/definitions/column_types.ts | 2 + .../operations/definitions/count.tsx | 1 + .../operations/definitions/index.ts | 9 ++ .../operations/definitions/metrics.tsx | 6 +- .../suffix_formatter.ts | 9 +- .../indexpattern_datasource/time_scale.ts | 3 +- .../indexpattern_datasource/to_expression.ts | 36 ++++- 10 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx diff --git a/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts b/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts index 99ad2098ab10a..82425b8f6509e 100644 --- a/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts +++ b/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts @@ -44,9 +44,13 @@ export function buildResultColumns( input: Datatable, outputColumnId: string, inputColumnId: string, - outputColumnName: string | undefined + outputColumnName: string | undefined, + options: { allowColumnOverride: boolean } = { allowColumnOverride: false } ) { - if (input.columns.some((column) => column.id === outputColumnId)) { + if ( + !options.allowColumnOverride && + input.columns.some((column) => column.id === outputColumnId) + ) { throw new Error( i18n.translate('expressions.functions.seriesCalculations.columnConflictMessage', { defaultMessage: 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 e5c05a1cf8c7a..9bb8f092c98c3 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 @@ -34,6 +34,7 @@ import { BucketNestingEditor } from './bucket_nesting_editor'; import { IndexPattern, IndexPatternLayer } from '../types'; import { trackUiEvent } from '../../lens_ui_telemetry'; import { FormatSelector } from './format_selector'; +import { TimeScaling } from './time_scaling'; const operationPanels = getOperationDisplay(); @@ -333,6 +334,16 @@ export function DimensionEditor(props: DimensionEditorProps) { ) : null} + {!currentFieldIsInvalid && !incompatibleSelectedOperationType && selectedColumn && ( + + )} + {!currentFieldIsInvalid && !incompatibleSelectedOperationType && selectedColumn && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx new file mode 100644 index 0000000000000..134fad0f523cb --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -0,0 +1,143 @@ +/* + * 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 { + EuiLink, + EuiFormRow, + EuiSelect, + EuiFlexItem, + EuiFlexGroup, + EuiButtonIcon, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import { StateSetter } from '../../types'; +import { IndexPatternColumn, operationDefinitionMap } from '../operations'; +import { mergeLayer } from '../state_helpers'; +import { unitSuffixesLong } from '../suffix_formatter'; +import { TimeScaleUnit } from '../time_scale'; +import { IndexPatternPrivateState } from '../types'; + +const DEFAULT_TIME_SCALE = 'm' as const; + +export function TimeScaling({ + selectedColumn, + columnId, + layerId, + state, + setState, +}: { + selectedColumn: IndexPatternColumn; + columnId: string; + layerId: string; + state: IndexPatternPrivateState; + setState: StateSetter; +}) { + const layer = state.layers[layerId]; + const hasDateHistogram = layer.columnOrder.some( + (colId) => layer.columns[colId].operationType === 'date_histogram' + ); + const selectedOperation = operationDefinitionMap[selectedColumn.operationType]; + if ( + !selectedOperation.timeScalingMode || + selectedOperation.timeScalingMode === 'disabled' || + !hasDateHistogram + ) { + return null; + } + + if (!selectedColumn.timeScale) { + return ( + { + setState( + mergeLayer({ + state, + layerId, + newLayer: { + ...state.layers[layerId], + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + timeScale: DEFAULT_TIME_SCALE, + }, + }, + }, + }) + ); + }} + > + {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { + defaultMessage: 'Show as rate', + })} + + ); + } + + return ( + + + + ({ + value: unit, + text, + }))} + value={selectedColumn.timeScale} + onChange={(e) => { + setState( + mergeLayer({ + state, + layerId, + newLayer: { + ...state.layers[layerId], + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + timeScale: e.target.value as TimeScaleUnit, + }, + }, + }, + }) + ); + }} + /> + + + {selectedOperation.timeScalingMode === 'optional' && ( + + { + setState( + mergeLayer({ + state, + layerId, + newLayer: { + ...state.layers[layerId], + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + timeScale: undefined, + }, + }, + }, + }) + ); + }} + iconType="cross" + /> + + )} + + ); +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts index 13bddc0c2ec26..4873fd729b75c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts @@ -5,11 +5,13 @@ */ import type { Operation } from '../../../types'; +import { TimeScaleUnit } from '../../time_scale'; export interface BaseIndexPatternColumn extends Operation { // Private operationType: string; customLabel?: boolean; + timeScale?: TimeScaleUnit; } // Formatting can optionally be added to any column diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index 30f64929fc1af..fae1fc53217e8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -69,4 +69,5 @@ export const countOperation: OperationDefinition { return true; }, + timeScalingMode: 'optional', }; 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 564a4ca340b40..c3241f83397c3 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 @@ -99,6 +99,8 @@ export interface ParamEditorProps { data: DataPublicPluginStart; } +export type TimeScalingMode = 'disabled' | 'mandatory' | 'optional'; + interface BaseOperationDefinitionProps { type: C['operationType']; /** @@ -146,6 +148,13 @@ interface BaseOperationDefinitionProps { * present on the new index pattern. */ transfer?: (column: C, newIndexPattern: IndexPattern) => C; + /** + * Flag whether this operation can be scaled by time unit if a date histogram is available. + * If set to mandatory or optional, a UI element is shown in the config flyout to configure the time unit + * to scale by. The chosen unit will be persisted as `timeScale` property of the column. + * If set to optional, time scaling won't be enabled by default and can be removed. + */ + timeScalingMode?: TimeScalingMode; } interface BaseBuildColumnArgs { 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 96df72ba8b7c1..5cbaa3914df78 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 @@ -5,7 +5,7 @@ */ import { i18n } from '@kbn/i18n'; -import { OperationDefinition } from './index'; +import { OperationDefinition, TimeScalingMode } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; type MetricColumn = FormattedIndexPatternColumn & @@ -18,17 +18,20 @@ function buildMetricOperation>({ displayName, ofName, priority, + timeScalingMode, }: { type: T['operationType']; displayName: string; ofName: (name: string) => string; priority?: number; + timeScalingMode?: TimeScalingMode; }) { return { type, priority, displayName, input: 'field', + timeScalingMode, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { if ( fieldType === 'number' && @@ -138,6 +141,7 @@ export const sumOperation = buildMetricOperation({ defaultMessage: 'Sum of {name}', values: { name }, }), + timeScalingMode: 'optional', }); export const medianOperation = buildMetricOperation({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/suffix_formatter.ts b/x-pack/plugins/lens/public/indexpattern_datasource/suffix_formatter.ts index 5594976738efe..f5d764acab086 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/suffix_formatter.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/suffix_formatter.ts @@ -10,12 +10,19 @@ import { FormatFactory } from '../types'; import { TimeScaleUnit } from './time_scale'; const unitSuffixes: Record = { - s: i18n.translate('xpack.lens.fieldFormats.suffix.s', { defaultMessage: '/h' }), + s: i18n.translate('xpack.lens.fieldFormats.suffix.s', { defaultMessage: '/s' }), m: i18n.translate('xpack.lens.fieldFormats.suffix.m', { defaultMessage: '/m' }), h: i18n.translate('xpack.lens.fieldFormats.suffix.h', { defaultMessage: '/h' }), d: i18n.translate('xpack.lens.fieldFormats.suffix.d', { defaultMessage: '/d' }), }; +export const unitSuffixesLong: Record = { + s: i18n.translate('xpack.lens.fieldFormats.longSuffix.s', { defaultMessage: 'per second' }), + m: i18n.translate('xpack.lens.fieldFormats.longSuffix.m', { defaultMessage: 'per minute' }), + h: i18n.translate('xpack.lens.fieldFormats.longSuffix.h', { defaultMessage: 'per hour' }), + d: i18n.translate('xpack.lens.fieldFormats.longSuffix.d', { defaultMessage: 'per day' }), +}; + export function getSuffixFormatter(formatFactory: FormatFactory) { return class SuffixFormatter extends FieldFormat { static id = 'suffix'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts b/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts index 06ff8058b1d09..16f8c24a9bfc9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts @@ -87,7 +87,8 @@ export function getTimeScaleFunction(data: DataPublicPluginStart) { input, outputColumnId, inputColumnId, - outputColumnName + outputColumnName, + { allowColumnOverride: true } ); if (!resultColumns) { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 5b66d4aae77ab..201ff406c44ca 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -59,7 +59,6 @@ function getExpressionForLayer(layer: IndexPatternLayer, indexPattern: IndexPatt } > >; - const columnsWithFormatters = columnEntries.filter( ([, col]) => col.params && @@ -87,6 +86,40 @@ function getExpressionForLayer(layer: IndexPatternLayer, indexPattern: IndexPatt } ); + const firstDateHistogramColumn = columnEntries.find( + ([, col]) => col.operationType === 'date_histogram' + ); + + const columnsWithTimeScale = firstDateHistogramColumn + ? columnEntries.filter(([, col]) => col.timeScale) + : []; + const timeScaleFunctions: ExpressionFunctionAST[] = columnsWithTimeScale.flatMap( + ([id, col]) => { + const scalingCall: ExpressionFunctionAST = { + type: 'function', + function: 'lens_time_scale', + arguments: { + dateColumnId: [firstDateHistogramColumn![0]], + inputColumnId: [id], + outputColumnId: [id], + targetUnit: [col.timeScale!], + }, + }; + + const formatCall: ExpressionFunctionAST = { + type: 'function', + function: 'lens_format_column', + arguments: { + format: [''], + columnId: [id], + parentFormat: [JSON.stringify({ id: 'suffix', params: { unit: col.timeScale } })], + }, + }; + + return [scalingCall, formatCall]; + } + ); + const allDateHistogramFields = Object.values(columns) .map((column) => column.operationType === dateHistogramOperation.type ? column.sourceField : null @@ -117,6 +150,7 @@ function getExpressionForLayer(layer: IndexPatternLayer, indexPattern: IndexPatt }, ...formatterOverrides, ...expressions, + ...timeScaleFunctions, ], }; } From de19d244e365fa076199c8f021543533b5bad8a6 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 20 Nov 2020 12:07:30 +0100 Subject: [PATCH 03/10] add tests and styling --- .../dimension_panel/dimension_panel.test.tsx | 150 +++++++++++++++- .../dimension_panel/time_scaling.tsx | 160 +++++++++++------- .../indexpattern.test.ts | 80 +++++++++ .../indexpattern_datasource/to_expression.ts | 7 +- 4 files changed, 335 insertions(+), 62 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 2e57ecee86033..03472a6200a15 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 @@ -5,7 +5,7 @@ */ import { ReactWrapper, ShallowWrapper } from 'enzyme'; -import React from 'react'; +import React, { ChangeEvent, MouseEvent } from 'react'; import { act } from 'react-dom/test-utils'; import { EuiComboBox, EuiListGroupItemProps, EuiListGroup, EuiRange } from '@elastic/eui'; import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; @@ -22,6 +22,10 @@ import { documentField } from '../document_field'; import { OperationMetadata } from '../../types'; import { DateHistogramIndexPatternColumn } from '../operations/definitions/date_histogram'; import { getFieldByNameFactory } from '../pure_helpers'; +import { TimeScaling } from './time_scaling'; +import { EuiSelect } from '@elastic/eui'; +import { EuiButtonIcon } from '@elastic/eui'; +import { DimensionEditor } from './dimension_editor'; jest.mock('../loader'); jest.mock('../operations'); @@ -111,7 +115,10 @@ describe('IndexPatternDimensionEditorPanel', () => { let defaultProps: IndexPatternDimensionEditorProps; function getStateWithColumns(columns: Record) { - return { ...state, layers: { first: { ...state.layers.first, columns } } }; + return { + ...state, + layers: { first: { ...state.layers.first, columns, columnOrder: Object.keys(columns) } }, + }; } beforeEach(() => { @@ -785,6 +792,143 @@ describe('IndexPatternDimensionEditorPanel', () => { }); }); + describe('time scaling', () => { + function getProps(colOverrides: Partial) { + return { + ...defaultProps, + state: getStateWithColumns({ + datecolumn: { + dataType: 'date', + isBucketed: true, + label: '', + operationType: 'date_histogram', + sourceField: 'ts', + params: { + interval: '1d', + }, + }, + col2: { + dataType: 'number', + isBucketed: false, + label: '', + operationType: 'count', + sourceField: 'Records', + ...colOverrides, + } as IndexPatternColumn, + }), + columnId: 'col2', + }; + } + it('should not show custom options if time scaling is not available', () => { + wrapper = mount( + + ); + expect(wrapper.find('[data-test-subj="indexPattern-time-scaling"]')).toHaveLength(0); + }); + + it('should show custom options if time scaling is available', () => { + wrapper = mount(); + expect( + wrapper + .find(TimeScaling) + .find('[data-test-subj="indexPattern-time-scaling-popover"]') + .exists() + ).toBe(true); + }); + + it('should show current time scaling if set', () => { + wrapper = mount(); + expect( + wrapper + .find('[data-test-subj="indexPattern-time-scaling-unit"]') + .find(EuiSelect) + .prop('value') + ).toEqual('d'); + }); + + it('should allow to set time scaling initially', () => { + const props = getProps({}); + wrapper = shallow(); + wrapper + .find(DimensionEditor) + .dive() + .find(TimeScaling) + .dive() + .find('[data-test-subj="indexPattern-time-scaling-enable"]') + .prop('onClick')!({} as MouseEvent); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: 'm', + }), + }, + }, + }, + }); + }); + + it('should allow to change time scaling', () => { + const props = getProps({ timeScale: 's' }); + wrapper = mount(); + wrapper + .find('[data-test-subj="indexPattern-time-scaling-unit"]') + .find(EuiSelect) + .prop('onChange')!(({ + target: { value: 'h' }, + } as unknown) as ChangeEvent); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: 'h', + }), + }, + }, + }, + }); + }); + + it('should allow to remove time scaling', () => { + const props = getProps({ timeScale: 's' }); + wrapper = mount(); + wrapper + .find('[data-test-subj="indexPattern-time-scaling-remove"]') + .find(EuiButtonIcon) + .prop('onClick')!( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + {} as any + ); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: undefined, + }), + }, + }, + }, + }); + }); + }); + it('should render invalid field if field reference is broken', () => { wrapper = mount( { act(() => { wrapper.find('[data-test-subj="lns-indexPatternDimension-min"]').first().prop('onClick')!( - {} as React.MouseEvent<{}, MouseEvent> + {} as MouseEvent ); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index 134fad0f523cb..5e8312b72e61f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -11,9 +11,13 @@ import { EuiFlexItem, EuiFlexGroup, EuiButtonIcon, + EuiText, + EuiPopover, + EuiButtonEmpty, + EuiSpacer, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React from 'react'; +import React, { useState } from 'react'; import { StateSetter } from '../../types'; import { IndexPatternColumn, operationDefinitionMap } from '../operations'; import { mergeLayer } from '../state_helpers'; @@ -36,6 +40,7 @@ export function TimeScaling({ state: IndexPatternPrivateState; setState: StateSetter; }) { + const [popoverOpen, setPopoverOpen] = useState(false); const layer = state.layers[layerId]; const hasDateHistogram = layer.columnOrder.some( (colId) => layer.columns[colId].operationType === 'date_histogram' @@ -51,46 +56,80 @@ export function TimeScaling({ if (!selectedColumn.timeScale) { return ( - { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - ...state.layers[layerId], - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - timeScale: DEFAULT_TIME_SCALE, - }, - }, - }, - }) - ); - }} - > - {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Show as rate', - })} - + + + { + setPopoverOpen(true); + }} + > + {i18n.translate('xpack.lens.indexPattern.timeScale.advancedSettings', { + defaultMessage: 'Add advanced options', + })} + + } + isOpen={popoverOpen} + closePopover={() => { + setPopoverOpen(false); + }} + > + + { + setState( + mergeLayer({ + state, + layerId, + newLayer: { + ...state.layers[layerId], + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + timeScale: DEFAULT_TIME_SCALE, + }, + }, + }, + }) + ); + }} + > + {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { + defaultMessage: 'Show as rate', + })} + + + + ); } return ( - - - + + + ({ value: unit, text, }))} + data-test-subj="indexPattern-time-scaling-unit" value={selectedColumn.timeScale} onChange={(e) => { setState( @@ -111,33 +150,38 @@ export function TimeScaling({ ); }} /> - - - {selectedOperation.timeScalingMode === 'optional' && ( - - { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - ...state.layers[layerId], - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - timeScale: undefined, + + {selectedOperation.timeScalingMode === 'optional' && ( + + { + setState( + mergeLayer({ + state, + layerId, + newLayer: { + ...state.layers[layerId], + columns: { + ...state.layers[layerId].columns, + [columnId]: { + ...selectedColumn, + timeScale: undefined, + }, }, }, - }, - }) - ); - }} - iconType="cross" - /> - - )} - + }) + ); + }} + iconType="cross" + /> + + )} + + ); } 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 3cf9bdc3a92f1..a6379e0ce7fcb 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -407,6 +407,86 @@ describe('IndexPattern Data Source', () => { expect(ast.chain[0].arguments.timeFields).toEqual(['timestamp', 'another_datefield']); }); + it('should add time_scale and format function if time scale is set and supported', async () => { + const queryBaseState: IndexPatternBaseState = { + currentIndexPatternId: '1', + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2', 'col3'], + columns: { + col1: { + label: 'Count of records', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + timeScale: 'h', + }, + col2: { + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + sourceField: 'bytes', + operationType: 'avg', + timeScale: 'h', + }, + col3: { + label: 'Date', + dataType: 'date', + isBucketed: true, + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'auto', + }, + }, + }, + }, + }, + }; + + const state = enrichBaseState(queryBaseState); + + const ast = indexPatternDatasource.toExpression(state, 'first') as Ast; + const timeScaleCalls = ast.chain.filter((fn) => fn.function === 'lens_time_scale'); + const formatCalls = ast.chain.filter((fn) => fn.function === 'lens_format_column'); + expect(timeScaleCalls).toHaveLength(1); + expect(timeScaleCalls[0].arguments).toMatchInlineSnapshot(` + Object { + "dateColumnId": Array [ + "col3", + ], + "inputColumnId": Array [ + "col1", + ], + "outputColumnId": Array [ + "col1", + ], + "targetUnit": Array [ + "h", + ], + } + `); + expect(formatCalls[0]).toMatchInlineSnapshot(` + Object { + "arguments": Object { + "columnId": Array [ + "col1", + ], + "format": Array [ + "", + ], + "parentFormat": Array [ + "{\\"id\\":\\"suffix\\",\\"params\\":{\\"unit\\":\\"h\\"}}", + ], + }, + "function": "lens_format_column", + "type": "function", + } + `); + }); + it('should rename the output from esaggs when using flat query', () => { const queryBaseState: IndexPatternBaseState = { currentIndexPatternId: '1', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 201ff406c44ca..7b7fc0468cf86 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -91,7 +91,12 @@ function getExpressionForLayer(layer: IndexPatternLayer, indexPattern: IndexPatt ); const columnsWithTimeScale = firstDateHistogramColumn - ? columnEntries.filter(([, col]) => col.timeScale) + ? columnEntries.filter( + ([, col]) => + col.timeScale && + operationDefinitionMap[col.operationType].timeScalingMode && + operationDefinitionMap[col.operationType].timeScalingMode !== 'disabled' + ) : []; const timeScaleFunctions: ExpressionFunctionAST[] = columnsWithTimeScale.flatMap( ([id, col]) => { From 5499eadd949907808176128dc26020bcc3974600 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 23 Nov 2020 12:43:36 +0100 Subject: [PATCH 04/10] remove integration logic --- .../operations/layer_helpers.ts | 74 ------------------- 1 file changed, 74 deletions(-) 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 00ca6b49e7372..1495a876a2c8e 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 @@ -152,80 +152,6 @@ export function insertNewColumn({ } } - if (operationDefinition.input === 'fullReference') { - let tempLayer = { ...layer }; - const referenceIds = operationDefinition.requiredReferences.map((validation) => { - const validOperations = Object.values(operationDefinitionMap).filter(({ type }) => - isOperationAllowedAsReference({ validation, operationType: type }) - ); - - if (!validOperations.length) { - throw new Error( - `Can't create reference, ${op} has a validation function which doesn't allow any operations` - ); - } - - const newId = generateId(); - if (validOperations.length === 1) { - const def = validOperations[0]; - - const validFields = - def.input === 'field' ? indexPattern.fields.filter(def.getPossibleOperationForField) : []; - - if (def.input === 'none') { - tempLayer = insertNewColumn({ - layer: tempLayer, - columnId: newId, - op: def.type, - indexPattern, - }); - } else if (validFields.length === 1) { - // Recursively update the layer for each new reference - tempLayer = insertNewColumn({ - layer: tempLayer, - columnId: newId, - op: def.type, - indexPattern, - field: validFields[0], - }); - } else { - tempLayer = { - ...tempLayer, - incompleteColumns: { - ...tempLayer.incompleteColumns, - [newId]: { operationType: def.type }, - }, - }; - } - } - return newId; - }); - - const possibleOperation = operationDefinition.getPossibleOperation(); - const isBucketed = Boolean(possibleOperation.isBucketed); - if (isBucketed) { - return addBucket( - tempLayer, - operationDefinition.buildColumn({ - ...baseOptions, - columnOrder: layer.columnOrder, - referenceIds, - }), - columnId - ); - } else { - return addMetric( - tempLayer, - operationDefinition.buildColumn({ - ...baseOptions, - columnOrder: layer.columnOrder, - referenceIds, - }), - columnId - ); - } - } - if (!field) { throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); } From 69ecfbdcd184e229fed61644b8642effebc2c783 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 24 Nov 2020 15:40:17 +0100 Subject: [PATCH 05/10] review comments --- .../series_calculation_helpers.ts | 5 +- .../dimension_panel/dimension_editor.tsx | 7 +- .../dimension_panel/dimension_panel.test.tsx | 37 +++++- .../dimension_panel/time_scaling.tsx | 117 ++++++++---------- .../operations/__mocks__/index.ts | 2 + .../operations/layer_helpers.test.ts | 97 +++++++++++++++ .../operations/layer_helpers.ts | 80 ++++++++++-- .../indexpattern_datasource/time_scale.ts | 2 +- 8 files changed, 261 insertions(+), 86 deletions(-) diff --git a/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts b/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts index 82425b8f6509e..71e528f78ec81 100644 --- a/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts +++ b/src/plugins/expressions/common/expression_functions/series_calculation_helpers.ts @@ -39,16 +39,17 @@ export function getBucketIdentifier(row: DatatableRow, groupColumns?: string[]) * @param outputColumnId Id of the output column * @param inputColumnId Id of the input column * @param outputColumnName Optional name of the output column + * @param options Optional options, set `allowColumnOverwrite` to true to not raise an exception if the output column exists already */ export function buildResultColumns( input: Datatable, outputColumnId: string, inputColumnId: string, outputColumnName: string | undefined, - options: { allowColumnOverride: boolean } = { allowColumnOverride: false } + options: { allowColumnOverwrite: boolean } = { allowColumnOverwrite: false } ) { if ( - !options.allowColumnOverride && + !options.allowColumnOverwrite && input.columns.some((column) => column.id === outputColumnId) ) { throw new Error( 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 9bb8f092c98c3..1faa9a0c89074 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 @@ -338,9 +338,10 @@ export function DimensionEditor(props: DimensionEditorProps) { + setState(mergeLayer({ layerId, state, newLayer })) + } /> )} 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 03472a6200a15..79102d4e34411 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 @@ -810,7 +810,7 @@ describe('IndexPatternDimensionEditorPanel', () => { col2: { dataType: 'number', isBucketed: false, - label: '', + label: 'Count of records', operationType: 'count', sourceField: 'Records', ...colOverrides, @@ -869,7 +869,8 @@ describe('IndexPatternDimensionEditorPanel', () => { columns: { ...props.state.layers.first.columns, col2: expect.objectContaining({ - timeScale: 'm', + timeScale: 's', + label: 'Count of records per second', }), }, }, @@ -878,7 +879,33 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should allow to change time scaling', () => { - const props = getProps({ timeScale: 's' }); + const props = getProps({ timeScale: 's', label: 'Count of records per second' }); + wrapper = mount(); + wrapper + .find('[data-test-subj="indexPattern-time-scaling-unit"]') + .find(EuiSelect) + .prop('onChange')!(({ + target: { value: 'h' }, + } as unknown) as ChangeEvent); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: 'h', + label: 'Count of records per hour', + }), + }, + }, + }, + }); + }); + + it('should not adjust label if it is custom', () => { + const props = getProps({ timeScale: 's', customLabel: true, label: 'My label' }); wrapper = mount(); wrapper .find('[data-test-subj="indexPattern-time-scaling-unit"]') @@ -895,6 +922,7 @@ describe('IndexPatternDimensionEditorPanel', () => { ...props.state.layers.first.columns, col2: expect.objectContaining({ timeScale: 'h', + label: 'My label', }), }, }, @@ -903,7 +931,7 @@ describe('IndexPatternDimensionEditorPanel', () => { }); it('should allow to remove time scaling', () => { - const props = getProps({ timeScale: 's' }); + const props = getProps({ timeScale: 's', label: 'Count of records per second' }); wrapper = mount(); wrapper .find('[data-test-subj="indexPattern-time-scaling-remove"]') @@ -921,6 +949,7 @@ describe('IndexPatternDimensionEditorPanel', () => { ...props.state.layers.first.columns, col2: expect.objectContaining({ timeScale: undefined, + label: 'Count of records', }), }, }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index 5e8312b72e61f..24fc006cdc8c0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { EuiToolTip } from '@elastic/eui'; import { EuiLink, EuiFormRow, @@ -18,30 +19,50 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React, { useState } from 'react'; -import { StateSetter } from '../../types'; -import { IndexPatternColumn, operationDefinitionMap } from '../operations'; -import { mergeLayer } from '../state_helpers'; +import { + adjustTimeScaleLabelSuffix, + DEFAULT_TIME_SCALE, + IndexPatternColumn, + operationDefinitionMap, +} from '../operations'; import { unitSuffixesLong } from '../suffix_formatter'; import { TimeScaleUnit } from '../time_scale'; -import { IndexPatternPrivateState } from '../types'; +import { IndexPatternLayer } from '../types'; -const DEFAULT_TIME_SCALE = 'm' as const; +export function setTimeScaling( + columnId: string, + layer: IndexPatternLayer, + timeScale: TimeScaleUnit | undefined +) { + const currentColumn = layer.columns[columnId]; + const label = currentColumn.customLabel + ? currentColumn.label + : adjustTimeScaleLabelSuffix(currentColumn.label, currentColumn.timeScale, timeScale); + return { + ...layer, + columns: { + ...layer.columns, + [columnId]: { + ...layer.columns[columnId], + label, + timeScale, + }, + }, + }; +} export function TimeScaling({ selectedColumn, columnId, - layerId, - state, - setState, + layer, + updateLayer, }: { selectedColumn: IndexPatternColumn; columnId: string; - layerId: string; - state: IndexPatternPrivateState; - setState: StateSetter; + layer: IndexPatternLayer; + updateLayer: (newLayer: IndexPatternLayer) => void; }) { const [popoverOpen, setPopoverOpen] = useState(false); - const layer = state.layers[layerId]; const hasDateHistogram = layer.columnOrder.some( (colId) => layer.columns[colId].operationType === 'date_histogram' ); @@ -85,26 +106,11 @@ export function TimeScaling({ data-test-subj="indexPattern-time-scaling-enable" color="text" onClick={() => { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - ...state.layers[layerId], - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - timeScale: DEFAULT_TIME_SCALE, - }, - }, - }, - }) - ); + updateLayer(setTimeScaling(columnId, layer, DEFAULT_TIME_SCALE)); }} > {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Show as rate', + defaultMessage: 'Show with normalized time units', })} @@ -117,9 +123,20 @@ export function TimeScaling({ + + {i18n.translate('xpack.lens.indexPattern.timeScale.label', { + defaultMessage: 'Normalize by time unit', + })} + + + } > @@ -132,22 +149,7 @@ export function TimeScaling({ data-test-subj="indexPattern-time-scaling-unit" value={selectedColumn.timeScale} onChange={(e) => { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - ...state.layers[layerId], - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - timeScale: e.target.value as TimeScaleUnit, - }, - }, - }, - }) - ); + updateLayer(setTimeScaling(columnId, layer, e.target.value as TimeScaleUnit)); }} /> @@ -157,25 +159,10 @@ export function TimeScaling({ data-test-subj="indexPattern-time-scaling-remove" color="danger" aria-label={i18n.translate('xpack.lens.timeScale.removeLabel', { - defaultMessage: 'Remove normalizing by unit', + defaultMessage: 'Remove normalizing by time unit', })} onClick={() => { - setState( - mergeLayer({ - state, - layerId, - newLayer: { - ...state.layers[layerId], - columns: { - ...state.layers[layerId].columns, - [columnId]: { - ...selectedColumn, - timeScale: undefined, - }, - }, - }, - }) - ); + updateLayer(setTimeScaling(columnId, layer, undefined)); }} iconType="cross" /> 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 f27fb8d4642f6..0e803be569d1f 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 @@ -39,6 +39,8 @@ export const { isColumnTransferable, getErrorMessages, isReferenced, + adjustTimeScaleLabelSuffix, + DEFAULT_TIME_SCALE, } = actualHelpers; export const { createMockedReferenceOperation } = actualMocks; 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 0d103a766c23a..31316f6e080bb 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 @@ -436,6 +436,8 @@ describe('state_helpers', () => { ); }); }); + + // TODO add test for mandatory time scale operation once counter rate is implemented }); describe('replaceColumn', () => { @@ -726,6 +728,101 @@ describe('state_helpers', () => { ); }); + describe('time scale transition', () => { + it('should carry over time scale and adjust label on operation', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Count of records per hour', + timeScale: 'h', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'sum', + field: indexPattern.fields[2], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + timeScale: 'h', + label: 'Sum of bytes per hour', + }) + ); + }); + + it('should carry over time scale and retain custom label', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Custom label', + customLabel: true, + timeScale: 'h', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'sum', + field: indexPattern.fields[2], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + timeScale: 'h', + label: 'Custom label', + }) + ); + }); + + it('should not carry over time scale if target does not support time scaling', () => { + const result = replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: '', + timeScale: 'h', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'avg', + field: indexPattern.fields[2], + }).columns.col1; + expect(result.timeScale).toBeUndefined(); + }); + + // TODO add test for mandatory time scale operation once counter rate is implemented + }); + it('should execute adjustments for other columns', () => { const termsColumn: TermsIndexPatternColumn = { label: 'Top values of source', 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 1495a876a2c8e..754fa37de2e15 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 @@ -22,6 +22,8 @@ import type { import { getSortScoreByPriority } from './operations'; import { mergeLayer } from '../state_helpers'; import { generateId } from '../../id_generator'; +import { TimeScaleUnit } from '../time_scale'; +import { unitSuffixesLong } from '../suffix_formatter'; interface ColumnChange { op: OperationType; @@ -237,11 +239,8 @@ export function replaceColumn({ } if (operationDefinition.input === 'none') { - const newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer }); - if (previousColumn.customLabel) { - newColumn.customLabel = true; - newColumn.label = previousColumn.label; - } + let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer }); + newColumn = adjustLabelAndTimeScale(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { @@ -255,12 +254,8 @@ export function replaceColumn({ throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); } - const newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, field }); - - if (previousColumn.customLabel) { - newColumn.customLabel = true; - newColumn.label = previousColumn.label; - } + let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, field }); + newColumn = adjustLabelAndTimeScale(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { @@ -280,6 +275,8 @@ export function replaceColumn({ if (previousColumn.customLabel) { newColumn.customLabel = true; newColumn.label = previousColumn.label; + } else { + newColumn.label = adjustTimeScaleLabelSuffix(newColumn.label, undefined, newColumn.timeScale); } const newColumns = { ...layer.columns, [columnId]: newColumn }; @@ -293,6 +290,35 @@ export function replaceColumn({ } } +function adjustLabelAndTimeScale( + newColumn: IndexPatternColumn, + previousColumn: IndexPatternColumn +) { + const adjustedColumn = { ...newColumn }; + if (previousColumn.customLabel) { + adjustedColumn.customLabel = true; + adjustedColumn.label = previousColumn.label; + } + const newOperationTimeScaleMode = + operationDefinitionMap[newColumn.operationType].timeScalingMode || 'disabled'; + + if ( + (previousColumn.timeScale && newOperationTimeScaleMode !== 'disabled') || + newOperationTimeScaleMode === 'mandatory' + ) { + adjustedColumn.timeScale = previousColumn.timeScale || DEFAULT_TIME_SCALE; + if (!adjustedColumn.customLabel) { + adjustedColumn.label = adjustTimeScaleLabelSuffix( + adjustedColumn.label, + undefined, + previousColumn.timeScale + ); + } + } + + return adjustedColumn; +} + function addBucket( layer: IndexPatternLayer, column: IndexPatternColumn, @@ -330,6 +356,16 @@ function addMetric( column: IndexPatternColumn, addedColumnId: string ): IndexPatternLayer { + const timeScaledColumn = { ...column }; + const operationDefinition = operationDefinitionMap[column.operationType]; + if (operationDefinition.timeScalingMode === 'mandatory') { + timeScaledColumn.timeScale = DEFAULT_TIME_SCALE; + timeScaledColumn.label = adjustTimeScaleLabelSuffix( + timeScaledColumn.label, + undefined, + DEFAULT_TIME_SCALE + ); + } return { ...layer, columns: { @@ -612,3 +648,25 @@ function isOperationAllowedAsReference({ hasValidMetadata ); } + +export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; + +export function adjustTimeScaleLabelSuffix( + oldLabel: string, + previousTimeScale: TimeScaleUnit | undefined, + newTimeScale: TimeScaleUnit | undefined +) { + let cleanedLabel = oldLabel; + // remove added suffix if column had a time scale previously + if (previousTimeScale) { + const suffixPosition = oldLabel.lastIndexOf(` ${unitSuffixesLong[previousTimeScale]}`); + if (suffixPosition !== -1) { + cleanedLabel = oldLabel.substring(0, suffixPosition); + } + } + if (!newTimeScale) { + return cleanedLabel; + } + // add new suffix if column has a time scale now + return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts b/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts index 16f8c24a9bfc9..ca0745493f768 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/time_scale.ts @@ -88,7 +88,7 @@ export function getTimeScaleFunction(data: DataPublicPluginStart) { outputColumnId, inputColumnId, outputColumnName, - { allowColumnOverride: true } + { allowColumnOverwrite: true } ); if (!resultColumns) { From 20db54f25e86b09ff64594cbe83feaa413de3966 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 26 Nov 2020 09:57:49 +0100 Subject: [PATCH 06/10] fix stuff --- .../dimension_panel/time_scaling.tsx | 3 + .../operations/__mocks__/index.ts | 5 +- .../definitions/calculations/counter_rate.tsx | 19 +++-- .../definitions/calculations/derivative.tsx | 17 +++-- .../calculations/moving_average.tsx | 17 +++-- .../definitions/calculations/utils.ts | 10 +++ .../operations/definitions/count.tsx | 8 ++- .../operations/definitions/metrics.tsx | 32 ++++++--- .../operations/index.ts | 1 + .../operations/layer_helpers.ts | 72 ++----------------- .../operations/time_scale_utils.ts | 30 ++++++++ 11 files changed, 119 insertions(+), 95 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index 24fc006cdc8c0..b8a134d9cb88b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -5,6 +5,7 @@ */ import { EuiToolTip } from '@elastic/eui'; +import { EuiIcon } from '@elastic/eui'; import { EuiLink, EuiFormRow, @@ -106,6 +107,7 @@ export function TimeScaling({ data-test-subj="indexPattern-time-scaling-enable" color="text" onClick={() => { + setPopoverOpen(false); updateLayer(setTimeScaling(columnId, layer, DEFAULT_TIME_SCALE)); }} > @@ -134,6 +136,7 @@ export function TimeScaling({ {i18n.translate('xpack.lens.indexPattern.timeScale.label', { defaultMessage: 'Normalize by time unit', })} + } 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 0e803be569d1f..385f2ab941ef2 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 @@ -6,6 +6,7 @@ const actualOperations = jest.requireActual('../operations'); const actualHelpers = jest.requireActual('../layer_helpers'); +const actualTimeScaleUtils = jest.requireActual('../time_scale_utils'); const actualMocks = jest.requireActual('../mocks'); jest.spyOn(actualOperations.operationDefinitionMap.date_histogram, 'paramEditor'); @@ -39,8 +40,8 @@ export const { isColumnTransferable, getErrorMessages, isReferenced, - adjustTimeScaleLabelSuffix, - DEFAULT_TIME_SCALE, } = actualHelpers; +export const { adjustTimeScaleLabelSuffix, DEFAULT_TIME_SCALE } = actualTimeScaleUtils; + export const { createMockedReferenceOperation } = actualMocks; 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 d256b74696a4c..0cfba4cfc739f 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 @@ -7,10 +7,16 @@ import { i18n } from '@kbn/i18n'; import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types'; import { IndexPatternLayer } from '../../../types'; -import { checkForDateHistogram, dateBasedOperationToExpression, hasDateField } from './utils'; +import { + buildLabelFunction, + checkForDateHistogram, + dateBasedOperationToExpression, + hasDateField, +} from './utils'; +import { DEFAULT_TIME_SCALE } from '../../time_scale_utils'; import { OperationDefinition } from '..'; -const ofName = (name?: string) => { +const ofName = buildLabelFunction((name?: string) => { return i18n.translate('xpack.lens.indexPattern.CounterRateOf', { defaultMessage: 'Counter rate of {name}', values: { @@ -21,7 +27,7 @@ const ofName = (name?: string) => { }), }, }); -}; +}); export type CounterRateIndexPatternColumn = FormattedIndexPatternColumn & ReferenceBasedIndexPatternColumn & { @@ -54,20 +60,22 @@ export const counterRateOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label); + return ofName(columns[column.references[0]]?.label, column.timeScale); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'lens_counter_rate'); }, buildColumn: ({ referenceIds, previousColumn, layer }) => { const metric = layer.columns[referenceIds[0]]; + const timeScale = previousColumn?.timeScale || DEFAULT_TIME_SCALE; return { - label: ofName(metric?.label), + label: ofName(metric?.label, timeScale), dataType: 'number', operationType: 'counter_rate', isBucketed: false, scale: 'ratio', references: referenceIds, + timeScale, params: previousColumn?.dataType === 'number' && previousColumn.params && @@ -88,4 +96,5 @@ export const counterRateOperation: OperationDefinition< }) ); }, + timeScalingMode: 'mandatory', }; 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 7398f7e07ea4e..12ca499f890b3 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 @@ -7,10 +7,15 @@ import { i18n } from '@kbn/i18n'; import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types'; import { IndexPatternLayer } from '../../../types'; -import { checkForDateHistogram, dateBasedOperationToExpression, hasDateField } from './utils'; +import { + buildLabelFunction, + checkForDateHistogram, + dateBasedOperationToExpression, + hasDateField, +} from './utils'; import { OperationDefinition } from '..'; -const ofName = (name?: string) => { +const ofName = buildLabelFunction((name?: string) => { return i18n.translate('xpack.lens.indexPattern.derivativeOf', { defaultMessage: 'Differences of {name}', values: { @@ -21,7 +26,7 @@ const ofName = (name?: string) => { }), }, }); -}; +}); export type DerivativeIndexPatternColumn = FormattedIndexPatternColumn & ReferenceBasedIndexPatternColumn & { @@ -53,7 +58,7 @@ export const derivativeOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label); + return ofName(columns[column.references[0]]?.label, column.timeScale); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'derivative'); @@ -61,12 +66,13 @@ export const derivativeOperation: OperationDefinition< buildColumn: ({ referenceIds, previousColumn, layer }) => { const metric = layer.columns[referenceIds[0]]; return { - label: ofName(metric?.label), + label: ofName(metric?.label, previousColumn?.timeScale), dataType: 'number', operationType: 'derivative', isBucketed: false, scale: 'ratio', references: referenceIds, + timeScale: previousColumn?.timeScale, params: previousColumn?.dataType === 'number' && previousColumn.params && @@ -87,4 +93,5 @@ export const derivativeOperation: OperationDefinition< }) ); }, + timeScalingMode: 'optional', }; 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 795281d0fd994..8e460e86919ed 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 @@ -11,12 +11,17 @@ import { EuiFormRow } from '@elastic/eui'; import { EuiFieldNumber } from '@elastic/eui'; import { FormattedIndexPatternColumn, ReferenceBasedIndexPatternColumn } from '../column_types'; import { IndexPatternLayer } from '../../../types'; -import { checkForDateHistogram, dateBasedOperationToExpression, hasDateField } from './utils'; +import { + buildLabelFunction, + checkForDateHistogram, + dateBasedOperationToExpression, + hasDateField, +} from './utils'; import { updateColumnParam } from '../../layer_helpers'; import { useDebounceWithOptions } from '../helpers'; import type { OperationDefinition, ParamEditorProps } from '..'; -const ofName = (name?: string) => { +const ofName = buildLabelFunction((name?: string) => { return i18n.translate('xpack.lens.indexPattern.movingAverageOf', { defaultMessage: 'Moving average of {name}', values: { @@ -27,7 +32,7 @@ const ofName = (name?: string) => { }), }, }); -}; +}); export type MovingAverageIndexPatternColumn = FormattedIndexPatternColumn & ReferenceBasedIndexPatternColumn & { @@ -62,7 +67,7 @@ export const movingAverageOperation: OperationDefinition< }; }, getDefaultLabel: (column, indexPattern, columns) => { - return ofName(columns[column.references[0]]?.label); + return ofName(columns[column.references[0]]?.label, column.timeScale); }, toExpression: (layer, columnId) => { return dateBasedOperationToExpression(layer, columnId, 'moving_average', { @@ -72,12 +77,13 @@ export const movingAverageOperation: OperationDefinition< buildColumn: ({ referenceIds, previousColumn, layer }) => { const metric = layer.columns[referenceIds[0]]; return { - label: ofName(metric?.label), + label: ofName(metric?.label, previousColumn?.timeScale), dataType: 'number', operationType: 'moving_average', isBucketed: false, scale: 'ratio', references: referenceIds, + timeScale: previousColumn?.timeScale, params: previousColumn?.dataType === 'number' && previousColumn.params && @@ -99,6 +105,7 @@ export const movingAverageOperation: OperationDefinition< }) ); }, + timeScalingMode: 'optional', }; function MovingAverageParamEditor({ 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 c64a292280603..bac45f683e444 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 @@ -6,9 +6,19 @@ import { i18n } from '@kbn/i18n'; import { ExpressionFunctionAST } from '@kbn/interpreter/common'; +import { TimeScaleUnit } from '../../../time_scale'; import { IndexPattern, IndexPatternLayer } from '../../../types'; +import { adjustTimeScaleLabelSuffix } from '../../time_scale_utils'; import { ReferenceBasedIndexPatternColumn } from '../column_types'; +export const buildLabelFunction = (ofName: (name?: string) => string) => ( + name?: string, + timeScale?: TimeScaleUnit +) => { + const rawLabel = ofName(name); + return adjustTimeScaleLabelSuffix(rawLabel, undefined, timeScale); +}; + /** * Checks whether the current layer includes a date histogram and returns an error otherwise */ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index fae1fc53217e8..4156441bc0632 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField } from '../../types'; +import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', @@ -28,7 +29,7 @@ export const countOperation: OperationDefinition { return { ...oldColumn, - label: field.displayName, + label: adjustTimeScaleLabelSuffix(field.displayName, undefined, oldColumn.timeScale), sourceField: field.name, }; }, @@ -41,15 +42,16 @@ export const countOperation: OperationDefinition countLabel, + getDefaultLabel: (column) => adjustTimeScaleLabelSuffix(countLabel, undefined, column.timeScale), buildColumn({ field, previousColumn }) { return { - label: countLabel, + label: adjustTimeScaleLabelSuffix(countLabel, undefined, previousColumn?.timeScale), dataType: 'number', operationType: 'count', isBucketed: false, scale: 'ratio', sourceField: field.name, + timeScale: previousColumn?.timeScale, params: previousColumn?.dataType === 'number' && previousColumn.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 5cbaa3914df78..7697c6928ec51 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 @@ -5,8 +5,13 @@ */ import { i18n } from '@kbn/i18n'; -import { OperationDefinition, TimeScalingMode } from './index'; -import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; +import { OperationDefinition } from './index'; +import { + FormattedIndexPatternColumn, + FieldBasedIndexPatternColumn, + BaseIndexPatternColumn, +} from './column_types'; +import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; type MetricColumn = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { @@ -18,20 +23,28 @@ function buildMetricOperation>({ displayName, ofName, priority, - timeScalingMode, + optionalTimeScaling, }: { type: T['operationType']; displayName: string; ofName: (name: string) => string; priority?: number; - timeScalingMode?: TimeScalingMode; + optionalTimeScaling?: boolean; }) { + const labelLookup = (name: string, column?: BaseIndexPatternColumn) => { + const rawLabel = ofName(name); + if (!optionalTimeScaling) { + return rawLabel; + } + return adjustTimeScaleLabelSuffix(rawLabel, undefined, column?.timeScale); + }; + return { type, priority, displayName, input: 'field', - timeScalingMode, + timeScalingMode: optionalTimeScaling ? 'optional' : undefined, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { if ( fieldType === 'number' && @@ -56,21 +69,22 @@ function buildMetricOperation>({ ); }, getDefaultLabel: (column, indexPattern, columns) => - ofName(indexPattern.getFieldByName(column.sourceField)!.displayName), + labelLookup(indexPattern.getFieldByName(column.sourceField)!.displayName, column), buildColumn: ({ field, previousColumn }) => ({ - label: ofName(field.displayName), + label: labelLookup(field.displayName, previousColumn), dataType: 'number', operationType: type, sourceField: field.name, isBucketed: false, scale: 'ratio', + timeScale: optionalTimeScaling ? previousColumn?.timeScale : undefined, params: previousColumn && previousColumn.dataType === 'number' ? previousColumn.params : undefined, }), onFieldChange: (oldColumn, field) => { return { ...oldColumn, - label: ofName(field.displayName), + label: labelLookup(field.displayName, oldColumn), sourceField: field.name, }; }, @@ -141,7 +155,7 @@ export const sumOperation = buildMetricOperation({ defaultMessage: 'Sum of {name}', values: { name }, }), - timeScalingMode: 'optional', + optionalTimeScaling: true, }); export const medianOperation = buildMetricOperation({ 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 3ad9a1e5b3674..7123becf71b4d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts @@ -6,6 +6,7 @@ export * from './operations'; export * from './layer_helpers'; +export * from './time_scale_utils'; export { OperationType, IndexPatternColumn, 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 74cb0e0a5072f..260ed180da921 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 @@ -22,8 +22,7 @@ import type { import { getSortScoreByPriority } from './operations'; import { mergeLayer } from '../state_helpers'; import { generateId } from '../../id_generator'; -import { TimeScaleUnit } from '../time_scale'; -import { unitSuffixesLong } from '../suffix_formatter'; +import { ReferenceBasedIndexPatternColumn } from './definitions/column_types'; interface ColumnChange { op: OperationType; @@ -210,8 +209,7 @@ export function replaceColumn({ let tempLayer = { ...layer }; if (previousDefinition.input === 'fullReference') { - // @ts-expect-error references are not statically analyzed - previousColumn.references.forEach((id: string) => { + (previousColumn as ReferenceBasedIndexPatternColumn).references.forEach((id: string) => { tempLayer = deleteColumn({ layer: tempLayer, columnId: id }); }); } @@ -240,7 +238,7 @@ export function replaceColumn({ if (operationDefinition.input === 'none') { let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer }); - newColumn = adjustLabelAndTimeScale(newColumn, previousColumn); + newColumn = adjustLabel(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { @@ -255,7 +253,7 @@ export function replaceColumn({ } let newColumn = operationDefinition.buildColumn({ ...baseOptions, layer: tempLayer, field }); - newColumn = adjustLabelAndTimeScale(newColumn, previousColumn); + newColumn = adjustLabel(newColumn, previousColumn); const newColumns = { ...tempLayer.columns, [columnId]: newColumn }; return { @@ -272,14 +270,7 @@ export function replaceColumn({ // Same operation, new field const newColumn = operationDefinition.onFieldChange(previousColumn, field); - if (previousColumn.customLabel) { - newColumn.customLabel = true; - newColumn.label = previousColumn.label; - } else { - newColumn.label = adjustTimeScaleLabelSuffix(newColumn.label, undefined, newColumn.timeScale); - } - - const newColumns = { ...layer.columns, [columnId]: newColumn }; + const newColumns = { ...layer.columns, [columnId]: adjustLabel(newColumn, previousColumn) }; return { ...layer, columnOrder: getColumnOrder({ ...layer, columns: newColumns }), @@ -290,31 +281,12 @@ export function replaceColumn({ } } -function adjustLabelAndTimeScale( - newColumn: IndexPatternColumn, - previousColumn: IndexPatternColumn -) { +function adjustLabel(newColumn: IndexPatternColumn, previousColumn: IndexPatternColumn) { const adjustedColumn = { ...newColumn }; if (previousColumn.customLabel) { adjustedColumn.customLabel = true; adjustedColumn.label = previousColumn.label; } - const newOperationTimeScaleMode = - operationDefinitionMap[newColumn.operationType].timeScalingMode || 'disabled'; - - if ( - (previousColumn.timeScale && newOperationTimeScaleMode !== 'disabled') || - newOperationTimeScaleMode === 'mandatory' - ) { - adjustedColumn.timeScale = previousColumn.timeScale || DEFAULT_TIME_SCALE; - if (!adjustedColumn.customLabel) { - adjustedColumn.label = adjustTimeScaleLabelSuffix( - adjustedColumn.label, - undefined, - previousColumn.timeScale - ); - } - } return adjustedColumn; } @@ -356,16 +328,6 @@ function addMetric( column: IndexPatternColumn, addedColumnId: string ): IndexPatternLayer { - const timeScaledColumn = { ...column }; - const operationDefinition = operationDefinitionMap[column.operationType]; - if (operationDefinition.timeScalingMode === 'mandatory') { - timeScaledColumn.timeScale = DEFAULT_TIME_SCALE; - timeScaledColumn.label = adjustTimeScaleLabelSuffix( - timeScaledColumn.label, - undefined, - DEFAULT_TIME_SCALE - ); - } return { ...layer, columns: { @@ -639,25 +601,3 @@ function isOperationAllowedAsReference({ hasValidMetadata ); } - -export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; - -export function adjustTimeScaleLabelSuffix( - oldLabel: string, - previousTimeScale: TimeScaleUnit | undefined, - newTimeScale: TimeScaleUnit | undefined -) { - let cleanedLabel = oldLabel; - // remove added suffix if column had a time scale previously - if (previousTimeScale) { - const suffixPosition = oldLabel.lastIndexOf(` ${unitSuffixesLong[previousTimeScale]}`); - if (suffixPosition !== -1) { - cleanedLabel = oldLabel.substring(0, suffixPosition); - } - } - if (!newTimeScale) { - return cleanedLabel; - } - // add new suffix if column has a time scale now - return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; -} 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 new file mode 100644 index 0000000000000..1d5ed796034d1 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts @@ -0,0 +1,30 @@ +/* + * 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 { unitSuffixesLong } from '../suffix_formatter'; +import { TimeScaleUnit } from '../time_scale'; + +export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; + +export function adjustTimeScaleLabelSuffix( + oldLabel: string, + previousTimeScale: TimeScaleUnit | undefined, + newTimeScale: TimeScaleUnit | undefined +) { + let cleanedLabel = oldLabel; + // remove added suffix if column had a time scale previously + if (previousTimeScale) { + const suffixPosition = oldLabel.lastIndexOf(` ${unitSuffixesLong[previousTimeScale]}`); + if (suffixPosition !== -1) { + cleanedLabel = oldLabel.substring(0, suffixPosition); + } + } + if (!newTimeScale) { + return cleanedLabel; + } + // add new suffix if column has a time scale now + return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; +} From bc5d789d917c0faa61ae5a06c632a5fd4da52426 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 26 Nov 2020 11:23:54 +0100 Subject: [PATCH 07/10] add tests and fix label input --- .../dimension_panel/dimension_editor.tsx | 24 ++++- .../dimension_panel/dimension_panel.test.tsx | 54 +++++++++++ .../dimension_panel/time_scaling.tsx | 6 +- .../definitions/calculations/derivative.tsx | 2 + .../calculations/moving_average.tsx | 2 + .../operations/definitions/count.tsx | 6 +- .../operations/definitions/metrics.tsx | 7 +- .../operations/time_scale_utils.test.ts | 92 +++++++++++++++++++ .../operations/time_scale_utils.ts | 24 +++++ 9 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 696984c19e744..62de601bb7888 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -6,7 +6,7 @@ import './dimension_editor.scss'; import _ from 'lodash'; -import React, { useState, useMemo } from 'react'; +import React, { useState, useMemo, useEffect, useRef } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiListGroup, @@ -44,10 +44,30 @@ export interface DimensionEditorProps extends IndexPatternDimensionEditorProps { currentIndexPattern: IndexPattern; } +/** + * This component shows a debounced input for the label of a dimension. It will update on root state changes + * if no debounced changes are in flight because the user is currently typing into the input. + */ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => { const [inputValue, setInputValue] = useState(value); + const unflushedChanges = useRef(false); + + const onChangeDebounced = useMemo(() => { + const callback = _.debounce((val: string) => { + onChange(val); + unflushedChanges.current = false; + }, 256); + return (val: string) => { + unflushedChanges.current = true; + callback(val); + }; + }, [onChange]); - const onChangeDebounced = useMemo(() => _.debounce(onChange, 256), [onChange]); + useEffect(() => { + if (!unflushedChanges.current && value !== inputValue) { + setInputValue(value); + } + }, [value, inputValue]); const handleInputChange = (e: React.ChangeEvent) => { const val = String(e.target.value); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx index 79102d4e34411..29a0586c92ffe 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx @@ -878,6 +878,60 @@ describe('IndexPatternDimensionEditorPanel', () => { }); }); + it('should carry over time scaling to other operation if possible', () => { + const props = getProps({ + timeScale: 'h', + sourceField: 'bytes', + operationType: 'sum', + label: 'Sum of bytes per hour', + }); + wrapper = mount(); + wrapper + .find('button[data-test-subj="lns-indexPatternDimension-count incompatible"]') + .simulate('click'); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: 'h', + label: 'Count of records per hour', + }), + }, + }, + }, + }); + }); + + it('should not carry over time scaling if the other operation does not support it', () => { + const props = getProps({ + timeScale: 'h', + sourceField: 'bytes', + operationType: 'sum', + label: 'Sum of bytes per hour', + }); + wrapper = mount(); + wrapper.find('button[data-test-subj="lns-indexPatternDimension-avg"]').simulate('click'); + expect(props.setState).toHaveBeenCalledWith({ + ...props.state, + layers: { + first: { + ...props.state.layers.first, + columns: { + ...props.state.layers.first.columns, + col2: expect.objectContaining({ + timeScale: undefined, + label: 'Average of bytes', + }), + }, + }, + }, + }); + }); + it('should allow to change time scaling', () => { const props = getProps({ timeScale: 's', label: 'Count of records per second' }); wrapper = mount(); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index b8a134d9cb88b..4a0bf39800e4f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -112,7 +112,7 @@ export function TimeScaling({ }} > {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Show with normalized time units', + defaultMessage: 'Show with normalized units', })} @@ -134,8 +134,8 @@ export function TimeScaling({ > {i18n.translate('xpack.lens.indexPattern.timeScale.label', { - defaultMessage: 'Normalize by time unit', - })} + defaultMessage: 'Normalize by unit', + })}{' '} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx index 12ca499f890b3..41fe361c7ba9c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx @@ -13,6 +13,7 @@ import { dateBasedOperationToExpression, hasDateField, } from './utils'; +import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils'; import { OperationDefinition } from '..'; const ofName = buildLabelFunction((name?: string) => { @@ -85,6 +86,7 @@ export const derivativeOperation: OperationDefinition< isTransferable: (column, newIndexPattern) => { return hasDateField(newIndexPattern); }, + onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, getErrorMessage: (layer: IndexPatternLayer) => { return checkForDateHistogram( layer, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx index 8e460e86919ed..522899662fbd1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx @@ -19,6 +19,7 @@ import { } from './utils'; import { updateColumnParam } from '../../layer_helpers'; import { useDebounceWithOptions } from '../helpers'; +import { adjustTimeScaleOnOtherColumnChange } from '../../time_scale_utils'; import type { OperationDefinition, ParamEditorProps } from '..'; const ofName = buildLabelFunction((name?: string) => { @@ -97,6 +98,7 @@ export const movingAverageOperation: OperationDefinition< isTransferable: (column, newIndexPattern) => { return hasDateField(newIndexPattern); }, + onOtherColumnChanged: adjustTimeScaleOnOtherColumnChange, getErrorMessage: (layer: IndexPatternLayer) => { return checkForDateHistogram( layer, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index 4156441bc0632..8cb95de72f97e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -8,7 +8,10 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField } from '../../types'; -import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; +import { + adjustTimeScaleLabelSuffix, + adjustTimeScaleOnOtherColumnChange, +} from '../time_scale_utils'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', @@ -61,6 +64,7 @@ export const countOperation: OperationDefinition ({ id: columnId, enabled: true, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index 7697c6928ec51..45ba721981ed5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -11,7 +11,10 @@ import { FieldBasedIndexPatternColumn, BaseIndexPatternColumn, } from './column_types'; -import { adjustTimeScaleLabelSuffix } from '../time_scale_utils'; +import { + adjustTimeScaleLabelSuffix, + adjustTimeScaleOnOtherColumnChange, +} from '../time_scale_utils'; type MetricColumn = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { @@ -68,6 +71,8 @@ function buildMetricOperation>({ (!newField.aggregationRestrictions || newField.aggregationRestrictions![type]) ); }, + onOtherColumnChanged: (column, otherColumns) => + optionalTimeScaling ? adjustTimeScaleOnOtherColumnChange(column, otherColumns) : column, getDefaultLabel: (column, indexPattern, columns) => labelLookup(indexPattern.getFieldByName(column.sourceField)!.displayName, column), buildColumn: ({ field, previousColumn }) => ({ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts new file mode 100644 index 0000000000000..841011c588433 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.test.ts @@ -0,0 +1,92 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { TimeScaleUnit } from '../time_scale'; +import { IndexPatternColumn } from './definitions'; +import { adjustTimeScaleLabelSuffix, adjustTimeScaleOnOtherColumnChange } from './time_scale_utils'; + +export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; + +describe('time scale utils', () => { + describe('adjustTimeScaleLabelSuffix', () => { + it('should should remove existing suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc per second', 's', undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc per hour', 'h', undefined)).toEqual('abc'); + }); + + it('should add suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc', undefined, 's')).toEqual('abc per second'); + expect(adjustTimeScaleLabelSuffix('abc', undefined, 'd')).toEqual('abc per day'); + }); + + it('should change suffix', () => { + expect(adjustTimeScaleLabelSuffix('abc per second', 's', 'd')).toEqual('abc per day'); + expect(adjustTimeScaleLabelSuffix('abc per day', 'd', 's')).toEqual('abc per second'); + }); + + it('should keep current state', () => { + expect(adjustTimeScaleLabelSuffix('abc', undefined, undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc per day', 'd', 'd')).toEqual('abc per day'); + }); + + it('should not fail on inconsistent input', () => { + expect(adjustTimeScaleLabelSuffix('abc', 's', undefined)).toEqual('abc'); + expect(adjustTimeScaleLabelSuffix('abc', 's', 'd')).toEqual('abc per day'); + expect(adjustTimeScaleLabelSuffix('abc per day', 's', undefined)).toEqual('abc per day'); + }); + }); + + describe('adjustTimeScaleOnOtherColumnChange', () => { + const baseColumn: IndexPatternColumn = { + operationType: 'count', + sourceField: 'Records', + label: 'Count of records per second', + dataType: 'number', + isBucketed: false, + timeScale: 's', + }; + it('should keep column if there is no time scale', () => { + const column = { ...baseColumn, timeScale: undefined }; + expect(adjustTimeScaleOnOtherColumnChange(column, { col1: column })).toBe(column); + }); + + it('should keep time scale if there is a date histogram', () => { + expect( + adjustTimeScaleOnOtherColumnChange(baseColumn, { + col1: baseColumn, + col2: { + operationType: 'date_histogram', + dataType: 'date', + isBucketed: true, + label: '', + }, + }) + ).toBe(baseColumn); + }); + + it('should remove time scale if there is no date histogram', () => { + expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( + 'timeScale', + undefined + ); + }); + + it('should remove suffix from label', () => { + expect(adjustTimeScaleOnOtherColumnChange(baseColumn, { col1: baseColumn })).toHaveProperty( + 'label', + 'Count of records' + ); + }); + + it('should keep custom label', () => { + const column = { ...baseColumn, label: 'abc', customLabel: true }; + expect(adjustTimeScaleOnOtherColumnChange(column, { col1: column })).toHaveProperty( + 'label', + 'abc' + ); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts index 1d5ed796034d1..5d525e573a617 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/time_scale_utils.ts @@ -6,6 +6,7 @@ import { unitSuffixesLong } from '../suffix_formatter'; import { TimeScaleUnit } from '../time_scale'; +import { BaseIndexPatternColumn } from './definitions/column_types'; export const DEFAULT_TIME_SCALE = 's' as TimeScaleUnit; @@ -28,3 +29,26 @@ export function adjustTimeScaleLabelSuffix( // add new suffix if column has a time scale now return `${cleanedLabel} ${unitSuffixesLong[newTimeScale]}`; } + +export function adjustTimeScaleOnOtherColumnChange( + column: T, + columns: Partial> +) { + if (!column.timeScale) { + return column; + } + const hasDateHistogram = Object.values(columns).some( + (col) => col?.operationType === 'date_histogram' + ); + if (hasDateHistogram) { + return column; + } + if (column.customLabel) { + return column; + } + return { + ...column, + timeScale: undefined, + label: adjustTimeScaleLabelSuffix(column.label, column.timeScale, undefined), + }; +} From 07fdddad0f75bb44f6914c0b0e591dd727d8ae52 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 26 Nov 2020 11:37:01 +0100 Subject: [PATCH 08/10] change wording --- .../indexpattern_datasource/dimension_panel/time_scaling.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index 4a0bf39800e4f..730de64f1a7dc 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -112,7 +112,7 @@ export function TimeScaling({ }} > {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Show with normalized units', + defaultMessage: 'Normalize by time unit', })} @@ -129,7 +129,7 @@ export function TimeScaling({ From 758311a9b61dadba98c0a2127b9d98bf490e0c81 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 26 Nov 2020 11:42:56 +0100 Subject: [PATCH 09/10] adjust wording --- .../indexpattern_datasource/dimension_panel/time_scaling.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx index 730de64f1a7dc..d5a90f7275279 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/time_scaling.tsx @@ -112,7 +112,7 @@ export function TimeScaling({ }} > {i18n.translate('xpack.lens.indexPattern.timeScale.enableTimeScale', { - defaultMessage: 'Normalize by time unit', + defaultMessage: 'Normalize by unit', })} @@ -129,7 +129,7 @@ export function TimeScaling({ From 6ab3f082d0e0ba6a843a79cc9fc629cf968c8aa6 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 30 Nov 2020 08:14:09 +0100 Subject: [PATCH 10/10] move tests --- .../operations/definitions.test.ts | 215 ++++++++++++++++++ .../operations/definitions/index.ts | 6 + .../operations/layer_helpers.test.ts | 97 -------- 3 files changed, 221 insertions(+), 97 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions.test.ts diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions.test.ts new file mode 100644 index 0000000000000..18aa15badec4f --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions.test.ts @@ -0,0 +1,215 @@ +/* + * 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 { + sumOperation, + averageOperation, + countOperation, + counterRateOperation, + movingAverageOperation, + derivativeOperation, +} from './definitions'; +import { getFieldByNameFactory } from '../pure_helpers'; +import { documentField } from '../document_field'; +import { IndexPattern, IndexPatternLayer, IndexPatternField } from '../types'; +import { IndexPatternColumn } from '.'; + +const indexPatternFields = [ + { + name: 'timestamp', + displayName: 'timestampLabel', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'start_date', + displayName: 'start_date', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'bytes', + displayName: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'memory', + displayName: 'memory', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'source', + displayName: 'source', + type: 'string', + aggregatable: true, + searchable: true, + }, + { + name: 'dest', + displayName: 'dest', + type: 'string', + aggregatable: true, + searchable: true, + }, + documentField, +]; + +const indexPattern = { + id: '1', + title: 'my-fake-index-pattern', + timeFieldName: 'timestamp', + hasRestrictions: false, + fields: indexPatternFields, + getFieldByName: getFieldByNameFactory([...indexPatternFields]), +}; + +const baseColumnArgs: { + previousColumn: IndexPatternColumn; + indexPattern: IndexPattern; + layer: IndexPatternLayer; + field: IndexPatternField; +} = { + previousColumn: { + label: 'Count of records per hour', + timeScale: 'h', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + indexPattern, + layer: { + columns: {}, + columnOrder: [], + indexPatternId: '1', + }, + field: indexPattern.fields[2], +}; + +describe('time scale transition', () => { + it('should carry over time scale and adjust label on operation from count to sum', () => { + expect( + sumOperation.buildColumn({ + ...baseColumnArgs, + }) + ).toEqual( + expect.objectContaining({ + timeScale: 'h', + label: 'Sum of bytes per hour', + }) + ); + }); + + it('should carry over time scale and adjust label on operation from count to calculation', () => { + [counterRateOperation, movingAverageOperation, derivativeOperation].forEach( + (calculationOperation) => { + const result = calculationOperation.buildColumn({ + ...baseColumnArgs, + referenceIds: [], + }); + expect(result.timeScale).toEqual('h'); + expect(result.label).toContain('per hour'); + } + ); + }); + + it('should carry over time scale and adjust label on operation from sum to count', () => { + expect( + countOperation.buildColumn({ + ...baseColumnArgs, + previousColumn: { + label: 'Sum of bytes per hour', + timeScale: 'h', + dataType: 'number', + isBucketed: false, + operationType: 'sum', + sourceField: 'bytes', + }, + }) + ).toEqual( + expect.objectContaining({ + timeScale: 'h', + label: 'Count of records per hour', + }) + ); + }); + + it('should not set time scale if it was not set previously', () => { + expect( + countOperation.buildColumn({ + ...baseColumnArgs, + previousColumn: { + label: 'Sum of bytes', + dataType: 'number', + isBucketed: false, + operationType: 'sum', + sourceField: 'bytes', + }, + }) + ).toEqual( + expect.objectContaining({ + timeScale: undefined, + label: 'Count of records', + }) + ); + }); + + it('should set time scale to default for counter rate', () => { + expect( + counterRateOperation.buildColumn({ + indexPattern, + layer: { + columns: {}, + columnOrder: [], + indexPatternId: '1', + }, + referenceIds: [], + }) + ).toEqual( + expect.objectContaining({ + timeScale: 's', + }) + ); + }); + + it('should adjust label on field change', () => { + expect( + sumOperation.onFieldChange( + { + label: 'Sum of bytes per hour', + timeScale: 'h', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'sum', + sourceField: 'bytes', + }, + indexPattern.fields[3] + ) + ).toEqual( + expect.objectContaining({ + timeScale: 'h', + label: 'Sum of memory per hour', + }) + ); + }); + + it('should not carry over time scale if target does not support time scaling', () => { + const result = averageOperation.buildColumn({ + ...baseColumnArgs, + }); + expect(result.timeScale).toBeUndefined(); + }); +}); 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 61f02dc7a706b..31bb332f791da 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 @@ -99,6 +99,12 @@ export { filtersOperation } from './filters'; export { dateHistogramOperation } from './date_histogram'; export { minOperation, averageOperation, sumOperation, maxOperation } from './metrics'; export { countOperation } from './count'; +export { + cumulativeSumOperation, + counterRateOperation, + derivativeOperation, + movingAverageOperation, +} from './calculations'; /** * Properties passed to the operation-specific part of the popover editor 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 31316f6e080bb..0d103a766c23a 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 @@ -436,8 +436,6 @@ describe('state_helpers', () => { ); }); }); - - // TODO add test for mandatory time scale operation once counter rate is implemented }); describe('replaceColumn', () => { @@ -728,101 +726,6 @@ describe('state_helpers', () => { ); }); - describe('time scale transition', () => { - it('should carry over time scale and adjust label on operation', () => { - expect( - replaceColumn({ - layer: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Count of records per hour', - timeScale: 'h', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'count', - sourceField: 'Records', - }, - }, - }, - indexPattern, - columnId: 'col1', - op: 'sum', - field: indexPattern.fields[2], - }).columns.col1 - ).toEqual( - expect.objectContaining({ - timeScale: 'h', - label: 'Sum of bytes per hour', - }) - ); - }); - - it('should carry over time scale and retain custom label', () => { - expect( - replaceColumn({ - layer: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Custom label', - customLabel: true, - timeScale: 'h', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'count', - sourceField: 'Records', - }, - }, - }, - indexPattern, - columnId: 'col1', - op: 'sum', - field: indexPattern.fields[2], - }).columns.col1 - ).toEqual( - expect.objectContaining({ - timeScale: 'h', - label: 'Custom label', - }) - ); - }); - - it('should not carry over time scale if target does not support time scaling', () => { - const result = replaceColumn({ - layer: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: '', - timeScale: 'h', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'count', - sourceField: 'Records', - }, - }, - }, - indexPattern, - columnId: 'col1', - op: 'avg', - field: indexPattern.fields[2], - }).columns.col1; - expect(result.timeScale).toBeUndefined(); - }); - - // TODO add test for mandatory time scale operation once counter rate is implemented - }); - it('should execute adjustments for other columns', () => { const termsColumn: TermsIndexPatternColumn = { label: 'Top values of source',