modify leaderboard #145
modify leaderboard #145
Conversation
jhheider
left a comment
jhheider
left a comment
There was a problem hiding this comment.
looks good, is limit needed?
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the leaderboard endpoint to accept a list of specific project IDs instead of returning all projects with a simple limit. The main changes transform the endpoint from a GET request with a path parameter to a POST request with a JSON body containing project IDs and limit parameters.
- Changed leaderboard endpoint from GET to POST with structured request body
- Added filtering by specific project IDs rather than returning all projects
- Enhanced validation with better error messages and consistent limits
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| api/src/handlers.rs | Modified leaderboard handler to accept POST requests with project ID filtering and improved validation |
| api/README.md | Updated documentation to reflect new leaderboard endpoint structure and added development task commands |
| WHERE ut_source.name = 'source' | ||
| WHERE | ||
| c.id = ANY($1::uuid[]) AND ut_source.name = 'source' | ||
| AND ut_source.name = 'source' |
There was a problem hiding this comment.
This condition is duplicated - 'ut_source.name = 'source'' appears on both line 391 and 392, which is redundant and should be removed.
| AND ut_source.name = 'source' |
| if req.project_ids.len() > RESPONSE_LIMIT as usize { | ||
| return HttpResponse::BadRequest().json(json!({ | ||
| "error": format!("Too many project IDs (maximum {} allowed)", RESPONSE_LIMIT) |
There was a problem hiding this comment.
[nitpick] Consider using a separate constant for the project IDs limit instead of reusing RESPONSE_LIMIT, as these may have different business requirements in the future.
|
actually limit is needed - we run it for all registered projects, and then limit the result set to how many we want to display on the leaderboard |
No description provided.