fix(tree): couple of issues found in Tree Data

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

Author: ghiscoding

src/app/examples/grid-resize-by-content.component.ts

@@ -54,7 +54,7 @@ const myCustomTitleValidator = (value: any, args: any) => {
 @Injectable()
 export class GridResizeByContentComponent implements OnInit {
   title = 'Example 30: Columns Resize by Content';
-  subTitle = `The grid below uses the optional resize by cell content (with a fixed 950px for demo purposes), you can click on the 2 buttons to see the difference. The "autosizeColumns" is really the default option used by SlickGrid-Universal, the resize by cell content is optional because it requires to read the first thousand rows and do extra width calculation.`;
+  subTitle = `The grid below uses the optional resize by cell content (with a fixed 950px for demo purposes), you can click on the 2 buttons to see the difference. The "autosizeColumns" is really the default option used by Angular-SlickGrid, the resize by cell content is optional because it requires to read the first thousand rows and do extra width calculation.`;
 
   angularGrid!: AngularGridInstance;
   gridOptions!: GridOption;

src/app/examples/grid-tree-data-hierarchical.component.html

@@ -26,10 +26,10 @@ <h2>
         <span>Expand All</span>
       </button>
       <button (click)="logFlatStructure()" class="btn btn-outline-secondary btn-sm">
-        <span>Log Flag Structure</span>
+        <span>Log Flat Structure</span>
       </button>
-      <button (click)="logExpandedStructure()" class="btn btn-outline-secondary btn-sm">
-        <span>Log Expanded Structure</span>
+      <button (click)="logHierarchicalStructure()" class="btn btn-outline-secondary btn-sm">
+        <span>Log Hierarchical Structure</span>
       </button>
     </div>
 
@@ -54,4 +54,4 @@ <h2>
                      [datasetHierarchical]="datasetHierarchical"
                      (onAngularGridCreated)="angularGridReady($event)">
   </angular-slickgrid>
-</div>
+</div>

src/app/examples/grid-tree-data-hierarchical.component.ts

@@ -218,7 +218,7 @@ export class GridTreeDataHierarchicalComponent implements OnInit {
     this.angularGrid.treeDataService.toggleTreeDataCollapse(false);
   }
 
-  logExpandedStructure() {
+  logHierarchicalStructure() {
     console.log('exploded array', this.angularGrid.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
   }
 

src/app/examples/grid-tree-data-parent-child.component.html

@@ -26,10 +26,10 @@ <h2>
         <span>Expand All</span>
       </button>
       <button (click)="logFlatStructure()" class="btn btn-outline-secondary btn-sm">
-        <span>Log Flag Structure</span>
+        <span>Log Flat Structure</span>
       </button>
-      <button (click)="logExpandedStructure()" class="btn btn-outline-secondary btn-sm">
-        <span>Log Expanded Structure</span>
+      <button (click)="logHierarchicalStructure()" class="btn btn-outline-secondary btn-sm">
+        <span>Log Hierarchical Structure</span>
       </button>
     </div>
   </div>
@@ -42,4 +42,4 @@ <h2>
                      [dataset]="dataset"
                      (onAngularGridCreated)="angularGridReady($event)">
   </angular-slickgrid>
-</div>
+</div>

src/app/examples/grid-tree-data-parent-child.component.ts

@@ -98,9 +98,16 @@ export class GridTreeDataParentChildComponent implements OnInit {
       enableTreeData: true, // you must enable this flag for the filtering & sorting to work as expected
       treeDataOptions: {
         columnId: 'title',
-        levelPropName: 'indent',
-        parentPropName: 'parentId'
+        levelPropName: 'indent', // this is optional, except that in our case we just need to define it because we are adding new item in the demo
+        parentPropName: 'parentId',
+
+        // you can optionally sort by a different column and/or sort direction
+        initialSort: {
+          columnId: 'title',
+          direction: 'ASC'
+        }
       },
+      showCustomFooter: true,
       multiColumnSort: false, // multi-column sorting is not supported with Tree Data, so you need to disable it
       // change header/cell row height for material design theme
       headerRowHeight: 45,
@@ -166,7 +173,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
       parentId: parentItemFound.id,
       title: `Task ${newId}`,
       duration: '1 day',
-      percentComplete: Math.round(Math.random() * 100),
+      percentComplete: 99,
       start: new Date(),
       finish: new Date(),
       effortDriven: false
@@ -176,11 +183,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
     this.dataset = [...dataset]; // make a copy to trigger a dataset refresh
 
     // add setTimeout to wait a full cycle because datasetChanged needs a full cycle
-    // force a resort because of the tree data structure
     setTimeout(() => {
-      const titleColumn = this.columnDefinitions.find(col => col.id === 'title') as Column;
-      this.angularGrid.sortService.onLocalSortChanged(this.gridObj, [{ columnId: 'title', sortCol: titleColumn, sortAsc: true }]);
-
       // scroll into the position, after insertion cycle, where the item was added
       const rowIndex = this.dataViewObj.getRowById(newItem.id);
       this.gridObj.scrollRowIntoView(rowIndex + 3);
@@ -195,7 +198,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
     this.angularGrid.treeDataService.toggleTreeDataCollapse(false);
   }
 
-  logExpandedStructure() {
+  logHierarchicalStructure() {
     console.log('exploded array', this.angularGrid.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
   }
 
@@ -232,8 +235,8 @@ export class GridTreeDataParentChildComponent implements OnInit {
       }
 
       d['id'] = i;
-      d['indent'] = indent;
       d['parentId'] = parentId;
+      d['indent'] = indent;
       d['title'] = 'Task ' + i;
       d['duration'] = '5 days';
       d['percentComplete'] = Math.round(Math.random() * 100);

src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts

@@ -97,6 +97,7 @@ const filterServiceStub = {
   bindLocalOnSort: jest.fn(),
   bindBackendOnSort: jest.fn(),
   populateColumnFilterSearchTermPresets: jest.fn(),
+  refreshTreeDataFilters: jest.fn(),
   getColumnFilters: jest.fn(),
 } as unknown as FilterService;
 
@@ -150,10 +151,13 @@ const sortServiceStub = {
   dispose: jest.fn(),
   loadGridSorters: jest.fn(),
   processTreeDataInitialSort: jest.fn(),
+  sortHierarchicalDataset: jest.fn(),
 } as unknown as SortService;
 
 const treeDataServiceStub = {
+  convertFlatDatasetConvertToHierarhicalView: jest.fn(),
   init: jest.fn(),
+  initializeHierarchicalDataset: jest.fn(),
   dispose: jest.fn(),
   handleOnCellClick: jest.fn(),
   toggleTreeDataCollapse: jest.fn(),
@@ -1633,16 +1637,22 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
         }
       });
 
-      it('should change flat dataset and expect  being called with other methods', () => {
+      it('should change flat dataset and expect being called with other methods', (done) => {
         const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
         const mockHierarchical = [{ id: 0, file: 'documents', files: [{ id: 1, file: 'vacation.txt' }] }];
         const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
+        jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: mockHierarchical, flat: mockFlatDataset });
+        const refreshTreeSpy = jest.spyOn(filterServiceStub, 'refreshTreeDataFilters');
 
         component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', parentPropName: 'parentId', childrenPropName: 'files' } } as GridOption;
         component.ngAfterViewInit();
         component.dataset = mockFlatDataset;
 
-        expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
+        setTimeout(() => {
+          expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
+          expect(refreshTreeSpy).not.toHaveBeenCalled();
+          done();
+        })
       });
 
       it('should change hierarchical dataset and expect processTreeDataInitialSort being called with other methods', (done) => {
@@ -1665,6 +1675,22 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
           done();
         }, 2);
       });
+
+      it('should expect "refreshTreeDataFilters" method to be called when our flat dataset was already set and it just got changed a 2nd time', () => {
+        const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
+        const mockHierarchical = [{ id: 0, file: 'documents', files: [{ id: 1, file: 'vacation.txt' }] }];
+        const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
+        jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: mockHierarchical, flat: mockFlatDataset });
+        const refreshTreeSpy = jest.spyOn(filterServiceStub, 'refreshTreeDataFilters');
+
+        component.dataset = [{ id: 0, file: 'documents' }];
+        component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', parentPropName: 'parentId', childrenPropName: 'files' } } as GridOption;
+        component.ngAfterViewInit();
+        component.dataset = mockFlatDataset;
+
+        expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
+        expect(refreshTreeSpy).toHaveBeenCalled();
+      });
     });
   });
 });

