fix(sorting): add cellValueCouldBeUndefined in grid option for sorting

- Salesforce very often removes property of an object if it's detected as null and this becomes undefined in slickgrid, add a salesforce global grid options to deal with these undefined property

packages/common/src/global-grid-options.ts

@@ -182,7 +182,7 @@ export const GlobalGridOptions: GridOption = {
     iconClearFilterCommand: 'fa fa-filter mdi mdi mdi-filter-remove-outline',
     iconClearSortCommand: 'fa fa-unsorted mdi mdi-swap-vertical',
     iconFreezeColumns: 'fa fa-thumb-tack mdi mdi-pin-outline',
-    iconSortAscCommand: 'fa fa-sort-amount-asc mdi mdi-sort-ascending',
+    iconSortAscCommand: 'fa fa-sort-amount-asc mdi mdi-flip-v mdi-sort-ascending',
     iconSortDescCommand: 'fa fa-sort-amount-desc mdi mdi-flip-v mdi-sort-descending',
     iconColumnHideCommand: 'fa fa-times mdi mdi-close',
     hideColumnHideCommand: false,

packages/common/src/interfaces/column.interface.ts

@@ -261,7 +261,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)
    */

packages/common/src/interfaces/gridOption.interface.ts

@@ -81,6 +81,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?: CheckboxSelectorOption;
 

packages/common/src/interfaces/sorter.interface.ts

@@ -1,4 +1,5 @@
 import { Column } from './column.interface';
+import { GridOption } from './gridOption.interface';
 import { SortDirectionNumber } from '../enums/sortDirectionNumber.enum';
 
-export type SortComparer = (value1: any, value2: any, sortDirection: SortDirectionNumber, sortColumn?: Column) => number;
+export type SortComparer = (value1: any, value2: any, sortDirection: SortDirectionNumber, sortColumn?: Column, gridOptions?: GridOption) => number;

packages/common/src/services/__tests__/collection.service.spec.ts

