-
Notifications
You must be signed in to change notification settings - Fork 377
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
feat: define filters and sort schemas for search #4270
feat: define filters and sort schemas for search #4270
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/responses-and-suggestion-filter #4270 +/- ##
============================================================================
+ Coverage 64.66% 91.72% +27.05%
============================================================================
Files 321 322 +1
Lines 18513 18558 +45
============================================================================
+ Hits 11972 17022 +5050
+ Misses 6541 1536 -5005 ☔ View full report in Codecov by Sentry. |
The URL of the deployed environment for this PR is https://argilla-quickstart-pr-4270-ki24f765kq-no.a.run.app |
@jfcalvo Yes, for me it's ok. Thanks Jose! |
I found some issues in your object scheme, {"type": "terms", "field": "suggestion.sentiment.values", "values": ["positive"]}, This example does not exist 👇 This example 👇 This example must be: |
The sort key is not necessary for this iteration. |
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.
I think this way is much more readable, comprehensible, parseable, and easy to validate
e95c1e1
into
feature/responses-and-suggestion-filter
Description
This is a proposal for adding
filters
andsort
attributes on the endpoints for search records.The changes allow us to define the following schema for searching:
Notice some changes from the Notion documentation:
field
instead ofpath
because we are already usingfield
in thequery
parameter.float
in the values in the case that we want to specify numbers.I'm adding a newterm
(singular) filter so is more explicit to filter by one specific value (this is optional we can remove it if not needed).With this PR we should ask several questions:
and
?and
will be enough for the frontend requirements by now.field
format should we follow?Making the "value" of the field to filter implicit likesuggestion.sentiment
instead ofsuggestion.sentiment.value
?suggestion.sentiment.value
instead ofsuggestion.sentiment
?value
(instead ofvalues
) at the end.@damianpumar should help us to answer the first question of this list.
Type of change
How Has This Been Tested
Checklist