Skip to content

Commit

Permalink
feat(sort): add valueCouldBeUndefined column flag to help sorting
Browse files Browse the repository at this point in the history
- this is an opt-in flag because it could bring unwanted behavior and for example it breaks the Row Detail UI when sorting undefined values, so user can opt-in but it's off by default
  • Loading branch information
ghiscoding-SE committed Apr 8, 2020
1 parent b1a552f commit 6d2b6a6
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 23 deletions.
7 changes: 7 additions & 0 deletions packages/common/src/interfaces/column.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,13 @@ export interface Column {
/** Defaults to false, when set to True will lead to the column being unselected in the UI */
unselectable?: boolean;

/**
* Can the value be undefined? Typically undefined values are disregarded when sorting, when set this flag will adds extra logic to Sorting and also sort undefined value.
* This is an extra flag that user has to enable by themselve because Sorting undefined values has unwanted behavior in some use case
* (for example Row Detail has UI inconsistencies since undefined is used in the plugin's logic)
*/
valueCouldBeUndefined?: boolean;

/** Editor Validator */
validator?: EditorValidator;

Expand Down
19 changes: 19 additions & 0 deletions packages/common/src/sorters/__tests__/dateIsoSorter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { sortByFieldType } from '../sorterUtilities';
import { FieldType, SortDirectionNumber } from '../../enums/index';
import { Column } from '../../interfaces/column.interface';

describe('the Date ISO (without time) Sorter', () => {
it('should return an array of US dates sorted ascending when only valid dates are provided', () => {
Expand Down Expand Up @@ -30,4 +31,22 @@ describe('the Date ISO (without time) Sorter', () => {
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction));
expect(inputArray).toEqual(['2001-01-01', '1998-12-14', '1998-10-08', '1998-08-08', null]);
});

it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.asc;
const inputArray = ['1998-10-08', undefined, '1998-08-08', '2001-01-01', '1998-12-14'];
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction, columnDef));
expect(inputArray).toEqual(['1998-08-08', '1998-10-08', '1998-12-14', '2001-01-01', undefined]);
});

it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.desc;
const inputArray = ['1998-10-08', undefined, '1998-08-08', '2001-01-01', '1998-12-14'];
inputArray.sort((value1, value2) => sortByFieldType(FieldType.dateIso, value1, value2, direction, columnDef));
expect(inputArray).toEqual(['2001-01-01', '1998-12-14', '1998-10-08', '1998-08-08', undefined]);
});
});
19 changes: 19 additions & 0 deletions packages/common/src/sorters/__tests__/numericSorter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { SortDirectionNumber } from '../../enums/sortDirectionNumber.enum';
import { numericSorter } from '../numericSorter';
import { Column } from '../../interfaces/column.interface';

describe('the Numeric Sorter', () => {
it('should return an array of numbers sorted ascending when only numbers are provided', () => {
Expand Down Expand Up @@ -37,4 +38,22 @@ describe('the Numeric Sorter', () => {
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction));
expect(inputArray).toEqual(['z', 'a', '', null]);
});

it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.asc;
const inputArray = [4, undefined, 39, 1, -15, -2, 0, 5, 500, 50];
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction, columnDef));
expect(inputArray).toEqual([-15, -2, 0, 1, 4, 5, 39, 50, 500, undefined]);
});

