diff --git a/x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx index ed93ddddd39f4d..f5d457db652341 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx @@ -19,18 +19,20 @@ function testSorting({ direction, type, keepLast, + reverseOutput = true, }: { input: unknown[]; output: unknown[]; direction: 'asc' | 'desc'; type: DatatableColumnType | 'range'; keepLast?: boolean; // special flag to handle values that should always be last no matter the direction + reverseOutput?: boolean; }) { const datatable = input.map((v) => ({ a: v, })); const sorted = output.map((v) => ({ a: v })); - if (direction === 'desc') { + if (direction === 'desc' && reverseOutput) { sorted.reverse(); if (keepLast) { // Cycle shift of the first element @@ -64,6 +66,44 @@ describe('Data sorting criteria', () => { }); }); } + + it(`should sort undefined and null to the end`, () => { + const now = Date.now(); + testSorting({ + input: [null, now, 0, undefined, null, now - 150000], + output: [0, now - 150000, now, null, undefined, null], + direction: 'asc', + type: 'date', + reverseOutput: false, + }); + + testSorting({ + input: [null, now, 0, undefined, null, now - 150000], + output: [now, now - 150000, 0, null, undefined, null], + direction: 'desc', + type: 'date', + reverseOutput: false, + }); + }); + + it(`should sort NaN to the end`, () => { + const now = Date.now(); + testSorting({ + input: [null, now, 0, undefined, Number.NaN, now - 150000], + output: [0, now - 150000, now, null, undefined, Number.NaN], + direction: 'asc', + type: 'number', + reverseOutput: false, + }); + + testSorting({ + input: [null, now, 0, undefined, Number.NaN, now - 150000], + output: [now, now - 150000, 0, null, undefined, Number.NaN], + direction: 'desc', + type: 'number', + reverseOutput: false, + }); + }); }); describe('String or anything else as string', () => { @@ -86,6 +126,39 @@ describe('Data sorting criteria', () => { }); }); } + + it('should sort undefined and null to the end', () => { + testSorting({ + input: ['a', null, 'b', 'c', undefined, 'd', '12'], + output: ['12', 'a', 'b', 'c', 'd', null, undefined], + direction: 'asc', + type: 'string', + reverseOutput: false, + }); + + testSorting({ + input: ['a', null, 'b', 'c', undefined, 'd', '12'], + output: ['d', 'c', 'b', 'a', '12', null, undefined], + direction: 'desc', + type: 'string', + reverseOutput: false, + }); + + testSorting({ + input: [true, null, false, undefined, false], + output: [false, false, true, null, undefined], + direction: 'asc', + type: 'boolean', + reverseOutput: false, + }); + testSorting({ + input: [true, null, false, undefined, false], + output: [true, false, false, null, undefined], + direction: 'desc', + type: 'boolean', + reverseOutput: false, + }); + }); }); describe('IP sorting', () => { @@ -154,6 +227,60 @@ describe('Data sorting criteria', () => { }); }); } + + it('should sort undefined and null to the end', () => { + testSorting({ + input: [ + 'fc00::123', + '192.168.1.50', + null, + undefined, + 'Other', + '10.0.1.76', + '8.8.8.8', + '::1', + ], + output: [ + '::1', + '8.8.8.8', + '10.0.1.76', + '192.168.1.50', + 'fc00::123', + 'Other', + null, + undefined, + ], + direction: 'asc', + type: 'ip', + reverseOutput: false, + }); + + testSorting({ + input: [ + 'fc00::123', + '192.168.1.50', + null, + undefined, + 'Other', + '10.0.1.76', + '8.8.8.8', + '::1', + ], + output: [ + 'fc00::123', + '192.168.1.50', + '10.0.1.76', + '8.8.8.8', + '::1', + 'Other', + null, + undefined, + ], + direction: 'desc', + type: 'ip', + reverseOutput: false, + }); + }); }); describe('Range sorting', () => { @@ -184,5 +311,22 @@ describe('Data sorting criteria', () => { }); }); } + + it('should sort undefined and null to the end', () => { + testSorting({ + input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }], + output: [{ gte: 0, lt: 5 }, { gte: 0 }, { gte: 1, lt: 5 }, undefined, null], + direction: 'asc', + type: 'range', + reverseOutput: false, + }); + testSorting({ + input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }], + output: [{ gte: 1, lt: 5 }, { gte: 0 }, { gte: 0, lt: 5 }, undefined, null], + direction: 'desc', + type: 'range', + reverseOutput: false, + }); + }); }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/sorting.tsx b/x-pack/plugins/lens/public/datatable_visualization/sorting.tsx index 9c3716827f54f2..0859ab5428c9ef 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/sorting.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/sorting.tsx @@ -62,6 +62,8 @@ function getRangeCriteria(sortBy: string, directionFactor: number) { }; } +type CompareFn = (rowA: Record, rowB: Record) => number; + export function getSortingCriteria( type: string | undefined, sortBy: string, @@ -71,22 +73,47 @@ export function getSortingCriteria( // handle the direction with a multiply factor. const directionFactor = direction === 'asc' ? 1 : -1; + let criteria: CompareFn; + if (['number', 'date'].includes(type || '')) { - return (rowA: Record, rowB: Record) => + criteria = (rowA: Record, rowB: Record) => directionFactor * ((rowA[sortBy] as number) - (rowB[sortBy] as number)); } // this is a custom type, and can safely assume the gte and lt fields are all numbers or undefined - if (type === 'range') { - return getRangeCriteria(sortBy, directionFactor); + else if (type === 'range') { + criteria = getRangeCriteria(sortBy, directionFactor); } // IP have a special sorting - if (type === 'ip') { - return getIPCriteria(sortBy, directionFactor); + else if (type === 'ip') { + criteria = getIPCriteria(sortBy, directionFactor); + } else { + // use a string sorter for the rest + criteria = (rowA: Record, rowB: Record) => { + const aString = formatter.convert(rowA[sortBy]); + const bString = formatter.convert(rowB[sortBy]); + return directionFactor * aString.localeCompare(bString); + }; } - // use a string sorter for the rest + return getUndefinedHandler(sortBy, criteria); +} + +function getUndefinedHandler( + sortBy: string, + sortingCriteria: (rowA: Record, rowB: Record) => number +) { return (rowA: Record, rowB: Record) => { - const aString = formatter.convert(rowA[sortBy]); - const bString = formatter.convert(rowB[sortBy]); - return directionFactor * aString.localeCompare(bString); + const valueA = rowA[sortBy]; + const valueB = rowB[sortBy]; + if (valueA != null && valueB != null && !Number.isNaN(valueA) && !Number.isNaN(valueB)) { + return sortingCriteria(rowA, rowB); + } + if (valueA == null || Number.isNaN(valueA)) { + return 1; + } + if (valueB == null || Number.isNaN(valueB)) { + return -1; + } + + return 0; }; }