Skip to content

Commit dff1aed

Browse files
viktorkombovviktorkombov
authored
fix(grids): getState no longer mutates live sorting/groupBy/filtering expressions - 21.1.x (#17195)
1 parent 8f131d0 commit dff1aed

2 files changed

Lines changed: 140 additions & 28 deletions

File tree

‎projects/igniteui-angular/grids/core/src/state-base.directive.ts‎

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,11 @@ export class IgxGridStateBaseDirective {
139139
private FEATURES = {
140140
sorting: {
141141
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
142-
const sortingState = context.currGrid.sortingExpressions;
143-
sortingState.forEach(s => {
144-
delete s.strategy;
145-
delete s.owner;
142+
const sortingState = context.currGrid.sortingExpressions.map(s => {
143+
const copy = { ...s };
144+
delete copy.strategy;
145+
delete copy.owner;
146+
return copy;
146147
});
147148
return { sorting: sortingState };
148149
},
@@ -154,10 +155,7 @@ export class IgxGridStateBaseDirective {
154155
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
155156
const filteringState = context.currGrid.filteringExpressionsTree;
156157
if (filteringState) {
157-
delete filteringState.owner;
158-
for (const item of filteringState.filteringOperands) {
159-
delete (item as IFilteringExpressionsTree).owner;
160-
}
158+
return { filtering: context.cloneFilteringTree(filteringState) };
161159
}
162160
return { filtering: filteringState };
163161
},
@@ -169,16 +167,7 @@ export class IgxGridStateBaseDirective {
169167
advancedFiltering: {
170168
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
171169
const filteringState = context.currGrid.advancedFilteringExpressionsTree;
172-
let advancedFiltering: any;
173-
if (filteringState) {
174-
delete filteringState.owner;
175-
for (const item of filteringState.filteringOperands) {
176-
delete (item as IFilteringExpressionsTree).owner;
177-
}
178-
advancedFiltering = filteringState;
179-
} else {
180-
advancedFiltering = {};
181-
}
170+
const advancedFiltering: any = filteringState ? context.cloneFilteringTree(filteringState) : {};
182171
return { advancedFiltering };
183172
},
184173
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IFilteringExpressionsTree): void => {
@@ -226,21 +215,21 @@ export class IgxGridStateBaseDirective {
226215
},
227216
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IColumnState[]): void => {
228217
const newColumns = [];
229-
218+
230219
// Helper to restore column state without auto-persisting widths
231220
const restoreColumnState = (column: IgxColumnComponent | IgxColumnGroupComponent, colState: IColumnState) => {
232221
// Extract width to handle it separately
233222
const width = colState.width;
234223
delete colState.width;
235-
224+
236225
Object.assign(column, colState);
237-
226+
238227
// Only restore width if it was explicitly set by the user (not undefined)
239228
if (width !== undefined) {
240229
column.width = width;
241230
}
242231
};
243-
232+
244233
state.forEach((colState) => {
245234
const hasColumnGroup = colState.columnGroup;
246235
const hasColumnLayouts = colState.columnLayout;
@@ -257,9 +246,9 @@ export class IgxGridStateBaseDirective {
257246
} else {
258247
ref1.children.reset([]);
259248
}
260-
249+
261250
restoreColumnState(ref1, colState);
262-
251+
263252
ref1.grid = context.currGrid;
264253
if (colState.parent || colState.parentKey) {
265254
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref1.parent));
@@ -277,7 +266,7 @@ export class IgxGridStateBaseDirective {
277266
}
278267

279268
restoreColumnState(ref, colState);
280-
269+
281270
ref.grid = context.currGrid;
282271
if (colState.parent || colState.parentKey) {
283272
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref.parent));
@@ -299,9 +288,10 @@ export class IgxGridStateBaseDirective {
299288
groupBy: {
300289
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
301290
const grid = context.currGrid;
302-
const groupingExpressions = grid.groupingExpressions;
303-
groupingExpressions.forEach(expr => {
304-
delete expr.strategy;
291+
const groupingExpressions = grid.groupingExpressions.map(expr => {
292+
const copy = { ...expr };
293+
delete copy.strategy;
294+
return copy;
305295
});
306296
const expansionState = grid.groupingExpansionState;
307297
const groupsExpanded = grid.groupsExpanded;
@@ -677,6 +667,22 @@ export class IgxGridStateBaseDirective {
677667
}
678668
}
679669

670+
/**
671+
* Recursively clones a filtering expression tree, stripping the `owner` property
672+
* from each cloned node so the live tree is never mutated.
673+
*/
674+
private cloneFilteringTree(tree: IFilteringExpressionsTree): IFilteringExpressionsTree {
675+
const copy: IFilteringExpressionsTree = { ...tree };
676+
delete copy.owner;
677+
copy.filteringOperands = tree.filteringOperands.map(item => {
678+
if ('filteringOperands' in item) {
679+
return this.cloneFilteringTree(item as IFilteringExpressionsTree);
680+
}
681+
return { ...item };
682+
});
683+
return copy;
684+
}
685+
680686
/**
681687
* This method builds a rehydrated IExpressionTree from a provided object.
682688
*/

‎projects/igniteui-angular/grids/core/src/state.directive.spec.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,112 @@ describe('IgxGridState - input properties #grid', () => {
803803
expect(prodIdColumn.colEnd).toBe(1);
804804
});
805805

806+
it('getState should not mutate live sorting expressions (strategy/owner)', () => {
807+
const fix = TestBed.createComponent(IgxGridStateComponent);
808+
fix.detectChanges();
809+
const grid = fix.componentInstance.grid;
810+
const state = fix.componentInstance.state;
811+
812+
const customStrategy = DefaultSortingStrategy.instance();
813+
const owner = {} as any;
814+
grid.sortingExpressions = [
815+
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
816+
];
817+
fix.detectChanges();
818+
819+
expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
820+
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should be set before getState');
821+
822+
state.getState(false, 'sorting');
823+
824+
expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live expressions after getState');
825+
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should not be removed from live expressions after getState');
826+
});
827+
828+
it('getState should not mutate live groupBy expressions (strategy/owner)', () => {
829+
const fix = TestBed.createComponent(IgxGridStateComponent);
830+
fix.detectChanges();
831+
const grid = fix.componentInstance.grid;
832+
const state = fix.componentInstance.state;
833+
834+
const customStrategy = DefaultSortingStrategy.instance();
835+
const owner = {} as any;
836+
grid.groupingExpressions = [
837+
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
838+
];
839+
fix.detectChanges();
840+
841+
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
842+
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should be set before getState');
843+
844+
state.getState(false, 'groupBy');
845+
846+
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live groupBy expressions after getState');
847+
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should not be removed from live groupBy expressions after getState');
848+
});
849+
850+
it('getState should not mutate live filtering expressions (owner)', () => {
851+
const fix = TestBed.createComponent(IgxGridStateComponent);
852+
fix.detectChanges();
853+
const grid = fix.componentInstance.grid;
854+
const state = fix.componentInstance.state;
855+
856+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
857+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
858+
productFilteringTree.filteringOperands.push({
859+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
860+
conditionName: 'true',
861+
fieldName: 'InStock',
862+
ignoreCase: true
863+
});
864+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
865+
filteringTree.filteringOperands.push(productFilteringTree);
866+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
867+
grid.filteringExpressionsTree = filteringTree;
868+
fix.detectChanges();
869+
870+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
871+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
872+
.toBe('nestedOwner', 'nested owner should be set before getState');
873+
874+
state.getState(false, 'filtering');
875+
876+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live filtering tree after getState');
877+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
878+
.toBe('nestedOwner', 'nested owner should not be removed from live filtering operand after getState');
879+
});
880+
881+
it('getState should not mutate live advancedFiltering expressions (owner)', () => {
882+
const fix = TestBed.createComponent(IgxGridStateComponent);
883+
fix.detectChanges();
884+
const grid = fix.componentInstance.grid;
885+
const state = fix.componentInstance.state;
886+
887+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
888+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
889+
productFilteringTree.filteringOperands.push({
890+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
891+
conditionName: 'true',
892+
fieldName: 'InStock',
893+
ignoreCase: true
894+
});
895+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
896+
filteringTree.filteringOperands.push(productFilteringTree);
897+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
898+
grid.advancedFilteringExpressionsTree = filteringTree;
899+
fix.detectChanges();
900+
901+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
902+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
903+
.toBe('nestedOwner', 'nested owner should be set before getState');
904+
905+
state.getState(false, 'advancedFiltering');
906+
907+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live advanced filtering tree after getState');
908+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
909+
.toBe('nestedOwner', 'nested owner should not be removed from live advanced filtering operand after getState');
910+
});
911+
806912
it('should preserve column widths when restoring state with all columns hidden', () => {
807913
const fix = TestBed.createComponent(IgxGridStateComponent);
808914
fix.detectChanges();

0 commit comments

Comments
 (0)