-
Notifications
You must be signed in to change notification settings - Fork 97
Add support for sorting on the documents API. #1163
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
Conversation
Walkthroughget_documents now accepts a string Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant get_documents
participant HTTP
participant Meilisearch
Client->>get_documents: call get_documents(uid, parameters)
rect rgb(245,250,255)
get_documents->>get_documents: if parameters["sort"] is string\nsplit on ',' and trim -> list
end
get_documents->>HTTP: send request with normalized parameters
HTTP->>Meilisearch: HTTP request
Meilisearch-->>HTTP: documents response
HTTP-->>get_documents: response
get_documents-->>Client: return documents
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Nitpick comments (1)
meilisearch/index.py (1)
406-406: Improve the docstring phrasing.The phrase "A list of attributes written as an array or as a comma-separated string" is somewhat awkward. Consider rephrasing for clarity.
Apply this diff:
- - sort: A list of attributes written as an array or as a comma-separated string + - sort: List of sort attributes. Can be provided as a list or comma-separated string (e.g., ["rating:desc", "date:asc"] or "rating:desc, date:asc")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
meilisearch/index.py(2 hunks)tests/index/test_index_document_meilisearch.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/index/test_index_document_meilisearch.py (2)
tests/conftest.py (1)
index_with_documents(121-128)meilisearch/index.py (4)
update_sortable_attributes(1510-1531)wait_for_task(233-260)add_documents(451-483)get_documents(387-425)
meilisearch/index.py (1)
meilisearch/_httprequests.py (1)
get(95-96)
🪛 GitHub Actions: Tests
tests/index/test_index_document_meilisearch.py
[error] Black formatting check failed: 1 file would be reformatted. Run 'pipenv run black' to format the code.
🔇 Additional comments (1)
meilisearch/index.py (1)
415-419: LGTM! Clean string-to-list conversion.The implementation correctly handles comma-separated sort strings, properly strips whitespace, and filters empty values. The type check ensures lists are passed through unchanged.
Strift
left a 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.
Thank you for your contribution 🙌
#1135
feat: Add support for sorting on the documents API.
Summary by CodeRabbit
Bug Fixes
Tests
Documentation