Skip to content

Commit

Permalink
fix(TreeData): auto-recalc should update totals for collapsed items t…
Browse files Browse the repository at this point in the history 
…oo (#1086)

* fix(TreeData): auto-recalc should update totals for collapsed items too
- this PR fixes an issue where the Tree Totals were not being updated for collapsed items when filtering
  • Loading branch information
@ghiscoding
ghiscoding authored Aug 21, 2023
1 parent 1a40c76 commit 25d39f2
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 17 deletions.
26 changes: 26 additions & 0 deletions packages/common/src/services/__tests__/filter.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1934,6 +1934,32 @@ describe('FilterService', () => {
expect(preFilterSpy).toHaveReturnedWith(initSetWithValues([21, 4, 5]));
});

it('should return False even when using "autoRecalcTotalsOnFilterChange" and item is found BUT its parent is collapsed', async () => {
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');
const preFilterSpy = jest.spyOn(service, 'preFilterTreeData');
jest.spyOn(dataViewStub, 'getItemById').mockReturnValueOnce({ ...dataset[4] as any, __collapsed: true })
.mockReturnValueOnce(dataset[5])
.mockReturnValueOnce(dataset[6]);

gridOptionMock.treeDataOptions!.autoRecalcTotalsOnFilterChange = true;
const mockItem1 = { __parentId: 4, id: 5, file: 'map.pdf', dateModified: '2015-05-21T10:22:00.123Z', size: 3.1 };

service.init(gridStub);
service.bindLocalOnFilter(gridStub);
gridStub.onHeaderRowCellRendered.notify(mockArgs1 as any, new Slick.EventData(), gridStub);
gridStub.onHeaderRowCellRendered.notify(mockArgs2 as any, new Slick.EventData(), gridStub);

const columnFilters = { file: { columnDef: mockColumn1, columnId: 'file', operator: 'Contains', searchTerms: ['map'], parsedSearchTerms: ['map'], targetSelector: '', type: FieldType.string } } as ColumnFilters;
await service.updateFilters([{ columnId: 'file', operator: '', searchTerms: ['map'] }], true, true, true);
const output = service.customLocalFilter(mockItem1, { dataView: dataViewStub, grid: gridStub, columnFilters });

expect(pubSubSpy).toHaveBeenCalledWith(`onBeforeFilterChange`, [{ columnId: 'file', operator: 'Contains', searchTerms: ['map'] }]);
expect(pubSubSpy).toHaveBeenCalledWith(`onFilterChanged`, [{ columnId: 'file', operator: 'Contains', searchTerms: ['map'] }]);
expect(output).toBe(false);
expect(preFilterSpy).toHaveBeenCalledWith(dataset, columnFilters);
expect(preFilterSpy).toHaveReturnedWith(initSetWithValues([21, 4, 5]));
});

it('should return False when item is not found in the dataset', async () => {
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');
const preFilterSpy = jest.spyOn(service, 'preFilterTreeData');
Expand Down
44 changes: 28 additions & 16 deletions packages/common/src/services/filter.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,28 +328,40 @@ export class FilterService {
const collapsedPropName = treeDataOptions.collapsedPropName ?? Constants.treeDataProperties.COLLAPSED_PROP;
const parentPropName = treeDataOptions.parentPropName ?? Constants.treeDataProperties.PARENT_PROP;
const childrenPropName = treeDataOptions?.childrenPropName ?? Constants.treeDataProperties.CHILDREN_PROP;
const dataViewIdIdentifier = this._gridOptions.datasetIdPropertyName ?? 'id';
const primaryDataId = this._gridOptions.datasetIdPropertyName ?? 'id';
const autoRecalcTotalsOnFilterChange = treeDataOptions.autoRecalcTotalsOnFilterChange ?? false;

// typically when a parent is collapsed we can exit early (by returning false) but we can't do that when we use auto-recalc totals
// if that happens, we need to keep a ref and recalculate total for all tree leafs then only after we can exit
let isParentCollapsed = false; // will be used only when auto-recalc is enabled
if (item[parentPropName] !== null) {
let parent = this._dataView.getItemById(item[parentPropName]);
while (parent) {
if (parent[collapsedPropName]) {
return false; // don't display any row that have their parent collapsed
if (autoRecalcTotalsOnFilterChange) {
isParentCollapsed = true; // when auto-recalc tree totals is enabled, we need to keep ref without exiting the loop just yet
} else {
// not using auto-recalc, we can exit early and not display any row that have their parent collapsed
return false;
}
}
parent = this._dataView.getItemById(parent[parentPropName]);
}
}

// filter out any row items that aren't part of our pre-processed "preFilterTreeData()" result
if (this._tmpPreFilteredData instanceof Set) {
const filtered = this._tmpPreFilteredData.has(item[dataViewIdIdentifier]); // return true when found, false otherwise
const filtered = this._tmpPreFilteredData.has(item[primaryDataId]); // return true when found, false otherwise

// when user enables Tree Data auto-recalc, we need to keep ref (only in hierarchical tree) of which datacontext was filtered or not
if (treeDataOptions?.autoRecalcTotalsOnFilterChange) {
const treeItem = findItemInTreeStructure(this.sharedService.hierarchicalDataset!, x => x[dataViewIdIdentifier] === item[dataViewIdIdentifier], childrenPropName);
if (autoRecalcTotalsOnFilterChange) {
const treeItem = findItemInTreeStructure(this.sharedService.hierarchicalDataset!, x => x[primaryDataId] === item[primaryDataId], childrenPropName);
if (treeItem) {
treeItem.__filteredOut = !filtered;
}
if (isParentCollapsed) {
return false; // now that we are done analyzing "__filteredOut", we can now return false to not show collapsed children
}
}
return filtered;
}
Expand Down Expand Up @@ -508,8 +520,8 @@ export class FilterService {
// when using localization (i18n), we should use the formatter output to search as the new cell value
if (columnDef?.params?.useFormatterOuputToFilter === true) {
const dataView = grid.getData() as SlickDataView;
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
const rowIndex = (dataView && typeof dataView.getIdxById === 'function') ? dataView.getIdxById(item[idPropName]) : 0;
const primaryDataId = this._gridOptions.datasetIdPropertyName || 'id';
const rowIndex = (dataView && typeof dataView.getIdxById === 'function') ? dataView.getIdxById(item[primaryDataId]) : 0;
const formattedCellValue = (columnDef && typeof columnDef.formatter === 'function') ? columnDef.formatter(rowIndex || 0, columnIndex, cellValue, columnDef, item, this._grid) : '';
cellValue = sanitizeHtmlToText(formattedCellValue as string);
}
Expand Down Expand Up @@ -542,7 +554,7 @@ export class FilterService {
const collapsedPropName = treeDataOptions?.collapsedPropName ?? Constants.treeDataProperties.COLLAPSED_PROP;
const parentPropName = treeDataOptions?.parentPropName ?? Constants.treeDataProperties.PARENT_PROP;
const hasChildrenPropName = treeDataOptions?.hasChildrenPropName ?? Constants.treeDataProperties.HAS_CHILDREN_PROP;
const dataViewIdIdentifier = this._gridOptions.datasetIdPropertyName ?? 'id';
const primaryDataId = this._gridOptions.datasetIdPropertyName ?? 'id';
const treeDataToggledItems = this._gridOptions.presets?.treeData?.toggledItems;
const isInitiallyCollapsed = this._gridOptions.treeDataOptions?.initiallyCollapsed ?? false;
const treeDataColumnId = this._gridOptions.treeDataOptions?.columnId;
Expand All @@ -558,11 +570,11 @@ export class FilterService {

if (Array.isArray(inputItems)) {
for (const inputItem of inputItems) {
(treeObj as any)[inputItem[dataViewIdIdentifier]] = inputItem;
(treeObj as any)[inputItem[primaryDataId]] = inputItem;
// as the filtered data is then used again as each subsequent letter
// we need to delete the .__used property, otherwise the logic below
// in the while loop (which checks for parents) doesn't work
delete (treeObj as any)[inputItem[dataViewIdIdentifier]].__used;
delete (treeObj as any)[inputItem[primaryDataId]].__used;
}

// Step 1. prepare search filter by getting their parsed value(s), for example if it's a date filter then parse it to a Moment object
Expand Down Expand Up @@ -598,15 +610,15 @@ export class FilterService {
// when using `excludeChildrenWhenFilteringTree: false`, we can auto-approve current item if it's the column holding the Tree structure and is a Parent that passes the first filter criteria
// in other words, if we're on the column with the Tree and its filter is valid (and is a parent), then skip any other filter(s)
if (conditionResult && isNotExcludingChildAndValidateOnlyTreeColumn && hasChildren && columnFilter.columnId === treeDataColumnId) {
filteredParents.set(item[dataViewIdIdentifier], true);
filteredParents.set(item[primaryDataId], true);
break;
}

// if item is valid OR we aren't excluding children and its parent is valid then we'll consider this valid
// however we don't return true, we need to continue and loop through next filter(s) since we still need to check other keys in columnFilters
if (conditionResult || (!excludeChildrenWhenFilteringTree && (filteredParents.get(item[parentPropName]) === true))) {
if (hasChildren && columnFilter.columnId === treeDataColumnId) {
filteredParents.set(item[dataViewIdIdentifier], true); // when it's a Parent item, we'll keep a Map ref as being a Parent with valid criteria
filteredParents.set(item[primaryDataId], true); // when it's a Parent item, we'll keep a Map ref as being a Parent with valid criteria
}
// if our filter is valid OR we're on the Tree column then let's continue
if (conditionResult || (!excludeChildrenWhenFilteringTree && columnFilter.columnId === treeDataColumnId)) {
Expand All @@ -616,7 +628,7 @@ export class FilterService {
// when it's a Parent item AND its Parent isn't valid AND we aren't on the Tree column
// we'll keep reference of the parent via a Map key/value pair and make its value as False because this Parent item is considered invalid
if (hasChildren && filteredParents.get(item[parentPropName]) !== true && columnFilter.columnId !== treeDataColumnId) {
filteredParents.set(item[dataViewIdIdentifier], false);
filteredParents.set(item[primaryDataId], false);
}
}
}
Expand All @@ -630,7 +642,7 @@ export class FilterService {
// will be pushed to the filteredChildrenAndParents array
if (matchFilter) {
// add child (id):
filteredChildrenAndParents.add(item[dataViewIdIdentifier]);
filteredChildrenAndParents.add(item[primaryDataId]);
let parent = (treeObj as any)[item[parentPropName]] ?? false;

// if there are any presets of collapsed parents, let's processed them
Expand All @@ -641,9 +653,9 @@ export class FilterService {

while (parent) {
// only add parent (id) if not already added:
parent.__used ?? filteredChildrenAndParents.add(parent[dataViewIdIdentifier]);
parent.__used ?? filteredChildrenAndParents.add(parent[primaryDataId]);
// mark each parent as used to not use them again later:
(treeObj as any)[parent[dataViewIdIdentifier]].__used = true;
(treeObj as any)[parent[primaryDataId]].__used = true;
// try to find parent of the current parent, if exists:
parent = (treeObj as any)[parent[parentPropName]] ?? false;
}
Expand Down
43 changes: 42 additions & 1 deletion test/cypress/e2e/example06.cy.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
describe('Example 06 - Tree Data (from a Hierarchical Dataset)', { retries: 0 }, () => {
describe('Example 06 - Tree Data (from a Hierarchical Dataset)', { retries: 1 }, () => {
const GRID_ROW_HEIGHT = 40;
const titles = ['Files', 'Date Modified', 'Description', 'Size'];
// const defaultSortAscList = ['bucket-list.txt', 'documents', 'misc', 'warranties.txt', 'pdf', 'internet-bill.pdf', 'map.pdf', 'map2.pdf', 'phone-bill.pdf', 'txt', 'todo.txt', 'unclassified.csv', 'unresolved.csv', 'xls', 'compilation.xls', 'music', 'mp3', 'other', 'pop', 'song.mp3', 'theme.mp3', 'rock', 'soft.mp3', 'something.txt'];
Expand Down Expand Up @@ -456,5 +456,46 @@ describe('Example 06 - Tree Data (from a Hierarchical Dataset)', { retries: 0 },
cy.get('.right-footer .item-count').contains('4');
cy.get('.right-footer .total-count').contains('31');
});

it('should clear all filters', () => {
cy.get('[data-test="clear-filters-btn"]')
.click();
});

it('should collapse "pdf" folder and filter with "b" again and expect same updated tree totals as earlier collapsed or expanded should still be Sum(2.8MB) / Avg(1.4MB)', () => {
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 4}px"] > .slick-cell:nth(0) .slick-group-toggle.expanded`)
.click();

cy.get('.search-filter.filter-file')
.type('b');

cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(0)`).should('contain', 'bucket-list.txt');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(0)`).should('contain', 'documents');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(3)`).should('contain', 'sum: 4.02 MB / avg: 1.34 MB (total)');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(0)`).should('contain', 'pdf');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(3)`).should('contain', 'sum: 2.8 MB / avg: 1.4 MB (sub-total)');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(0)`).should('contain', 'zebra.dll');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(3)`).should('contain', '1.22 MB');

cy.get('.right-footer .item-count').contains('4');
cy.get('.right-footer .total-count').contains('31');
});

it('should clear all filters and collapse all Tree groups (folders) then type "so" and expect updated "music" totals Sum(104.3MB) / Avg(52.15MB)', () => {
cy.get('[data-test="clear-filters-btn"]').click();
cy.get('[data-test="collapse-all-btn"]').click();

cy.get('.search-filter.filter-file').type('so');

cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(0)`).should('contain', 'documents');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(3)`).should('contain', 'sum: 0.79 MB / avg: 0.79 MB (total)');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(0)`).should('contain', 'music');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(3)`).should('contain', 'sum: 104.3 MB / avg: 52.15 MB (total)');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(0)`).should('contain', 'something.txt');
cy.get(`.grid6 [style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(3)`).should('contain', '90 MB');

cy.get('.right-footer .item-count').contains('3');
cy.get('.right-footer .total-count').contains('31');
});
});
});

0 comments on commit 25d39f2

Please sign in to comment.