Skip to content

Conversation

@ahmadbky
Copy link
Member

@ahmadbky ahmadbky commented Dec 6, 2025

This PR fixes a bug of the /overview endpoint, that was returning an empty leaderboard for maps in their event version (e.g. in Benchmark 2).

Closes #103.

@ahmadbky ahmadbky requested a review from Copilot December 6, 2025 19:40
@ahmadbky ahmadbky added the bug Something isn't working label Dec 6, 2025
@ahmadbky ahmadbky linked an issue Dec 6, 2025 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug in the /overview endpoint where the leaderboard was returning empty results for maps in their event version (e.g., Benchmark 2). The fix refactors database queries across three files to use SeaORM's query builder methods instead of raw SQL query construction, and updates the query logic to properly use MIN(time) aggregation with GROUP BY on player IDs.

Key changes:

  • Replaced manual SQL Query::select() construction with SeaORM's fluent query builder API
  • Added proper MIN(time) aggregation when grouping by RecordPlayerId to get best times per player
  • Removed dependencies on deprecated global_event_records and global_records entities in favor of event_edition_records

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
crates/records_lib/src/ranks.rs Updated leaderboard query to use MIN(time) for ordering instead of direct time column ordering
crates/records_lib/src/leaderboard.rs Refactored from manual SQL query construction to SeaORM query builder, applying event filters and aggregations
crates/game_api/src/http/overview.rs Refactored rank retrieval query from manual SQL to SeaORM query builder with proper event filtering

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* Fixes the `leaderboard_into` function from the `records_lib` crate,
  that was using the wrong source of records to retrieve those of a map
  that is in its event version, which is thus hidden from the global
  records (those shown on the website).
* Instead of using the global records sources, use the `records` table
  and join with the `event_edition_records` table if we're in an event
  context.

Closes: #103.
@ahmadbky ahmadbky force-pushed the 103-fix-overview-endpoint branch from bbf3b78 to 185a595 Compare December 6, 2025 19:43
ahmadbky and others added 2 commits December 6, 2025 20:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ahmadbky ahmadbky merged commit de0b63a into master Dec 6, 2025
3 checks passed
@ahmadbky ahmadbky deleted the 103-fix-overview-endpoint branch December 6, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/overview endpoint on Benchmark 2 maps returns an empty leaderboard

2 participants