fix(sorting): multi-column sort shouldn't work when option is disabled

- with Tree Data only supporting single sort, I found out that we could bypass the single sort by using the header menu
- also found that sort presets with multiple columns were also loading as multiple even with the multiColumnSort disabled and now it's fixed

packages/common/src/extensions/headerMenuExtension.ts

@@ -383,30 +383,31 @@ export class HeaderMenuExtension implements Extension {
     if (args && args.column) {
       // get previously sorted columns
       const columnDef = args.column;
-      const sortedColsWithoutCurrent = this.sortService.getCurrentColumnSorts(columnDef.id + '');
+
+      // 1- get the sort columns without the current column, in the case of a single sort that would equal to an empty array
+      const tmpSortedColumns = !this.sharedService.gridOptions.multiColumnSort ? [] : this.sortService.getCurrentColumnSorts(columnDef.id + '');
 
       let emitterType: EmitterType = EmitterType.local;
 
-      // add to the column array, the column sorted by the header menu
-      sortedColsWithoutCurrent.push({ columnId: columnDef.id, sortCol: columnDef, sortAsc: isSortingAsc });
+      // 2- add to the column array, the new sorted column by the header menu
+      tmpSortedColumns.push({ columnId: columnDef.id, sortCol: columnDef, sortAsc: isSortingAsc });
+
       if (this.sharedService.gridOptions.backendServiceApi) {
-        this.sortService.onBackendSortChanged(event, { multiColumnSort: true, sortCols: sortedColsWithoutCurrent, grid: this.sharedService.slickGrid });
+        this.sortService.onBackendSortChanged(event, { multiColumnSort: true, sortCols: tmpSortedColumns, grid: this.sharedService.slickGrid });
         emitterType = EmitterType.remote;
       } else if (this.sharedService.dataView) {
-        this.sortService.onLocalSortChanged(this.sharedService.slickGrid, sortedColsWithoutCurrent);
+        this.sortService.onLocalSortChanged(this.sharedService.slickGrid, tmpSortedColumns);
         emitterType = EmitterType.local;
       } else {
         // when using customDataView, we will simply send it as a onSort event with notify
-        const isMultiSort = this.sharedService && this.sharedService.gridOptions && this.sharedService.gridOptions.multiColumnSort || false;
-        const sortOutput = isMultiSort ? sortedColsWithoutCurrent : sortedColsWithoutCurrent[0];
-        args.grid.onSort.notify(sortOutput);
+        args.grid.onSort.notify(tmpSortedColumns);
       }
 
       // update the sharedService.slickGrid sortColumns array which will at the same add the visual sort icon(s) on the UI
-      const newSortColumns = sortedColsWithoutCurrent.map((col) => {
+      const newSortColumns = tmpSortedColumns.map(col => {
         return {
-          columnId: col && col.sortCol && col.sortCol.id,
-          sortAsc: col && col.sortAsc,
+          columnId: col?.sortCol?.id ?? '',
+          sortAsc: col?.sortAsc ?? true,
         };
       });
 

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

@@ -656,6 +656,7 @@ describe('SortService', () => {
 
   describe('toggleSortFunctionality method', () => {
     beforeEach(() => {
+      gridOptionMock.multiColumnSort = true;
       gridOptionMock.enableSorting = true;
     });
 
@@ -696,7 +697,7 @@ describe('SortService', () => {
       jest.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
     });
 
-    it('should load local grid presets', () => {
+    it('should load local grid multiple presets sorting when multiColumnSort is enabled', () => {
       const spySetCols = jest.spyOn(gridStub, 'setSortColumns');
       const spySortChanged = jest.spyOn(service, 'onLocalSortChanged');
       const expectation = [
@@ -713,6 +714,22 @@ describe('SortService', () => {
       ]);
       expect(spySortChanged).toHaveBeenCalledWith(gridStub, expectation);
     });
+
+    it('should load local grid with only a single sort when multiColumnSort is disabled even when passing multiple column sorters', () => {
+      const spySetCols = jest.spyOn(gridStub, 'setSortColumns');
+      const spySortChanged = jest.spyOn(service, 'onLocalSortChanged');
+      const expectation = [
+        { columnId: 'firstName', sortAsc: true, sortCol: { id: 'firstName', field: 'firstName' } },
+        { columnId: 'lastName', sortAsc: false, sortCol: { id: 'lastName', field: 'lastName' } },
+      ];
+
+      gridOptionMock.multiColumnSort = false;
+      service.bindLocalOnSort(gridStub);
+      service.loadGridSorters(gridOptionMock.presets.sorters);
+
+      expect(spySetCols).toHaveBeenCalledWith([{ columnId: 'firstName', sortAsc: true }]);
+      expect(spySortChanged).toHaveBeenCalledWith(gridStub, [expectation[0]]);
+    });
   });
 
   describe('undefined getColumns & getOptions', () => {
@@ -909,6 +926,7 @@ describe('SortService', () => {
       gridStub.getOptions = () => gridOptionMock;
       gridOptionMock.enableSorting = true;
       gridOptionMock.backendServiceApi = undefined;
+      gridOptionMock.multiColumnSort = true;
 
       mockNewSorters = [
         { columnId: 'firstName', direction: 'ASC' },

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

@@ -286,7 +286,9 @@ export class SortService {
     const sortCols: ColumnSort[] = [];
 
     if (Array.isArray(sorters)) {
-      sorters.forEach((sorter: CurrentSorter) => {
+      const tmpSorters = this._gridOptions.multiColumnSort ? sorters : sorters.slice(0, 1);
+
+      tmpSorters.forEach((sorter: CurrentSorter) => {
         const gridColumn = this._columnDefinitions.find((col: Column) => col.id === sorter.columnId);
         if (gridColumn) {
           sortCols.push({

packages/vanilla-bundle/src/components/__tests__/slick-vanilla-grid.spec.ts

@@ -1209,16 +1209,29 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
         expect(spy).toHaveBeenCalledWith(mockFilters, true);
       });
 
-      it('should call the "updateSorters" method when filters are defined in the "presets" property', () => {
+      it('should call the "updateSorters" method when sorters are defined in the "presets" property with multi-column sort enabled', () => {
         jest.spyOn(mockGrid, 'getSelectionModel').mockReturnValue(true as any);
         const spy = jest.spyOn(mockGraphqlService, 'updateSorters');
-        const mockSorters = [{ columnId: 'name', direction: 'asc' }] as CurrentSorter[];
+        const mockSorters = [{ columnId: 'firstName', direction: 'asc' }, { columnId: 'lastName', direction: 'desc' }] as CurrentSorter[];
+
         component.gridOptions.presets = { sorters: mockSorters };
         component.initialization(divContainer, slickEventHandler);
 
         expect(spy).toHaveBeenCalledWith(undefined, mockSorters);
       });
 
+      it('should call the "updateSorters" method with ONLY 1 column sort when multi-column sort is disabled and user provided multiple sorters in the "presets" property', () => {
+        jest.spyOn(mockGrid, 'getSelectionModel').mockReturnValue(true as any);
+        const spy = jest.spyOn(mockGraphqlService, 'updateSorters');
+        const mockSorters = [{ columnId: 'firstName', direction: 'asc' }, { columnId: 'lastName', direction: 'desc' }] as CurrentSorter[];
+
+        component.gridOptions.multiColumnSort = false;
+        component.gridOptions.presets = { sorters: mockSorters };
+        component.initialization(divContainer, slickEventHandler);
+
+        expect(spy).toHaveBeenCalledWith(undefined, [mockSorters[0]]);
+      });
+
       it('should call the "updatePagination" method when filters are defined in the "presets" property', () => {
         const spy = jest.spyOn(mockGraphqlService, 'updatePagination');
 

packages/vanilla-bundle/src/components/slick-vanilla-grid-bundle.ts

@@ -898,7 +898,9 @@ export class SlickVanillaGridBundle {
         }
         // Sorters "presets"
         if (backendApiService.updateSorters && Array.isArray(gridOptions.presets.sorters) && gridOptions.presets.sorters.length > 0) {
-          backendApiService.updateSorters(undefined, gridOptions.presets.sorters);
+          // when using multi-column sort, we can have multiple but on single sort then only grab the first sort provided
+          const sortColumns = this._gridOptions.multiColumnSort ? gridOptions.presets.sorters : gridOptions.presets.sorters.slice(0, 1);
+          backendApiService.updateSorters(undefined, sortColumns);
         }
         // Pagination "presets"
         if (backendApiService.updatePagination && gridOptions.presets.pagination) {
@@ -975,7 +977,9 @@ export class SlickVanillaGridBundle {
     // if user entered some Sort "presets", we need to reflect them all in the DOM
     if (gridOptions.enableSorting) {
       if (gridOptions.presets && Array.isArray(gridOptions.presets.sorters)) {
-        this.sortService.loadGridSorters(gridOptions.presets.sorters);
+        // when using multi-column sort, we can have multiple but on single sort then only grab the first sort provided
+        const sortColumns = this._gridOptions.multiColumnSort ? gridOptions.presets.sorters : gridOptions.presets.sorters.slice(0, 1);
+        this.sortService.loadGridSorters(sortColumns);
       }
     }
   }