Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Fix sorting undefined, null and NaN values #92575

Merged
merged 2 commits into from
Feb 24, 2021
Merged

[Lens] Fix sorting undefined, null and NaN values #92575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f0d906
fix sorting undefined and null values
flash1293 Feb 24, 2021
9b43ea2
add NaN handling
flash1293 Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 145 additions & 1 deletion x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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,
});
});
});
});
45 changes: 36 additions & 9 deletions x-pack/plugins/lens/public/datatable_visualization/sorting.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ function getRangeCriteria(sortBy: string, directionFactor: number) {
};
}

type CompareFn = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number;

export function getSortingCriteria(
type: string | undefined,
sortBy: string,
Expand All @@ -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<string, unknown>, rowB: Record<string, unknown>) =>
criteria = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) =>
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<string, unknown>, rowB: Record<string, unknown>) => {
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<string, unknown>, rowB: Record<string, unknown>) => number
) {
return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
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;
};
}