it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.desc;
const inputArray = [4, undefined, 39, 1, -15, -2, 0, 5, 500, 50];
inputArray.sort((value1, value2) => numericSorter(value1, value2, direction, columnDef));
expect(inputArray).toEqual([500, 50, 39, 5, 4, 1, 0, -2, -15, undefined]);
});
});
10 changes: 5 additions & 5 deletions packages/common/src/sorters/__tests__/sorterUtilities.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@ describe('sorterUtilities', () => {
it('should call the Sorters.numeric when FieldType is number', () => {
const spy = jest.spyOn(Sorters, 'numeric');
sortByFieldType(FieldType.number, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
});

it('should call the Sorters.numeric when FieldType is integer', () => {
const spy = jest.spyOn(Sorters, 'numeric');
sortByFieldType(FieldType.integer, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
});

it('should call the Sorters.numeric when FieldType is float', () => {
const spy = jest.spyOn(Sorters, 'numeric');
sortByFieldType(FieldType.float, 0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc);
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
});

it('should call the Sorters.string when FieldType is a string (which is also the default)', () => {
const string1 = 'John';
const string2 = 'Jane';
const spy = jest.spyOn(Sorters, 'string');
sortByFieldType(FieldType.string, string1, string2, SortDirectionNumber.asc);
expect(spy).toHaveBeenCalledWith(string1, string2, SortDirectionNumber.asc);
sortByFieldType(FieldType.string, string1, string2, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(string1, string2, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
});

it('should call the Sorters.objectString when FieldType is objectString', () => {
Expand Down
19 changes: 19 additions & 0 deletions packages/common/src/sorters/__tests__/stringSorter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { SortDirectionNumber } from '../../enums/index';
import { stringSorter } from '../stringSorter';
import { Column } from '../../interfaces/column.interface';

describe('the String Sorter', () => {
it('should return original unsorted array when no direction is provided', () => {
Expand Down Expand Up @@ -43,4 +44,22 @@ describe('the String Sorter', () => {
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction));
expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', null, null]);
});

it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.asc;
const inputArray = ['amazon', undefined, 'zebra', undefined, '', '@at', 'John', 'Abe', 'abc'];
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction, columnDef));
expect(inputArray).toEqual(['', '@at', 'Abe', 'John', 'abc', 'amazon', 'zebra', undefined, undefined]);
});

it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name', valueCouldBeUndefined: true } as Column;
const direction = SortDirectionNumber.desc;
const inputArray = ['amazon', undefined, 'zebra', undefined, '', '@at', 'John', 'Abe', 'abc'];
inputArray.sort((value1, value2) => stringSorter(value1, value2, direction, columnDef));
expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', undefined, undefined]);
});
});
15 changes: 8 additions & 7 deletions packages/common/src/sorters/dateUtilities.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { mapMomentDateFormatWithFieldType } from '../services/utilities';
import { FieldType } from '../enums/fieldType.enum';
import { Sorter } from '../interfaces/index';
import { Column, Sorter } from '../interfaces/index';
import * as moment_ from 'moment-mini';
const moment = moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670

export function compareDates(value1: any, value2: any, sortDirection: number, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
export function compareDates(value1: any, value2: any, sortDirection: number, sortColumn: Column, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
let diff = 0;
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;

if (value1 === null || value1 === '' || !moment(value1, format, strict).isValid()) {
if (value1 === null || value1 === '' || (checkForUndefinedValues && value1 === undefined) || !moment(value1, format, strict).isValid()) {
diff = -1;
} else if (value2 === null || value2 === '' || !moment(value2, format, strict).isValid()) {
} else if (value2 === null || value2 === '' || (checkForUndefinedValues && value2 === undefined) || !moment(value2, format, strict).isValid()) {
diff = 1;
} else {
const date1 = moment(value1, format, strict);
Expand All @@ -24,10 +25,10 @@ export function compareDates(value1: any, value2: any, sortDirection: number, fo
export function getAssociatedDateSorter(fieldType: FieldType): Sorter {
const FORMAT = (fieldType === FieldType.date) ? moment.ISO_8601 : mapMomentDateFormatWithFieldType(fieldType);

return (value1: any, value2: any, sortDirection: number) => {
return (value1: any, value2: any, sortDirection: number, sortColumn: Column) => {
if (FORMAT === moment.ISO_8601) {
return compareDates(value1, value2, sortDirection, FORMAT, false);
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, false);
}
return compareDates(value1, value2, sortDirection, FORMAT, true);
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, true);
};
}
9 changes: 5 additions & 4 deletions packages/common/src/sorters/numericSorter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Sorter } from '../interfaces/sorter.interface';
import { Column, Sorter } from '../interfaces/index';

export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number) => {
const x = (isNaN(value1) || value1 === '' || value1 === null) ? -99e+10 : parseFloat(value1);
const y = (isNaN(value2) || value2 === '' || value2 === null) ? -99e+10 : parseFloat(value2);
export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number, sortColumn?: Column) => {
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
const x = (isNaN(value1) || value1 === '' || value1 === null || (checkForUndefinedValues && value1 === undefined)) ? -99e+10 : parseFloat(value1);
const y = (isNaN(value2) || value2 === '' || value2 === null || (checkForUndefinedValues && value2 === undefined)) ? -99e+10 : parseFloat(value2);
return sortDirection * (x === y ? 0 : (x > y ? 1 : -1));
};
6 changes: 3 additions & 3 deletions packages/common/src/sorters/sorterUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function sortByFieldType(fieldType: FieldType, value1: any, value2: any,
case FieldType.float:
case FieldType.integer:
case FieldType.number:
sortResult = Sorters.numeric(value1, value2, sortDirection);
sortResult = Sorters.numeric(value1, value2, sortDirection, sortColumn);
break;
case FieldType.date:
case FieldType.dateIso:
Expand Down Expand Up @@ -39,14 +39,14 @@ export function sortByFieldType(fieldType: FieldType, value1: any, value2: any,
case FieldType.dateTimeUsShort:
case FieldType.dateTimeUsShortAmPm:
case FieldType.dateTimeUsShortAM_PM:
sortResult = getAssociatedDateSorter(fieldType).call(this, value1, value2, sortDirection);
sortResult = getAssociatedDateSorter(fieldType).call(this, value1, value2, sortDirection, sortColumn);
break;
case FieldType.object:
sortResult = Sorters.objectString(value1, value2, sortDirection, sortColumn);
break;
case FieldType.string:
default:
sortResult = Sorters.string(value1, value2, sortDirection);
sortResult = Sorters.string(value1, value2, sortDirection, sortColumn);
break;
}

Expand Down
9 changes: 5 additions & 4 deletions packages/common/src/sorters/stringSorter.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { Sorter } from '../interfaces/index';
import { Column, Sorter } from '../interfaces/index';
import { SortDirectionNumber } from '../enums/sortDirectionNumber.enum';

export const stringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber) => {
export const stringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn?: Column) => {
if (sortDirection === undefined || sortDirection === null) {
sortDirection = SortDirectionNumber.neutral;
}
let position = 0;
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;

if (value1 === value2) {
position = 0;
} else if (value1 === null || value1 === undefined) {
} else if (value1 === null || (checkForUndefinedValues && value1 === undefined)) {
position = -1;
} else if (value2 === null || value2 === undefined) {
} else if (value2 === null || (checkForUndefinedValues && value2 === undefined)) {
position = 1;
} else if (sortDirection) {
position = value1 < value2 ? -1 : 1;
Expand Down

0 comments on commit 6d2b6a6

Please sign in to comment.