feat: improve filter to match title and values - #110
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSearch filtering now uses a recursive schema filter that matches property keys, titles, nested objects, arrays, and runtime model values. ChangesProperty search
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@src/form/fields/ObjectField/ObjectFieldGrouping.tsx`:
- Around line 16-23: Split the combined memo in the ObjectFieldGrouping
component into separate filtering and grouping memos. Keep the
filtered-properties memo dependent on filteredFieldText, schema.properties,
model, and propName, then memoize getFieldGroups using only the
filtered-properties result so unchanged properties preserve groupedProperties
identity during unrelated model updates.
In `@src/form/utils/get-filtered-properties.ts`:
- Around line 33-39: Guard the model value in the valueMatched logic of
getFilteredProperties before calling .some, ensuring iteration only occurs when
model[property] is actually an array. Preserve the existing recursive filtering
and null-item handling, while treating non-array values as unmatched rather than
throwing.
- Around line 4-42: Update getFilteredObjectProperty and
getFilteredArrayProperty to first match the container’s own property key or
definition.title against filter, returning the original container definition
when matched. Preserve the existing descendant schema/model matching behavior
when the container itself does not match, and add coverage for object and array
key/title searches with no matching child.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3a58bf6-f27a-4d6e-8f64-aa39dc0de719
📒 Files selected for processing (4)
src/form/fields/ObjectField/ObjectFieldGrouping.test.tsxsrc/form/fields/ObjectField/ObjectFieldGrouping.tsxsrc/form/utils/get-filtered-properties.test.tssrc/form/utils/get-filtered-properties.ts
ce55903 to
1562811
Compare
#1) Replace functional implementation with SchemaPropertyFilter class for better code organization and maintainability. Changes: - Rename get-filtered-properties.ts → SchemaPropertyFilter.ts - Convert from functional to class-based static methods - Add type guards (hasNestedProperties, hasArrayItemProperties, isMatchablePrimitive) - Extract helper methods for better separation of concerns: - matchesKeyOrTitle() - centralized key/title matching - matchesModelValue() - value-based matching logic - getNestedModelSlice() - type-safe model extraction for objects - getArrayModelSlice() - type-safe model extraction for arrays - filterObjectProperty() - dedicated object filtering - filterArrayProperty() - dedicated array filtering - matchesPrimitiveProperty() - primitive property matching - Improve documentation with JSDoc comments - Enhance type safety throughout the implementation - Maintain all existing functionality and test coverage The new structure makes the code more maintainable by breaking down complex logic into focused, well-documented methods while preserving the original filtering behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/form/utils/SchemaPropertyFilter.ts (1)
26-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCase-insensitivity is only true if the caller already lowercased
filter.The docstring says the
filterparam is "case-insensitive", butmatchesKeyOrTitleandmatchesModelValueonly lowercase the key/title/value side of the comparison —filteritself is never normalized inside this class. Today's sole caller (ObjectFieldGrouping.tsx) happens to lowercase before calling, so it works, but any future caller that passes a mixed-case term (e.g.SchemaPropertyFilter.filter(props, 'UserName')) will silently get zero matches even though the docstring promises case-insensitive matching. There's also no test exercising a raw, non-pre-loweredfilterargument, so this gap isn't caught by the current suite.Normalize once at the entry point so every recursive call (which re-enters
filter()) benefits automatically:🛡️ Proposed fix
// Early return for undefined properties if (!isDefined(properties)) { return {}; } + // Normalize case here so callers don't need to pre-lowercase, and all + // recursive matchers (key/title/model-value) stay consistent. + filter = filter.toLowerCase(); + // Early return for empty filter (optimization) if (filter.length === 0) {Also applies to: 121-135
🤖 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 `@src/form/utils/SchemaPropertyFilter.ts` around lines 26 - 30, Normalize the filter term once at the entry point of SchemaPropertyFilter.filter before matching or recursive calls, so mixed-case input is handled case-insensitively throughout matchesKeyOrTitle and matchesModelValue. Preserve the existing space-removal contract and ensure recursive filter calls reuse the normalized term.
🤖 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.
Inline comments:
In `@src/form/utils/SchemaPropertyFilter.ts`:
- Around line 91-109: Update hasNestedProperties to recognize any schema with
defined properties, regardless of whether schema.type is 'object'; preserve the
existing type guard return shape and isDefined(properties) check so schemas with
omitted type recurse through nested matching.
---
Nitpick comments:
In `@src/form/utils/SchemaPropertyFilter.ts`:
- Around line 26-30: Normalize the filter term once at the entry point of
SchemaPropertyFilter.filter before matching or recursive calls, so mixed-case
input is handled case-insensitively throughout matchesKeyOrTitle and
matchesModelValue. Preserve the existing space-removal contract and ensure
recursive filter calls reuse the normalized term.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e62b48ec-a389-426c-8534-6580292a228d
📒 Files selected for processing (6)
src/form/fields/ObjectField/ObjectFieldGrouping.test.tsxsrc/form/fields/ObjectField/ObjectFieldGrouping.tsxsrc/form/utils/SchemaPropertyFilter.test.tssrc/form/utils/SchemaPropertyFilter.tssrc/form/utils/get-filtered-properties.test.tssrc/form/utils/get-filtered-properties.ts
💤 Files with no reviewable changes (2)
- src/form/utils/get-filtered-properties.test.ts
- src/form/utils/get-filtered-properties.ts
|
What changed
getFilteredPropertiesnow accepts an optionalmodelparameter (current form data) and matches properties on three criteria: property key, schematitle, and the current runtime value.propertiesreference unchanged when there's nothing to filter or omit (avoids unnecessary re-renders).ObjectFieldGroupingpasses the relevant model slice down togetFilteredPropertiesand strips spaces from the query term before matching.KaotoSchemaDefinitionwith the standardJSONSchema4type in tests.Tests added
get-filtered-properties.test.ts: 10 new cases covering title matching, value matching, object/array recursion, empty-filter short-circuit, and edge cases (null array items, plain-string arrays).ObjectFieldGrouping.test.tsx: 2 new cases — space-stripping in the filter and required-field indicator rendering.Screen.Recording.2026-07-28.at.16.11.32.mov
fix: KaotoIO/kaoto#3068
Summary by CodeRabbit
New Features
Bug Fixes
Tests