Fix: Incorrect filtering of properties (#13009) - #13011
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes property filtering by matching text values by content and improving filter-clause management.
Changes:
- Bypasses identity fast path for content-matched properties.
- Replaces duplicate property filters and removes chips by position.
- Needs correction to preserve valid repeated filters and regression tests.
Verification: Static review completed; runtime verification was unavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/main/frontend/components/views.cljs |
Updates filter creation and removal behavior. |
deps/db/src/logseq/db/common/view.cljs |
Falls back to content matching for text-like property values. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Track the filter created by each picker session, keep content matching on the fast path, and add regression coverage.
|
@tiensonqin Thank you so much for refining the code to handle those edge cases and adding the regression tests! I really appreciate you taking the time to polish this with me. I just updated the branch with the latest master, so it looks like the GitHub Action workflows are currently paused and awaiting a maintainer's approval to run. Let me know if there's anything else you need from my end! |
Description
Fixes #13009
What is the problem?
As reported in #13009, databases don't correctly filter certain text properties (like a custom 'author' property) on the first try, hiding valid results. If you filter the exact same property again, all the expected results suddenly show up, but the UI displays duplicate filter chips (e.g., "Author is..." twice). Trying to remove one of these duplicate filters removes both of them at once. Properties with set options (like status or priority) do not experience this bug.
How does this PR fix it?
Duplicate Filter Prevention (views.cljs): Updated the filter selection handler to actively remove any existing :is filter for a property before adding the newly selected value. This prevents the UI from stacking duplicate clauses when selecting options.
Text Property Matching (view.cljs): The root cause of the missing results on the first click was a backend optimization that tried to match property values by their exact database ID. Since custom text properties don't always share IDs (unlike closed-value options like status), it failed. I added a match-by-content? check so text properties bypass this fast path and correctly evaluate based on their actual content.
Tested manually:
Created a database view querying a custom text property (e.g., 'author').
Clicked the filter icon and filtered by a specific author.
Confirmed all blocks with that property show up correctly on the first try.
Confirmed duplicate filter chips are no longer created when adjusting the filter.
Note: I used Claude Code to help investigate the root cause and write this fix. I manually tested the described behavior myself on a local build before opening this PR.