Skip to content

Commit

Permalink
fix(sorting): add cellValueCouldBeUndefined in grid option for sorting
Browse files Browse the repository at this point in the history
- Some systems like Salesforce sometime remove properties of an object if it's detected as null and this becomes undefined in slickgrid, add a  global grid options to deal with these undefined properties
  • Loading branch information
ghiscoding committed Dec 17, 2020
1 parent 12638cc commit 24584b1
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 38 deletions.
3 changes: 2 additions & 1 deletion src/app/modules/angular-slickgrid/models/column.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ export interface Column<T = any> {
validator?: EditorValidator;

/**
* 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.
* Defaults to false, can the value be undefined?
* Typically undefined values are disregarded when sorting, when setting this flag it 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)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ export interface GridOption {
/** Cell menu options (Action menu) */
cellMenu?: CellMenu;

/**
* Defaults to false, can the cell value (dataContext) be undefined?
* Typically undefined values are disregarded when sorting, when setting this flag it 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)
*/
cellValueCouldBeUndefined?: boolean;

/** Checkbox Select Plugin options (columnId, cssClass, toolTip, width) */
checkboxSelector?: CheckboxSelector;

Expand Down
3 changes: 2 additions & 1 deletion src/app/modules/angular-slickgrid/models/sorter.interface.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Column } from './column.interface';
import { GridOption } from './gridOption.interface';
import { SortDirectionNumber } from './sortDirectionNumber.enum';

