Skip to content

Commit b867b35

Browse files
viktorkombovviktorkombov
authored
fix(grids): getState no longer mutates live sorting/groupBy/filtering expressions (#17198)
1 parent 28a06b9 commit b867b35

2 files changed

Lines changed: 141 additions & 28 deletions

File tree

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

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,11 @@ export class IgxGridStateBaseDirective {
144144
private FEATURES = {
145145
sorting: {
146146
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
147-
const sortingState = context.currGrid.sortingExpressions;
148-
sortingState.forEach(s => {
149-
delete s.strategy;
150-
delete s.owner;
147+
const sortingState = context.currGrid.sortingExpressions.map(s => {
148+
const copy = { ...s };
149+
delete copy.strategy;
150+
delete copy.owner;
151+
return copy;
151152
});
152153
return { sorting: sortingState };
153154
},
@@ -159,10 +160,7 @@ export class IgxGridStateBaseDirective {
159160
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
160161
const filteringState = context.currGrid.filteringExpressionsTree;
161162
if (filteringState) {
162-
delete filteringState.owner;
163-
for (const item of filteringState.filteringOperands) {
164-
delete (item as IFilteringExpressionsTree).owner;
165-
}
163+
return { filtering: context.cloneFilteringTree(filteringState) };
166164
}
167165
return { filtering: filteringState };
168166
},
@@ -174,16 +172,7 @@ export class IgxGridStateBaseDirective {
174172
advancedFiltering: {
175173
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
176174
const filteringState = context.currGrid.advancedFilteringExpressionsTree;
177-
let advancedFiltering: any;
178-
if (filteringState) {
179-
delete filteringState.owner;
180-
for (const item of filteringState.filteringOperands) {
181-
delete (item as IFilteringExpressionsTree).owner;
182-
}
183-
advancedFiltering = filteringState;
184-
} else {
185-
advancedFiltering = {};
186-
}
175+
const advancedFiltering: any = filteringState ? context.cloneFilteringTree(filteringState) : {};
187176
return { advancedFiltering };
188177
},
189178
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IFilteringExpressionsTree): void => {
@@ -231,21 +220,21 @@ export class IgxGridStateBaseDirective {
231220
},
232221
restoreFeatureState: (context: IgxGridStateBaseDirective, state: IColumnState[]): void => {
233222
const newColumns = [];
234-
223+
235224
// Helper to restore column state without auto-persisting widths
236225
const restoreColumnState = (column: IgxColumnComponent | IgxColumnGroupComponent, colState: IColumnState) => {
237226
// Extract width to handle it separately
238227
const width = colState.width;
239228
delete colState.width;
240-
229+
241230
Object.assign(column, colState);
242-
231+
243232
// Only restore width if it was explicitly set by the user (not undefined)
244233
if (width !== undefined) {
245234
column.width = width;
246235
}
247236
};
248-
237+
249238
state.forEach((colState) => {
250239
const hasColumnGroup = colState.columnGroup;
251240
const hasColumnLayouts = colState.columnLayout;
@@ -262,9 +251,9 @@ export class IgxGridStateBaseDirective {
262251
} else {
263252
ref1.children.reset([]);
264253
}
265-
254+
266255
restoreColumnState(ref1, colState);
267-
256+
268257
ref1.grid = context.currGrid;
269258
if (colState.parent || colState.parentKey) {
270259
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref1.parent));
@@ -282,7 +271,7 @@ export class IgxGridStateBaseDirective {
282271
}
283272

284273
restoreColumnState(ref, colState);
285-
274+
286275
ref.grid = context.currGrid;
287276
if (colState.parent || colState.parentKey) {
288277
const columnGroup: IgxColumnGroupComponent = newColumns.find(e => e.columnGroup && (e.key ? e.key === colState.parentKey : e.header === ref.parent));
@@ -304,9 +293,11 @@ export class IgxGridStateBaseDirective {
304293
groupBy: {
305294
getFeatureState: (context: IgxGridStateBaseDirective): IGridState => {
306295
const grid = context.currGrid as IgxGridComponent;
307-
const groupingExpressions = grid.groupingExpressions;
308-
groupingExpressions.forEach(expr => {
309-
delete expr.strategy;
296+
const groupingExpressions = grid.groupingExpressions.map(expr => {
297+
const copy = { ...expr };
298+
delete copy.strategy;
299+
delete copy.owner;
300+
return copy;
310301
});
311302
const expansionState = grid.groupingExpansionState;
312303
const groupsExpanded = grid.groupsExpanded;
@@ -689,6 +680,22 @@ export class IgxGridStateBaseDirective {
689680
}
690681
}
691682

683+
/**
684+
* Recursively clones a filtering expression tree, stripping the `owner` property
685+
* from each cloned node so the live tree is never mutated.
686+
*/
687+
private cloneFilteringTree(tree: IFilteringExpressionsTree): IFilteringExpressionsTree {
688+
const copy: IFilteringExpressionsTree = { ...tree };
689+
delete copy.owner;
690+
copy.filteringOperands = tree.filteringOperands.map(item => {
691+
if ('filteringOperands' in item) {
692+
return this.cloneFilteringTree(item as IFilteringExpressionsTree);
693+
}
694+
return { ...item };
695+
});
696+
return copy;
697+
}
698+
692699
/**
693700
* This method builds a rehydrated IExpressionTree from a provided object.
694701
*/

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

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,112 @@ describe('IgxGridState - input properties #grid', () => {
848848
const allSameWidth = calcWidths.every(w => w === calcWidths[0]);
849849
expect(allSameWidth).toBe(false, 'Columns should not all have the same width');
850850
});
851+
852+
it('getState should not mutate live sorting expressions (strategy/owner)', () => {
853+
const fix = TestBed.createComponent(IgxGridStateComponent);
854+
fix.detectChanges();
855+
const grid = fix.componentInstance.grid;
856+
const state = fix.componentInstance.state;
857+
858+
const customStrategy = DefaultSortingStrategy.instance();
859+
const owner = {} as any;
860+
grid.sortingExpressions = [
861+
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
862+
];
863+
fix.detectChanges();
864+
865+
expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
866+
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should be set before getState');
867+
868+
state.getState(false, 'sorting');
869+
870+
expect(grid.sortingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live expressions after getState');
871+
expect(grid.sortingExpressions[0].owner).toBe(owner, 'owner should not be removed from live expressions after getState');
872+
});
873+
874+
it('getState should not mutate live groupBy expressions (strategy/owner)', () => {
875+
const fix = TestBed.createComponent(IgxGridStateComponent);
876+
fix.detectChanges();
877+
const grid = fix.componentInstance.grid;
878+
const state = fix.componentInstance.state;
879+
880+
const customStrategy = DefaultSortingStrategy.instance();
881+
const owner = {} as any;
882+
grid.groupingExpressions = [
883+
{ fieldName: 'ProductID', dir: SortingDirection.Asc, ignoreCase: false, strategy: customStrategy, owner }
884+
];
885+
fix.detectChanges();
886+
887+
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should be set before getState');
888+
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should be set before getState');
889+
890+
state.getState(false, 'groupBy');
891+
892+
expect(grid.groupingExpressions[0].strategy).toBe(customStrategy, 'strategy should not be removed from live groupBy expressions after getState');
893+
expect(grid.groupingExpressions[0].owner).toBe(owner, 'owner should not be removed from live groupBy expressions after getState');
894+
});
895+
896+
it('getState should not mutate live filtering expressions (owner)', () => {
897+
const fix = TestBed.createComponent(IgxGridStateComponent);
898+
fix.detectChanges();
899+
const grid = fix.componentInstance.grid;
900+
const state = fix.componentInstance.state;
901+
902+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
903+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
904+
productFilteringTree.filteringOperands.push({
905+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
906+
conditionName: 'true',
907+
fieldName: 'InStock',
908+
ignoreCase: true
909+
});
910+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
911+
filteringTree.filteringOperands.push(productFilteringTree);
912+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
913+
grid.filteringExpressionsTree = filteringTree;
914+
fix.detectChanges();
915+
916+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
917+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
918+
.toBe('nestedOwner', 'nested owner should be set before getState');
919+
920+
state.getState(false, 'filtering');
921+
922+
expect(grid.filteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live filtering tree after getState');
923+
expect((grid.filteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
924+
.toBe('nestedOwner', 'nested owner should not be removed from live filtering operand after getState');
925+
});
926+
927+
it('getState should not mutate live advancedFiltering expressions (owner)', () => {
928+
const fix = TestBed.createComponent(IgxGridStateComponent);
929+
fix.detectChanges();
930+
const grid = fix.componentInstance.grid;
931+
const state = fix.componentInstance.state;
932+
933+
const filteringTree = new FilteringExpressionsTree(FilteringLogic.And);
934+
const productFilteringTree = new FilteringExpressionsTree(FilteringLogic.And, 'ProductName');
935+
productFilteringTree.filteringOperands.push({
936+
condition: IgxBooleanFilteringOperand.instance().condition('true'),
937+
conditionName: 'true',
938+
fieldName: 'InStock',
939+
ignoreCase: true
940+
});
941+
(productFilteringTree as IFilteringExpressionsTree).owner = 'nestedOwner';
942+
filteringTree.filteringOperands.push(productFilteringTree);
943+
(filteringTree as IFilteringExpressionsTree).owner = 'rootOwner';
944+
grid.advancedFilteringExpressionsTree = filteringTree;
945+
fix.detectChanges();
946+
947+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should be set before getState');
948+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
949+
.toBe('nestedOwner', 'nested owner should be set before getState');
950+
951+
state.getState(false, 'advancedFiltering');
952+
953+
expect(grid.advancedFilteringExpressionsTree.owner).toBe('rootOwner', 'root owner should not be removed from live advanced filtering tree after getState');
954+
expect((grid.advancedFilteringExpressionsTree.filteringOperands[0] as IFilteringExpressionsTree).owner)
955+
.toBe('nestedOwner', 'nested owner should not be removed from live advanced filtering operand after getState');
956+
});
851957
});
852958

853959
class HelperFunctions {

0 commit comments

Comments
 (0)