-
Notifications
You must be signed in to change notification settings - Fork 102
Add filterableAttributes syntax API to settings #730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds mixed-syntax support for Meilisearch v1.14 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Index as Index API
participant Server as Meilisearch
rect rgba(135,206,250,0.08)
Note over Client,Index: Update filterable attributes (mixed syntax)
Client->>Index: set_filterable_attributes_advanced([ "author", { attributePatterns:[...], features:{...} } ])
Index->>Server: POST /settings/filterable-attributes (body: Vec<FilterableAttribute>)
Server-->>Index: TaskInfo
Index-->>Client: Result<TaskInfo]
end
rect rgba(220,220,220,0.06)
Note over Client,Index: Backward-compatible string usage
Client->>Index: set_filterable_attributes(["author","genre"])
Index->>Index: Wrap strings -> FilterableAttribute::Attribute(...)
Index->>Server: POST /settings/filterable-attributes (body: Vec<FilterableAttribute>)
Server-->>Index: TaskInfo
Index-->>Client: Result<TaskInfo]
end
rect rgba(144,238,144,0.06)
Note over Client,Index: Retrieve mixed syntax
Client->>Index: get_filterable_attributes_advanced()
Index->>Server: GET /settings/filterable-attributes
Server-->>Index: Vec<FilterableAttribute> (mixed)
Index-->>Client: Result<Vec<FilterableAttribute>>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/settings.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/settings.rs (1)
1614-1633: Consider more flexible signature forset_filterable_attributes_advanced.The current signature
impl IntoIterator<Item = FilterableAttribute>requires owned values, which prevents passing borrowed slices like&[FilterableAttribute]. This differs from other setter methods (e.g.,set_stop_wordsat line 1497) that useItem = impl AsRef<str>to accept both owned and borrowed values.While the current design is simpler and works correctly, consider whether accepting borrowed values would improve ergonomics:
pub async fn set_filterable_attributes_advanced( &self, filterable_attributes: &[FilterableAttribute], ) -> Result<TaskInfo, Error> { self.client .http_client .request::<(), Vec<FilterableAttribute>, TaskInfo>( &format!(...), Method::Put { query: (), body: filterable_attributes.to_vec(), }, 202, ) .await }This would allow callers to retain ownership of their configurations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.code-samples.meilisearch.yaml(1 hunks)src/documents.rs(1 hunks)src/settings.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/settings.rs (1)
src/indexes.rs (13)
client(186-188)client(232-234)client(320-322)client(376-378)client(428-430)client(473-475)client(522-525)client(556-558)client(634-636)client(700-702)client(970-972)client(1038-1040)client(1089-1091)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration-tests
🔇 Additional comments (5)
.code-samples.meilisearch.yaml (1)
549-567: LGTM: Imports and mixed-syntax demonstration are correct.The new types are properly imported and the example correctly demonstrates both legacy syntax (using
.into()for string conversion) and the new settings object syntax.src/documents.rs (1)
706-713: LGTM: Test correctly updated for new FilterableAttribute type.The test properly imports and uses the new
FilterableAttributeenum, wrapping attribute names in theAttributevariant. This correctly reflects the changes toSettings::filterable_attributesfromOption<Vec<String>>toOption<Vec<FilterableAttribute>>.src/settings.rs (3)
67-114: LGTM: Well-structured types for mixed-syntax filterable attributes.The new types properly model Meilisearch v1.14's advanced filterable attributes syntax:
- Hierarchical structure with
FilterFeatureModes→FilterFeatures→FilterableAttributesSettings- Untagged enum allows seamless JSON serialization of mixed plain strings and settings objects
Fromimplementations provide ergonomic string-to-attribute conversion- Proper camelCase JSON mapping via serde annotations
244-399: LGTM: Backward-compatible API design with advanced method.The changes maintain backward compatibility:
- Existing
with_filterable_attributesaccepts strings and converts internally- New
with_filterable_attributes_advancedaccepts mixedFilterableAttributeitems- Clear documentation distinguishes the two approaches
This design allows gradual adoption of the new syntax while preserving existing code.
767-799: LGTM: Dual getter methods provide both legacy and advanced access.The addition of
get_filterable_attributes_advancedalongside the existingget_filterable_attributesenables:
- Legacy code continues using
Vec<String>return type- New code can retrieve mixed-syntax
Vec<FilterableAttribute>Both methods query the same endpoint but deserialize to different types, maintaining backward compatibility.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #730 +/- ##
==========================================
+ Coverage 86.18% 86.42% +0.23%
==========================================
Files 20 20
Lines 6263 6395 +132
==========================================
+ Hits 5398 5527 +129
- Misses 865 868 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request
Related issue
Fixes #660
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Backward Compatibility
Documentation / Tests