Skip to content

Conversation

kumarUjjawal
Copy link
Contributor

@kumarUjjawal kumarUjjawal commented Sep 26, 2025

Pull Request

Related issue

Fixes #699

What does this PR do?

As per the requirements of the Meilisearch 1.16 the goal was to update the SDK to allow the sorting on the documents API.

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Summary by CodeRabbit

  • New Features
    • Added sorting options to document retrieval, allowing results to be ordered by specified attributes when sorting is configured. Supports multi-attribute sorting. Default behavior remains unchanged if no sort is provided.
  • Tests
    • Added test coverage to verify correct ordering of documents when sorting is applied.

Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Added optional sorting support to DocumentsQuery by introducing a public sort field, initializing it in the constructor, and providing a with_sort builder method. Tests were added within the same module to validate sorted retrieval when sortable attributes are configured.

Changes

Cohort / File(s) Summary
Documents query sorting
src/documents.rs
Added public field sort: Option<Vec<&'a str>> to DocumentsQuery<'a, Http>, initialized to None in DocumentsQuery::new. Implemented with_sort(...) -> &mut DocumentsQuery<'a, Http> to set sort order. Added tests in-module to verify sorted document retrieval.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Client App
  participant SDK as SDK DocumentsQuery
  participant HTTP as HTTP Client
  participant MS as Meilisearch Server

  Dev->>SDK: DocumentsQuery::new(index)
  Dev->>SDK: with_sort(["price:asc","rating:desc"])
  Dev->>SDK: execute()
  SDK->>HTTP: GET /indexes/{uid}/documents?sort=price:asc,rating:desc
  HTTP->>MS: Request with sort param
  MS-->>HTTP: 200 OK (documents in requested order)
  HTTP-->>SDK: Response
  SDK-->>Dev: Documents (sorted)
  note over MS,SDK: If sortable attributes not configured, server may error per API rules
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers, sort by price,
Ascend, descend—so neat, so nice.
A hop, a skip, a query’s art,
With tidy lists I play my part.
Carrots aligned, results in line—
Version 1.16, crisp and fine. 🍃🐇

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely and accurately describes the primary change by indicating that sorting support has been added to the documents API, which aligns directly with the code modifications in this pull request.
Linked Issues Check ✅ Passed The pull request implements the sort parameter in the document query builder and adds a corresponding test case, directly fulfilling the tasks specified in issue #699 to update documents methods to accept sorting and include a new test.
Out of Scope Changes Check ✅ Passed All changes are confined to src/documents.rs and related tests to support sorting, matching the objectives of the linked issue and introducing no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/documents.rs (3)

193-198: Add sort field: LGTM. Consider API consistency with SearchQuery.

Works as intended. Optional: align the type with SearchQuery (Option<&'a [&'a str]>) for cross-API consistency. Both serialize fine; this is mostly about uniformity. Based on relevant code snippets


287-311: with_sort builder: LGTM. Optional parity with SearchQuery.

Current iterator-based API is ergonomic. If desired, add a slice-based overload (accepts &'a [&'a str]) to mirror SearchQuery::with_sort for parity. Based on relevant code snippets


573-599: Sorting test: LGTM. Optional extra assertions.

Test validates descending order. Consider adding:

  • An ascending case (id:asc) to cover both orders.
  • A negative case (sorting on a non-sortable attribute) to assert the expected Meilisearch error.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 910f9e9 and 0329ee4.

📒 Files selected for processing (1)
  • src/documents.rs (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/documents.rs (2)
src/search.rs (8)
  • with_sort (571-574)
  • index (724-724)
  • index (2050-2053)
  • setup_test_index (1160-1183)
  • new (39-41)
  • new (425-457)
  • new (744-749)
  • new (996-1007)
src/indexes.rs (26)
  • 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)
  • client (1133-1135)
  • client (1186-1188)
  • client (1245-1247)
  • index (2171-2171)
  • index (2188-2189)
  • index (2226-2226)
  • index (2298-2298)
  • index (2326-2326)
  • index (2352-2352)
  • index (2379-2379)
  • new (81-89)
  • new (1783-1790)
  • new (1994-2000)
⏰ 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 (1)
src/documents.rs (1)

215-215: Constructor initializes sort to None: LGTM.

Copy link

codecov bot commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.95%. Comparing base (910f9e9) to head (0329ee4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
+ Coverage   85.89%   85.95%   +0.05%     
==========================================
  Files          19       19              
  Lines        5950     5975      +25     
==========================================
+ Hits         5111     5136      +25     
  Misses        839      839              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.16.0] Add support for sorting on the documents API
1 participant