Skip to content

Commit 61608b1

Browse files
authored
fix(grids): getState no longer mutates live sorting/groupBy/filtering expressions (#17194)
1 parent d822f07 commit 61608b1

2 files changed

Lines changed: 145 additions & 28 deletions

File tree

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

Lines changed: 35 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,11 @@ 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+
delete copy.owner;
295+
return copy;
305296
});
306297
const expansionState = grid.groupingExpansionState;
307298
const groupsExpanded = grid.groupsExpanded;
@@ -677,6 +668,22 @@ export class IgxGridStateBaseDirective {
677668
}
678669
}
679670

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

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,116 @@ 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+
const serializedState = state.getState(false, 'groupBy') as IGridState;
845+
const serializedGroupBy = (serializedState.groupBy?.expressions ?? []) as Array<IGroupingExpression & { owner?: unknown }>;
846+
847+
expect(serializedGroupBy.length).toBe(1, 'serialized groupBy state should contain the configured expression');
848+
expect(serializedGroupBy[0].strategy).toBeUndefined('strategy should be removed from serialized groupBy expressions');
849+
expect(serializedGroupBy[0].owner).toBeUndefined('owner should be removed from serialized groupBy expressions');
850+
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live groupBy expressions after getState');
851+
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should not be removed from live groupBy expressions after getState');
852+
});
853+
854+
it('getState should not mutate live filtering expressions (owner)', () => {
855+
const fix = TestBed.createComponent(IgxGridStateComponent);
856+
fix.detectChanges();
857+
const grid = fix.componentInstance.grid;
858+
const state = fix.componentInstance.state;
859+
860+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
861+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
862+
productFilteringTree.filteringOperands.push({
863+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
864+
conditionName: 'true',
865+
fieldName: 'InStock',
866+
ignoreCase: true
867+
});
868+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
869+
filteringTree.filteringOperands.push(productFilteringTree);
870+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
871+
grid.filteringExpressionsTree = filteringTree;
872+
fix.detectChanges();
873+
874+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
875+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
876+
.toBe('nestedOwner', 'nested owner should be set before getState');
877+
878+
state.getState(false, 'filtering');
879+
880+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live filtering tree after getState');
881+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
882+
.toBe('nestedOwner', 'nested owner should not be removed from live filtering operand after getState');
883+
});
884+
885+
it('getState should not mutate live advancedFiltering expressions (owner)', () => {
886+
const fix = TestBed.createComponent(IgxGridStateComponent);
887+
fix.detectChanges();
888+
const grid = fix.componentInstance.grid;
889+
const state = fix.componentInstance.state;
890+
891+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
892+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
893+
productFilteringTree.filteringOperands.push({
894+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
895+
conditionName: 'true',
896+
fieldName: 'InStock',
897+
ignoreCase: true
898+
});
899+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
900+
filteringTree.filteringOperands.push(productFilteringTree);
901+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
902+
grid.advancedFilteringExpressionsTree = filteringTree;
903+
fix.detectChanges();
904+
905+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
906+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
907+
.toBe('nestedOwner', 'nested owner should be set before getState');
908+
909+
state.getState(false, 'advancedFiltering');
910+
911+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live advanced filtering tree after getState');
912+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
913+
.toBe('nestedOwner', 'nested owner should not be removed from live advanced filtering operand after getState');
914+
});
915+
806916
it('should preserve column widths when restoring state with all columns hidden', () => {
807917
const fix = TestBed.createComponent(IgxGridStateComponent);
808918
fix.detectChanges();

0 commit comments

Comments
 (0)