Conversation
Signed-off-by: Will Velida <willvelida@hotmail.co.uk>
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive date range querying functionality to the Biotrackr Activity API, enabling clients to retrieve activity documents within specified date ranges with full pagination support and robust validation.
- Implements a new REST endpoint
/range/{startDate}/{endDate}with optional pagination query parameters - Adds complete validation for date formats and logical date range constraints in the handler layer
- Extends the repository layer with efficient Cosmos DB querying using proper partitioning and count operations
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ICosmosRepository.cs | Adds interface method signature for date range querying with pagination |
| CosmosRepository.cs | Implements date range query logic with separate count and data retrieval operations |
| EndpointRouteBuilderExtensions.cs | Registers new REST endpoint with OpenAPI documentation |
| ActivityHandlers.cs | Implements request handler with date validation and pagination defaults |
| CosmosRepositoryShould.cs | Comprehensive unit tests covering repository functionality, edge cases, and error scenarios |
| ActivityHandlersShould.cs | Unit tests for handler validation logic, pagination behavior, and error cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| try | ||
| { | ||
| QueryDefinition queryDefinition = new QueryDefinition("SELECT VALUE COUNT(1) FROM c WHERE c.date >= @startDate AND c.date <= @endDate") |
There was a problem hiding this comment.
The count query is missing the document type filter that's present in the main query. This could return incorrect counts if the container has other document types. Add AND c.documentType = 'Activity' to match the filtering logic in the main query.
| QueryDefinition queryDefinition = new QueryDefinition("SELECT VALUE COUNT(1) FROM c WHERE c.date >= @startDate AND c.date <= @endDate") | |
| QueryDefinition queryDefinition = new QueryDefinition("SELECT VALUE COUNT(1) FROM c WHERE c.date >= @startDate AND c.date <= @endDate AND c.documentType = 'Activity'") |
|
|
||
| // Get paginated results | ||
| QueryDefinition queryDefinition = new QueryDefinition( | ||
| "SELECT * FROM c WHERE c.documentType = 'Activity' AND c.date >= @startDate AND c.date <= @endDate ORDER BY c.date ASC OFFSET @offset LIMIT @limit") |
There was a problem hiding this comment.
[nitpick] Using SELECT * may retrieve unnecessary data and impact performance. Consider selecting only the required fields if the ActivityDocument contains large properties that aren't always needed.
| "SELECT * FROM c WHERE c.documentType = 'Activity' AND c.date >= @startDate AND c.date <= @endDate ORDER BY c.date ASC OFFSET @offset LIMIT @limit") | |
| "SELECT c.id, c.date, c.documentType FROM c WHERE c.documentType = 'Activity' AND c.date >= @startDate AND c.date <= @endDate ORDER BY c.date ASC OFFSET @offset LIMIT @limit") |
| while (iterator.HasMoreResults) | ||
| { | ||
| var response = await iterator.ReadNextAsync(); | ||
| results.AddRange(response.ToList()); |
There was a problem hiding this comment.
The iterator loop pattern may return more results than the LIMIT clause specifies due to how Cosmos DB pagination works. Consider breaking the loop once the desired page size is reached to avoid processing extra data.
| results.AddRange(response.ToList()); | |
| while (iterator.HasMoreResults && results.Count < paginationRequest.PageSize) | |
| { | |
| var response = await iterator.ReadNextAsync(); | |
| var batch = response.ToList(); | |
| int remaining = paginationRequest.PageSize - results.Count; | |
| if (batch.Count > remaining) | |
| { | |
| results.AddRange(batch.Take(remaining)); | |
| break; | |
| } | |
| else | |
| { | |
| results.AddRange(batch); | |
| } |
Signed-off-by: Will Velida <willvelida@hotmail.co.uk>
This pull request adds comprehensive support for retrieving activity documents within a given date range, including pagination and robust validation, and thoroughly tests this new functionality. The changes cover endpoint registration, handler logic, repository querying, and extensive unit tests to ensure correct behavior and error handling.
New Feature: Date Range Activity Retrieval
GetActivitiesByDateRangeendpoint toEndpointRouteBuilderExtensions, allowing clients to fetch activity documents for a specified date range, with optional pagination parameters.GetActivitiesByDateRangehandler inActivityHandlers, including validation for date formats and date range logic, and default pagination support.Unit Tests: Endpoint and Handler
ActivityHandlersShouldto verify correct responses for valid and invalid date ranges, pagination defaults and overrides, and edge cases such as same-day ranges and invalid dates.Unit Tests: Repository Layer
CosmosRepositoryShouldto verify correct query construction, partition key usage, pagination metadata, logging, error handling, and behavior for empty or single-day results.These changes ensure that the date range retrieval feature is robust, well-validated, and thoroughly tested across all layers of the application.