@@ -288,7 +288,7 @@ describe('CollectionService', () => {
       describe('sortCollection method', () => {
         it('should return a collection of numbers sorted', () => {
           translateService.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);
@@ -300,7 +300,7 @@ describe('CollectionService', () => {
         it('should return a collection of translation values sorted', () => {
           translateService.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);

packages/common/src/services/collection.service.ts

@@ -110,13 +110,13 @@ export class CollectionService<T = any> {
           for (let i = 0, l = sortByOptions.length; i < l; i++) {
             const sortBy = sortByOptions[i];
 
-            if (sortBy && sortBy.property) {
+            if (sortBy?.property) {
               // 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 value1 = (enableTranslateLabel) ? this.translaterService && this.translaterService.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow1[objectProperty] || ' ') : dataRow1[objectProperty];
-              const value2 = (enableTranslateLabel) ? this.translaterService && this.translaterService.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow2[objectProperty] || ' ') : dataRow2[objectProperty];
+              const fieldType = sortBy?.fieldType ?? columnDef?.type ?? FieldType.string;
+              const value1 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow1[objectProperty] || ' ') : dataRow1[objectProperty];
+              const value2 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow2[objectProperty] || ' ') : dataRow2[objectProperty];
 
               const sortResult = sortByFieldType(fieldType, value1, value2, sortDirection, columnDef);
               if (sortResult !== SortDirectionNumber.neutral) {
@@ -126,16 +126,16 @@ export class CollectionService<T = any> {
           }
           return SortDirectionNumber.neutral;
         });
-      } else if (sortByOptions && sortByOptions.property) {
+      } else if (sortByOptions?.property) {
         // single sort
         // 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?.type ?? FieldType.string;
 
         sortedCollection = collection.sort((dataRow1: T, dataRow2: T) => {
-          const value1 = (enableTranslateLabel) ? this.translaterService && this.translaterService.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow1[objectProperty] || ' ') : dataRow1[objectProperty];
-          const value2 = (enableTranslateLabel) ? this.translaterService && this.translaterService.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow2[objectProperty] || ' ') : dataRow2[objectProperty];
+          const value1 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow1[objectProperty] || ' ') : dataRow1[objectProperty];
+          const value2 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow2[objectProperty] || ' ') : dataRow2[objectProperty];
           const sortResult = sortByFieldType(fieldType, value1, value2, sortDirection, columnDef);
           if (sortResult !== SortDirectionNumber.neutral) {
             return sortResult;
@@ -144,11 +144,11 @@ 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?.type ?? FieldType.string;
 
         sortedCollection = collection.sort((dataRow1: any, dataRow2: any) => {
-          const value1 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow1 || ' ') : dataRow1;
-          const value2 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.getCurrentLanguage && this.translaterService.getCurrentLanguage() && this.translaterService.translate(dataRow2 || ' ') : dataRow2;
+          const value1 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow1 || ' ') : dataRow1;
+          const value2 = (enableTranslateLabel) ? this.translaterService?.translate && this.translaterService.translate(dataRow2 || ' ') : dataRow2;
           const sortResult = sortByFieldType(fieldType, value1, value2, sortDirection, columnDef);
           if (sortResult !== SortDirectionNumber.neutral) {
             return sortResult;

packages/common/src/services/sort.service.ts

@@ -416,7 +416,7 @@ export class SortService {
   }
 
   sortComparer(sortColumn: ColumnSort, dataRow1: any, dataRow2: any, querySortField?: string): number | undefined {
-    if (sortColumn && sortColumn.sortCol) {
+    if (sortColumn?.sortCol) {
       const columnDef = sortColumn.sortCol;
       const sortDirection = sortColumn.sortAsc ? SortDirectionNumber.asc : SortDirectionNumber.desc;
       let queryFieldName1 = querySortField || columnDef.queryFieldSorter || columnDef.queryField || columnDef.field;
@@ -442,12 +442,12 @@ export class SortService {
 
       // user could provide his own custom Sorter
       if (columnDef.sortComparer) {
-        const customSortResult = columnDef.sortComparer(value1, value2, sortDirection, columnDef);
+        const customSortResult = columnDef.sortComparer(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;
         }

packages/common/src/sortComparers/__tests__/sortUtilities.spec.ts

@@ -7,27 +7,27 @@ describe('sortUtilities', () => {
   it('should call the SortComparers.numeric when FieldType is number', () => {
     const spy = jest.spyOn(SortComparers, '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 SortComparers.numeric when FieldType is integer', () => {
     const spy = jest.spyOn(SortComparers, '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 SortComparers.numeric when FieldType is float', () => {
     const spy = jest.spyOn(SortComparers, '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 SortComparers.string when FieldType is a string (which is also the default)', () => {
     const string1 = 'John';
     const string2 = 'Jane';
     const spy = jest.spyOn(SortComparers, 'string');
     sortByFieldType(FieldType.string, string1, string2, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
-    expect(spy).toHaveBeenCalledWith(string1, string2, SortDirectionNumber.asc, { id: 'field1', field: 'field1' });
+    expect(spy).toHaveBeenCalledWith(string1, string2, SortDirectionNumber.asc, { id: 'field1', field: 'field1' }, undefined);
   });
 
   it('should call the SortComparers.objectString when FieldType is objectString', () => {
@@ -36,6 +36,6 @@ describe('sortUtilities', () => {
     const mockColumn = { id: 'field1', field: 'field1', dataKey: 'firstName' } as Column;
     const spy = jest.spyOn(SortComparers, '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);
   });
 });

packages/common/src/sortComparers/__tests__/stringSortComparer.spec.ts

@@ -1,6 +1,6 @@
 import { SortDirectionNumber } from '../../enums/index';
 import { stringSortComparer } from '../stringSortComparer';
-import { Column } from '../../interfaces/column.interface';
+import { Column, GridOption } from '../../interfaces/index';
 
 describe('the String SortComparer', () => {
   it('should return original unsorted array when no direction is provided', () => {
@@ -45,7 +45,7 @@ describe('the String SortComparer', () => {
     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;
@@ -54,12 +54,30 @@ describe('the String SortComparer', () => {
     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) => stringSortComparer(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) => stringSortComparer(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) => stringSortComparer(value1, value2, direction, columnDef, { cellValueCouldBeUndefined: true } as GridOption));
+    expect(inputArray).toEqual(['zebra', 'amazon', 'abc', 'John', 'Abe', '@at', '', undefined, undefined]);
+  });
 });

packages/common/src/sortComparers/dateUtilities.ts

@@ -1,12 +1,12 @@
 import { mapMomentDateFormatWithFieldType } from '../services/utilities';
 import { FieldType } from '../enums/fieldType.enum';
-import { Column, SortComparer } from '../interfaces/index';
+import { Column, GridOption, SortComparer } 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, 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?.valueCouldBeUndefined ?? gridOptions?.cellValueCouldBeUndefined ?? false;
 
   if (value1 === null || value1 === '' || (checkForUndefinedValues && value1 === undefined) || !moment(value1, format, strict).isValid()) {
     diff = -1;
@@ -25,10 +25,10 @@ export function compareDates(value1: any, value2: any, sortDirection: number, so
 export function getAssociatedDateSortComparer(fieldType: typeof FieldType[keyof typeof FieldType]): SortComparer {
   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);
   };
 }

packages/common/src/sortComparers/numericSortComparer.ts

@@ -1,7 +1,7 @@
-import { Column, SortComparer } from '../interfaces/index';
+import { Column, GridOption, SortComparer } from '../interfaces/index';
 
-export const numericSortComparer: SortComparer = (value1: any, value2: any, sortDirection: number, sortColumn?: Column) => {
-  const checkForUndefinedValues = sortColumn && sortColumn.valueCouldBeUndefined || false;
+export const numericSortComparer: SortComparer = (value1: any, value2: any, sortDirection: number, sortColumn?: Column, gridOptions?: GridOption) => {
+  const checkForUndefinedValues = sortColumn?.valueCouldBeUndefined ?? 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));