fix(tree): couple of issues found in Tree Data, fixes #307

1. initial sort not always working
2. tree level property should not be required while providing a parentId relation
3. tree data was loading and rendering the grid more than once (at least 1x time before the sort and another time after the tree was built) while it should only be rendered once

examples/webpack-demo-vanilla-bundle/src/examples/example05.html

@@ -28,13 +28,13 @@ <h6 class="title is-6 italic">
       <span>Expand All</span>
     </button>
     <button onclick.delegate="logFlatStructure()" class="button is-small">
-      <span>Log Flag Structure</span>
+      <span>Log Flat Structure</span>
     </button>
-    <button onclick.delegate="logExpandedStructure()" class="button is-small">
-      <span>Log Expanded Structure</span>
+    <button onclick.delegate="logHierarchicalStructure()" class="button is-small">
+      <span>Log Hierarchical Structure</span>
     </button>
   </div>
 </div>
 
 <div class="grid5">
-</div>
+</div>

examples/webpack-demo-vanilla-bundle/src/examples/example05.ts

@@ -92,8 +92,13 @@ export class Example5 {
       enableTreeData: true, // you must enable this flag for the filtering & sorting to work as expected
       treeDataOptions: {
         columnId: 'title',
-        levelPropName: 'indent',
-        parentPropName: 'parentId'
+        parentPropName: 'parentId',
+
+        // you can optionally sort by a different column and/or sort direction
+        initialSort: {
+          columnId: 'title',
+          direction: 'ASC'
+        }
       },
       multiColumnSort: false, // multi-column sorting is not supported with Tree Data, so you need to disable it
       presets: {
@@ -151,8 +156,8 @@ export class Example5 {
     this.sgb.treeDataService.toggleTreeDataCollapse(false);
   }
 
-  logExpandedStructure() {
-    console.log('exploded array', this.sgb.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
+  logHierarchicalStructure() {
+    console.log('hierarchical array', this.sgb.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
   }
 
   logFlatStructure() {
@@ -188,9 +193,9 @@ export class Example5 {
       }
 
       d['id'] = i;
-      d['indent'] = indent;
       d['parentId'] = parentId;
-      d['title'] = 'Task ' + i;
+      // d['title'] = `Task ${i}    -   [P]: ${parentId}`;
+      d['title'] = `Task ${i}`;
       d['duration'] = '5 days';
       d['percentComplete'] = Math.round(Math.random() * 100);
       d['start'] = new Date(randomYear, randomMonth, randomDay);

examples/webpack-demo-vanilla-bundle/src/examples/example06.html

@@ -27,10 +27,10 @@ <h6 class="title is-6 italic">
       <span>Expand All</span>
     </button>
     <button onclick.delegate="logFlatStructure()" class="button is-small">
-      <span>Log Flag Structure</span>
+      <span>Log Flat Structure</span>
     </button>
-    <button onclick.delegate="logExpandedStructure()" class="button is-small">
-      <span>Log Expanded Structure</span>
+    <button onclick.delegate="logHierarchicalStructure()" class="button is-small">
+      <span>Log Hierarchical Structure</span>
     </button>
   </div>
   <div class="column is-6">
@@ -58,4 +58,4 @@ <h6 class="title is-6 italic">
 </div>
 
 <div class="grid6">
-</div>
+</div>

examples/webpack-demo-vanilla-bundle/src/examples/example06.ts

@@ -172,8 +172,8 @@ export class Example6 {
     this.sgb.treeDataService.toggleTreeDataCollapse(false);
   }
 
-  logExpandedStructure() {
-    console.log('exploded array', this.sgb.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
+  logHierarchicalStructure() {
+    console.log('hierarchical array', this.sgb.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
   }
 
   logFlatStructure() {

packages/common/src/formatters/treeFormatter.ts

@@ -26,16 +26,17 @@ export const treeFormatter: Formatter = (_row, _cell, value, columnDef, dataCont
     throw new Error('You must provide valid "treeDataOptions" in your Grid Options and it seems that there are no tree level found in this row');
   }
 
-  if (dataView && dataView.getIdxById && dataView.getItemByIdx) {
+  if (dataView?.getItemByIdx) {
     if (typeof outputValue === 'string') {
       outputValue = htmlEncode(outputValue);
     }
     const identifierPropName = dataView.getIdPropertyName() || 'id';
-    const spacer = `<span style="display:inline-block; width:${indentMarginLeft * dataContext[treeLevelPropName]}px;"></span>`;
+    const treeLevel = dataContext[treeLevelPropName] || 0;
+    const spacer = `<span style="display:inline-block; width:${indentMarginLeft * treeLevel}px;"></span>`;
     const idx = dataView.getIdxById(dataContext[identifierPropName]);
     const nextItemRow = dataView.getItemByIdx((idx || 0) + 1);
 
-    if (nextItemRow && nextItemRow[treeLevelPropName] > dataContext[treeLevelPropName]) {
+    if (nextItemRow?.[treeLevelPropName] > treeLevel) {
       if (dataContext.__collapsed) {
         return `${spacer}<span class="slick-group-toggle collapsed"></span>&nbsp;${outputValue}`;
       } else {

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

@@ -989,6 +989,7 @@ describe('SortService', () => {
         { columnId: 'file', sortAsc: false, sortCol: { id: 'file', field: 'file', width: 75 } }
       ];
 
+      sharedService.hierarchicalDataset = [];
       service.bindLocalOnSort(gridStub);
       gridStub.onSort.notify({ multiColumnSort: true, sortCols: mockSortedCols, grid: gridStub }, new Slick.EventData(), gridStub);
 
@@ -1009,11 +1010,13 @@ describe('SortService', () => {
       const spyCurrentSort = jest.spyOn(service, 'getCurrentLocalSorters');
       const spyOnLocalSort = jest.spyOn(service, 'onLocalSortChanged');
       const spyUpdateSorting = jest.spyOn(service, 'updateSorting');
+
       const mockSortedCols: ColumnSort[] = [
         { columnId: 'lastName', sortAsc: true, sortCol: { id: 'lastName', field: 'lastName', width: 100 } },
         { columnId: 'file', sortAsc: false, sortCol: { id: 'file', field: 'file', width: 75 } }
       ];
 
+      sharedService.hierarchicalDataset = [];
       service.bindLocalOnSort(gridStub);
       gridStub.onSort.notify({ multiColumnSort: true, sortCols: mockSortedCols, grid: gridStub }, new Slick.EventData(), gridStub);
 

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

@@ -1,7 +1,11 @@
+
 import { Column, SlickDataView, GridOption, SlickEventHandler, SlickGrid, SlickNamespace } from '../../interfaces/index';
 import { SharedService } from '../shared.service';
+import { SortService } from '../sort.service';
 import { TreeDataService } from '../treeData.service';
+import * as utilities from '../utilities';
 
+const mockConvertParentChildArray = jest.fn();
 declare const Slick: SlickNamespace;
 
 const gridOptionsMock = {
@@ -35,13 +39,18 @@ const gridStub = {
   setSortColumns: jest.fn(),
 } as unknown as SlickGrid;
 
+const sortServiceStub = {
+  clearSorting: jest.fn(),
+  sortHierarchicalDataset: jest.fn(),
+} as unknown as SortService;
+
 describe('TreeData Service', () => {
   let service: TreeDataService;
   let slickgridEventHandler: SlickEventHandler;
   const sharedService = new SharedService();
 
   beforeEach(() => {
-    service = new TreeDataService(sharedService);
+    service = new TreeDataService(sharedService, sortServiceStub);
     slickgridEventHandler = service.eventHandler;
     jest.spyOn(gridStub, 'getData').mockReturnValue(dataViewStub);
   });
@@ -160,9 +169,7 @@ describe('TreeData Service', () => {
 
       beforeEach(() => {
         itemsMock = [{ file: 'myFile.txt', size: 0.5 }, { file: 'myMusic.txt', size: 5.3 }];
-        gridOptionsMock.treeDataOptions = {
-          columnId: 'file'
-        };
+        gridOptionsMock.treeDataOptions = { columnId: 'file' };
         jest.clearAllMocks();
       });
 
@@ -209,5 +216,63 @@ describe('TreeData Service', () => {
         ]);
       });
     });
+
+    describe('initializeHierarchicalDataset method', () => {
+      let mockColumns: Column[];
+      let mockFlatDataset;
+
+      beforeEach(() => {
+        mockColumns = [{ id: 'file', field: 'file', }, { id: 'size', field: 'size', }] as Column[];
+        mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', size: 1.2, parentId: 0 }, { id: 2, file: 'todo.txt', size: 2.3, parentId: 0 }];
+        gridOptionsMock.treeDataOptions = { columnId: 'file', parentPropName: 'parentId' };
+        jest.clearAllMocks();
+      });
+
+      it('should sort by the Tree column when there is no initial sort provided', () => {
+        const mockHierarchical = [{
+          id: 0,
+          file: 'documents',
+          files: [{ id: 2, file: 'todo.txt', size: 2.3, }, { id: 1, file: 'vacation.txt', size: 1.2, }]
+        }];
+        const setSortSpy = jest.spyOn(gridStub, 'setSortColumns');
+        jest.spyOn(gridStub, 'getColumnIndex').mockReturnValue(0);
+        jest.spyOn(sortServiceStub, 'sortHierarchicalDataset').mockReturnValue({ flat: mockFlatDataset, hierarchical: mockHierarchical });
+
+        service.init(gridStub);
+        const result = service.initializeHierarchicalDataset(mockFlatDataset, [mockColumn]);
+
+        expect(setSortSpy).toHaveBeenCalledWith([{
+          columnId: 'file',
+          sortAsc: true,
+          sortCol: mockColumn
+        }]);
+        expect(result).toEqual({ flat: mockFlatDataset, hierarchical: mockHierarchical });
+      });
+
+      it('should sort by the Tree column by the "initialSort" provided', () => {
+        gridOptionsMock.treeDataOptions.initialSort = {
+          columnId: 'size',
+          direction: 'desc'
+        };
+        const mockHierarchical = [{
+          id: 0,
+          file: 'documents',
+          files: [{ id: 1, file: 'vacation.txt', size: 1.2, }, { id: 2, file: 'todo.txt', size: 2.3, }]
+        }];
+        const setSortSpy = jest.spyOn(gridStub, 'setSortColumns');
+        jest.spyOn(gridStub, 'getColumnIndex').mockReturnValue(0);
+        jest.spyOn(sortServiceStub, 'sortHierarchicalDataset').mockReturnValue({ flat: mockFlatDataset, hierarchical: mockHierarchical });
+
+        service.init(gridStub);
+        const result = service.initializeHierarchicalDataset(mockFlatDataset, [mockColumn]);
+
+        expect(setSortSpy).toHaveBeenCalledWith([{
+          columnId: 'size',
+          sortAsc: false,
+          sortCol: mockColumn
+        }]);
+        expect(result).toEqual({ flat: mockFlatDataset, hierarchical: mockHierarchical });
+      });
+    });
   });
 });

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

@@ -323,24 +323,24 @@ export class SortService {
   /** Process the initial sort, typically it will sort ascending by the column that has the Tree Data unless user specifies a different initialSort */
   processTreeDataInitialSort() {
     // when a Tree Data view is defined, we must sort the data so that the UI works correctly
-    if (this._gridOptions && this._gridOptions.enableTreeData && this._gridOptions.treeDataOptions) {
+    if (this._gridOptions?.enableTreeData && this._gridOptions.treeDataOptions) {
       // first presort it once by tree level
       const treeDataOptions = this._gridOptions.treeDataOptions;
-      const columnWithTreeData = this._columnDefinitions.find((col: Column) => col && col.id === treeDataOptions.columnId);
+      const columnWithTreeData = this._columnDefinitions.find((col: Column) => col.id === treeDataOptions.columnId);
       if (columnWithTreeData) {
         let sortDirection = SortDirection.ASC;
         let sortTreeLevelColumn: ColumnSort = { columnId: treeDataOptions.columnId, sortCol: columnWithTreeData, sortAsc: true };
 
         // user could provide a custom sort field id, if so get that column and sort by it
-        if (treeDataOptions && treeDataOptions.initialSort && treeDataOptions.initialSort.columnId) {
+        if (treeDataOptions?.initialSort?.columnId) {
           const initialSortColumnId = treeDataOptions.initialSort.columnId;
           const initialSortColumn = this._columnDefinitions.find((col: Column) => col.id === initialSortColumnId);
           sortDirection = (treeDataOptions.initialSort.direction || SortDirection.ASC).toUpperCase() as SortDirection;
           sortTreeLevelColumn = { columnId: initialSortColumnId, sortCol: initialSortColumn, sortAsc: (sortDirection === SortDirection.ASC) } as ColumnSort;
         }
 
         // when we have a valid column with Tree Data, we can sort by that column
-        if (sortTreeLevelColumn && sortTreeLevelColumn.columnId) {
+        if (sortTreeLevelColumn?.columnId && this.sharedService?.hierarchicalDataset) {
           this.updateSorting([{ columnId: sortTreeLevelColumn.columnId || '', direction: sortDirection }]);
         }
       }
@@ -383,12 +383,8 @@ export class SortService {
 
       if (isTreeDataEnabled && this.sharedService && Array.isArray(this.sharedService.hierarchicalDataset)) {
         const hierarchicalDataset = this.sharedService.hierarchicalDataset;
-        this.sortTreeData(hierarchicalDataset, sortColumns);
-        const dataViewIdIdentifier = this._gridOptions?.datasetIdPropertyName ?? 'id';
-        const treeDataOpt: TreeDataOption = this._gridOptions?.treeDataOptions ?? { columnId: '' };
-        const treeDataOptions = { ...treeDataOpt, identifierPropName: treeDataOpt.identifierPropName ?? dataViewIdIdentifier };
-        const sortedFlatArray = convertHierarchicalViewToParentChildArray(hierarchicalDataset, treeDataOptions);
-        dataView.setItems(sortedFlatArray, this._gridOptions?.datasetIdPropertyName ?? 'id');
+        const datasetSortResult = this.sortHierarchicalDataset(hierarchicalDataset, sortColumns);
+        this._dataView.setItems(datasetSortResult.flat, this._gridOptions?.datasetIdPropertyName ?? 'id');
       } else {
         dataView.sort(this.sortComparers.bind(this, sortColumns));
       }
@@ -406,6 +402,17 @@ export class SortService {
     }
   }
 
+  /** Takes a hierarchical dataset and sort it recursively,  */
+  sortHierarchicalDataset(hierarchicalDataset: any[], sortColumns: Array<ColumnSort & { clearSortTriggered?: boolean; }>): { hierarchical: any[]; flat: any[]; } {
+    this.sortTreeData(hierarchicalDataset, sortColumns);
+    const dataViewIdIdentifier = this._gridOptions?.datasetIdPropertyName ?? 'id';
+    const treeDataOpt: TreeDataOption = this._gridOptions?.treeDataOptions ?? { columnId: '' };
+    const treeDataOptions = { ...treeDataOpt, identifierPropName: treeDataOpt.identifierPropName ?? dataViewIdIdentifier };
+    const sortedFlatArray = convertHierarchicalViewToParentChildArray(hierarchicalDataset, treeDataOptions);
+
+    return { hierarchical: hierarchicalDataset, flat: sortedFlatArray };
+  }
+
   /** Call a local grid sort by its default sort field id (user can customize default field by configuring "defaultColumnSortFieldId" in the grid options, defaults to "id") */
   sortLocalGridByDefaultSortFieldId() {
     const sortColFieldId = this._gridOptions && this._gridOptions.defaultColumnSortFieldId || this._gridOptions.datasetIdPropertyName || 'id';
@@ -510,11 +517,11 @@ export class SortService {
     }
 
     if (Array.isArray(sorters)) {
-      const backendApi = this._gridOptions && this._gridOptions.backendServiceApi;
+      const backendApi = this._gridOptions?.backendServiceApi;
 
       if (backendApi) {
-        const backendApiService = backendApi && backendApi.service;
-        if (backendApiService && backendApiService.updateSorters) {
+        const backendApiService = backendApi?.service;
+        if (backendApiService?.updateSorters) {
           backendApiService.updateSorters(undefined, sorters);
           if (triggerBackendQuery) {
             this.backendUtilities?.refreshBackendDataset(this._gridOptions);

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

@@ -1,5 +1,7 @@
-import { SlickDataView, GridOption, SlickEventHandler, SlickGrid, SlickNamespace, GetSlickEventType, OnClickEventArgs, SlickEventData } from '../interfaces/index';
+import { Column, ColumnSort, GetSlickEventType, GridOption, OnClickEventArgs, SlickDataView, SlickEventData, SlickEventHandler, SlickGrid, SlickNamespace, TreeDataOption, } from '../interfaces/index';
+import { convertParentChildArrayToHierarchicalView } from './utilities';
 import { SharedService } from './shared.service';
+import { SortService } from './sort.service';
 
 // using external non-typed js libraries
 declare const Slick: SlickNamespace;
@@ -8,7 +10,7 @@ export class TreeDataService {
   private _grid!: SlickGrid;
   private _eventHandler: SlickEventHandler;
 
-  constructor(private sharedService: SharedService) {
+  constructor(private sharedService: SharedService, private sortService: SortService) {
     this._eventHandler = new Slick.EventHandler();
   }
 
@@ -51,6 +53,35 @@ export class TreeDataService {
     }
   }
 
+  /** Takes a flat dataset, converts it into a hierarchical dataset, sort it by recursion and finally return back the final and sorted flat array */
+  initializeHierarchicalDataset(flatDataset: any[], columnDefinitions: Column[]) {
+    // 1- convert the flat array into a hierarchical array
+    const datasetHierarchical = this.convertFlatDatasetConvertToHierarhicalView(flatDataset);
+
+    // 2- sort the hierarchical array recursively by an optional "initialSort" OR if nothing is provided we'll sort by the column defined as the Tree column
+    // also note that multi-column is not currently supported with Tree Data
+    const treeDataOptions = this.gridOptions?.treeDataOptions;
+    const initialColumnSort = treeDataOptions?.initialSort ?? { columnId: treeDataOptions?.columnId ?? '', direction: 'ASC' };
+    const columnSort: ColumnSort = {
+      columnId: initialColumnSort.columnId,
+      sortAsc: initialColumnSort?.direction?.toUpperCase() !== 'DESC',
+      sortCol: columnDefinitions[this._grid.getColumnIndex(initialColumnSort.columnId || '')],
+    };
+    const datasetSortResult = this.sortService.sortHierarchicalDataset(datasetHierarchical, [columnSort]);
+
+    // and finally add the sorting icon (this has to be done manually in SlickGrid) to the column we used for the sorting
+    this._grid.setSortColumns([columnSort]);
+
+    return datasetSortResult;
+  }
+
+  convertFlatDatasetConvertToHierarhicalView(flatDataset: any[]): any[] {
+    const dataViewIdIdentifier = this.gridOptions?.datasetIdPropertyName ?? 'id';
+    const treeDataOpt: TreeDataOption = this.gridOptions?.treeDataOptions ?? { columnId: 'id' };
+    const treeDataOptions = { ...treeDataOpt, identifierPropName: treeDataOpt.identifierPropName ?? dataViewIdIdentifier };
+    return convertParentChildArrayToHierarchicalView(flatDataset, treeDataOptions);
+  }
+
   handleOnCellClick(event: SlickEventData, args: OnClickEventArgs) {
     if (event && args) {
       const targetElm: any = event.target || {};