fix global filtering with undefined first values - #6438
Conversation
📝 WalkthroughWalkthroughGlobal filtering now honors explicit enablement and checks the first non-null value in a column when determining filterability. A regression test covers filtering data where the first row’s value is ChangesGlobal filtering fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 19ea699
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts (1)
551-577: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the undefined-first-row regression directly.
This test exercises the explicit
enableGlobalFilter: truebranch, so it never validates the new first non-nullish lookup. Add a case with the first row’s valueundefined, a later searchable string, and no explicit opt-in; assert that the later row is returned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts` around lines 551 - 577, Extend the global filtering tests near the explicit opt-in case with a column whose first row value is undefined and a later row contains a searchable string, without setting enableGlobalFilter. Configure the filter to match that string and assert that only the later row is returned, covering the first non-nullish value lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts`:
- Around line 551-577: Extend the global filtering tests near the explicit
opt-in case with a column whose first row value is undefined and a later row
contains a searchable string, without setting enableGlobalFilter. Configure the
filter to match that string and assert that only the later row is returned,
covering the first non-nullish value lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4287a756-7fce-4b86-aada-f6438bc84c77
📒 Files selected for processing (2)
packages/table-core/src/features/global-filtering/globalFilteringFeature.tspackages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts
1 participant
What changed
Why
The default eligibility predicate sampled only the first row. When that row had an undefined value, the entire column was excluded from global filtering even when later rows contained valid strings or numbers. This made results depend on row order.
Validation
Closes #4783
Summary by CodeRabbit
Bug Fixes
Tests