From efa89e6b96fa41345fc3f15e2435f28da3792f56 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 11 Feb 2022 16:14:42 +0100 Subject: [PATCH] [Lens] Version-aware sorting to data table (#125361) --- .../expressions/datatable/datatable_column.ts | 5 ++- .../expressions/datatable/datatable_fn.ts | 8 +++-- .../expressions/datatable/sorting.test.tsx | 33 ++++++++++++++++++- .../common/expressions/datatable/sorting.tsx | 24 ++++++++++++++ x-pack/plugins/lens/common/types.ts | 2 ++ .../datatable_visualization/visualization.tsx | 2 ++ .../indexpattern_datasource/indexpattern.tsx | 24 +++++++++++--- x-pack/plugins/lens/public/types.ts | 3 +- 8 files changed, 91 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts index 892631c3b0f45..7d9954697f493 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts @@ -6,6 +6,7 @@ */ import type { Direction } from '@elastic/eui'; +import { SortingHint } from '../..'; import type { CustomPaletteState, PaletteOutput, @@ -28,6 +29,7 @@ export type ColumnConfigArg = Omit & { type: 'lens_datatable_column'; palette?: PaletteOutput; summaryRowValue?: unknown; + sortingHint?: SortingHint; }; export interface ColumnState { @@ -53,7 +55,7 @@ export type DatatableColumnResult = ColumnState & { type: 'lens_datatable_column export const datatableColumn: ExpressionFunctionDefinition< 'lens_datatable_column', null, - ColumnState, + ColumnState & { sortingHint?: SortingHint }, DatatableColumnResult > = { name: 'lens_datatable_column', @@ -64,6 +66,7 @@ export const datatableColumn: ExpressionFunctionDefinition< args: { columnId: { types: ['string'], help: '' }, alignment: { types: ['string'], help: '' }, + sortingHint: { types: ['string'], help: '' }, hidden: { types: ['boolean'], help: '' }, width: { types: ['number'], help: '' }, isTransposed: { types: ['boolean'], help: '' }, diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts index 4cd2a57cbc429..9d9e666e12762 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts @@ -64,11 +64,13 @@ export const datatableFn = } if (sortBy && columnsReverseLookup[sortBy] && sortDirection !== 'none') { + const sortingHint = args.columns.find((col) => col.columnId === sortBy)?.sortingHint; // Sort on raw values for these types, while use the formatted value for the rest const sortingCriteria = getSortingCriteria( - isRange(columnsReverseLookup[sortBy]?.meta) - ? 'range' - : columnsReverseLookup[sortBy]?.meta?.type, + sortingHint ?? + (isRange(columnsReverseLookup[sortBy]?.meta) + ? 'range' + : columnsReverseLookup[sortBy]?.meta?.type), sortBy, formatters[sortBy], sortDirection diff --git a/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx b/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx index b5ff0a7e6915b..24dca87c7f138 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx +++ b/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx @@ -24,7 +24,7 @@ function testSorting({ input: unknown[]; output: unknown[]; direction: 'asc' | 'desc'; - type: DatatableColumnType | 'range'; + type: DatatableColumnType | 'range' | 'version'; keepLast?: boolean; // special flag to handle values that should always be last no matter the direction reverseOutput?: boolean; }) { @@ -106,6 +106,37 @@ describe('Data sorting criteria', () => { }); }); + describe('Version sorting', () => { + for (const direction of ['asc', 'desc'] as const) { + it(`should provide the version criteria for terms values (${direction})`, () => { + testSorting({ + input: ['1.21.0', '1.1.0', '1.112.0', '1.0.0'], + output: ['1.0.0', '1.1.0', '1.21.0', '1.112.0'], + direction, + type: 'version', + }); + }); + } + + it('should sort non-version stuff to the end', () => { + testSorting({ + input: ['1.21.0', undefined, '1.1.0', null, '1.112.0', '__other__', '1.0.0'], + output: ['1.0.0', '1.1.0', '1.21.0', '1.112.0', undefined, null, '__other__'], + direction: 'asc', + type: 'version', + reverseOutput: false, + }); + + testSorting({ + input: ['1.21.0', undefined, '1.1.0', null, '1.112.0', '__other__', '1.0.0'], + output: ['1.112.0', '1.21.0', '1.1.0', '1.0.0', undefined, null, '__other__'], + direction: 'desc', + type: 'version', + reverseOutput: false, + }); + }); + }); + describe('String or anything else as string', () => { for (const direction of ['asc', 'desc'] as const) { it(`should provide the string criteria for terms values (${direction})`, () => { diff --git a/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx b/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx index 30060c10ea37e..d4a3ba0ea6908 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx +++ b/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx @@ -5,6 +5,8 @@ * 2.0. */ +import versionCompare from 'compare-versions'; +import valid from 'semver/functions/valid'; import ipaddr from 'ipaddr.js'; import type { IPv4, IPv6 } from 'ipaddr.js'; import type { FieldFormat } from '../../../../../../src/plugins/field_formats/common'; @@ -48,6 +50,25 @@ function getIPCriteria(sortBy: string, directionFactor: number) { }; } +function getVersionCriteria(sortBy: string, directionFactor: number) { + return (rowA: Record, rowB: Record) => { + const valueA = String(rowA[sortBy] ?? ''); + const valueB = String(rowB[sortBy] ?? ''); + const aInvalid = !valueA || !valid(valueA); + const bInvalid = !valueB || !valid(valueB); + if (aInvalid && bInvalid) { + return 0; + } + if (aInvalid) { + return 1; + } + if (bInvalid) { + return -1; + } + return directionFactor * versionCompare(valueA, valueB); + }; +} + function getRangeCriteria(sortBy: string, directionFactor: number) { // fill missing fields with these open bounds to perform number sorting const openRange = { gte: -Infinity, lt: Infinity }; @@ -86,6 +107,9 @@ export function getSortingCriteria( // IP have a special sorting else if (type === 'ip') { criteria = getIPCriteria(sortBy, directionFactor); + } else if (type === 'version') { + // do not wrap in undefined behandler because of special invalid-case handling + return getVersionCriteria(sortBy, directionFactor); } else { // use a string sorter for the rest criteria = (rowA: Record, rowB: Record) => { diff --git a/x-pack/plugins/lens/common/types.ts b/x-pack/plugins/lens/common/types.ts index 0243aeef41c2d..f3572fea90f9e 100644 --- a/x-pack/plugins/lens/common/types.ts +++ b/x-pack/plugins/lens/common/types.ts @@ -47,6 +47,8 @@ export interface ColorStop { stop: number; } +export type SortingHint = 'version'; + export interface CustomPaletteParams { name?: string; reverse?: boolean; diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index ccfd6a49a2d8b..6f5fb9c17a5aa 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -363,6 +363,7 @@ export const getDatatableVisualization = ({ : [], reverse: false, // managed at UI level }; + const sortingHint = datasource!.getOperationForColumnId(column.columnId)!.sortingHint; const hasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; @@ -388,6 +389,7 @@ export const getDatatableVisualization = ({ summaryLabel: hasNoSummaryRow ? [] : [column.summaryLabel ?? getDefaultSummaryLabel(column.summaryRow!)], + sortingHint: sortingHint ? [sortingHint] : [], }, }, ], diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 005cbd4b11977..8efb667120f77 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -48,12 +48,17 @@ import { getVisualDefaultsForLayer, isColumnInvalid } from './utils'; import { normalizeOperationDataType, isDraggedField } from './pure_utils'; import { LayerPanel } from './layerpanel'; import { GenericIndexPatternColumn, getErrorMessages, insertNewColumn } from './operations'; -import { IndexPatternField, IndexPatternPrivateState, IndexPatternPersistedState } from './types'; +import { + IndexPatternField, + IndexPatternPrivateState, + IndexPatternPersistedState, + IndexPattern, +} from './types'; import { KibanaContextProvider, KibanaThemeProvider, } from '../../../../../src/plugins/kibana_react/public'; -import { DataPublicPluginStart } from '../../../../../src/plugins/data/public'; +import { DataPublicPluginStart, ES_FIELD_TYPES } from '../../../../../src/plugins/data/public'; import { VisualizeFieldContext } from '../../../../../src/plugins/ui_actions/public'; import { mergeLayer } from './state_helpers'; import { Datasource, StateSetter } from '../types'; @@ -69,15 +74,22 @@ export { deleteColumn } from './operations'; export function columnToOperation( column: GenericIndexPatternColumn, - uniqueLabel?: string + uniqueLabel?: string, + dataView?: IndexPattern ): Operation { const { dataType, label, isBucketed, scale, operationType } = column; + const fieldTypes = + 'sourceField' in column ? dataView?.getFieldByName(column.sourceField)?.esTypes : undefined; return { dataType: normalizeOperationDataType(dataType), isBucketed, scale, label: uniqueLabel || label, isStaticValue: operationType === 'static_value', + sortingHint: + column.dataType === 'string' && fieldTypes?.includes(ES_FIELD_TYPES.VERSION) + ? 'version' + : undefined, }; } @@ -446,7 +458,11 @@ export function getIndexPatternDatasource({ if (layer && layer.columns[columnId]) { if (!isReferenced(layer, columnId)) { - return columnToOperation(layer.columns[columnId], columnLabelMap[columnId]); + return columnToOperation( + layer.columns[columnId], + columnLabelMap[columnId], + state.indexPatterns[layer.indexPatternId] + ); } } return null; diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index fd5eb473c42d7..7cffd7bd88c17 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -18,7 +18,7 @@ import type { Datatable, } from '../../../../src/plugins/expressions/public'; import { DraggingIdentifier, DragDropIdentifier, DragContextState } from './drag_drop'; -import type { DateRange, LayerType } from '../common'; +import type { DateRange, LayerType, SortingHint } from '../common'; import type { Query } from '../../../../src/plugins/data/public'; import type { RangeSelectContext, @@ -436,6 +436,7 @@ export type DataType = 'string' | 'number' | 'date' | 'boolean' | FieldOnlyDataT export interface Operation extends OperationMetadata { // User-facing label for the operation label: string; + sortingHint?: SortingHint; } export interface OperationMetadata {