src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid.component.spec.ts

@@ -43,18 +43,9 @@ import {
   RowMoveManagerExtension,
   RowSelectionExtension,
 } from '../../extensions';
-import * as utilities from '../../services/utilities';
 
 declare const Slick: any;
 
-function removeExtraSpaces(text: string) {
-  return `${text}`.replace(/\s{2,}/g, '');
-}
-
-const mockConvertParentChildArray = jest.fn();
-// @ts-ignore
-utilities.convertParentChildArrayToHierarchicalView = mockConvertParentChildArray;
-
 const excelExportServiceStub = {
   init: jest.fn(),
   dispose: jest.fn(),
@@ -123,7 +114,9 @@ const sortServiceStub = {
 } as unknown as SortService;
 
 const treeDataServiceStub = {
+  convertFlatDatasetConvertToHierarhicalView: jest.fn(),
   init: jest.fn(),
+  initializeHierarchicalDataset: jest.fn(),
   dispose: jest.fn(),
   handleOnCellClick: jest.fn(),
   toggleTreeDataCollapse: jest.fn(),
@@ -237,6 +230,7 @@ describe('App Component', () => {
   it('should convert parent/child dataset to hierarchical dataset when Tree Data is enabled and "onRowsChanged" was triggered', async (done) => {
     const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
     const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
+    jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: [], flat: mockFlatDataset });
 
     component.gridId = 'grid1';
     component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file' } } as GridOption;
@@ -250,7 +244,6 @@ describe('App Component', () => {
 
     setTimeout(() => {
       expect(hierarchicalSpy).toHaveBeenCalled();
-      expect(mockConvertParentChildArray).toHaveBeenCalled();
       done();
     }, 51)
   });

src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts

@@ -35,7 +35,6 @@ import {
   Pagination,
   ServicePagination,
   SlickEventHandler,
-  TreeDataOption,
 } from './../models/index';
 import { FilterFactory } from '../filters/filterFactory';
 import { autoAddEditorFormatterToColumnsWithEditor } from './slick-vanilla-utilities';
@@ -124,6 +123,7 @@ const slickgridEventPrefix = 'sg';
 export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnInit {
   private _dataset?: any[] | null;
   private _columnDefinitions!: Column[];
+  private _currentDatasetLength = 0;
   private _eventHandler: SlickEventHandler = new Slick.EventHandler();
   private _angularGridInstances: AngularGridInstance | undefined;
   private _fixedHeight?: number | null;
@@ -213,9 +213,24 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   get dataset(): any[] {
     return this.dataView.getItems();
   }
-  set dataset(dataset: any[]) {
-    this._dataset = dataset;
-    this.refreshGridData(dataset);
+  set dataset(newDataset: any[]) {
+    const prevDatasetLn = this._currentDatasetLength;
+    let data = newDataset;
+    this._dataset = data;
+
+    // when Tree Data is enabled and we don't yet have the hierarchical dataset filled, we can force a convert & sort of the array
+    if (this.gridOptions?.enableTreeData && Array.isArray(newDataset) && (newDataset.length > 0 || newDataset.length !== prevDatasetLn)) {
+      const sortedDatasetResult = this.treeDataService.initializeHierarchicalDataset(data, this._columnDefinitions);
+      this.sharedService.hierarchicalDataset = sortedDatasetResult.hierarchical;
+      data = sortedDatasetResult.flat;
+
+      // if we add/remove item(s) from the dataset, we need to also refresh our tree data filters
+      if (newDataset.length > 0 && prevDatasetLn > 0 && newDataset.length !== prevDatasetLn) {
+        this.filterService.refreshTreeDataFilters();
+      }
+    }
+    this.refreshGridData(data || []);
+    this._currentDatasetLength = newDataset.length;
   }
 
   @Input()
@@ -225,13 +240,13 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
   set datasetHierarchical(newHierarchicalDataset: any[] | undefined) {
     this.sharedService.hierarchicalDataset = newHierarchicalDataset;
 
-    if (newHierarchicalDataset && this.columnDefinitions && this.filterService && this.filterService.clearFilters) {
+    if (newHierarchicalDataset && this.columnDefinitions && this.filterService?.clearFilters) {
       this.filterService.clearFilters();
     }
 
     // when a hierarchical dataset is set afterward, we can reset the flat dataset and call a tree data sort that will overwrite the flat dataset
     setTimeout(() => {
-      if (newHierarchicalDataset && this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
+      if (newHierarchicalDataset && this.dataView && this.sortService?.processTreeDataInitialSort && this.gridOptions?.enableTreeData) {
         this.dataView.setItems([], this.gridOptions.datasetIdPropertyName);
         this.sortService.processTreeDataInitialSort();
       }
@@ -426,7 +441,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
 
     if (Array.isArray(dataset) && this.grid && this.dataView && typeof this.dataView.setItems === 'function') {
       this.dataView.setItems(dataset, this.gridOptions.datasetIdPropertyName);
-      if (!this.gridOptions.backendServiceApi) {
+      if (!this.gridOptions.backendServiceApi && !this.gridOptions.enableTreeData) {
         this.dataView.reSort();
       }
 
@@ -439,11 +454,6 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
           }
         }
         this._isDatasetInitialized = true;
-
-        // also update the hierarchical dataset
-        if (dataset.length > 0 && this.gridOptions.treeDataOptions) {
-          this.sharedService.hierarchicalDataset = this.treeDataSortComparer(dataset);
-        }
       }
 
       if (dataset) {
@@ -667,16 +677,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
         throw new Error('[Angular-Slickgrid] When enabling tree data, you must also provide the "treeDataOption" property in your Grid Options with "childrenPropName" or "parentPropName" (depending if your array is hierarchical or flat) for the Tree Data to work properly');
       }
 
-      this._eventHandler.subscribe(dataView.onRowsChanged, (e: any, args: any) => {
-        // when dealing with Tree Data, anytime the flat dataset changes, we need to update our hierarchical dataset
-        // this could be triggered by a DataView setItems or updateItem
-        if (this.gridOptions && this.gridOptions.enableTreeData) {
-          const items = this.dataView.getItems();
-          if (Array.isArray(items) && items.length > 0 && !this._isDatasetInitialized) {
-            this.sharedService.hierarchicalDataset = this.treeDataSortComparer(items);
-          }
-        }
-
+      this._eventHandler.subscribe(dataView.onRowsChanged, (_e: Event, args: any) => {
         // filtering data with local dataset will not always show correctly unless we call this updateRow/render
         // also don't use "invalidateRows" since it destroys the entire row and as bad user experience when updating a row
         // see commit: https://github.com/ghiscoding/Angular-Slickgrid/commit/bb62c0aa2314a5d61188ff005ccb564577f08805
@@ -805,6 +806,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
 
   /** When data changes in the DataView, we'll refresh the metrics and/or display a warning if the dataset is empty */
   private handleOnItemCountChanged(currentPageRowItemCount: number, totalItemCount: number) {
+    this._currentDatasetLength = totalItemCount;
     this.metrics = {
       startTime: new Date(),
       endTime: new Date(),
@@ -1218,13 +1220,6 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
     }
   }
 
-  private treeDataSortComparer(flatDataset: any[]): any[] {
-    const dataViewIdIdentifier = this.gridOptions && this.gridOptions.datasetIdPropertyName || 'id';
-    const treeDataOpt: TreeDataOption = this.gridOptions && this.gridOptions.treeDataOptions || { columnId: '' };
-    const treeDataOptions = { ...treeDataOpt, identifierPropName: treeDataOpt.identifierPropName || dataViewIdIdentifier };
-    return convertParentChildArrayToHierarchicalView(flatDataset, treeDataOptions);
-  }
-
   /**
    * For convenience to the user, we provide the property "editor" as an Angular-Slickgrid editor complex object
    * however "editor" is used internally by SlickGrid for it's own Editor Factory