Skip to content

Commit 1e75cd4

Browse files
flash1293flash1293
committed
[Lens] Fix sorting undefined, null and NaN values (#92575)
1 parent 179f6cb commit 1e75cd4

2 files changed

Lines changed: 181 additions & 10 deletions

File tree

‎x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx‎

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,20 @@ function testSorting({
1919
direction,
2020
type,
2121
keepLast,
22+
reverseOutput = true,
2223
}: {
2324
input: unknown[];
2425
output: unknown[];
2526
direction: 'asc' | 'desc';
2627
type: DatatableColumnType | 'range';
2728
keepLast?: boolean; // special flag to handle values that should always be last no matter the direction
29+
reverseOutput?: boolean;
2830
}) {
2931
const datatable = input.map((v) => ({
3032
a: v,
3133
}));
3234
const sorted = output.map((v) => ({ a: v }));
33-
if (direction === 'desc') {
35+
if (direction === 'desc' && reverseOutput) {
3436
sorted.reverse();
3537
if (keepLast) {
3638
// Cycle shift of the first element
@@ -64,6 +66,44 @@ describe('Data sorting criteria', () => {
6466
});
6567
});
6668
}
69+
70+
it(`should sort undefined and null to the end`, () => {
71+
const now = Date.now();
72+
testSorting({
73+
input: [null, now, 0, undefined, null, now - 150000],
74+
output: [0, now - 150000, now, null, undefined, null],
75+
direction: 'asc',
76+
type: 'date',
77+
reverseOutput: false,
78+
});
79+
80+
testSorting({
81+
input: [null, now, 0, undefined, null, now - 150000],
82+
output: [now, now - 150000, 0, null, undefined, null],
83+
direction: 'desc',
84+
type: 'date',
85+
reverseOutput: false,
86+
});
87+
});
88+
89+
it(`should sort NaN to the end`, () => {
90+
const now = Date.now();
91+
testSorting({
92+
input: [null, now, 0, undefined, Number.NaN, now - 150000],
93+
output: [0, now - 150000, now, null, undefined, Number.NaN],
94+
direction: 'asc',
95+
type: 'number',
96+
reverseOutput: false,
97+
});
98+
99+
testSorting({
100+
input: [null, now, 0, undefined, Number.NaN, now - 150000],
101+
output: [now, now - 150000, 0, null, undefined, Number.NaN],
102+
direction: 'desc',
103+
type: 'number',
104+
reverseOutput: false,
105+
});
106+
});
67107
});
68108

69109
describe('String or anything else as string', () => {
@@ -86,6 +126,39 @@ describe('Data sorting criteria', () => {
86126
});
87127
});
88128
}
129+
130+
it('should sort undefined and null to the end', () => {
131+
testSorting({
132+
input: ['a', null, 'b', 'c', undefined, 'd', '12'],
133+
output: ['12', 'a', 'b', 'c', 'd', null, undefined],
134+
direction: 'asc',
135+
type: 'string',
136+
reverseOutput: false,
137+
});
138+
139+
testSorting({
140+
input: ['a', null, 'b', 'c', undefined, 'd', '12'],
141+
output: ['d', 'c', 'b', 'a', '12', null, undefined],
142+
direction: 'desc',
143+
type: 'string',
144+
reverseOutput: false,
145+
});
146+
147+
testSorting({
148+
input: [true, null, false, undefined, false],
149+
output: [false, false, true, null, undefined],
150+
direction: 'asc',
151+
type: 'boolean',
152+
reverseOutput: false,
153+
});
154+
testSorting({
155+
input: [true, null, false, undefined, false],
156+
output: [true, false, false, null, undefined],
157+
direction: 'desc',
158+
type: 'boolean',
159+
reverseOutput: false,
160+
});
161+
});
89162
});
90163

