From e2354f5650da63d48fcfdd3a8b83325a83aa13d1 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 16 Nov 2020 16:44:21 +0100 Subject: [PATCH 01/27] feat: last value sketch --- .../operations/definitions/index.ts | 6 +- .../definitions/last_value.test.tsx | 5 + .../operations/definitions/last_value.tsx | 192 ++++++++++++++++++ .../operations/definitions/terms/index.tsx | 2 +- 4 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx 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..bd8c5bdfba7d7 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 @@ -23,6 +23,7 @@ import { } from './metrics'; import { dateHistogramOperation, DateHistogramIndexPatternColumn } from './date_histogram'; import { countOperation, CountIndexPatternColumn } from './count'; +import { lastValueOperation, LastValueIndexPatternColumn } from './last_value'; import { StateSetter, OperationMetadata } from '../../../types'; import { BaseIndexPatternColumn } from './column_types'; import { IndexPatternPrivateState, IndexPattern, IndexPatternField } from '../../types'; @@ -46,7 +47,8 @@ export type IndexPatternColumn = | CardinalityIndexPatternColumn | SumIndexPatternColumn | MedianIndexPatternColumn - | CountIndexPatternColumn; + | CountIndexPatternColumn + | LastValueIndexPatternColumn; export type FieldBasedIndexPatternColumn = Extract; @@ -63,6 +65,7 @@ const internalOperationDefinitions = [ cardinalityOperation, sumOperation, medianOperation, + lastValueOperation, countOperation, rangeOperation, ]; @@ -73,6 +76,7 @@ export { filtersOperation } from './filters'; export { dateHistogramOperation } from './date_histogram'; export { minOperation, averageOperation, sumOperation, maxOperation } from './metrics'; export { countOperation } from './count'; +export { lastValueOperation } from './last_value'; /** * Properties passed to the operation-specific part of the popover editor diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx new file mode 100644 index 0000000000000..41bc2aa258807 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -0,0 +1,5 @@ +/* + * 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. + */ diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx new file mode 100644 index 0000000000000..46a59e21a25e0 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -0,0 +1,192 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import React from 'react'; +import { i18n } from '@kbn/i18n'; +import { EuiFormRow, EuiSelect, EuiSwitch } from '@elastic/eui'; +import { OperationDefinition } from './index'; +import { FieldBasedIndexPatternColumn } from './column_types'; +import { IndexPatternField, IndexPattern } from '../../types'; +import { updateColumnParam } from '../layer_helpers'; +import { DataType } from '../../../types'; + +function ofName(name: string) { + return i18n.translate('xpack.lens.indexPattern.lastValueOf', { + defaultMessage: 'Last value of {name}', + values: { name }, + }); +} + +const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); + +function getDateFields(indexPattern: IndexPattern): IndexPatternField[] { + if (indexPattern.timeFieldName) { + return [ + ...indexPattern.fields.filter((field) => field.name === indexPattern.timeFieldName), + ...indexPattern.fields.filter( + (field) => field.type === 'date' && indexPattern.timeFieldName !== field.name + ), + ]; + } else { + return indexPattern.fields.filter((field) => field.type === 'date'); + } +} + +export interface LastValueIndexPatternColumn extends FieldBasedIndexPatternColumn { + operationType: 'last_value'; + params: { + sortField: string; + sortOrder: 'asc' | 'desc'; + // last value on numeric fields can be formatted + format?: { + id: string; + params?: { + decimals: number; + }; + }; + }; +} + +export const lastValueOperation: OperationDefinition = { + type: 'last_value', + displayName: i18n.translate('xpack.lens.indexPattern.lastValue', { + defaultMessage: 'Last value', + }), + input: 'field', + onFieldChange: (oldColumn, field) => { + const newParams = { ...oldColumn.params }; + + if ('format' in newParams && field.type !== 'number') { + delete newParams.format; + } + return { + ...oldColumn, + dataType: field.type as DataType, + label: ofName(field.displayName), + sourceField: field.name, + params: newParams, + }; + }, + getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { + if (supportedTypes.has(type)) { + return { dataType: type as DataType, isBucketed: false, scale: 'ordinal' }; + } + }, + buildColumn({ field, previousColumn, indexPattern }) { + console.log(indexPattern); + return { + label: ofName(field.displayName), + dataType: 'number', + operationType: 'last_value', + isBucketed: false, + scale: 'ratio', + sourceField: field.name, + params: { + sortOrder: + (previousColumn?.params && + 'sortOrder' in previousColumn.params && + previousColumn?.params?.sortOrder) || + 'desc', + sortField: + indexPattern.timeFieldName || + indexPattern.fields.filter((field) => field.type === 'date')[0] || + field.name, + }, + }; + }, + toEsAggsConfig: (column, columnId) => ({ + id: columnId, + enabled: true, + schema: 'metric', + type: 'top_hits', + params: { + field: column.sourceField, + aggregate: 'concat', + size: 1, + sortOrder: column.params?.sortOrder, + sortField: column.params.sortField, + }, + }), + + isTransferable: (column, newIndexPattern) => { + const newField = newIndexPattern.getFieldByName(column.sourceField); + return Boolean(newField && newField.type === 'number'); + }, + + paramEditor: ({ state, setState, currentColumn, layerId }) => { + const dateFields = getDateFields(state.indexPatterns[state.layers[layerId].indexPatternId]); + if (!dateFields.length) { + } + // console.log(state.indexPatterns[state.layers[layerId].indexPatternId]); + return ( + <> + + { + console.log('chlen'); + setState( + updateColumnParam({ + state, + layerId, + currentColumn, + paramName: 'sortOrder', + value: currentColumn.params?.sortOrder === 'desc' ? 'asc' : 'desc', + }) + ); + }} + /> + + {dateFields.length ? ( + + { + return { + value: field.name, + text: field.displayName, + }; + })} + value={currentColumn.params.sortField} + onChange={(e: React.ChangeEvent) => + setState( + updateColumnParam({ + state, + layerId, + currentColumn, + paramName: 'sortField', + value: e.target.value, + }) + ) + } + aria-label={i18n.translate('xpack.lens.indexPattern.terms.orderBy', { + defaultMessage: 'Order by', + })} + /> + + ) : null} + + ); + }, +}; 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..8d2b4182ac3e8 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 @@ -31,7 +31,7 @@ function ofName(name: string) { } function isSortableByColumn(column: IndexPatternColumn) { - return !column.isBucketed; + return !column.isBucketed && column.operationType !== 'last_value'; } const DEFAULT_SIZE = 3; From 6a6dd4e510705e3461c7d0b5669ba5b77be12de6 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 17 Nov 2020 15:01:49 +0100 Subject: [PATCH 02/27] feat: rollup indices --- .../operations/definitions/last_value.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 46a59e21a25e0..2b5b7d78b469e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -69,8 +69,8 @@ export const lastValueOperation: OperationDefinition { - if (supportedTypes.has(type)) { + getPossibleOperationForField: ({ aggregationRestrictions, type }) => { + if (supportedTypes.has(type) && !aggregationRestrictions) { return { dataType: type as DataType, isBucketed: false, scale: 'ordinal' }; } }, From f80785877e8320bf9dee797cdfab14273f3f3aab Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 18 Nov 2020 10:56:06 +0100 Subject: [PATCH 03/27] added disabledStatus --- .../dimension_panel/dimension_editor.scss | 4 + .../dimension_panel/dimension_editor.tsx | 24 ++- .../operations/definitions/index.ts | 6 + .../operations/definitions/last_value.tsx | 152 ++++++++++-------- .../definitions/terms/terms.test.tsx | 37 ++++- 5 files changed, 154 insertions(+), 69 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.scss b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.scss index 096047da681b9..6bd6808f17b35 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.scss +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.scss @@ -15,6 +15,10 @@ column-gap: $euiSizeXL; } +.lnsIndexPatternDimensionEditor__operation .euiListGroupItem__label { + width: 100%; +} + .lnsIndexPatternDimensionEditor__operation > button { padding-top: 0; padding-bottom: 0; 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..a35c854891fcc 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 @@ -15,6 +15,10 @@ import { EuiSpacer, EuiListGroupItemProps, EuiFormLabel, + EuiFlexGroup, + EuiFlexItem, + EuiToolTip, + EuiIcon, } from '@elastic/eui'; import { IndexPatternDimensionEditorProps } from './dimension_panel'; import { OperationSupportMatrix } from './operation_support'; @@ -122,6 +126,9 @@ export function DimensionEditor(props: DimensionEditorProps) { definition.input === 'field' && fieldByOperation[operationType]?.has(selectedColumn.sourceField)) || (selectedColumn && !hasField(selectedColumn) && definition.input !== 'field'), + disabledStatus: + definition.getDisabledStatus && + definition.getDisabledStatus(state.indexPatterns[state.currentIndexPatternId]), }; }); @@ -135,7 +142,7 @@ export function DimensionEditor(props: DimensionEditorProps) { ); const sideNavItems: EuiListGroupItemProps[] = operationsWithCompatibility.map( - ({ operationType, compatibleWithCurrentField }) => { + ({ operationType, compatibleWithCurrentField, disabledStatus }) => { const isActive = Boolean( incompatibleSelectedOperationType === operationType || (!incompatibleSelectedOperationType && @@ -151,7 +158,18 @@ export function DimensionEditor(props: DimensionEditorProps) { } let label: EuiListGroupItemProps['label'] = operationPanels[operationType].displayName; - if (isActive) { + if (disabledStatus) { + label = ( + + {operationPanels[operationType].displayName} + + + + + + + ); + } else if (isActive) { label = {operationPanels[operationType].displayName}; } @@ -161,6 +179,7 @@ export function DimensionEditor(props: DimensionEditorProps) { color, isActive, size: 's', + isDisabled: !!disabledStatus, className: 'lnsIndexPatternDimensionEditor__operation', 'data-test-subj': `lns-indexPatternDimension-${operationType}${ compatibleWithCurrentField ? '' : ' incompatible' @@ -220,7 +239,6 @@ export function DimensionEditor(props: DimensionEditorProps) { ? currentIndexPattern.getFieldByName(selectedColumn.sourceField) : undefined, }); - setState(mergeLayer({ state, layerId, newLayer })); }, }; 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 bd8c5bdfba7d7..9e32d40b705d1 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 @@ -139,6 +139,12 @@ interface BaseOperationDefinitionProps { * present on the new index pattern. */ transfer?: (column: C, newIndexPattern: IndexPattern) => C; + /** + * if there is some reason to display the operation in the operations list + * but disable it from usage, this function returns the string describing + * the status. Otherwise it returns undefined + */ + getDisabledStatus?: (indexPattern: IndexPattern) => string | undefined; } interface BaseBuildColumnArgs { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 2b5b7d78b469e..6d71d7983e638 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -5,7 +5,7 @@ */ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiFormRow, EuiSelect, EuiSwitch } from '@elastic/eui'; +import { EuiFormRow, EuiSelect, EuiButtonGroup } from '@elastic/eui'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; @@ -74,25 +74,36 @@ export const lastValueOperation: OperationDefinition f.type === 'date')[0].name || + field.name; + + const sortOrder = + (previousColumn?.params && + 'sortOrder' in previousColumn.params && + previousColumn?.params?.sortOrder) || + 'desc'; + return { label: ofName(field.displayName), - dataType: 'number', + dataType: field.type as DataType, operationType: 'last_value', isBucketed: false, scale: 'ratio', sourceField: field.name, params: { - sortOrder: - (previousColumn?.params && - 'sortOrder' in previousColumn.params && - previousColumn?.params?.sortOrder) || - 'desc', - sortField: - indexPattern.timeFieldName || - indexPattern.fields.filter((field) => field.type === 'date')[0] || - field.name, + sortOrder, + sortField, }, }; }, @@ -105,87 +116,98 @@ export const lastValueOperation: OperationDefinition { const newField = newIndexPattern.getFieldByName(column.sourceField); - return Boolean(newField && newField.type === 'number'); + return Boolean(newField && newField.type === column.dataType); }, paramEditor: ({ state, setState, currentColumn, layerId }) => { const dateFields = getDateFields(state.indexPatterns[state.layers[layerId].indexPatternId]); - if (!dateFields.length) { - } - // console.log(state.indexPatterns[state.layers[layerId].indexPatternId]); + const sortOrderButtons = [ + { + id: `lns-lastValue-ascending`, + label: 'Ascending', + value: 'asc', + }, + { + id: `lns-lastValue-descending`, + label: 'Descending', + value: 'desc', + }, + ]; return ( <> - { + return { + value: field.name, + text: field.displayName, + }; })} - showLabel={false} - data-test-subj="indexPattern-lastValue-switch" - name="lastValueSwitch" - checked={currentColumn.params?.sortOrder === 'desc'} - onChange={() => { - console.log('chlen'); + value={currentColumn.params.sortField} + onChange={(e: React.ChangeEvent) => + setState( + updateColumnParam({ + state, + layerId, + currentColumn, + paramName: 'sortField', + value: e.target.value, + }) + ) + } + aria-label={i18n.translate('xpack.lens.indexPattern.terms.orderBy', { + defaultMessage: 'Order by', + })} + /> + + + value === currentColumn.params?.sortOrder)!.id + } + onChange={(optionId: string) => { + const value = sortOrderButtons.find(({ id }) => id === optionId)!.value; setState( updateColumnParam({ state, layerId, currentColumn, paramName: 'sortOrder', - value: currentColumn.params?.sortOrder === 'desc' ? 'asc' : 'desc', + value, }) ); }} /> - {dateFields.length ? ( - - { - return { - value: field.name, - text: field.displayName, - }; - })} - value={currentColumn.params.sortField} - onChange={(e: React.ChangeEvent) => - setState( - updateColumnParam({ - state, - layerId, - currentColumn, - paramName: 'sortField', - value: e.target.value, - }) - ) - } - aria-label={i18n.translate('xpack.lens.indexPattern.terms.orderBy', { - defaultMessage: 'Order by', - })} - /> - - ) : null} ); }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index bba7bda308b72..ab3cc45491cf4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -364,7 +364,7 @@ describe('terms', () => { }); describe('onOtherColumnChanged', () => { - it('should keep the column if order by column still exists and is metric', () => { + it('should keep the column if order by column still exists and is isSortableByColumn metric', () => { const initialColumn: TermsIndexPatternColumn = { label: 'Top value of category', dataType: 'string', @@ -391,6 +391,41 @@ describe('terms', () => { expect(updatedColumn).toBe(initialColumn); }); + it('should switch to alphabetical ordering if metric is of type last_value', () => { + const initialColumn: TermsIndexPatternColumn = { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'terms', + params: { + orderBy: { type: 'column', columnId: 'col1' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }; + const updatedColumn = termsOperation.onOtherColumnChanged!(initialColumn, { + col1: { + label: 'Last Value', + dataType: 'number', + isBucketed: false, + sourceField: 'bytes', + operationType: 'last_value', + params: { + sortOrder: 'desc', + sortField: 'time', + }, + }, + }); + expect(updatedColumn.params).toEqual( + expect.objectContaining({ + orderBy: { type: 'alphabetical' }, + }) + ); + }); + it('should switch to alphabetical ordering if there are no columns to order by', () => { const termsColumn = termsOperation.onOtherColumnChanged!( { From 4555d51818a220f9b678e41632f4c20485eb4634 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 18 Nov 2020 11:00:17 +0100 Subject: [PATCH 04/27] update old tests --- .../dimension_panel/dimension_panel.test.tsx | 2 +- .../operations/operations.test.ts | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) 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..7855852223b6d 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 @@ -975,12 +975,12 @@ describe('IndexPatternDimensionEditorPanel', () => { expect(items.map(({ label }: { label: React.ReactNode }) => label)).toEqual([ 'Average', 'Count', + 'Last value', 'Maximum', 'Median', 'Minimum', 'Sum', 'Unique count', - '\u00a0', ]); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index d6f5b10cf64e1..5570b0d76e1c0 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -279,6 +279,34 @@ describe('getOperationTypesForField', () => { }, ], }, + Object { + "operationMetaData": Object { + "dataType": "number", + "isBucketed": false, + "scale": "ordinal", + }, + "operations": Array [ + Object { + "field": "bytes", + "operationType": "last_value", + "type": "field", + }, + ], + }, + Object { + "operationMetaData": Object { + "dataType": "string", + "isBucketed": false, + "scale": "ordinal", + }, + "operations": Array [ + Object { + "field": "source", + "operationType": "last_value", + "type": "field", + }, + ], + }, ] `); }); From 2e3708f1510a9719eabed79457a149e0c708192d Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 18 Nov 2020 14:25:05 +0100 Subject: [PATCH 05/27] tests added --- .../definitions/last_value.test.tsx | 469 ++++++++++++++++++ .../operations/definitions/last_value.tsx | 2 +- 2 files changed, 470 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 41bc2aa258807..cf49b4dd6a124 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -3,3 +3,472 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + +import React from 'react'; +import { shallow } from 'enzyme'; +import { EuiButtonGroup } from '@elastic/eui'; +import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; +import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; +import { dataPluginMock } from '../../../../../../../src/plugins/data/public/mocks'; +import { createMockedIndexPattern } from '../../mocks'; +import { LastValueIndexPatternColumn } from './last_value'; +import { lastValueOperation } from './index'; +import { IndexPatternPrivateState, IndexPattern } from '../../types'; + +const defaultProps = { + storage: {} as IStorageWrapper, + uiSettings: {} as IUiSettingsClient, + savedObjectsClient: {} as SavedObjectsClientContract, + dateRange: { fromDate: 'now-1d', toDate: 'now' }, + data: dataPluginMock.createStartContract(), + http: {} as HttpSetup, +}; + +describe('last_value', () => { + let state: IndexPatternPrivateState; + const InlineOptions = lastValueOperation.paramEditor!; + + beforeEach(() => { + const indexPattern = createMockedIndexPattern(); + state = { + indexPatternRefs: [], + indexPatterns: { + '1': { + ...indexPattern, + hasRestrictions: false, + } as IndexPattern, + }, + existingFields: {}, + currentIndexPatternId: '1', + isFirstExistenceFetch: false, + layers: { + first: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Top value of category', + dataType: 'string', + isBucketed: true, + operationType: 'terms', + params: { + orderBy: { type: 'alphabetical' }, + size: 3, + orderDirection: 'asc', + }, + sourceField: 'category', + }, + col2: { + label: 'Last value of a', + dataType: 'number', + isBucketed: false, + sourceField: 'a', + operationType: 'last_value', + params: { + sortField: 'datefield', + sortOrder: 'asc', + }, + }, + }, + }, + }, + }; + }); + + describe('toEsAggsConfig', () => { + it('should reflect params correctly', () => { + const lastValueColumn = state.layers.first.columns.col2 as LastValueIndexPatternColumn; + const esAggsConfig = lastValueOperation.toEsAggsConfig( + { ...lastValueColumn, params: { ...lastValueColumn.params } }, + 'col1', + {} as IndexPattern + ); + expect(esAggsConfig).toEqual( + expect.objectContaining({ + params: expect.objectContaining({ + aggregate: 'concat', + field: 'a', + size: 1, + sortField: 'datefield', + sortOrder: 'asc', + }), + }) + ); + }); + }); + + describe('onFieldChange', () => { + it('should change correctly to new field', () => { + const oldColumn: LastValueIndexPatternColumn = { + operationType: 'last_value', + sourceField: 'source', + label: 'Last value of source', + isBucketed: true, + dataType: 'string', + params: { + sortField: 'datefield', + sortOrder: 'asc', + }, + }; + const indexPattern = createMockedIndexPattern(); + const newNumberField = indexPattern.getFieldByName('bytes')!; + const column = lastValueOperation.onFieldChange(oldColumn, newNumberField); + + expect(column).toEqual( + expect.objectContaining({ + dataType: 'number', + sourceField: 'bytes', + params: expect.objectContaining({ + sortField: 'datefield', + sortOrder: 'asc', + }), + }) + ); + expect(column.label).toContain('bytes'); + }); + + it('should remove numeric parameters when changing away from number', () => { + const oldColumn: LastValueIndexPatternColumn = { + operationType: 'last_value', + sourceField: 'bytes', + label: 'Last value of bytes', + isBucketed: false, + dataType: 'number', + params: { + sortField: 'datefield', + sortOrder: 'asc', + }, + }; + const indexPattern = createMockedIndexPattern(); + const newStringField = indexPattern.fields.find((i) => i.name === 'source')!; + + const column = lastValueOperation.onFieldChange(oldColumn, newStringField); + expect(column).toHaveProperty('dataType', 'string'); + expect(column).toHaveProperty('sourceField', 'source'); + expect(column.params.format).toBeUndefined(); + }); + }); + + describe('getPossibleOperationForField', () => { + it('should return operation with the right type', () => { + expect( + lastValueOperation.getPossibleOperationForField({ + aggregatable: true, + searchable: true, + name: 'test', + displayName: 'test', + type: 'boolean', + }) + ).toEqual({ + dataType: 'boolean', + isBucketed: false, + scale: 'ordinal', + }); + + expect( + lastValueOperation.getPossibleOperationForField({ + aggregatable: true, + searchable: true, + name: 'test', + displayName: 'test', + type: 'ip', + }) + ).toEqual({ + dataType: 'ip', + isBucketed: false, + scale: 'ordinal', + }); + }); + + it('should not return an operation if restrictions prevent terms', () => { + expect( + lastValueOperation.getPossibleOperationForField({ + aggregatable: true, + searchable: true, + name: 'test', + displayName: 'test', + type: 'string', + aggregationRestrictions: { + terms: { + agg: 'terms', + }, + }, + }) + ).toEqual(undefined); + + expect( + lastValueOperation.getPossibleOperationForField({ + aggregatable: true, + aggregationRestrictions: {}, + searchable: true, + name: 'test', + displayName: 'test', + type: 'string', + }) + ).toEqual(undefined); + // does it have to be aggregatable? + expect( + lastValueOperation.getPossibleOperationForField({ + aggregatable: false, + searchable: true, + name: 'test', + displayName: 'test', + type: 'string', + }) + ).toEqual({ dataType: 'string', isBucketed: false, scale: 'ordinal' }); + }); + }); + + describe('buildColumn', () => { + it('should use type from the passed field', () => { + const lastValueColumn = lastValueOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + columns: {}, + }); + expect(lastValueColumn.dataType).toEqual('boolean'); + }); + + it('should set sortOrder to desc by default', () => { + const lastValueColumn = lastValueOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + columns: {}, + }); + expect(lastValueColumn.params.sortOrder).toEqual('desc'); + }); + + it('should use indexPattern timeFieldName as a default sortField', () => { + const lastValueColumn = lastValueOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + }); + expect(lastValueColumn.params).toEqual( + expect.objectContaining({ + sortField: 'timestamp', + }) + ); + }); + + it('should use first indexPattern date field if there is no default timefieldName', () => { + const indexPattern = createMockedIndexPattern(); + const indexPatternNoTimeField = { + ...indexPattern, + timeFieldName: undefined, + fields: [ + { + aggregatable: true, + searchable: true, + type: 'date', + name: 'datefield', + displayName: 'datefield', + }, + { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + ], + }; + const lastValueColumn = lastValueOperation.buildColumn({ + indexPattern: indexPatternNoTimeField, + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + }); + expect(lastValueColumn.params).toEqual( + expect.objectContaining({ + sortField: 'datefield', + }) + ); + }); + }); + + it('should return disabledStatus if indexPattern does contain date field', () => { + const indexPattern = createMockedIndexPattern(); + + expect(lastValueOperation.getDisabledStatus!(indexPattern)).toEqual(undefined); + + const indexPatternWithoutTimeFieldName = { + ...indexPattern, + timeFieldName: undefined, + }; + expect(lastValueOperation.getDisabledStatus!(indexPatternWithoutTimeFieldName)).toEqual( + undefined + ); + + const indexPatternWithoutTimefields = { + ...indexPatternWithoutTimeFieldName, + fields: indexPattern.fields.filter((f) => f.type !== 'date'), + }; + + const disabledStatus = lastValueOperation.getDisabledStatus!(indexPatternWithoutTimefields); + expect(disabledStatus).toEqual( + 'This function requires the presence of a date field in your index' + ); + }); + + describe('param editor', () => { + it('should render current sortField', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + + const select = instance.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]'); + + expect(select.prop('value')).toEqual('datefield'); + }); + + it('should update state when changing sortField', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + + instance.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').prop('onChange')!({ + target: { + value: 'datefield2', + }, + } as React.ChangeEvent); + + expect(setStateSpy).toHaveBeenCalledWith({ + ...state, + layers: { + first: { + ...state.layers.first, + columns: { + ...state.layers.first.columns, + col2: { + ...state.layers.first.columns.col2, + params: { + ...(state.layers.first.columns.col2 as LastValueIndexPatternColumn).params, + sortField: 'datefield2', + }, + }, + }, + }, + }, + }); + }); + + it('should render sortOrder value and options', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + + const select = instance + .find('[data-test-subj="lns-indexPattern-lastValue-sortOrder"]') + .find(EuiButtonGroup); + + expect(select.prop('idSelected')).toEqual('lns-lastValue-ascending'); + + expect(select.prop('options')!.map(({ value }: { value?: string }) => value)).toEqual([ + 'asc', + 'desc', + ]); + }); + + it('should update state when changing sortOrder', () => { + const setStateSpy = jest.fn(); + const instance = shallow( + + ); + + const select = instance + .find('[data-test-subj="lns-indexPattern-lastValue-sortOrder"]') + .find(EuiButtonGroup); + + select.prop('onChange')!('lns-lastValue-descending'); + expect(setStateSpy).toHaveBeenCalledWith({ + ...state, + layers: { + first: { + ...state.layers.first, + columns: { + ...state.layers.first.columns, + col2: { + ...state.layers.first.columns.col2, + params: { + ...(state.layers.first.columns.col2 as LastValueIndexPatternColumn).params, + sortOrder: 'desc', + }, + }, + }, + }, + }, + }); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 6d71d7983e638..ebe3b63ee1c92 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -151,7 +151,7 @@ export const lastValueOperation: OperationDefinition { return { value: field.name, From df8c5eeaf8cab6b2a0e901d535042e3a055dc7e3 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 18 Nov 2020 17:42:18 +0100 Subject: [PATCH 06/27] fix i18n message --- .../operations/definitions/last_value.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index ebe3b63ee1c92..96a54717273db 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -170,8 +170,8 @@ export const lastValueOperation: OperationDefinition From 0c14e4cfcb16e9b257250d0a55539959848d9dfa Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Fri, 20 Nov 2020 09:59:33 +0100 Subject: [PATCH 07/27] cr --- .../operations/definitions/last_value.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 96a54717273db..f606e6640bf74 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -22,16 +22,19 @@ function ofName(name: string) { const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); function getDateFields(indexPattern: IndexPattern): IndexPatternField[] { + const dateFields = indexPattern.fields.filter((field) => field.type === 'date'); if (indexPattern.timeFieldName) { - return [ - ...indexPattern.fields.filter((field) => field.name === indexPattern.timeFieldName), - ...indexPattern.fields.filter( - (field) => field.type === 'date' && indexPattern.timeFieldName !== field.name - ), - ]; - } else { - return indexPattern.fields.filter((field) => field.type === 'date'); + dateFields.sort(({ name: nameA }, { name: nameB }) => { + if (nameA === indexPattern.timeFieldName) { + return -1; + } + if (nameB === indexPattern.timeFieldName) { + return 1; + } + return 0; + }); } + return dateFields; } export interface LastValueIndexPatternColumn extends FieldBasedIndexPatternColumn { @@ -85,7 +88,7 @@ export const lastValueOperation: OperationDefinition f.type === 'date')[0].name || + indexPattern.fields.find((f) => f.type === 'date')?.name || field.name; const sortOrder = From dba38a51252f80aba619e513d1ad43bbc06c4ef5 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Fri, 20 Nov 2020 11:27:30 +0100 Subject: [PATCH 08/27] (experimental) don't add icon --- .../dimension_panel/dimension_editor.tsx | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index a35c854891fcc..23b76cab53be5 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 @@ -15,10 +15,7 @@ import { EuiSpacer, EuiListGroupItemProps, EuiFormLabel, - EuiFlexGroup, - EuiFlexItem, EuiToolTip, - EuiIcon, } from '@elastic/eui'; import { IndexPatternDimensionEditorProps } from './dimension_panel'; import { OperationSupportMatrix } from './operation_support'; @@ -160,14 +157,9 @@ export function DimensionEditor(props: DimensionEditorProps) { let label: EuiListGroupItemProps['label'] = operationPanels[operationType].displayName; if (disabledStatus) { label = ( - - {operationPanels[operationType].displayName} - - - - - - + + {operationPanels[operationType].displayName} + ); } else if (isActive) { label = {operationPanels[operationType].displayName}; From 1357868be2d8bd7fec6de9e442392539859d8b88 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 23 Nov 2020 09:05:52 +0100 Subject: [PATCH 09/27] fix transferable --- .../operations/definitions/last_value.tsx | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index f606e6640bf74..53579d014416b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -87,9 +87,15 @@ export const lastValueOperation: OperationDefinition f.type === 'date')?.name || - field.name; + indexPattern.timeFieldName || indexPattern.fields.find((f) => f.type === 'date')?.name; + + if (!sortField) { + throw new Error( + i18n.translate('xpack.lens.functions.lastValue.missingSortField', { + defaultMessage: 'This index pattern does not contain any date fields', + }) + ); + } const sortOrder = (previousColumn?.params && @@ -126,7 +132,13 @@ export const lastValueOperation: OperationDefinition { const newField = newIndexPattern.getFieldByName(column.sourceField); - return Boolean(newField && newField.type === column.dataType); + const newTimeField = newIndexPattern.getFieldByName(column.params.sortField); + return Boolean( + newField && + newField.type === column.dataType && + !newField.aggregationRestrictions && + newTimeField?.type === 'date' + ); }, paramEditor: ({ state, setState, currentColumn, layerId }) => { From 3c76ab4136d82b53464ca4525a428f4e65579b0c Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 23 Nov 2020 14:56:55 +0100 Subject: [PATCH 10/27] error handling, tests in progress --- .../dimension_panel/dimension_editor.tsx | 14 ++-- .../dimension_panel/dimension_panel.tsx | 9 +-- .../indexpattern_datasource/indexpattern.tsx | 41 +++++----- .../operations/definitions/cardinality.tsx | 3 + .../operations/definitions/date_histogram.tsx | 2 + .../operations/definitions/index.ts | 4 + .../definitions/last_value.test.tsx | 40 +++++----- .../operations/definitions/last_value.tsx | 79 +++++++++++++++---- .../operations/definitions/metrics.tsx | 2 + .../operations/definitions/ranges/ranges.tsx | 2 + .../operations/definitions/terms/index.tsx | 2 + .../public/indexpattern_datasource/utils.ts | 43 +++++----- 12 files changed, 148 insertions(+), 93 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 81a0ce6c9a080..e3a17b03faf60 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 @@ -107,7 +107,7 @@ export function DimensionEditor(props: DimensionEditorProps) { }) .map((def) => def.type) .filter((type) => fieldByOperation[type]?.size || operationWithoutField.has(type)); - }, [fieldByOperation, operationWithoutField]); + }, [operationWithoutField]); // Operations are compatible if they match inputs. They are always compatible in // the empty state. Field-based operations are not compatible with field-less operations. @@ -129,14 +129,10 @@ export function DimensionEditor(props: DimensionEditorProps) { }; }); - const selectedColumnSourceField = - selectedColumn && 'sourceField' in selectedColumn ? selectedColumn.sourceField : undefined; - - const currentFieldIsInvalid = useMemo( - () => - fieldIsInvalid(selectedColumnSourceField, selectedColumn?.operationType, currentIndexPattern), - [selectedColumnSourceField, selectedColumn?.operationType, currentIndexPattern] - ); + const currentFieldIsInvalid = useMemo(() => fieldIsInvalid(selectedColumn, currentIndexPattern), [ + selectedColumn, + currentIndexPattern, + ]); const sideNavItems: EuiListGroupItemProps[] = operationsWithCompatibility.map( ({ operationType, compatibleWithCurrentField, disabledStatus }) => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx index 94018bd84b517..3ba780c9a53b7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx @@ -12,7 +12,7 @@ import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { DatasourceDimensionTriggerProps, DatasourceDimensionEditorProps } from '../../types'; import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; import { IndexPatternColumn } from '../indexpattern'; -import { fieldIsInvalid } from '../utils'; +import { isColumnInvalid } from '../utils'; import { IndexPatternPrivateState } from '../types'; import { DimensionEditor } from './dimension_editor'; import { DateRange } from '../../../common'; @@ -52,12 +52,9 @@ export const IndexPatternDimensionTriggerComponent = function IndexPatternDimens const selectedColumn: IndexPatternColumn | null = layer.columns[props.columnId] || null; const currentIndexPattern = props.state.indexPatterns[layer.indexPatternId]; - const selectedColumnSourceField = - selectedColumn && 'sourceField' in selectedColumn ? selectedColumn.sourceField : undefined; const currentFieldIsInvalid = useMemo( - () => - fieldIsInvalid(selectedColumnSourceField, selectedColumn?.operationType, currentIndexPattern), - [selectedColumnSourceField, selectedColumn?.operationType, currentIndexPattern] + () => isColumnInvalid(selectedColumn, currentIndexPattern), + [selectedColumn, currentIndexPattern] ); const { columnId, uniqueLabel } = props; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 2c64431867df0..e44f547d09159 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -40,7 +40,7 @@ import { } from './indexpattern_suggestions'; import { - getInvalidFieldsForLayer, + getInvalidColumnsForLayer, getInvalidLayers, isDraggedField, normalizeOperationDataType, @@ -372,7 +372,7 @@ export function getIndexPatternDatasource({ } }) .filter(Boolean) as Array<[number, number]>; - const invalidFieldsPerLayer: string[][] = getInvalidFieldsForLayer( + const invalidColumnsForLayer: string[][] = getInvalidColumnsForLayer( invalidLayers, state.indexPatterns ); @@ -382,33 +382,34 @@ export function getIndexPatternDatasource({ return [ ...layerErrors, ...realIndex.map(([filteredIndex, layerIndex]) => { - const fieldsWithBrokenReferences: string[] = invalidFieldsPerLayer[filteredIndex].map( - (columnId) => { - const column = invalidLayers[filteredIndex].columns[ - columnId - ] as FieldBasedIndexPatternColumn; - return column.sourceField; - } - ); + const columnLabelsWithBrokenReferences: string[] = invalidColumnsForLayer[ + filteredIndex + ].map((columnId) => { + const column = invalidLayers[filteredIndex].columns[ + columnId + ] as FieldBasedIndexPatternColumn; + return column.label; + }); if (originalLayersList.length === 1) { return { shortMessage: i18n.translate( 'xpack.lens.indexPattern.dataReferenceFailureShortSingleLayer', { - defaultMessage: 'Invalid {fields, plural, one {reference} other {references}}.', + defaultMessage: + 'Invalid {columns, plural, one {reference} other {references}}.', values: { - fields: fieldsWithBrokenReferences.length, + columns: columnLabelsWithBrokenReferences.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.`, + defaultMessage: `"{columns}" {columnsLength, plural, one {has an} other {have}} invalid reference.`, values: { - fields: fieldsWithBrokenReferences.join('", "'), - fieldsLength: fieldsWithBrokenReferences.length, + columns: columnLabelsWithBrokenReferences.join('", "'), + columnsLength: columnLabelsWithBrokenReferences.length, }, } ), @@ -417,18 +418,18 @@ export function getIndexPatternDatasource({ return { shortMessage: i18n.translate('xpack.lens.indexPattern.dataReferenceFailureShort', { defaultMessage: - 'Invalid {fieldsLength, plural, one {reference} other {references}} on Layer {layer}.', + 'Invalid {columnsLength, plural, one {reference} other {references}} on Layer {layer}.', values: { layer: layerIndex, - fieldsLength: fieldsWithBrokenReferences.length, + columnsLength: columnLabelsWithBrokenReferences.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}".`, + defaultMessage: `Layer {layer} has {columnsLength, plural, one {an invalid} other {invalid}} {columnsLength, plural, one {reference} other {references}} in "{columns}".`, values: { layer: layerIndex, - fields: fieldsWithBrokenReferences.join('", "'), - fieldsLength: fieldsWithBrokenReferences.length, + columns: columnLabelsWithBrokenReferences.join('", "'), + columnsLength: columnLabelsWithBrokenReferences.length, }, }), }; 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 fd3ca4319669e..21ecd259fb5af 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 @@ -8,6 +8,8 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; +import { fieldIsInvalid } from '../../utils'; + const supportedTypes = new Set(['string', 'boolean', 'number', 'ip', 'date']); const SCALE = 'ratio'; @@ -42,6 +44,7 @@ export const cardinalityOperation: OperationDefinition { const newField = newIndexPattern.getFieldByName(column.sourceField); 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 efac9c151a435..7ca604b31b036 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 @@ -23,6 +23,7 @@ import { updateColumnParam } from '../layer_helpers'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternAggRestrictions, search } from '../../../../../../../src/plugins/data/public'; +import { fieldIsInvalid } from '../../utils'; const { isValidInterval } = search.aggs; const autoInterval = 'auto'; @@ -46,6 +47,7 @@ export const dateHistogramOperation: OperationDefinition< }), input: 'field', priority: 5, // Highest priority level used + hasInvalidReferences: fieldIsInvalid, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( type === 'date' && 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 593e075c758f7..de3004f0c5a90 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 @@ -156,6 +156,10 @@ interface BaseOperationDefinitionProps { * the status. Otherwise it returns undefined */ getDisabledStatus?: (indexPattern: IndexPattern) => string | undefined; + /** + * Returns true if column contains invalid references + */ + hasInvalidReferences?: (column: C, indexPattern: IndexPattern) => boolean; } interface BaseBuildColumnArgs { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index cf49b4dd6a124..3fbfc4228042b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -230,7 +230,7 @@ describe('last_value', () => { name: 'test', displayName: 'test', }, - columns: {}, + layer: { columns: {}, columnOrder: [], indexPatternId: '' }, }); expect(lastValueColumn.dataType).toEqual('boolean'); }); @@ -245,7 +245,7 @@ describe('last_value', () => { name: 'test', displayName: 'test', }, - columns: {}, + layer: { columns: {}, columnOrder: [], indexPatternId: '' }, }); expect(lastValueColumn.params.sortOrder).toEqual('desc'); }); @@ -253,15 +253,15 @@ describe('last_value', () => { it('should use indexPattern timeFieldName as a default sortField', () => { const lastValueColumn = lastValueOperation.buildColumn({ indexPattern: createMockedIndexPattern(), - columns: { - col1: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, - }, + + layer: { columns: { col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + },}, columnOrder: [], indexPatternId: '' }, + field: { aggregatable: true, searchable: true, @@ -301,15 +301,15 @@ describe('last_value', () => { }; const lastValueColumn = lastValueOperation.buildColumn({ indexPattern: indexPatternNoTimeField, - columns: { - col1: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, - }, + + layer: { columns: { col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }}, columnOrder: [], indexPatternId: '' }, + field: { aggregatable: true, searchable: true, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 53579d014416b..6e328d3e1dbc6 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -5,12 +5,13 @@ */ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiFormRow, EuiSelect, EuiButtonGroup } from '@elastic/eui'; +import { EuiFormRow, EuiComboBox, EuiButtonGroup, EuiComboBoxOptionOption } from '@elastic/eui'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; import { updateColumnParam } from '../layer_helpers'; import { DataType } from '../../../types'; +import { fieldIsInvalid } from '../../utils'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.lastValueOf', { @@ -21,9 +22,23 @@ function ofName(name: string) { const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); +export function sortFieldIsInvalid(sortField: string, indexPattern: IndexPattern) { + const field = indexPattern.getFieldByName(sortField); + return !(field && field.type === 'date'); +} + +function isTimeFieldNameDateField(indexPattern: IndexPattern) { + return ( + indexPattern.timeFieldName && + indexPattern.fields.find( + (field) => field.name === indexPattern.timeFieldName && field.type === 'date' + ) + ); +} + function getDateFields(indexPattern: IndexPattern): IndexPatternField[] { const dateFields = indexPattern.fields.filter((field) => field.type === 'date'); - if (indexPattern.timeFieldName) { + if (isTimeFieldNameDateField(indexPattern)) { dateFields.sort(({ name: nameA }, { name: nameB }) => { if (nameA === indexPattern.timeFieldName) { return -1; @@ -57,6 +72,8 @@ export const lastValueOperation: OperationDefinition + indexPattern.getFieldByName(column.sourceField)!.displayName, input: 'field', onFieldChange: (oldColumn, field) => { const newParams = { ...oldColumn.params }; @@ -85,9 +102,16 @@ export const lastValueOperation: OperationDefinition f.type === 'date')?.name; + const sortField = isTimeFieldNameDateField(indexPattern) + ? indexPattern.timeFieldName + : indexPattern.fields.find((f) => f.type === 'date')?.name; if (!sortField) { throw new Error( @@ -142,7 +166,8 @@ export const lastValueOperation: OperationDefinition { - const dateFields = getDateFields(state.indexPatterns[state.layers[layerId].indexPatternId]); + const currentIndexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; + const dateFields = getDateFields(currentIndexPattern); const sortOrderButtons = [ { id: `lns-lastValue-ascending`, @@ -155,6 +180,10 @@ export const lastValueOperation: OperationDefinition - { return { value: field.name, - text: field.displayName, + label: field.displayName, }; })} - value={currentColumn.params.sortField} - onChange={(e: React.ChangeEvent) => + onChange={(choices) => { + if (choices.length === 0) { + return; + } setState( updateColumnParam({ state, layerId, currentColumn, paramName: 'sortField', - value: e.target.value, + value: choices[0].value, }) - ) + ); + }} + selectedOptions={ + ((currentColumn.params?.sortField + ? [ + { + label: currentColumn.params.sortField, + value: currentColumn.params.sortField, + }, + ] + : []) as unknown) as EuiComboBoxOptionOption[] } - aria-label={i18n.translate('xpack.lens.indexPattern.lastValue.sortField', { - defaultMessage: 'Sort by date field', - })} /> = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { @@ -29,6 +30,7 @@ function buildMetricOperation>({ priority, displayName, input: 'field', + hasInvalidReferences: fieldIsInvalid, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { if ( fieldType === '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 d2456e1c8d375..dae4db42faf20 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 @@ -17,6 +17,7 @@ import { mergeLayer } from '../../../state_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; +import { fieldIsInvalid } from '../../../utils'; type RangeType = Omit; // Try to cover all possible serialized states for ranges @@ -109,6 +110,7 @@ export const rangeOperation: OperationDefinition { if ( type === 'number' && 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 ffddc48de87fc..ef46cfd539c45 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 @@ -22,6 +22,7 @@ import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesRangeInput } from './values_range_input'; +import { fieldIsInvalid } from '../../../utils'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { @@ -71,6 +72,7 @@ export const termsOperation: OperationDefinition { const newField = newIndexPattern.getFieldByName(column.sourceField); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index 01b834610eb1a..8b531860935c7 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -11,7 +11,7 @@ import { BaseIndexPatternColumn, FieldBasedIndexPatternColumn, } from './operations/definitions/column_types'; -import { operationDefinitionMap, OperationType } from './operations'; +import { operationDefinitionMap, IndexPatternColumn } from './operations'; /** * Normalizes the specified operation type. (e.g. document operations @@ -50,44 +50,39 @@ export function getInvalidLayers(state: IndexPatternPrivateState) { return Object.values(state.layers).filter((layer) => { return layer.columnOrder.some((columnId) => { const column = layer.columns[columnId]; - return ( - hasField(column) && - fieldIsInvalid( - column.sourceField, - column.operationType, - state.indexPatterns[layer.indexPatternId] - ) - ); + return isColumnInvalid(column, state.indexPatterns[layer.indexPatternId]); }); }); } -export function getInvalidFieldsForLayer( +export function getInvalidColumnsForLayer( layers: IndexPatternLayer[], indexPatternMap: Record ) { return layers.map((layer) => { return layer.columnOrder.filter((columnId) => { const column = layer.columns[columnId]; - return ( - hasField(column) && - fieldIsInvalid( - column.sourceField, - column.operationType, - indexPatternMap[layer.indexPatternId] - ) - ); + return isColumnInvalid(column, indexPatternMap[layer.indexPatternId]); }); }); } -export function fieldIsInvalid( - sourceField: string | undefined, - operationType: OperationType | undefined, - indexPattern: IndexPattern -) { - const operationDefinition = operationType && operationDefinitionMap[operationType]; +export function isColumnInvalid(column: IndexPatternColumn, indexPattern: IndexPattern) { + const operationDefinition = column.operationType && operationDefinitionMap[column.operationType]; + return ( + operationDefinition.hasInvalidReferences && + operationDefinition.hasInvalidReferences(column, indexPattern) + ); +} + +export function fieldIsInvalid(column: IndexPatternColumn | undefined, indexPattern: IndexPattern) { + if (!column || !hasField(column)) { + return false; + } + + const { sourceField, operationType } = column; const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; + const operationDefinition = operationType && operationDefinitionMap[operationType]; return Boolean( sourceField && From 7ee756f1cf025471c32af114d1c2dc074356fec7 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 23 Nov 2020 21:38:18 +0100 Subject: [PATCH 11/27] tests for hasInvalidReferences --- .../definitions/last_value.test.tsx | 103 ++++++++++++++---- .../operations/definitions/last_value.tsx | 2 +- .../definitions/terms/terms.test.tsx | 33 ++++++ 3 files changed, 113 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 3fbfc4228042b..516dcad302ab3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { EuiButtonGroup } from '@elastic/eui'; +import { EuiButtonGroup, EuiComboBox } from '@elastic/eui'; import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { dataPluginMock } from '../../../../../../../src/plugins/data/public/mocks'; @@ -254,14 +254,20 @@ describe('last_value', () => { const lastValueColumn = lastValueOperation.buildColumn({ indexPattern: createMockedIndexPattern(), - layer: { columns: { col1: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - },}, columnOrder: [], indexPatternId: '' }, - + layer: { + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + columnOrder: [], + indexPatternId: '', + }, + field: { aggregatable: true, searchable: true, @@ -301,15 +307,21 @@ describe('last_value', () => { }; const lastValueColumn = lastValueOperation.buildColumn({ indexPattern: indexPatternNoTimeField, - - layer: { columns: { col1: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }}, columnOrder: [], indexPatternId: '' }, - + + layer: { + columns: { + col1: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + columnOrder: [], + indexPatternId: '', + }, + field: { aggregatable: true, searchable: true, @@ -366,7 +378,7 @@ describe('last_value', () => { const select = instance.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]'); - expect(select.prop('value')).toEqual('datefield'); + expect(select.prop('selectedOptions')).toEqual([{ value: 'datefield' }]); }); it('should update state when changing sortField', () => { @@ -382,11 +394,10 @@ describe('last_value', () => { /> ); - instance.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]').prop('onChange')!({ - target: { - value: 'datefield2', - }, - } as React.ChangeEvent); + instance + .find('[data-test-subj="lns-indexPattern-lastValue-sortField"]') + .find(EuiComboBox) + .prop('onChange')!([{ label: 'datefield2', value: 'datefield2' }]); expect(setStateSpy).toHaveBeenCalledWith({ ...state, @@ -471,4 +482,48 @@ describe('last_value', () => { }); }); }); + + describe('hasInvalidReferences', () => { + let indexPattern: IndexPattern; + let column: LastValueIndexPatternColumn; + beforeEach(() => { + indexPattern = createMockedIndexPattern(); + column = { + dataType: 'boolean', + isBucketed: false, + label: 'Last value of test', + operationType: 'last_value', + params: { sortField: 'timestamp', sortOrder: 'desc' }, + scale: 'ratio', + sourceField: 'bytes', + }; + }); + it('returns false if sourceField exists and sortField is of type date ', () => { + expect(lastValueOperation.hasInvalidReferences!(column, indexPattern)).toEqual(false); + }); + it('returns true if the sourceField does not exist in index pattern', () => { + expect( + lastValueOperation.hasInvalidReferences!( + { ...column, sourceField: 'notExisting' }, + indexPattern + ) + ).toEqual(true); + }); + it('returns true if the sortField does not exist in index pattern', () => { + expect( + lastValueOperation.hasInvalidReferences!( + { ...column, params: { ...column.params, sortField: 'notExisting' } }, + indexPattern + ) + ).toEqual(true); + }); + it('returns true if the sortField is not date', () => { + expect( + lastValueOperation.hasInvalidReferences!( + { ...column, params: { ...column.params, sortField: 'bytes' } }, + indexPattern + ) + ).toEqual(true); + }); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 6e328d3e1dbc6..b7691c3a56f08 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -233,7 +233,7 @@ export const lastValueOperation: OperationDefinition { }); }); }); + describe('hasInvalidReferences', () => { + let indexPattern: IndexPattern; + let column: TermsIndexPatternColumn; + beforeEach(() => { + indexPattern = createMockedIndexPattern(); + column = { + dataType: 'boolean', + isBucketed: true, + label: 'Top values of bytes', + operationType: 'terms', + params: { + missingBucket: false, + orderBy: { type: 'alphabetical' }, + orderDirection: 'asc', + otherBucket: true, + size: 5, + }, + scale: 'ordinal', + sourceField: 'bytes', + }; + }); + it('returns false if sourceField exists in index pattern', () => { + expect(termsOperation.hasInvalidReferences!(column, indexPattern)).toEqual(false); + }); + it('returns true if the sourceField does not exist in index pattern', () => { + expect( + termsOperation.hasInvalidReferences!( + { ...column, sourceField: 'notExisting' }, + indexPattern + ) + ).toEqual(true); + }); + }); }); From 3f1991e71ce3fd65fd59c0ccf0da1d6092d6c451 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 23 Nov 2020 21:46:19 +0100 Subject: [PATCH 12/27] change name --- .../indexpattern_datasource/indexpattern_suggestions.ts | 6 +++--- x-pack/plugins/lens/public/indexpattern_datasource/utils.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 263b4646c9feb..ebac396210a5c 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, hasInvalidFields } from './utils'; +import { hasField, hasInvalidColumns } from './utils'; import { IndexPattern, IndexPatternPrivateState, @@ -90,7 +90,7 @@ export function getDatasourceSuggestionsForField( indexPatternId: string, field: IndexPatternField ): IndexPatternSugestion[] { - if (hasInvalidFields(state)) return []; + if (hasInvalidColumns(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 (hasInvalidFields(state)) return []; + if (hasInvalidColumns(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/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index 8b531860935c7..c1957b8c5717f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -42,7 +42,7 @@ export function isDraggedField(fieldCandidate: unknown): fieldCandidate is Dragg ); } -export function hasInvalidFields(state: IndexPatternPrivateState) { +export function hasInvalidColumns(state: IndexPatternPrivateState) { return getInvalidLayers(state).length > 0; } From de882138ff215cfb0afe3cd2d30c3b7cd59b0e55 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 23 Nov 2020 21:52:57 +0100 Subject: [PATCH 13/27] correct displayName --- .../operations/definitions/last_value.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index b7691c3a56f08..26c9a33f3ddc4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -233,7 +233,9 @@ export const lastValueOperation: OperationDefinition Date: Mon, 23 Nov 2020 21:56:12 +0100 Subject: [PATCH 14/27] test corrected last value --- .../operations/definitions/last_value.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 516dcad302ab3..cbe009a651703 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -378,7 +378,7 @@ describe('last_value', () => { const select = instance.find('[data-test-subj="lns-indexPattern-lastValue-sortField"]'); - expect(select.prop('selectedOptions')).toEqual([{ value: 'datefield' }]); + expect(select.prop('selectedOptions')).toEqual([{ label: 'datefield', value: 'datefield' }]); }); it('should update state when changing sortField', () => { From 86ee0167503583b4574187811a875be21be5788b Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 09:15:13 +0100 Subject: [PATCH 15/27] fix tests and types --- .../dimension_panel/droppable.test.ts | 2 -- .../indexpattern.test.ts | 12 +++++----- .../indexpattern_datasource/loader.test.ts | 2 -- .../operations/__mocks__/index.ts | 1 + .../operations/definitions/cardinality.tsx | 2 +- .../operations/definitions/count.tsx | 2 ++ .../operations/definitions/date_histogram.tsx | 2 +- .../definitions/filters/filters.tsx | 1 + .../operations/definitions/helpers.tsx | 19 ++++++++++++++++ .../operations/definitions/index.ts | 18 ++++++++++++++- .../operations/definitions/last_value.tsx | 2 +- .../operations/definitions/metrics.tsx | 11 ++++++---- .../operations/definitions/ranges/ranges.tsx | 2 +- .../operations/definitions/terms/index.tsx | 2 +- .../operations/mocks.ts | 1 + .../public/indexpattern_datasource/utils.ts | 22 ++++--------------- 16 files changed, 63 insertions(+), 38 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts index 48240a5417108..54da245bc8e1a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts @@ -17,8 +17,6 @@ import { OperationMetadata } from '../../types'; import { IndexPatternColumn } from '../operations'; import { getFieldByNameFactory } from '../pure_helpers'; -jest.mock('../operations'); - const fields = [ { name: 'timestamp', 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..e1d671f2eeb2a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts @@ -728,7 +728,7 @@ describe('IndexPattern Data Source', () => { expect(messages).toHaveLength(1); expect(messages![0]).toEqual({ shortMessage: 'Invalid reference.', - longMessage: 'Field "bytes" has an invalid reference.', + longMessage: '"Foo" has an invalid reference.', }); }); @@ -753,7 +753,7 @@ describe('IndexPattern Data Source', () => { col2: { dataType: 'number', isBucketed: false, - label: 'Foo', + label: 'Foo2', operationType: 'count', // <= invalid sourceField: 'memory', }, @@ -766,7 +766,7 @@ describe('IndexPattern Data Source', () => { expect(messages).toHaveLength(1); expect(messages![0]).toEqual({ shortMessage: 'Invalid references.', - longMessage: 'Fields "bytes", "memory" have invalid reference.', + longMessage: '"Foo", "Foo2" have invalid reference.', }); }); @@ -791,7 +791,7 @@ describe('IndexPattern Data Source', () => { col2: { dataType: 'number', isBucketed: false, - label: 'Foo', + label: 'Foo2', operationType: 'count', // <= invalid sourceField: 'memory', }, @@ -818,11 +818,11 @@ describe('IndexPattern Data Source', () => { expect(messages).toEqual([ { shortMessage: 'Invalid references on Layer 1.', - longMessage: 'Layer 1 has invalid references in fields "bytes", "memory".', + longMessage: 'Layer 1 has invalid references in "Foo", "Foo2".', }, { shortMessage: 'Invalid reference on Layer 2.', - longMessage: 'Layer 2 has an invalid reference in field "source".', + longMessage: 'Layer 2 has an invalid reference in "Foo".', }, ]); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts index adb86253ab28c..29786d9bc68f3 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts @@ -25,8 +25,6 @@ import { import { createMockedRestrictedIndexPattern, createMockedIndexPattern } from './mocks'; import { documentField } from './document_field'; -jest.mock('./operations'); - const createMockStorage = (lastData?: Record) => { return { get: jest.fn().mockImplementation(() => lastData), 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..64fe51dfd63b4 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 @@ -23,6 +23,7 @@ export const { getOperationResultType, operationDefinitionMap, operationDefinitions, + fieldIsInvalid, } = actualOperations; export const { 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 21ecd259fb5af..f05e069e0cd9b 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 @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; -import { fieldIsInvalid } from '../../utils'; +import { fieldIsInvalid } from '.'; const supportedTypes = new Set(['string', 'boolean', 'number', 'ip', 'date']); 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..df0b0b09c080f 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 { fieldIsInvalid } from '.'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', @@ -25,6 +26,7 @@ export const countOperation: OperationDefinition { return { ...oldColumn, 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 7ca604b31b036..2a2f0733d8837 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 @@ -23,7 +23,7 @@ import { updateColumnParam } from '../layer_helpers'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternAggRestrictions, search } from '../../../../../../../src/plugins/data/public'; -import { fieldIsInvalid } from '../../utils'; +import { fieldIsInvalid } from '.'; const { isValidInterval } = search.aggs; const autoInterval = 'auto'; 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 1b0452d18a79c..f77e5e2fdbd42 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 @@ -76,6 +76,7 @@ export const filtersOperation: OperationDefinition true, getDefaultLabel: () => filtersLabel, + hasInvalidReferences: () => false, buildColumn({ previousColumn }) { let params = { filters: [defaultFilter] }; if (previousColumn?.operationType === 'terms') { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index a5c08a93467af..c51797cb215b1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -6,6 +6,9 @@ import { useRef } from 'react'; import useDebounce from 'react-use/lib/useDebounce'; +import { operationDefinitionMap } from '.'; +import { FieldBasedIndexPatternColumn } from './column_types'; +import { IndexPattern } from '../../types'; export const useDebounceWithOptions = ( fn: Function, @@ -28,3 +31,19 @@ export const useDebounceWithOptions = ( newDeps ); }; + +export function fieldIsInvalid(column: FieldBasedIndexPatternColumn, indexPattern: IndexPattern) { + const { sourceField, operationType } = column; + const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; + const operationDefinition = operationType && operationDefinitionMap[operationType]; + + return Boolean( + sourceField && + operationDefinition && + !( + field && + operationDefinition?.input === 'field' && + operationDefinition.getPossibleOperationForField(field) !== undefined + ) + ); +} 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 de3004f0c5a90..85b52f91490bb 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 @@ -159,7 +159,7 @@ interface BaseOperationDefinitionProps { /** * Returns true if column contains invalid references */ - hasInvalidReferences?: (column: C, indexPattern: IndexPattern) => boolean; + hasInvalidReferences: (column: C, indexPattern: IndexPattern) => boolean; } interface BaseBuildColumnArgs { @@ -341,3 +341,19 @@ export const operationDefinitionMap: Record< (definitionMap, definition) => ({ ...definitionMap, [definition.type]: definition }), {} ); + +export function fieldIsInvalid(column: FieldBasedIndexPatternColumn, indexPattern: IndexPattern) { + const { sourceField, operationType } = column; + const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; + const operationDefinition = operationType && operationDefinitionMap[operationType]; + + return Boolean( + sourceField && + operationDefinition && + !( + field && + operationDefinition?.input === 'field' && + operationDefinition.getPossibleOperationForField(field) !== undefined + ) + ); +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 26c9a33f3ddc4..c09cf9ec96338 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -11,7 +11,7 @@ import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; import { updateColumnParam } from '../layer_helpers'; import { DataType } from '../../../types'; -import { fieldIsInvalid } from '../../utils'; +import { fieldIsInvalid } from '.'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.lastValueOf', { 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 59c44d8d40e2c..f216a19a5ef20 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -7,11 +7,13 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; -import { fieldIsInvalid } from '../../utils'; +import { fieldIsInvalid } from '.'; -type MetricColumn = FormattedIndexPatternColumn & +type MetricType = 'sum' | 'avg' | 'min' | 'max' | 'median'; + +type MetricColumn = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { - operationType: T; + operationType: MetricType; }; function buildMetricOperation>({ @@ -30,7 +32,6 @@ function buildMetricOperation>({ priority, displayName, input: 'field', - hasInvalidReferences: fieldIsInvalid, getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type: fieldType }) => { if ( fieldType === 'number' && @@ -83,6 +84,8 @@ function buildMetricOperation>({ missing: 0, }, }), + hasInvalidReferences: (column, _indexPattern) => + fieldIsInvalid(column as MetricColumn, _indexPattern), } as OperationDefinition; } 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 dae4db42faf20..ca7d5810b531c 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 @@ -17,7 +17,7 @@ import { mergeLayer } from '../../../state_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; -import { fieldIsInvalid } from '../../../utils'; +import { fieldIsInvalid } from '..'; type RangeType = Omit; // Try to cover all possible serialized states for ranges 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 ef46cfd539c45..d4e4fdea5ad15 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 @@ -22,7 +22,7 @@ import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesRangeInput } from './values_range_input'; -import { fieldIsInvalid } from '../../../utils'; +import { fieldIsInvalid } from '..'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts index c3f7dac03ada3..18dbd3dc4ab3f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts @@ -35,5 +35,6 @@ export const createMockedReferenceOperation = () => { toExpression: jest.fn().mockReturnValue([]), getPossibleOperation: jest.fn().mockReturnValue({ dataType: 'number', isBucketed: false }), getDefaultLabel: jest.fn().mockReturnValue('Default label'), + hasInvalidReferences: jest.fn(), }; }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index c1957b8c5717f..75cdc30b9a160 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -13,6 +13,8 @@ import { } from './operations/definitions/column_types'; import { operationDefinitionMap, IndexPatternColumn } from './operations'; +import { fieldIsInvalid as fieldIsInvalidHelper } from './operations/definitions'; + /** * Normalizes the specified operation type. (e.g. document operations * produce 'number') @@ -69,28 +71,12 @@ export function getInvalidColumnsForLayer( export function isColumnInvalid(column: IndexPatternColumn, indexPattern: IndexPattern) { const operationDefinition = column.operationType && operationDefinitionMap[column.operationType]; - return ( - operationDefinition.hasInvalidReferences && - operationDefinition.hasInvalidReferences(column, indexPattern) - ); + return operationDefinition.hasInvalidReferences(column, indexPattern); } export function fieldIsInvalid(column: IndexPatternColumn | undefined, indexPattern: IndexPattern) { if (!column || !hasField(column)) { return false; } - - const { sourceField, operationType } = column; - const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; - const operationDefinition = operationType && operationDefinitionMap[operationType]; - - return Boolean( - sourceField && - operationDefinition && - !( - field && - operationDefinition?.input === 'field' && - operationDefinition.getPossibleOperationForField(field) !== undefined - ) - ); + return fieldIsInvalidHelper(column, indexPattern); } From 3455a7d4a5ed323de4f3f42cbcc7177701084eb9 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 09:21:47 +0100 Subject: [PATCH 16/27] no need to make a function obligatory --- .../operations/definitions/filters/filters.tsx | 1 - x-pack/plugins/lens/public/indexpattern_datasource/utils.ts | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 f77e5e2fdbd42..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 @@ -76,7 +76,6 @@ export const filtersOperation: OperationDefinition true, getDefaultLabel: () => filtersLabel, - hasInvalidReferences: () => false, buildColumn({ previousColumn }) { let params = { filters: [defaultFilter] }; if (previousColumn?.operationType === 'terms') { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index 75cdc30b9a160..980025f5a0db4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -71,7 +71,10 @@ export function getInvalidColumnsForLayer( export function isColumnInvalid(column: IndexPatternColumn, indexPattern: IndexPattern) { const operationDefinition = column.operationType && operationDefinitionMap[column.operationType]; - return operationDefinition.hasInvalidReferences(column, indexPattern); + return !!( + operationDefinition.hasInvalidReferences && + operationDefinition.hasInvalidReferences(column, indexPattern) + ); } export function fieldIsInvalid(column: IndexPatternColumn | undefined, indexPattern: IndexPattern) { From b54c73d4e451342ad1c1f577d0f168754882e2ce Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 14:51:38 +0100 Subject: [PATCH 17/27] hasInvalidreferenced not necessary --- .../indexpattern_datasource/operations/definitions/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 85b52f91490bb..cfdc4f2774f65 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 @@ -159,7 +159,7 @@ interface BaseOperationDefinitionProps { /** * Returns true if column contains invalid references */ - hasInvalidReferences: (column: C, indexPattern: IndexPattern) => boolean; + hasInvalidReferences?: (column: C, indexPattern: IndexPattern) => boolean; } interface BaseBuildColumnArgs { From 8b90db74adedf81520cedc5a790d1966104777d3 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 16:23:54 +0100 Subject: [PATCH 18/27] eslint --- .../operations/definitions/metrics.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 f216a19a5ef20..fda3f300753aa 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,9 +11,9 @@ import { fieldIsInvalid } from '.'; type MetricType = 'sum' | 'avg' | 'min' | 'max' | 'median'; -type MetricColumn = FormattedIndexPatternColumn & +type MetricColumn = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { - operationType: MetricType; + operationType: T; }; function buildMetricOperation>({ From 71e82649b64a1a343c40d5e790a31eb725e0bc75 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 16:47:30 +0100 Subject: [PATCH 19/27] addded warnings for pie and xy --- .../workspace_panel/warnings_popover.tsx | 54 +++++++++++++++ .../workspace_panel_wrapper.scss | 4 ++ .../workspace_panel_wrapper.tsx | 66 ++++++++++++------- .../pie_visualization/visualization.tsx | 28 ++++++++ x-pack/plugins/lens/public/types.ts | 5 ++ .../xy_visualization/visualization.test.ts | 65 ++++++++++++++++++ .../public/xy_visualization/visualization.tsx | 31 +++++++++ 7 files changed, 230 insertions(+), 23 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx new file mode 100644 index 0000000000000..5a3949f2e121b --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx @@ -0,0 +1,54 @@ +/* + * 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 './workspace_panel_wrapper.scss'; + +import React, { useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { EuiPopover, EuiText, EuiButtonEmpty, EuiHorizontalRule } from '@elastic/eui'; + +export const WarningsPopover = ({ + children, +}: { + children?: React.ReactNode | React.ReactNode[]; +}) => { + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + + if (!children) { + return null; + } + + const onButtonClick = () => setIsPopoverOpen((isOpen) => !isOpen); + const closePopover = () => setIsPopoverOpen(false); + const warningsCount = React.Children.count(children); + return ( + + {i18n.translate('xpack.lens.chartWarnings.number', { + defaultMessage: `{warningsCount} {warningsCount, plural, one {warning} other {warnings}}`, + values: { + warningsCount, + }, + })} + + } + isOpen={isPopoverOpen} + closePopover={closePopover} + > + {React.Children.map(children, (child, index) => ( + <> + {child} + {warningsCount - 1 !== index && } + + ))} + + ); +}; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss index ae9294c474b42..cec0f2fbea884 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss @@ -115,3 +115,7 @@ 75% { transform: translateY(15%); } 100% { transform: translateY(10%); } } + +.lnsWorkspaceWarningButton { + color: $euiColorWarningText; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx index 33ddc23312a96..fa5d0046853ab 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx @@ -19,6 +19,7 @@ import { Datasource, FramePublicAPI, Visualization } from '../../../types'; import { NativeRenderer } from '../../../native_renderer'; import { Action } from '../state_management'; import { ChartSwitch } from './chart_switch'; +import { WarningsPopover } from './warnings_popover'; export interface WorkspacePanelWrapperProps { children: React.ReactNode | React.ReactNode[]; @@ -64,40 +65,59 @@ export function WorkspacePanelWrapper({ }, [dispatch, activeVisualization] ); + const warningMessages = + activeVisualization?.getWarningMessages && + activeVisualization.getWarningMessages(visualizationState, framePublicAPI); return ( <>
- + + + + + {activeVisualization && activeVisualization.renderToolbar && ( + + + + )} + + + + {warningMessages && warningMessages.length ? ( + {warningMessages} + ) : null} - {activeVisualization && activeVisualization.renderToolbar && ( - - - - )}
diff --git a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx index 91f0ddb54ad41..2d9a345b978ec 100644 --- a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx @@ -245,6 +245,34 @@ export const getPieVisualization = ({ ); }, + getWarningMessages(state, frame) { + if (state?.layers.length === 0 || !frame.activeData) { + return; + } + + const metricColumnsWithArrayValues = []; + + for (const layer of state.layers) { + const { layerId, metric } = layer; + const rows = frame.activeData[layerId] && frame.activeData[layerId].rows; + if (!rows || !metric) { + break; + } + const columnToLabel = frame.datasourceLayers[layerId].getOperationForColumnId(metric)?.label; + + const hasArrayValues = rows.some((row) => Array.isArray(row[metric])); + if (hasArrayValues) { + metricColumnsWithArrayValues.push(columnToLabel || metric); + } + } + return metricColumnsWithArrayValues.map((label) => ( + <> + {label} contains array values. Your visualization may not render as + expected. + + )); + }, + getErrorMessages(state, frame) { // not possible to break it? return undefined; diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 225fedb987c76..042130d2d69f4 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -593,6 +593,11 @@ export interface Visualization { state: T, frame: FramePublicAPI ) => Array<{ shortMessage: string; longMessage: string }> | undefined; + + /** + * The frame calls this function to display warnings about visualization + */ + getWarningMessages?: (state: T, frame: FramePublicAPI) => React.ReactNode[] | undefined; } export interface LensFilterEvent { diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index 546cf06d4014e..d780ce85bad69 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -775,4 +775,69 @@ describe('xy_visualization', () => { ]); }); }); + + describe('#getWarningMessages', () => { + let mockDatasource: ReturnType; + let frame: ReturnType; + + beforeEach(() => { + frame = createMockFramePublicAPI(); + mockDatasource = createMockDatasource('testDatasource'); + + mockDatasource.publicAPIMock.getTableSpec.mockReturnValue([ + { columnId: 'd' }, + { columnId: 'a' }, + { columnId: 'b' }, + { columnId: 'c' }, + ]); + + frame.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + + frame.activeData = { + first: { + type: 'datatable', + columns: [ + { id: 'a', name: 'A', meta: { type: 'number' } }, + { id: 'b', name: 'B', meta: { type: 'number' } }, + ], + rows: [ + { a: 1, b: [2, 0] }, + { a: 3, b: 4 }, + { a: 5, b: 6 }, + { a: 7, b: 8 }, + ], + }, + }; + }); + it('should return a warning when numeric accessors contain array', () => { + (frame.datasourceLayers.first.getOperationForColumnId as jest.Mock).mockReturnValue({ + label: 'Label B', + }); + const warningMessages = xyVisualization.getWarningMessages!( + { + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: ['b'], + }, + ], + }, + frame + ); + expect(warningMessages).toHaveLength(1); + expect(warningMessages && warningMessages[0]).toMatchInlineSnapshot(` + + + Label B + + contains array values. Your visualization may not render as expected. + + `); + }); + }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index 5748e649c181e..c8dc880363a97 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -369,6 +369,37 @@ export const getXyVisualization = ({ return errors.length ? errors : undefined; }, + + getWarningMessages(state, frame) { + if (state?.layers.length === 0 || !frame.activeData) { + return; + } + + const layers = state.layers; + + const filteredLayers = layers.filter(({ accessors }: LayerConfig) => accessors.length > 0); + const accessorsWithArrayValues = []; + for (const layer of filteredLayers) { + const { layerId, accessors } = layer; + const rows = frame.activeData[layerId] && frame.activeData[layerId].rows; + if (!rows) { + break; + } + const columnToLabel = getColumnToLabelMap(layer, frame.datasourceLayers[layerId]); + for (const accessor of accessors) { + const hasArrayValues = rows.some((row) => Array.isArray(row[accessor])); + if (hasArrayValues) { + accessorsWithArrayValues.push(columnToLabel[accessor]); + } + } + } + return accessorsWithArrayValues.map((label) => ( + <> + {label} contains array values. Your visualization may not render as + expected. + + )); + }, }); function getAccessorColorConfig( From 3e4b5f73805f1b826ddc2c89511d657ee585e069 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 24 Nov 2020 17:48:52 +0100 Subject: [PATCH 20/27] Update x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx --- .../dimension_panel/dimension_editor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e3a17b03faf60..f5e32ab373d88 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 @@ -107,7 +107,7 @@ export function DimensionEditor(props: DimensionEditorProps) { }) .map((def) => def.type) .filter((type) => fieldByOperation[type]?.size || operationWithoutField.has(type)); - }, [operationWithoutField]); + }, [fieldByOperation, operationWithoutField]); // Operations are compatible if they match inputs. They are always compatible in // the empty state. Field-based operations are not compatible with field-less operations. From 2f844b8fad8ba0d125ba14e27a1cf29f9abd32a6 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 26 Nov 2020 17:59:04 +0100 Subject: [PATCH 21/27] refactor hasInvalidReferences to getErrorMessage --- .../dimension_panel/dimension_panel.tsx | 13 +-- .../operations/__mocks__/index.ts | 2 +- .../operations/definitions/cardinality.tsx | 5 +- .../operations/definitions/count.tsx | 5 +- .../operations/definitions/date_histogram.tsx | 5 +- .../operations/definitions/helpers.tsx | 19 +++- .../operations/definitions/index.ts | 45 ++++---- .../definitions/last_value.test.tsx | 102 +++++++++++------- .../operations/definitions/last_value.tsx | 42 ++++++-- .../operations/definitions/metrics.tsx | 8 +- .../operations/definitions/ranges/ranges.tsx | 5 +- .../operations/definitions/terms/index.tsx | 5 +- .../definitions/terms/terms.test.tsx | 62 ++++++----- .../operations/mocks.ts | 2 +- .../operations/operations.test.ts | 9 -- .../public/indexpattern_datasource/utils.ts | 30 +++--- 16 files changed, 216 insertions(+), 143 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx index 57efd58d284b7..20134699d2067 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.tsx @@ -45,21 +45,22 @@ export const IndexPatternDimensionTriggerComponent = function IndexPatternDimens ) { const layerId = props.layerId; const layer = props.state.layers[layerId]; - const selectedColumn: IndexPatternColumn | null = layer.columns[props.columnId] || null; const currentIndexPattern = props.state.indexPatterns[layer.indexPatternId]; + const { columnId, uniqueLabel } = props; - const currentFieldIsInvalid = useMemo( - () => isColumnInvalid(selectedColumn, currentIndexPattern), - [selectedColumn, currentIndexPattern] + const currentColumnHasErrors = useMemo( + () => isColumnInvalid(layer, columnId, currentIndexPattern), + [layer, columnId, currentIndexPattern] ); - const { columnId, uniqueLabel } = props; + const selectedColumn: IndexPatternColumn | null = layer.columns[props.columnId] || null; + if (!selectedColumn) { return null; } const formattedLabel = wrapOnDot(uniqueLabel); - if (currentFieldIsInvalid) { + if (currentColumnHasErrors) { return ( + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), isTransferable: (column, newIndexPattern) => { const newField = newIndexPattern.getFieldByName(column.sourceField); 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 df0b0b09c080f..a608a3b67c56d 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,7 @@ import { i18n } from '@kbn/i18n'; import { OperationDefinition } from './index'; import { FormattedIndexPatternColumn, FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField } from '../../types'; -import { fieldIsInvalid } from '.'; +import { getInvalidFieldMessage } from './helpers'; const countLabel = i18n.translate('xpack.lens.indexPattern.countOf', { defaultMessage: 'Count of records', @@ -26,7 +26,8 @@ export const countOperation: OperationDefinition + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), onFieldChange: (oldColumn, field) => { return { ...oldColumn, 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 2a2f0733d8837..ca426fb53a3ab 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 @@ -23,7 +23,7 @@ import { updateColumnParam } from '../layer_helpers'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternAggRestrictions, search } from '../../../../../../../src/plugins/data/public'; -import { fieldIsInvalid } from '.'; +import { getInvalidFieldMessage } from './helpers'; const { isValidInterval } = search.aggs; const autoInterval = 'auto'; @@ -47,7 +47,8 @@ export const dateHistogramOperation: OperationDefinition< }), input: 'field', priority: 5, // Highest priority level used - hasInvalidReferences: fieldIsInvalid, + getErrorMessage: (layer, columnId, indexPattern) => + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( type === 'date' && diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index c51797cb215b1..c84325ccc5549 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -6,6 +6,7 @@ import { useRef } from 'react'; import useDebounce from 'react-use/lib/useDebounce'; +import { i18n } from '@kbn/i18n'; import { operationDefinitionMap } from '.'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPattern } from '../../types'; @@ -32,12 +33,18 @@ export const useDebounceWithOptions = ( ); }; -export function fieldIsInvalid(column: FieldBasedIndexPatternColumn, indexPattern: IndexPattern) { +export function getInvalidFieldMessage( + column: FieldBasedIndexPatternColumn, + indexPattern?: IndexPattern +) { + if (!indexPattern) { + return; + } const { sourceField, operationType } = column; const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; const operationDefinition = operationType && operationDefinitionMap[operationType]; - return Boolean( + const isInvalid = Boolean( sourceField && operationDefinition && !( @@ -46,4 +53,12 @@ export function fieldIsInvalid(column: FieldBasedIndexPatternColumn, indexPatter operationDefinition.getPossibleOperationForField(field) !== undefined ) ); + return isInvalid + ? [ + i18n.translate('xpack.lens.indexPattern.invalidSourceField', { + defaultMessage: 'Field {invalidField} has an invalid reference', + values: { invalidField: sourceField }, + }), + ] + : undefined; } 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 10f16a41d075f..c37254b239e8b 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 @@ -175,9 +175,16 @@ interface BaseOperationDefinitionProps { */ getDisabledStatus?: (indexPattern: IndexPattern) => string | undefined; /** - * Returns true if column contains invalid references + * 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 */ - hasInvalidReferences?: (column: C, indexPattern: IndexPattern) => boolean; + getErrorMessage?: ( + layer: IndexPatternLayer, + columnId: string, + indexPattern?: IndexPattern + ) => string[] | undefined; } interface BaseBuildColumnArgs { @@ -244,6 +251,17 @@ interface FieldBasedOperationDefinition { * together with the agg configs returned from other columns. */ toEsAggsConfig: (column: C, columnId: string, indexPattern: IndexPattern) => unknown; + /** + * 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, + indexPattern?: IndexPattern + ) => string[] | undefined; } export interface RequiredReference { @@ -296,13 +314,6 @@ interface FullReferenceOperationDefinition { 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 { @@ -359,19 +370,3 @@ export const operationDefinitionMap: Record< (definitionMap, definition) => ({ ...definitionMap, [definition.type]: definition }), {} ); - -export function fieldIsInvalid(column: FieldBasedIndexPatternColumn, indexPattern: IndexPattern) { - const { sourceField, operationType } = column; - const field = sourceField ? indexPattern.getFieldByName(sourceField) : undefined; - const operationDefinition = operationType && operationDefinitionMap[operationType]; - - return Boolean( - sourceField && - operationDefinition && - !( - field && - operationDefinition?.input === 'field' && - operationDefinition.getPossibleOperationForField(field) !== undefined - ) - ); -} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index cbe009a651703..7535b303eec1f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -13,7 +13,7 @@ import { dataPluginMock } from '../../../../../../../src/plugins/data/public/moc import { createMockedIndexPattern } from '../../mocks'; import { LastValueIndexPatternColumn } from './last_value'; import { lastValueOperation } from './index'; -import { IndexPatternPrivateState, IndexPattern } from '../../types'; +import { IndexPatternPrivateState, IndexPattern, IndexPatternLayer } from '../../types'; const defaultProps = { storage: {} as IStorageWrapper, @@ -162,7 +162,7 @@ describe('last_value', () => { ).toEqual({ dataType: 'boolean', isBucketed: false, - scale: 'ordinal', + scale: 'ratio', }); expect( @@ -176,7 +176,7 @@ describe('last_value', () => { ).toEqual({ dataType: 'ip', isBucketed: false, - scale: 'ordinal', + scale: 'ratio', }); }); @@ -483,47 +483,77 @@ describe('last_value', () => { }); }); - describe('hasInvalidReferences', () => { + describe('getErrorMessage', () => { let indexPattern: IndexPattern; - let column: LastValueIndexPatternColumn; + let layer: IndexPatternLayer; beforeEach(() => { indexPattern = createMockedIndexPattern(); - column = { - dataType: 'boolean', - isBucketed: false, - label: 'Last value of test', - operationType: 'last_value', - params: { sortField: 'timestamp', sortOrder: 'desc' }, - scale: 'ratio', - sourceField: 'bytes', + layer = { + columns: { + col1: { + dataType: 'boolean', + isBucketed: false, + label: 'Last value of test', + operationType: 'last_value', + params: { sortField: 'timestamp', sortOrder: 'desc' }, + scale: 'ratio', + sourceField: 'bytes', + }, + }, + columnOrder: [], + indexPatternId: '', }; }); - it('returns false if sourceField exists and sortField is of type date ', () => { - expect(lastValueOperation.hasInvalidReferences!(column, indexPattern)).toEqual(false); + it('returns undefined if sourceField exists and sortField is of type date ', () => { + expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual(undefined); }); - it('returns true if the sourceField does not exist in index pattern', () => { - expect( - lastValueOperation.hasInvalidReferences!( - { ...column, sourceField: 'notExisting' }, - indexPattern - ) - ).toEqual(true); + it('shows error message if the sourceField does not exist in index pattern', () => { + layer = { + ...layer, + columns: { + col1: { + ...layer.columns.col1, + sourceField: 'notExisting', + } as LastValueIndexPatternColumn, + }, + }; + expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + 'Field notExisting has an invalid reference', + ]); }); - it('returns true if the sortField does not exist in index pattern', () => { - expect( - lastValueOperation.hasInvalidReferences!( - { ...column, params: { ...column.params, sortField: 'notExisting' } }, - indexPattern - ) - ).toEqual(true); + it('shows error message if the sortField does not exist in index pattern', () => { + layer = { + ...layer, + columns: { + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + sortField: 'notExisting', + }, + } as LastValueIndexPatternColumn, + }, + }; + expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + 'Field notExisting has an invalid reference', + ]); }); - it('returns true if the sortField is not date', () => { - expect( - lastValueOperation.hasInvalidReferences!( - { ...column, params: { ...column.params, sortField: 'bytes' } }, - indexPattern - ) - ).toEqual(true); + it('shows error message if the sortField is not date', () => { + layer = { + ...layer, + columns: { + col1: { + ...layer.columns.col1, + params: { + ...layer.columns.col1.params, + sortField: 'bytes', + }, + } as LastValueIndexPatternColumn, + }, + }; + expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + 'Field bytes has an invalid reference', + ]); }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index c09cf9ec96338..e26f256609502 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -11,7 +11,7 @@ import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; import { updateColumnParam } from '../layer_helpers'; import { DataType } from '../../../types'; -import { fieldIsInvalid } from '.'; +import { getInvalidFieldMessage } from './helpers'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.lastValueOf', { @@ -22,9 +22,17 @@ function ofName(name: string) { const supportedTypes = new Set(['string', 'boolean', 'number', 'ip']); -export function sortFieldIsInvalid(sortField: string, indexPattern: IndexPattern) { +export function getInvalidSortFieldMessage(sortField: string, indexPattern?: IndexPattern) { + if (!indexPattern) { + return; + } const field = indexPattern.getFieldByName(sortField); - return !(field && field.type === 'date'); + if (!(field && field.type === 'date')) { + return i18n.translate('xpack.lens.indexPattern.lastValue.invalidSortField', { + defaultMessage: 'Field {invalidField} has an invalid reference', + values: { invalidField: sortField }, + }); + } } function isTimeFieldNameDateField(indexPattern: IndexPattern) { @@ -91,7 +99,11 @@ export const lastValueOperation: OperationDefinition { if (supportedTypes.has(type) && !aggregationRestrictions) { - return { dataType: type as DataType, isBucketed: false, scale: 'ordinal' }; + return { + dataType: type as DataType, + isBucketed: false, + scale: type === 'string' ? 'ordinal' : 'ratio', + }; } }, getDisabledStatus(indexPattern: IndexPattern) { @@ -102,11 +114,21 @@ export const lastValueOperation: OperationDefinition = FormattedIndexPatternColumn & FieldBasedIndexPatternColumn & { @@ -84,8 +82,8 @@ function buildMetricOperation>({ missing: 0, }, }), - hasInvalidReferences: (column, _indexPattern) => - fieldIsInvalid(column as MetricColumn, _indexPattern), + getErrorMessage: (layer, columnId, indexPattern) => + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), } as OperationDefinition; } 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 ca7d5810b531c..f2d3435cc52c0 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 @@ -17,7 +17,7 @@ import { mergeLayer } from '../../../state_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; -import { fieldIsInvalid } from '..'; +import { getInvalidFieldMessage } from '../helpers'; type RangeType = Omit; // Try to cover all possible serialized states for ranges @@ -110,7 +110,8 @@ export const rangeOperation: OperationDefinition + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), getPossibleOperationForField: ({ aggregationRestrictions, aggregatable, type }) => { if ( type === 'number' && 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 d4e4fdea5ad15..e8351ea1e1d09 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 @@ -22,7 +22,7 @@ import { DataType } from '../../../../types'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { ValuesRangeInput } from './values_range_input'; -import { fieldIsInvalid } from '..'; +import { getInvalidFieldMessage } from '../helpers'; function ofName(name: string) { return i18n.translate('xpack.lens.indexPattern.termsOf', { @@ -72,7 +72,8 @@ export const termsOperation: OperationDefinition + getInvalidFieldMessage(layer.columns[columnId] as FieldBasedIndexPatternColumn, indexPattern), isTransferable: (column, newIndexPattern) => { const newField = newIndexPattern.getFieldByName(column.sourceField); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index e17a0ecf06c80..c0aaa1ead0d56 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -15,7 +15,7 @@ import { createMockedIndexPattern } from '../../../mocks'; import { ValuesRangeInput } from './values_range_input'; import { TermsIndexPatternColumn } from '.'; import { termsOperation } from '../index'; -import { IndexPatternPrivateState, IndexPattern } from '../../../types'; +import { IndexPatternPrivateState, IndexPattern, IndexPatternLayer } from '../../../types'; const defaultProps = { storage: {} as IStorageWrapper, @@ -805,37 +805,49 @@ describe('terms', () => { }); }); }); - describe('hasInvalidReferences', () => { + describe('getErrorMessage', () => { let indexPattern: IndexPattern; - let column: TermsIndexPatternColumn; + let layer: IndexPatternLayer; beforeEach(() => { indexPattern = createMockedIndexPattern(); - column = { - dataType: 'boolean', - isBucketed: true, - label: 'Top values of bytes', - operationType: 'terms', - params: { - missingBucket: false, - orderBy: { type: 'alphabetical' }, - orderDirection: 'asc', - otherBucket: true, - size: 5, + layer = { + columns: { + col1: { + dataType: 'boolean', + isBucketed: true, + label: 'Top values of bytes', + operationType: 'terms', + params: { + missingBucket: false, + orderBy: { type: 'alphabetical' }, + orderDirection: 'asc', + otherBucket: true, + size: 5, + }, + scale: 'ordinal', + sourceField: 'bytes', + }, }, - scale: 'ordinal', - sourceField: 'bytes', + columnOrder: [], + indexPatternId: '', }; }); - it('returns false if sourceField exists in index pattern', () => { - expect(termsOperation.hasInvalidReferences!(column, indexPattern)).toEqual(false); + it('returns undefined if sourceField exists in index pattern', () => { + expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual(undefined); }); - it('returns true if the sourceField does not exist in index pattern', () => { - expect( - termsOperation.hasInvalidReferences!( - { ...column, sourceField: 'notExisting' }, - indexPattern - ) - ).toEqual(true); + it('returns error message if the sourceField does not exist in index pattern', () => { + layer = { + ...layer, + columns: { + col1: { + ...layer.columns.col1, + sourceField: 'notExisting', + } as TermsIndexPatternColumn, + }, + }; + expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ + 'Field notExisting has an invalid reference', + ]); }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts index 18dbd3dc4ab3f..33af8842648f8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/mocks.ts @@ -35,6 +35,6 @@ export const createMockedReferenceOperation = () => { toExpression: jest.fn().mockReturnValue([]), getPossibleOperation: jest.fn().mockReturnValue({ dataType: 'number', isBucketed: false }), getDefaultLabel: jest.fn().mockReturnValue('Default label'), - hasInvalidReferences: jest.fn(), + getErrorMessage: jest.fn(), }; }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index 8bef3edd20b8c..9f2b8eab4e09b 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -293,15 +293,6 @@ describe('getOperationTypesForField', () => { "operationType": "median", "type": "field", }, - ], - }, - Object { - "operationMetaData": Object { - "dataType": "number", - "isBucketed": false, - "scale": "ordinal", - }, - "operations": Array [ Object { "field": "bytes", "operationType": "last_value", diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts index 980025f5a0db4..5f4865ca0ac32 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/utils.ts @@ -13,7 +13,7 @@ import { } from './operations/definitions/column_types'; import { operationDefinitionMap, IndexPatternColumn } from './operations'; -import { fieldIsInvalid as fieldIsInvalidHelper } from './operations/definitions'; +import { getInvalidFieldMessage } from './operations/definitions/helpers'; /** * Normalizes the specified operation type. (e.g. document operations @@ -50,10 +50,9 @@ export function hasInvalidColumns(state: IndexPatternPrivateState) { export function getInvalidLayers(state: IndexPatternPrivateState) { return Object.values(state.layers).filter((layer) => { - return layer.columnOrder.some((columnId) => { - const column = layer.columns[columnId]; - return isColumnInvalid(column, state.indexPatterns[layer.indexPatternId]); - }); + return layer.columnOrder.some((columnId) => + isColumnInvalid(layer, columnId, state.indexPatterns[layer.indexPatternId]) + ); }); } @@ -62,18 +61,23 @@ export function getInvalidColumnsForLayer( indexPatternMap: Record ) { return layers.map((layer) => { - return layer.columnOrder.filter((columnId) => { - const column = layer.columns[columnId]; - return isColumnInvalid(column, indexPatternMap[layer.indexPatternId]); - }); + return layer.columnOrder.filter((columnId) => + isColumnInvalid(layer, columnId, indexPatternMap[layer.indexPatternId]) + ); }); } -export function isColumnInvalid(column: IndexPatternColumn, indexPattern: IndexPattern) { +export function isColumnInvalid( + layer: IndexPatternLayer, + columnId: string, + indexPattern: IndexPattern +) { + const column = layer.columns[columnId]; + const operationDefinition = column.operationType && operationDefinitionMap[column.operationType]; return !!( - operationDefinition.hasInvalidReferences && - operationDefinition.hasInvalidReferences(column, indexPattern) + operationDefinition.getErrorMessage && + operationDefinition.getErrorMessage(layer, columnId, indexPattern) ); } @@ -81,5 +85,5 @@ export function fieldIsInvalid(column: IndexPatternColumn | undefined, indexPatt if (!column || !hasField(column)) { return false; } - return fieldIsInvalidHelper(column, indexPattern); + return !!getInvalidFieldMessage(column, indexPattern)?.length; } From 85f3503263179de22f7b68434741680baa393c2f Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 26 Nov 2020 18:32:38 +0100 Subject: [PATCH 22/27] ts --- .../indexpattern_datasource/operations/layer_helpers.test.ts | 1 - 1 file changed, 1 deletion(-) 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..48dc43b85a3ed 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 @@ -1686,7 +1686,6 @@ 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', From 808bc29d63c8e72e1b33bf978f5b5afec071a0cc Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 30 Nov 2020 09:31:14 +0100 Subject: [PATCH 23/27] copy change to more human --- .../operations/definitions/helpers.tsx | 4 ++-- .../operations/definitions/last_value.test.tsx | 6 +++--- .../operations/definitions/last_value.tsx | 12 +++++++++--- .../operations/definitions/terms/terms.test.tsx | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx index c84325ccc5549..640a357d9a7a4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/helpers.tsx @@ -55,8 +55,8 @@ export function getInvalidFieldMessage( ); return isInvalid ? [ - i18n.translate('xpack.lens.indexPattern.invalidSourceField', { - defaultMessage: 'Field {invalidField} has an invalid reference', + i18n.translate('xpack.lens.indexPattern.fieldNotFound', { + defaultMessage: 'Field {invalidField} was not found', values: { invalidField: sourceField }, }), ] diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index 7535b303eec1f..c411c54f20e01 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -518,7 +518,7 @@ describe('last_value', () => { }, }; expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ - 'Field notExisting has an invalid reference', + 'Field notExisting was not found', ]); }); it('shows error message if the sortField does not exist in index pattern', () => { @@ -535,7 +535,7 @@ describe('last_value', () => { }, }; expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ - 'Field notExisting has an invalid reference', + 'Field notExisting was not found', ]); }); it('shows error message if the sortField is not date', () => { @@ -552,7 +552,7 @@ describe('last_value', () => { }, }; expect(lastValueOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ - 'Field bytes has an invalid reference', + 'Field bytes is not a date field and cannot be used for sorting', ]); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index e26f256609502..48976d98ce3c1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -27,9 +27,15 @@ export function getInvalidSortFieldMessage(sortField: string, indexPattern?: Ind return; } const field = indexPattern.getFieldByName(sortField); - if (!(field && field.type === 'date')) { - return i18n.translate('xpack.lens.indexPattern.lastValue.invalidSortField', { - defaultMessage: 'Field {invalidField} has an invalid reference', + if (!field) { + return i18n.translate('xpack.lens.indexPattern.lastValue.sortFieldNotFound', { + defaultMessage: 'Field {invalidField} was not found', + values: { invalidField: sortField }, + }); + } + if (field.type !== 'date') { + return i18n.translate('xpack.lens.indexPattern.lastValue.invalidTypeSortField', { + defaultMessage: 'Field {invalidField} is not a date field and cannot be used for sorting', values: { invalidField: sortField }, }); } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index c0aaa1ead0d56..d2141e3a69cb4 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -846,7 +846,7 @@ describe('terms', () => { }, }; expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([ - 'Field notExisting has an invalid reference', + 'Field notExisting was not found', ]); }); }); From 15ba028ae27b8cb4e1eb89e77afba217bb9f021f Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 1 Dec 2020 11:09:44 +0100 Subject: [PATCH 24/27] design changes --- .../workspace_panel/warnings_popover.scss | 15 +++++++++++++++ .../workspace_panel/warnings_popover.tsx | 18 ++++++++++-------- .../workspace_panel_wrapper.scss | 4 ---- 3 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss new file mode 100644 index 0000000000000..265b2cb11c0e5 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss @@ -0,0 +1,15 @@ +.lnsWorkspaceWarning__button { + color: $euiColorWarningText; +} + +.lnsWorkspaceWarningList { + overflow-y: auto; + max-height: $euiSize * 20; + width: $euiSize * 16; +} + +.lnsWorkspaceWarningList__item:not(:last-child) { + border-bottom: 1px $euiColorLightShade solid; + padding-bottom: $euiSize; + margin-bottom: $euiSize; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx index 5a3949f2e121b..da8d58d90c271 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx @@ -5,10 +5,11 @@ */ import './workspace_panel_wrapper.scss'; +import './warnings_popover.scss'; import React, { useState } from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiPopover, EuiText, EuiButtonEmpty, EuiHorizontalRule } from '@elastic/eui'; +import { EuiPopover, EuiText, EuiButtonEmpty } from '@elastic/eui'; export const WarningsPopover = ({ children, @@ -30,7 +31,7 @@ export const WarningsPopover = ({ {i18n.translate('xpack.lens.chartWarnings.number', { defaultMessage: `{warningsCount} {warningsCount, plural, one {warning} other {warnings}}`, @@ -43,12 +44,13 @@ export const WarningsPopover = ({ isOpen={isPopoverOpen} closePopover={closePopover} > - {React.Children.map(children, (child, index) => ( - <> - {child} - {warningsCount - 1 !== index && } - - ))} +
    + {React.Children.map(children, (child, index) => ( +
  • + {child} +
    • + ))} +
); }; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss index cec0f2fbea884..ae9294c474b42 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss @@ -115,7 +115,3 @@ 75% { transform: translateY(15%); } 100% { transform: translateY(10%); } } - -.lnsWorkspaceWarningButton { - color: $euiColorWarningText; -} From b42d0c5fb7fb8cad9141cff726ecf00b165de88c Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 1 Dec 2020 15:54:28 +0100 Subject: [PATCH 25/27] designs --- .../workspace_panel/warnings_popover.scss | 12 +++++++----- .../workspace_panel/warnings_popover.tsx | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss index 265b2cb11c0e5..19f815dfb7114 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.scss @@ -3,13 +3,15 @@ } .lnsWorkspaceWarningList { - overflow-y: auto; + @include euiYScroll; max-height: $euiSize * 20; width: $euiSize * 16; } -.lnsWorkspaceWarningList__item:not(:last-child) { - border-bottom: 1px $euiColorLightShade solid; - padding-bottom: $euiSize; - margin-bottom: $euiSize; +.lnsWorkspaceWarningList__item { + padding: $euiSize; + + & + & { + border-top: $euiBorderThin; + } } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx index da8d58d90c271..cb414972e84af 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/warnings_popover.tsx @@ -27,6 +27,7 @@ export const WarningsPopover = ({ const warningsCount = React.Children.count(children); return ( Date: Wed, 2 Dec 2020 15:48:28 +0100 Subject: [PATCH 26/27] design --- .../dimension_panel/dimension_editor.tsx | 2 +- .../operations/definitions/last_value.tsx | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index b29c8aa1cb447..b4231dadf54c0 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 @@ -170,7 +170,7 @@ export function DimensionEditor(props: DimensionEditorProps) { let label: EuiListGroupItemProps['label'] = operationPanels[operationType].displayName; if (disabledStatus) { label = ( - + {operationPanels[operationType].displayName} ); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index 48976d98ce3c1..a895336482bc1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -199,12 +199,16 @@ export const lastValueOperation: OperationDefinition Date: Wed, 2 Dec 2020 16:49:08 +0100 Subject: [PATCH 27/27] remove sortOrder --- .../definitions/last_value.test.tsx | 88 +------------------ .../operations/definitions/last_value.tsx | 61 +------------ .../definitions/terms/terms.test.tsx | 1 - 3 files changed, 5 insertions(+), 145 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx index c411c54f20e01..09b68e78d3469 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { EuiButtonGroup, EuiComboBox } from '@elastic/eui'; +import { EuiComboBox } from '@elastic/eui'; import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public'; import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { dataPluginMock } from '../../../../../../../src/plugins/data/public/mocks'; @@ -66,7 +66,6 @@ describe('last_value', () => { operationType: 'last_value', params: { sortField: 'datefield', - sortOrder: 'asc', }, }, }, @@ -90,7 +89,7 @@ describe('last_value', () => { field: 'a', size: 1, sortField: 'datefield', - sortOrder: 'asc', + sortOrder: 'desc', }), }) ); @@ -107,7 +106,6 @@ describe('last_value', () => { dataType: 'string', params: { sortField: 'datefield', - sortOrder: 'asc', }, }; const indexPattern = createMockedIndexPattern(); @@ -120,7 +118,6 @@ describe('last_value', () => { sourceField: 'bytes', params: expect.objectContaining({ sortField: 'datefield', - sortOrder: 'asc', }), }) ); @@ -136,7 +133,6 @@ describe('last_value', () => { dataType: 'number', params: { sortField: 'datefield', - sortOrder: 'asc', }, }; const indexPattern = createMockedIndexPattern(); @@ -235,21 +231,6 @@ describe('last_value', () => { expect(lastValueColumn.dataType).toEqual('boolean'); }); - it('should set sortOrder to desc by default', () => { - const lastValueColumn = lastValueOperation.buildColumn({ - indexPattern: createMockedIndexPattern(), - field: { - aggregatable: true, - searchable: true, - type: 'boolean', - name: 'test', - displayName: 'test', - }, - layer: { columns: {}, columnOrder: [], indexPatternId: '' }, - }); - expect(lastValueColumn.params.sortOrder).toEqual('desc'); - }); - it('should use indexPattern timeFieldName as a default sortField', () => { const lastValueColumn = lastValueOperation.buildColumn({ indexPattern: createMockedIndexPattern(), @@ -418,69 +399,6 @@ describe('last_value', () => { }, }); }); - - it('should render sortOrder value and options', () => { - const setStateSpy = jest.fn(); - const instance = shallow( - - ); - - const select = instance - .find('[data-test-subj="lns-indexPattern-lastValue-sortOrder"]') - .find(EuiButtonGroup); - - expect(select.prop('idSelected')).toEqual('lns-lastValue-ascending'); - - expect(select.prop('options')!.map(({ value }: { value?: string }) => value)).toEqual([ - 'asc', - 'desc', - ]); - }); - - it('should update state when changing sortOrder', () => { - const setStateSpy = jest.fn(); - const instance = shallow( - - ); - - const select = instance - .find('[data-test-subj="lns-indexPattern-lastValue-sortOrder"]') - .find(EuiButtonGroup); - - select.prop('onChange')!('lns-lastValue-descending'); - expect(setStateSpy).toHaveBeenCalledWith({ - ...state, - layers: { - first: { - ...state.layers.first, - columns: { - ...state.layers.first.columns, - col2: { - ...state.layers.first.columns.col2, - params: { - ...(state.layers.first.columns.col2 as LastValueIndexPatternColumn).params, - sortOrder: 'desc', - }, - }, - }, - }, - }, - }); - }); }); describe('getErrorMessage', () => { @@ -495,7 +413,7 @@ describe('last_value', () => { isBucketed: false, label: 'Last value of test', operationType: 'last_value', - params: { sortField: 'timestamp', sortOrder: 'desc' }, + params: { sortField: 'timestamp' }, scale: 'ratio', sourceField: 'bytes', }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx index a895336482bc1..5ae5dd472ce22 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.tsx @@ -5,7 +5,7 @@ */ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiFormRow, EuiComboBox, EuiButtonGroup, EuiComboBoxOptionOption } from '@elastic/eui'; +import { EuiFormRow, EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternField, IndexPattern } from '../../types'; @@ -70,7 +70,6 @@ export interface LastValueIndexPatternColumn extends FieldBasedIndexPatternColum operationType: 'last_value'; params: { sortField: string; - sortOrder: 'asc' | 'desc'; // last value on numeric fields can be formatted format?: { id: string; @@ -149,12 +148,6 @@ export const lastValueOperation: OperationDefinition { const currentIndexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; const dateFields = getDateFields(currentIndexPattern); - const sortOrderButtons = [ - { - id: `lns-lastValue-ascending`, - label: i18n.translate('xpack.lens.indexPattern.lastValue.sortFieldAscending', { - defaultMessage: 'Ascending', - }), - value: 'asc', - }, - { - id: `lns-lastValue-descending`, - label: i18n.translate('xpack.lens.indexPattern.lastValue.sortField', { - defaultMessage: 'Descending', - }), - value: 'desc', - }, - ]; const isSortFieldInvalid = !!getInvalidSortFieldMessage( currentColumn.params.sortField, currentIndexPattern @@ -275,39 +251,6 @@ export const lastValueOperation: OperationDefinition - - value === currentColumn.params?.sortOrder)!.id - } - onChange={(optionId: string) => { - const value = sortOrderButtons.find(({ id }) => id === optionId)!.value; - setState( - updateColumnParam({ - state, - layerId, - currentColumn, - paramName: 'sortOrder', - value, - }) - ); - }} - /> - ); }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index d2141e3a69cb4..0af0f9a9d8613 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -418,7 +418,6 @@ describe('terms', () => { sourceField: 'bytes', operationType: 'last_value', params: { - sortOrder: 'desc', sortField: 'time', }, },