fix(data-view): preserve filtering on hidden properties#14500
Merged
darkskygit merged 3 commits intotoeverything:canaryfrom Feb 23, 2026
Merged
fix(data-view): preserve filtering on hidden properties#14500darkskygit merged 3 commits intotoeverything:canaryfrom
darkskygit merged 3 commits intotoeverything:canaryfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFilter evaluation in KanbanSingleView and TableSingleView now builds the row property map from all raw properties ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
blocksuite/affine/data-view/src/__tests__/table.unit.spec.ts (1)
100-144: Good regression test — mirrors the kanban test for consistency.Same feedback as the kanban test: consider adding a negative case (hidden column value doesn't match filter →
isShowreturnsfalse) to fully cover the filtering path.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blocksuite/affine/data-view/src/__tests__/table.unit.spec.ts` around lines 100 - 144, Add a negative test mirroring the existing "evaluates filters with hidden columns" case: create a FilterGroup that checks status is "Done", but make the statusProperty.jsonValue$.value something else (e.g., "In Progress"), keep properties$ showing only title and propertiesRaw$ containing title and status, and assert TableSingleView.prototype.isShow.call(view, 'row-1') returns false; this ensures the hidden-column filter path is covered for non-matching values.blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts (1)
277-321: Good regression test for the hidden-column filtering fix.The test correctly simulates a hidden column scenario and validates that
isShowevaluates the filter usingpropertiesRaw$rather thanproperties$.Consider adding a complementary test case where the hidden column's value does not match the filter (e.g.,
value: 'In Progress'), assertingisShowreturnsfalse. This would confirm both the positive and negative paths through the filter with hidden columns.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts` around lines 277 - 321, Add a complementary unit test in the same describe('filtering') block that mirrors the existing "evaluates filters with hidden columns" case but sets the hidden statusProperty's jsonValue$.value to 'In Progress' (so it does not match the filter) and assert that KanbanSingleView.prototype.isShow.call(view, 'row-1') returns false; reuse the same filter variable and view setup but change statusProperty value to 'In Progress' and keep properties$ containing only titleProperty and propertiesRaw$ containing both properties to ensure isShow evaluates against propertiesRaw$.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts`:
- Around line 277-321: Add a complementary unit test in the same
describe('filtering') block that mirrors the existing "evaluates filters with
hidden columns" case but sets the hidden statusProperty's jsonValue$.value to
'In Progress' (so it does not match the filter) and assert that
KanbanSingleView.prototype.isShow.call(view, 'row-1') returns false; reuse the
same filter variable and view setup but change statusProperty value to 'In
Progress' and keep properties$ containing only titleProperty and propertiesRaw$
containing both properties to ensure isShow evaluates against propertiesRaw$.
In `@blocksuite/affine/data-view/src/__tests__/table.unit.spec.ts`:
- Around line 100-144: Add a negative test mirroring the existing "evaluates
filters with hidden columns" case: create a FilterGroup that checks status is
"Done", but make the statusProperty.jsonValue$.value something else (e.g., "In
Progress"), keep properties$ showing only title and propertiesRaw$ containing
title and status, and assert TableSingleView.prototype.isShow.call(view,
'row-1') returns false; this ensures the hidden-column filter path is covered
for non-matching values.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts (1)
277-365: Extract sharedfilterandtitlePropertymocks to reduce duplication.Both test cases contain an identical
filterdefinition (lines 279–293 and 323–337) and an identicaltitlePropertymock (lines 295–302 and 339–346). Hoisting these to sharedconsts within thedescribe('filtering', ...)scope removes copy-paste drift risk if the filter structure ever changes.♻️ Proposed refactor
describe('filtering', () => { + const sharedFilter: FilterGroup = { + type: 'group', + op: 'and', + conditions: [ + { + type: 'filter', + left: { type: 'ref', name: 'status' }, + function: 'is', + args: [{ type: 'literal', value: 'Done' }], + }, + ], + }; + + const titleProperty = { + id: 'title', + cellGetOrCreate: () => ({ jsonValue$: { value: 'Task 1' } }), + }; + it('evaluates filters with hidden columns', () => { - const filter: FilterGroup = { - type: 'group', - op: 'and', - conditions: [ - { - type: 'filter', - left: { type: 'ref', name: 'status' }, - function: 'is', - args: [{ type: 'literal', value: 'Done' }], - }, - ], - }; - - const titleProperty = { - id: 'title', - cellGetOrCreate: () => ({ jsonValue$: { value: 'Task 1' } }), - }; const statusProperty = { id: 'status', cellGetOrCreate: () => ({ jsonValue$: { value: 'Done' } }), }; const view = { - filter$: { value: filter }, + filter$: { value: sharedFilter }, properties$: { value: [titleProperty] }, propertiesRaw$: { value: [titleProperty, statusProperty] }, } as unknown as KanbanSingleView; expect(KanbanSingleView.prototype.isShow.call(view, 'row-1')).toBe(true); }); it('returns false when hidden filtered column does not match', () => { - const filter: FilterGroup = { - type: 'group', - op: 'and', - conditions: [ - { - type: 'filter', - left: { type: 'ref', name: 'status' }, - function: 'is', - args: [{ type: 'literal', value: 'Done' }], - }, - ], - }; - - const titleProperty = { - id: 'title', - cellGetOrCreate: () => ({ jsonValue$: { value: 'Task 1' } }), - }; const statusProperty = { id: 'status', cellGetOrCreate: () => ({ jsonValue$: { value: 'In Progress' } }), }; const view = { - filter$: { value: filter }, + filter$: { value: sharedFilter }, properties$: { value: [titleProperty] }, propertiesRaw$: { value: [titleProperty, statusProperty] }, } as unknown as KanbanSingleView; expect(KanbanSingleView.prototype.isShow.call(view, 'row-1')).toBe(false); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts` around lines 277 - 365, Duplicate filter and titleProperty mocks are repeated in the two tests; hoist them into shared constants inside the describe('filtering', ...) scope (e.g., const sharedFilter = { ... } and const sharedTitleProperty = { ... }) and replace the inline definitions in both it(...) blocks with references to sharedFilter and sharedTitleProperty; keep the per-test statusProperty definitions local and continue to call KanbanSingleView.prototype.isShow.call(view, 'row-1') as before.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.tsblocksuite/affine/data-view/src/__tests__/table.unit.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- blocksuite/affine/data-view/src/tests/table.unit.spec.ts
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@blocksuite/affine/data-view/src/__tests__/kanban.unit.spec.ts`:
- Around line 277-365: Duplicate filter and titleProperty mocks are repeated in
the two tests; hoist them into shared constants inside the describe('filtering',
...) scope (e.g., const sharedFilter = { ... } and const sharedTitleProperty = {
... }) and replace the inline definitions in both it(...) blocks with references
to sharedFilter and sharedTitleProperty; keep the per-test statusProperty
definitions local and continue to call
KanbanSingleView.prototype.isShow.call(view, 'row-1') as before.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #14500 +/- ##
==========================================
- Coverage 57.46% 57.07% -0.39%
==========================================
Files 2860 2860
Lines 154583 154583
Branches 23368 23243 -125
==========================================
- Hits 88829 88227 -602
- Misses 62845 63348 +503
- Partials 2909 3008 +99
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
darkskygit
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes issue #14036 where hiding a column used in filters caused empty table/kanban results.
Root cause: filter evaluation built the row map from visible properties only.
Change: evaluate filters using full property set (propertiesRaw$) so hidden filtered columns still participate.
Added unit regressions for both table and kanban hidden-column filtering behavior.
Verified this does fix the filtering issue for hidden columns:
Summary by CodeRabbit