feat: implement database indexing and premium UI for historical game …#706
Open
AbuJulaybeeb wants to merge 2 commits into
Open
feat: implement database indexing and premium UI for historical game …#706AbuJulaybeeb wants to merge 2 commits into
AbuJulaybeeb wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughExtends game-list filtering end-to-end and adds DB indexes: API and DTO accept opponent/variant/result/date filters, GameService applies head-to-head, variant, result and date constraints, a migration adds result/variant/head-to-head indexes, and RecentMatches gains client-side interactive filtering. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend UI
participant API as API Handler
participant Service as GameService
participant DB as Database
User->>Frontend: set opponent / result / date filters
Frontend->>Frontend: update state & compute filteredMatches
Frontend->>API: GET /games?player_id&opponent_id&variant&result_side&from_date&to_date
API->>Service: list_games(player_id, opponent_id, variant, result_side, from_date, to_date)
Service->>Service: build query (player/head‑to‑head, variant, result, created_at bounds)
Service->>DB: SELECT ... FROM game WHERE ... (uses indexes)
DB-->>Service: return rows
Service-->>API: games[]
API-->>Frontend: games[] (JSON)
Frontend->>Frontend: render latest 10 of filteredGames
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
@AbuJulaybeeb Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/modules/service/src/games.rs (1)
397-465:⚠️ Potential issue | 🟡 MinorAdd coverage for the new filter branches.
These updated calls only prove the old happy path still compiles. Please add cases for
opponent_id,variant,result_side, and the date range so the new SQL branches are exercised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/service/src/games.rs` around lines 397 - 465, Add tests that call GameService::list_games with non-None values for opponent_id, variant, result_side, and the date range so the new SQL branches are exercised: create new tokio::test functions (or extend existing tests test_list_games and test_list_games_with_cursor) that set opponent_id (Uuid), variant (db_entity::game::GameVariant::...), result_side (appropriate enum/value), and from_timestamp/to_timestamp (Utc times or encoded cursor) and supply a MockDatabase with append_query_results followed by into_connection; then invoke GameService::list_games with those parameters and assert the transaction_log contains the expected WHERE clauses (references to "opponent", variant, result filtering, and date range conditions) to ensure the new branches hit in GameService::list_games.backend/modules/api/src/games.rs (1)
183-188:⚠️ Potential issue | 🟡 MinorKeep the OpenAPI params in sync with the handler.
list_gameshandler acceptsopponent_id,variant,result_side,from_date, andto_datevia theListGamesQuerystruct, but the#[utoipa::path(params(...))]block at lines 183–188 does not document them. Generated API docs will be incomplete for these historical-search filters.📚 Proposed fix
params( ("status" = Option<String>, Query, description = "Filter by status (waiting, in_progress, completed, aborted)"), ("player_id" = Option<String>, Query, description = "Filter by player UUID", format = "uuid"), + ("opponent_id" = Option<String>, Query, description = "Filter by opponent UUID", format = "uuid"), + ("variant" = Option<String>, Query, description = "Filter by variant"), + ("result_side" = Option<String>, Query, description = "Filter by result side"), + ("from_date" = Option<String>, Query, description = "Inclusive start date", format = "date-time"), + ("to_date" = Option<String>, Query, description = "Inclusive end date", format = "date-time"), ("page" = Option<i32>, Query, description = "Page number"), ("limit" = Option<i32>, Query, description = "Items per page") ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/modules/api/src/games.rs` around lines 183 - 188, The OpenAPI params in the #[utoipa::path(...)] block are missing filters that the list_games handler accepts; update the params to mirror the ListGamesQuery fields (opponent_id, variant, result_side, from_date, to_date) so generated docs match the handler. Locate the utoipa path for list_games and add param entries for ("opponent_id" = Option<String>, Query, format = "uuid"), ("variant" = Option<String>, Query), ("result_side" = Option<String>, Query), ("from_date" = Option<String>, Query, format = "date-time"), and ("to_date" = Option<String>, Query, format = "date-time") to match ListGamesQuery types and descriptions. Ensure names and types match the ListGamesQuery struct and keep existing params (status, player_id, page, limit) intact. Verify documentation builds and the generated OpenAPI spec includes all these fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/modules/db/migrations/src/m20260428_100000_add_historical_search_indexes.rs`:
- Around line 20-27: The migration creates a one-direction composite index
"idx_games_head_to_head" on ("white_player", "black_player", "created_at" DESC)
but GameService::list_games queries both (white,black) and the reverse order;
add a second, mirrored composite index on ("black_player", "white_player",
"created_at" DESC) (e.g., name it "idx_games_head_to_head_rev") by issuing a
second execute_unprepared call on manager.get_connection() to CREATE INDEX for
"smdb"."game" so both search branches are optimized.
- Around line 48-51: The DROP INDEX in the down() migration is missing the
schema qualifier for idx_games_head_to_head; update the execute_unprepared call
that currently runs DROP INDEX IF EXISTS "idx_games_head_to_head" (inside
down()) to explicitly reference the smdb schema (e.g. DROP INDEX IF EXISTS
"smdb"."idx_games_head_to_head") so it matches the other schema-qualified drops
and avoids search_path dependency; modify the string passed to
manager.get_connection().execute_unprepared(...) accordingly.
In `@backend/modules/service/src/games.rs`:
- Around line 213-221: The variant mapping silently defaults unknown or mistyped
inputs to Standard and misses several GameVariant members; update the mapping
where variant is handled (the block that builds v_enum from variant.as_str() and
then calls query.filter(game::Column::Variant.eq(v_enum))) to include all enum
string forms used by db_entity::game::GameVariant (Standard, Chess960,
ThreeCheck (use "three_check"), Blitz, Rapid, Classical) and ensure the match
arm uses the correct string keys (e.g., "three_check" not "three-check");
alternatively, return an error or ignore the filter for any unrecognized input
instead of defaulting to Standard so typos do not produce incorrect results.
- Around line 223-233: The current mapping of result_side silently coerces
unknown values to ResultSide::Ongoing; instead validate result_side, explicitly
handle "ongoing" by filtering for NULL with query =
query.filter(game::Column::Result.is_null()), map
"white_wins"/"black_wins"/"draw"/"abandoned" to db_entity::game::ResultSide
variants as before, and return an error (or early-bail) for any other string
rather than defaulting to Ongoing; update the code around the result_side
handling (the result_side variable, the r_enum match, and the
query.filter(game::Column::Result.eq(...)) call) to implement these checks and
the NULL filter.
In `@frontend/components/dashboard/RecentMatches.tsx`:
- Around line 58-88: The search input and icon-only filter toggle need
accessible names: add an explicit aria-label (or a visually-hidden <label> tied
to the input id) to the input controlled by searchQuery/setSearchQuery (e.g.,
"Search opponents"), add an aria-label to the clear button (the X button) such
as "Clear search", and add aria-label (e.g., "Toggle filters") plus
aria-expanded={showFilters} to the filter toggle button that calls
setShowFilters; ensure labels are descriptive for screen readers and that
aria-expanded reflects the showFilters state.
- Around line 1-4: This file defines the RecentMatches component and uses React
hooks (useState, useMemo) but lacks the client directive; add the "use client"
directive as the very first line of
frontend/components/dashboard/RecentMatches.tsx so the component runs as a
Client Component and can use hooks (locate the file around the import block
referencing useState/useMemo and symbols like RecentMatches, EloDataPoint, cn,
and lucide-react imports).
---
Outside diff comments:
In `@backend/modules/api/src/games.rs`:
- Around line 183-188: The OpenAPI params in the #[utoipa::path(...)] block are
missing filters that the list_games handler accepts; update the params to mirror
the ListGamesQuery fields (opponent_id, variant, result_side, from_date,
to_date) so generated docs match the handler. Locate the utoipa path for
list_games and add param entries for ("opponent_id" = Option<String>, Query,
format = "uuid"), ("variant" = Option<String>, Query), ("result_side" =
Option<String>, Query), ("from_date" = Option<String>, Query, format =
"date-time"), and ("to_date" = Option<String>, Query, format = "date-time") to
match ListGamesQuery types and descriptions. Ensure names and types match the
ListGamesQuery struct and keep existing params (status, player_id, page, limit)
intact. Verify documentation builds and the generated OpenAPI spec includes all
these fields.
In `@backend/modules/service/src/games.rs`:
- Around line 397-465: Add tests that call GameService::list_games with non-None
values for opponent_id, variant, result_side, and the date range so the new SQL
branches are exercised: create new tokio::test functions (or extend existing
tests test_list_games and test_list_games_with_cursor) that set opponent_id
(Uuid), variant (db_entity::game::GameVariant::...), result_side (appropriate
enum/value), and from_timestamp/to_timestamp (Utc times or encoded cursor) and
supply a MockDatabase with append_query_results followed by into_connection;
then invoke GameService::list_games with those parameters and assert the
transaction_log contains the expected WHERE clauses (references to "opponent",
variant, result filtering, and date range conditions) to ensure the new branches
hit in GameService::list_games.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45eb751a-1234-4bec-be01-3f95be9e7e44
📒 Files selected for processing (6)
backend/modules/api/src/games.rsbackend/modules/db/migrations/src/lib.rsbackend/modules/db/migrations/src/m20260428_100000_add_historical_search_indexes.rsbackend/modules/dto/src/games.rsbackend/modules/service/src/games.rsfrontend/components/dashboard/RecentMatches.tsx
…ce component accessibility
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Implemented advanced historical game search functionality across the stack. This includes database-level optimizations for high-speed queries and a premium Next.js UI for the user dashboard.
Changes
result,variant, and composite head-to-head indexes in thesmdbschema.list_gamesendpoint with advanced filtering parameters (opponent, result, variant, date range).RecentMatchesdashboard component with a high-contrast search and filter system, ensuring a premium feel consistent with XLMate's aesthetic.Verification
MockDatabaseunit tests inGameService.closes Backend: Database indexing for historical game search #554
Summary by CodeRabbit
New Features
Performance