Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Adding appliedFilters to column-change payloads breaks existing event-shape consumers and tests.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR decouples ColumnsController from DataController and moves outdated-filter detection into DataController.
Changes:
- Adds outdated-filter detection and regression tests.
- Carries applied-filter metadata through column changes.
- Removes the
DataControllerdependency fromColumnsController.
File summaries
| File | Summary |
|---|---|
packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts |
Detects outdated filters and reloads data. |
packages/devextreme/js/__internal/grids/grid_core/data_controller/__tests__/data_controller.outdated_filter.test.ts |
Tests outdated-filter reload behavior. |
packages/devextreme/js/__internal/grids/grid_core/columns_controller/types.ts |
Adds applied-filter change metadata. |
packages/devextreme/js/__internal/grids/grid_core/columns_controller/m_columns_controller.ts |
Removes the data-controller dependency and records applied filters. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| updateColumnChanges(this, 'columns'); | ||
|
|
||
| if (this._columnChanges) { |
There was a problem hiding this comment.
It looks like this._columnChanges will always be defined here, since updateColumnChanges is called above and always assigns an object to this._columnChanges.
Wouldn’t it be better to check for parameters.filtering here instead?
There was a problem hiding this comment.
this if is for ts. So I can replace it by using !.
But checking for parameters.filtering would be a behavior change, undefined value for columnChages.appliedFilter is a legit value meaning that dataSource was loaded without any filter. With this check, the condition will change:
No description provided.