Add support to sort in /documents endpoint
#666
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds support for specifying sort on document queries by introducing a Sort property on DocumentsQuery, updates README with a "Search with sort" example, adds tests covering id and multi-field sorting plus query-string serialization, and bumps MEILISEARCH_VERSION in .env. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SDK
participant Meilisearch
rect rgb(220, 240, 255)
Note left of Client: Build DocumentsQuery\nwith Sort (e.g. ["genre:asc","name:desc"])
end
Client->>SDK: Call Documents API (Search/Query) with DocumentsQuery
SDK->>Meilisearch: HTTP request with JSON body/query including "sort"
Meilisearch-->>SDK: Sorted results response
SDK-->>Client: Return typed results
note right of SDK #e8f5e9: New DocumentsQuery.Sort\nis serialized into request
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
kashifsoofi
force-pushed
the
#665-add-support-for-sorting-on-documents-api
branch
6 times, most recently
from
a06db50 to
c95e43a
Compare
kashifsoofi
force-pushed
the
#665-add-support-for-sorting-on-documents-api
branch
from
c95e43a to
a4a2f2a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/Meilisearch.Tests/DocumentTests.cs (3)
646-662: Pluralize test name, use a unique index UID, and avoid multiple enumerations.
- Naming: other tests use “Documents” (plural). Align for consistency.
- Index UID: reusing "GetOneExistingDocumentWithIntegerIdTest" can make failures harder to triage; give each test its own UID.
- Results: materialize once to a list to avoid repeated enumeration of Results.
Apply:
- public async Task GetMultipleExistingDocumentWithAscSort() + public async Task GetMultipleExistingDocumentsWithAscSort() { - var index = await _fixture.SetUpBasicIndexWithIntId("GetOneExistingDocumentWithIntegerIdTest"); + var index = await _fixture.SetUpBasicIndexWithIntId("GetMultipleExistingDocumentsWithAscSortTest"); var updateSortable = await index.UpdateSortableAttributesAsync(new[] { "id" }); updateSortable.TaskUid.Should().BeGreaterOrEqualTo(0); await index.WaitForTaskAsync(updateSortable.TaskUid); - var documents = - await index.GetDocumentsAsync<MovieWithIntId>(new DocumentsQuery() - { - Sort = new List<string> { "id:asc" } - }); + var results = (await index.GetDocumentsAsync<MovieWithIntId>( + new DocumentsQuery { Sort = new List<string> { "id:asc" } } + )).Results.ToList(); - Assert.Equal(7, documents.Results.Count()); - documents.Results.Should().BeInAscendingOrder(x => x.Id); + Assert.Equal(7, results.Count); + results.Should().BeInAscendingOrder(x => x.Id); }
664-680: Same refinements for the DESC test; consider consolidating ASC/DESC with a Theory.
- Consistency and isolation: pluralize the method name and use a unique index UID.
- Minor style: materialize results once.
- Optional: reduce duplication by converting ASC/DESC into a single [Theory] with InlineData.
Apply:
- public async Task GetMultipleExistingDocumentWithDescSort() + public async Task GetMultipleExistingDocumentsWithDescSort() { - var index = await _fixture.SetUpBasicIndexWithIntId("GetOneExistingDocumentWithIntegerIdTest"); + var index = await _fixture.SetUpBasicIndexWithIntId("GetMultipleExistingDocumentsWithDescSortTest"); var updateSortable = await index.UpdateSortableAttributesAsync(new[] { "id" }); updateSortable.TaskUid.Should().BeGreaterOrEqualTo(0); await index.WaitForTaskAsync(updateSortable.TaskUid); - var documents = - await index.GetDocumentsAsync<MovieWithIntId>(new DocumentsQuery() - { - Sort = new List<string> { "id:desc" } - }); + var results = (await index.GetDocumentsAsync<MovieWithIntId>( + new DocumentsQuery { Sort = new List<string> { "id:desc" } } + )).Results.ToList(); - Assert.Equal(7, documents.Results.Count()); - documents.Results.Should().BeInDescendingOrder(x => x.Id); + Assert.Equal(7, results.Count); + results.Should().BeInDescendingOrder(x => x.Id); }Optional consolidation sketch:
[Theory] [InlineData("id:asc", true)] [InlineData("id:desc", false)] public async Task GetMultipleExistingDocumentsWithIdSort(string sort, bool ascending) { var index = await _fixture.SetUpBasicIndexWithIntId($"GetMultipleExistingDocumentsWithIdSort_{(ascending ? "Asc" : "Desc")}Test"); var updateSortable = await index.UpdateSortableAttributesAsync(new[] { "id" }); updateSortable.TaskUid.Should().BeGreaterOrEqualTo(0); await index.WaitForTaskAsync(updateSortable.TaskUid); var results = (await index.GetDocumentsAsync<MovieWithIntId>(new DocumentsQuery { Sort = new() { sort } })) .Results.ToList(); Assert.Equal(7, results.Count); if (ascending) results.Should().BeInAscendingOrder(x => x.Id); else results.Should().BeInDescendingOrder(x => x.Id); }
682-708: Multi-sort test: unique index UID, pluralized name, and optionally strengthen the ordering assertions.
- Consistency/isolation: pluralize the method name and use a unique index UID.
- Optional: in addition to checking first/last, verify group boundaries and within-group descending order on name to more directly exercise multi-key sort.
Apply:
- public async Task GetMultipleExistingDocumentWithMultiSort() + public async Task GetMultipleExistingDocumentsWithMultiSort() { - var index = await _fixture.SetUpBasicIndexWithIntId("GetOneExistingDocumentWithIntegerIdTest"); + var index = await _fixture.SetUpBasicIndexWithIntId("GetMultipleExistingDocumentsWithMultiSortTest"); var updateFilterable = await index.UpdateFilterableAttributesAsync(new[] { "genre" }); updateFilterable.TaskUid.Should().BeGreaterOrEqualTo(0); await index.WaitForTaskAsync(updateFilterable.TaskUid); var updateSortable = await index.UpdateSortableAttributesAsync(new[] { "genre", "name" }); updateSortable.TaskUid.Should().BeGreaterOrEqualTo(0); await index.WaitForTaskAsync(updateSortable.TaskUid); - var documents = - await index.GetDocumentsAsync<MovieWithIntId>(new DocumentsQuery() - { - Filter = "genre IN ['SF','Action']", - Sort = new List<string> { "genre:asc", "name:desc" } - }); + var results = (await index.GetDocumentsAsync<MovieWithIntId>( + new DocumentsQuery + { + Filter = "genre IN ['SF','Action']", + Sort = new List<string> { "genre:asc", "name:desc" } + } + )).Results.ToList(); - Assert.Equal(4, documents.Results.Count()); - var first = documents.Results.First(); + Assert.Equal(4, results.Count); + var first = results.First(); first.Genre.Should().Be("Action"); first.Name.Should().Be("Spider-Man"); - var last = documents.Results.Last(); + var last = results.Last(); last.Genre.Should().Be("SF"); last.Name.Should().Be("Harry Potter"); }Optional additional checks to more explicitly validate multi-key ordering:
+ // Optional: validate grouping and within-group sort + var split = results.FindIndex(x => x.Genre == "SF"); + split.Should().BeGreaterThan(0); // at least one Action before SF + results.Take(split).Should().OnlyContain(x => x.Genre == "Action"); + results.Skip(split).Should().OnlyContain(x => x.Genre == "SF"); + results.Take(split).Should().BeInDescendingOrder(x => x.Name); + results.Skip(split).Should().BeInDescendingOrder(x => x.Name);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.env(1 hunks)README.md(1 hunks)src/Meilisearch/QueryParameters/DocumentsQuery.cs(1 hunks)tests/Meilisearch.Tests/DocumentTests.cs(1 hunks)tests/Meilisearch.Tests/ObjectExtensionsTests.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/Meilisearch.Tests/ObjectExtensionsTests.cs
- .env
- README.md
- src/Meilisearch/QueryParameters/DocumentsQuery.cs
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Meilisearch.Tests/DocumentTests.cs (2)
tests/Meilisearch.Tests/Movie.cs (1)
MovieWithIntId(40-47)src/Meilisearch/QueryParameters/DocumentsQuery.cs (1)
DocumentsQuery(9-40)
curquiza
changed the title
curquiza
changed the title
/documents endpoint
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
Related issue
Fixes #665
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
Documentation
Tests
Chores