export type Sorter = (value1: any, value2: any, sortDirection: SortDirectionNumber, sortColumn?: Column) => number;
export type Sorter = (value1: any, value2: any, sortDirection: SortDirectionNumber, sortColumn?: Column, gridOptions?: GridOption) => number;
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe('CollectionService', () => {
describe('sortCollection method', () => {
it('should return a collection of numbers sorted', () => {
translate.use('en');
const columnDef = { id: 'count', field: 'count', fieldType: FieldType.number } as Column;
const columnDef = { id: 'count', field: 'count', type: FieldType.number } as Column;

const result1 = service.sortCollection(columnDef, [0, -11, 3, 99999, -200], { sortDesc: false } as CollectionSortBy);
const result2 = service.sortCollection(columnDef, [0, -11, 3, 99999, -200], { sortDesc: true } as CollectionSortBy);
Expand All @@ -328,7 +328,7 @@ describe('CollectionService', () => {
it('should return a collection of translation values sorted', () => {
translate.use('en');
const roleCollection = ['SALES_REP', 'DEVELOPER', 'SALES_REP', null, 'HUMAN_RESOURCES', 'FINANCE_MANAGER', 'UNKNOWN'];
const columnDef = { id: 'count', field: 'count', fieldType: FieldType.string } as Column;
const columnDef = { id: 'count', field: 'count', type: FieldType.string } as Column;

const result1 = service.sortCollection(columnDef, [...roleCollection], { sortDesc: false } as CollectionSortBy, true);
const result2 = service.sortCollection(columnDef, [...roleCollection], { sortDesc: true } as CollectionSortBy, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class CollectionService<T = any> {
// collection of objects with a property name provided
const sortDirection = sortBy.sortDesc ? SortDirectionNumber.desc : SortDirectionNumber.asc;
const objectProperty = sortBy.property;
const fieldType = sortBy.fieldType || FieldType.string;
const fieldType = sortBy.fieldType || columnDef && columnDef.type || FieldType.string;
const value1 = (enableTranslateLabel) ? this.translate && this.translate.currentLang && this.translate.instant(dataRow1[objectProperty] || ' ') : dataRow1[objectProperty];
const value2 = (enableTranslateLabel) ? this.translate && this.translate.currentLang && this.translate.instant(dataRow2[objectProperty] || ' ') : dataRow2[objectProperty];

Expand All @@ -136,7 +136,7 @@ export class CollectionService<T = any> {
// collection of objects with a property name provided
const objectProperty = sortByOptions.property;
const sortDirection = sortByOptions.sortDesc ? SortDirectionNumber.desc : SortDirectionNumber.asc;
const fieldType = sortByOptions.fieldType || FieldType.string;
const fieldType = sortByOptions.fieldType || columnDef && columnDef.type || FieldType.string;

if (objectProperty) {
sortedCollection = collection.sort((dataRow1: any, dataRow2: any) => {
Expand All @@ -151,7 +151,7 @@ export class CollectionService<T = any> {
}
} else if (sortByOptions && !sortByOptions.property) {
const sortDirection = sortByOptions.sortDesc ? SortDirectionNumber.desc : SortDirectionNumber.asc;
const fieldType = sortByOptions.fieldType || FieldType.string;
const fieldType = sortByOptions.fieldType || columnDef && columnDef.type || FieldType.string;

sortedCollection = collection.sort((dataRow1: any, dataRow2: any) => {
const value1 = (enableTranslateLabel) ? this.translate && this.translate.currentLang && this.translate.instant(dataRow1 || ' ') : dataRow1;
Expand Down
4 changes: 2 additions & 2 deletions src/app/modules/angular-slickgrid/services/sort.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,12 +442,12 @@ export class SortService {

// user could provide his own custom Sorter
if (columnDef.sorter) {
const customSortResult = columnDef.sorter(value1, value2, sortDirection, columnDef);
const customSortResult = columnDef.sorter(value1, value2, sortDirection, columnDef, this._gridOptions);
if (customSortResult !== SortDirectionNumber.neutral) {
return customSortResult;
}
} else {
const sortResult = sortByFieldType(fieldType, value1, value2, sortDirection, columnDef);
const sortResult = sortByFieldType(fieldType, value1, value2, sortDirection, columnDef, this._gridOptions);
if (sortResult !== SortDirectionNumber.neutral) {
return sortResult;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ 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, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' }, undefined);
});

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, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' }, undefined);
});

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, { id: 'field1', field: 'field1' });
expect(spy).toHaveBeenCalledWith(0, 4, SortDirectionNumber.asc, { id: 'field1', field: 'field1' }, undefined);
});

it('should call the Sorters.objectString when FieldType is objectString', () => {
Expand All @@ -27,6 +27,6 @@ describe('sorterUtilities', () => {
const mockColumn = { id: 'field1', field: 'field1', dataKey: 'firstName' } as Column;
const spy = jest.spyOn(Sorters, 'objectString');
sortByFieldType(FieldType.object, object1, object2, SortDirectionNumber.asc, mockColumn);
expect(spy).toHaveBeenCalledWith(object1, object2, SortDirectionNumber.asc, mockColumn);
expect(spy).toHaveBeenCalledWith(object1, object2, SortDirectionNumber.asc, mockColumn, undefined);
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Column, SortDirectionNumber } from '../../models';
import { Column, GridOption, SortDirectionNumber } from '../../models';
import { stringSorter } from '../stringSorter';

describe('the String Sorter', () => {
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('the String Sorter', () => {
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', () => {
it('should return a sorted ascending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set in the column definition', () => {
// 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;
Expand All @@ -53,12 +53,30 @@ describe('the String Sorter', () => {
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', () => {
it('should return a sorted descending array and move the undefined values to the end of the array when "valueCouldBeUndefined" is set in the column definition', () => {
// 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]);
});

it('should return a sorted ascending array and move the undefined values to the end of the array when "cellValueCouldBeUndefined" is set in the grid options', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name' } 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, { cellValueCouldBeUndefined: true } as GridOption));
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 "cellValueCouldBeUndefined" is set in the grid options', () => {
// from MDN specification quote: All undefined elements are sorted to the end of the array.
const columnDef = { id: 'name', field: 'name' } 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, { cellValueCouldBeUndefined: true } as GridOption));
expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', undefined, undefined]);
});
});
12 changes: 6 additions & 6 deletions src/app/modules/angular-slickgrid/sorters/dateUtilities.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { mapMomentDateFormatWithFieldType } from '../services/utilities';
import { Column, FieldType, Sorter } from '../models/index';
import { Column, FieldType, GridOption, Sorter } from '../models/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, sortColumn: Column, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
export function compareDates(value1: any, value2: any, sortDirection: number, sortColumn: Column, gridOptions: GridOption, format: string | moment_.MomentBuiltinFormat, strict?: boolean) {
let diff = 0;
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || gridOptions && gridOptions.cellValueCouldBeUndefined || false;

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

return (value1: any, value2: any, sortDirection: number, sortColumn: Column) => {
return (value1: any, value2: any, sortDirection: number, sortColumn: Column, gridOptions: GridOption) => {
if (FORMAT === moment.ISO_8601) {
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, false);
return compareDates(value1, value2, sortDirection, sortColumn, gridOptions, FORMAT, false);
}
return compareDates(value1, value2, sortDirection, sortColumn, FORMAT, true);
return compareDates(value1, value2, sortDirection, sortColumn, gridOptions, FORMAT, true);
};
}
6 changes: 3 additions & 3 deletions src/app/modules/angular-slickgrid/sorters/numericSorter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Column, Sorter } from './../models/index';
import { Column, GridOption, Sorter } from './../models/index';

export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number, sortColumn?: Column) => {
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
export const numericSorter: Sorter = (value1: any, value2: any, sortDirection: number, sortColumn?: Column, gridOptions?: GridOption) => {
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || gridOptions && gridOptions.cellValueCouldBeUndefined || 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));
Expand Down
10 changes: 6 additions & 4 deletions src/app/modules/angular-slickgrid/sorters/objectStringSorter.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Column, Sorter, SortDirectionNumber } from './../models/index';
import { Column, GridOption, Sorter, SortDirectionNumber } from './../models/index';

export const objectStringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn: Column) => {
export const objectStringSorter: Sorter = (value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn: Column, gridOptions?: GridOption) => {
if (!sortColumn || !sortColumn.dataKey) {
throw new Error('Sorting a "FieldType.object" requires you to provide the "dataKey" (object property name) of the object so that we can use it to sort correctly');
}

const stringValue1 = (value1 && value1.hasOwnProperty(sortColumn.dataKey)) ? value1[sortColumn.dataKey] : value1;
const stringValue2 = (value2 && value2.hasOwnProperty(sortColumn.dataKey)) ? value2[sortColumn.dataKey] : value2;
const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || gridOptions && gridOptions.cellValueCouldBeUndefined || false;

if (sortDirection === undefined || sortDirection === null) {
sortDirection = SortDirectionNumber.neutral;
}
Expand All @@ -16,9 +18,9 @@ export const objectStringSorter: Sorter = (value1: any, value2: any, sortDirecti
position = -99e+10;
} else if (typeof value2 !== 'object') {
position = 99e+10;
} else if (!stringValue1) {
} else if (stringValue1 === null || (checkForUndefinedValues && stringValue1 === undefined)) {
position = -1;
} else if (!stringValue2) {
} else if (stringValue2 === null || (checkForUndefinedValues && stringValue2 === undefined)) {
position = 1;
} else if (stringValue1 === stringValue2) {
position = 0;
Expand Down
12 changes: 6 additions & 6 deletions src/app/modules/angular-slickgrid/sorters/sorterUtilities.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { Column, FieldType, SortDirectionNumber, Sorter } from './../models/index';
import { Column, FieldType, GridOption, SortDirectionNumber } from './../models/index';
import { Sorters } from './index';
import { getAssociatedDateSorter } from './dateUtilities';

export function sortByFieldType(fieldType: FieldType, value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn?: Column) {
export function sortByFieldType(fieldType: FieldType, value1: any, value2: any, sortDirection: number | SortDirectionNumber, sortColumn?: Column, gridOptions?: GridOption) {
let sortResult = 0;

switch (fieldType) {
case FieldType.float:
case FieldType.integer:
case FieldType.number:
sortResult = Sorters.numeric(value1, value2, sortDirection, sortColumn);
sortResult = Sorters.numeric(value1, value2, sortDirection, sortColumn, gridOptions);
break;
case FieldType.date:
case FieldType.dateIso:
Expand Down Expand Up @@ -37,17 +37,17 @@ 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, sortColumn);
sortResult = getAssociatedDateSorter(fieldType).call(this, value1, value2, sortDirection, sortColumn, gridOptions);
break;
case FieldType.object:
sortResult = Sorters.objectString(value1, value2, sortDirection, sortColumn);
sortResult = Sorters.objectString(value1, value2, sortDirection, sortColumn, gridOptions);
break;
case FieldType.string:
case FieldType.text:
case FieldType.password:
case FieldType.readonly:
default:
sortResult = Sorters.string(value1, value2, sortDirection, sortColumn);
sortResult = Sorters.string(value1, value2, sortDirection, sortColumn, gridOptions);
break;
}

Expand Down
6 changes: 3 additions & 3 deletions src/app/modules/angular-slickgrid/sorters/stringSorter.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Column, Sorter, SortDirectionNumber } from './../models/index';
import { Column, GridOption, Sorter, SortDirectionNumber } from './../models/index';

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

if (value1 === value2) {
position = 0;
Expand Down

0 comments on commit 24584b1

Please sign in to comment.