91164
describe('IP sorting', () => {
@@ -154,6 +227,60 @@ describe('Data sorting criteria', () => {
154227
});
155228
});
156229
}
230+
231+
it('should sort undefined and null to the end', () => {
232+
testSorting({
233+
input: [
234+
'fc00::123',
235+
'192.168.1.50',
236+
null,
237+
undefined,
238+
'Other',
239+
'10.0.1.76',
240+
'8.8.8.8',
241+
'::1',
242+
],
243+
output: [
244+
'::1',
245+
'8.8.8.8',
246+
'10.0.1.76',
247+
'192.168.1.50',
248+
'fc00::123',
249+
'Other',
250+
null,
251+
undefined,
252+
],
253+
direction: 'asc',
254+
type: 'ip',
255+
reverseOutput: false,
256+
});
257+
258+
testSorting({
259+
input: [
260+
'fc00::123',
261+
'192.168.1.50',
262+
null,
263+
undefined,
264+
'Other',
265+
'10.0.1.76',
266+
'8.8.8.8',
267+
'::1',
268+
],
269+
output: [
270+
'fc00::123',
271+
'192.168.1.50',
272+
'10.0.1.76',
273+
'8.8.8.8',
274+
'::1',
275+
'Other',
276+
null,
277+
undefined,
278+
],
279+
direction: 'desc',
280+
type: 'ip',
281+
reverseOutput: false,
282+
});
283+
});
157284
});
158285

159286
describe('Range sorting', () => {
@@ -184,5 +311,22 @@ describe('Data sorting criteria', () => {
184311
});
185312
});
186313
}
314+
315+
it('should sort undefined and null to the end', () => {
316+
testSorting({
317+
input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }],
318+
output: [{ gte: 0, lt: 5 }, { gte: 0 }, { gte: 1, lt: 5 }, undefined, null],
319+
direction: 'asc',
320+
type: 'range',
321+
reverseOutput: false,
322+
});
323+
testSorting({
324+
input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }],
325+
output: [{ gte: 1, lt: 5 }, { gte: 0 }, { gte: 0, lt: 5 }, undefined, null],
326+
direction: 'desc',
327+
type: 'range',
328+
reverseOutput: false,
329+
});
330+
});
187331
});
188332
});

‎x-pack/plugins/lens/public/datatable_visualization/sorting.tsx‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ function getRangeCriteria(sortBy: string, directionFactor: number) {
6262
};
6363
}
6464

65+
type CompareFn = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number;
66+
6567
export function getSortingCriteria(
6668
type: string | undefined,
6769
sortBy: string,
@@ -71,22 +73,47 @@ export function getSortingCriteria(
7173
// handle the direction with a multiply factor.
7274
const directionFactor = direction === 'asc' ? 1 : -1;
7375

76+
let criteria: CompareFn;
77+
7478
if (['number', 'date'].includes(type || '')) {
75-
return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) =>
79+
criteria = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) =>
7680
directionFactor * ((rowA[sortBy] as number) - (rowB[sortBy] as number));
7781
}
7882
// this is a custom type, and can safely assume the gte and lt fields are all numbers or undefined
79-
if (type === 'range') {
80-
return getRangeCriteria(sortBy, directionFactor);
83+
else if (type === 'range') {
84+
criteria = getRangeCriteria(sortBy, directionFactor);
8185
}
8286
// IP have a special sorting
83-
if (type === 'ip') {
84-
return getIPCriteria(sortBy, directionFactor);
87+
else if (type === 'ip') {
88+
criteria = getIPCriteria(sortBy, directionFactor);
89+
} else {
90+
// use a string sorter for the rest
91+
criteria = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
92+
const aString = formatter.convert(rowA[sortBy]);
93+
const bString = formatter.convert(rowB[sortBy]);
94+
return directionFactor * aString.localeCompare(bString);
95+
};
8596
}
86-
// use a string sorter for the rest
97+
return getUndefinedHandler(sortBy, criteria);
98+
}
99+
100+
function getUndefinedHandler(
101+
sortBy: string,
102+
sortingCriteria: (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number
103+
) {
87104
return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
88-
const aString = formatter.convert(rowA[sortBy]);
89-
const bString = formatter.convert(rowB[sortBy]);
90-
return directionFactor * aString.localeCompare(bString);
105+
const valueA = rowA[sortBy];
106+
const valueB = rowB[sortBy];
107+
if (valueA != null && valueB != null && !Number.isNaN(valueA) && !Number.isNaN(valueB)) {
108+
return sortingCriteria(rowA, rowB);
109+
}
110+
if (valueA == null || Number.isNaN(valueA)) {
111+
return 1;
112+
}
113+
if (valueB == null || Number.isNaN(valueB)) {
114+
return -1;
115+
}
116+
117+
return 0;
91118
};
92119
}

0 commit comments

Comments
 (0)