Added filtering games by location (on/off-campus)#68
Added filtering games by location (on/off-campus)#68andrewp0809 wants to merge 2 commits intomasterfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a location-based game filtering feature by adding a MongoDB index on the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant GraphQL as GraphQL Query
participant Service as GameService
participant Repo as GameRepository
participant DB as MongoDB
Client->>GraphQL: Query games_by_location(onCampus=true)
GraphQL->>GraphQL: resolve_games_by_location(onCampus)
GraphQL->>Service: get_games_by_location(onCampus)
Service->>Repo: find_by_location(onCampus)
Repo->>DB: Query: city == "Ithaca" OR city != "Ithaca"
DB-->>Repo: Return matching Game documents
Repo-->>Service: Return List[Game]
Service-->>GraphQL: Return List[Game]
GraphQL-->>Client: Return games filtered by location
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/queries/game_query.py (1)
33-33: Consider adding a description to theonCampusargument.Other fields in this class don't have argument descriptions either, but for clarity, consider documenting what
onCampusmeans (e.g., games in Ithaca vs. away games):games_by_location = List( GameType, onCampus=Boolean(required=True, description="True for games in Ithaca, False for away games") )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/queries/game_query.py` at line 33, Add a description to the onCampus argument on the games_by_location field so callers know its meaning: update the games_by_location declaration (the List[GameType] field named games_by_location) to include a description on the onCampus=Boolean(...) argument (e.g., "True for games in Ithaca, False for away games") so the schema docs clearly convey what onCampus represents.src/repositories/game_repository.py (2)
250-251: Consider using snake_case for parameter naming.The parameter
onCampususes camelCase, which differs from Python naming conventions (snake_case). While this matches the GraphQL API naming, Python methods typically useon_campus. This is a minor consistency issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/game_repository.py` around lines 250 - 251, Rename the parameter onCampus to on_campus in the find_by_location static method and update all internal references inside find_by_location and any callers to use the new snake_case name; if the method is part of a public API that must accept camelCase (e.g., GraphQL resolver), add a thin adapter that maps the incoming onCampus argument to the on_campus parameter before calling GameRepository.find_by_location to preserve external compatibility. Ensure the function signature and all variable references use on_campus and update any unit tests or call sites accordingly.
259-262: Extract hardcoded city value to constant and fix null city handling in off-campus query.Two refactoring recommendations:
The city value
"Ithaca"is hardcoded. Extract it to a module-level constant for maintainability.When
onCampus=False, the$neoperator will match documents wherecityisnullor missing. If games with unknown locations should be excluded from off-campus results (consistent with how null cities are explicitly handled infind_by_exact_match), use$exists: trueand$ne:♻️ Suggested approach
+CAMPUS_CITY = "Ithaca" + `@staticmethod` def find_by_location(onCampus): ... if onCampus: - query = {"city": "Ithaca"} + query = {"city": CAMPUS_CITY} else: - query = {"city": {"$ne": "Ithaca"}} + query = {"city": {"$ne": CAMPUS_CITY, "$exists": true}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repositories/game_repository.py` around lines 259 - 262, Extract the hardcoded "Ithaca" into a module-level constant (e.g., CAMPUS_CITY or CITY_ITHACA) and use that constant when building the query; then change the off-campus branch that currently sets query = {"city": {"$ne": "Ithaca"}} to exclude documents with missing/null cities by using both $exists and $ne (e.g., {"city": {"$exists": True, "$ne": CITY_ITHACA}}), matching the null-handling used in find_by_exact_match; update any references to the literal string in this module to use the new constant.src/services/game_service.py (1)
123-128: Consider adding pagination for large result sets.This method can return all games matching the location filter without limits. While this is consistent with similar methods like
get_games_by_sportandget_games_by_gender, consider adding optionallimitandoffsetparameters (similar toget_all_games) to prevent unbounded result sets in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/game_service.py` around lines 123 - 128, get_games_by_location currently returns all matching rows; change the signature of GameService.get_games_by_location to accept optional pagination parameters (e.g., limit and offset with sensible defaults) and pass them through to GameRepository.find_by_location (update that method signature too) so the DB query can apply LIMIT/OFFSET; update any callers of get_games_by_location to provide or honor the new params and ensure behavior matches get_all_games/get_games_by_sport patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/queries/game_query.py`:
- Line 33: Add a description to the onCampus argument on the games_by_location
field so callers know its meaning: update the games_by_location declaration (the
List[GameType] field named games_by_location) to include a description on the
onCampus=Boolean(...) argument (e.g., "True for games in Ithaca, False for away
games") so the schema docs clearly convey what onCampus represents.
In `@src/repositories/game_repository.py`:
- Around line 250-251: Rename the parameter onCampus to on_campus in the
find_by_location static method and update all internal references inside
find_by_location and any callers to use the new snake_case name; if the method
is part of a public API that must accept camelCase (e.g., GraphQL resolver), add
a thin adapter that maps the incoming onCampus argument to the on_campus
parameter before calling GameRepository.find_by_location to preserve external
compatibility. Ensure the function signature and all variable references use
on_campus and update any unit tests or call sites accordingly.
- Around line 259-262: Extract the hardcoded "Ithaca" into a module-level
constant (e.g., CAMPUS_CITY or CITY_ITHACA) and use that constant when building
the query; then change the off-campus branch that currently sets query =
{"city": {"$ne": "Ithaca"}} to exclude documents with missing/null cities by
using both $exists and $ne (e.g., {"city": {"$exists": True, "$ne":
CITY_ITHACA}}), matching the null-handling used in find_by_exact_match; update
any references to the literal string in this module to use the new constant.
In `@src/services/game_service.py`:
- Around line 123-128: get_games_by_location currently returns all matching
rows; change the signature of GameService.get_games_by_location to accept
optional pagination parameters (e.g., limit and offset with sensible defaults)
and pass them through to GameRepository.find_by_location (update that method
signature too) so the DB query can apply LIMIT/OFFSET; update any callers of
get_games_by_location to provide or honor the new params and ensure behavior
matches get_all_games/get_games_by_sport patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d67e7e3-c0dd-42ba-bddc-db32c6486330
📒 Files selected for processing (4)
src/database.pysrc/queries/game_query.pysrc/repositories/game_repository.pysrc/services/game_service.py
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
5 participants
##Overview
Added support for filtering games by on-campus vs off-campus locations through a new GraphQL query. This allows clients to easily retrieve games based on whether they are played in Ithaca (on-campus) or elsewhere.
##Changes Made
Implemented a new repository method find_by_location(onCampus) that queries MongoDB using:
"city": "Ithaca" for on-campus games
"city": { "$ne": "Ithaca" } for off-campus games
##Test Coverage
Manual Testing, ran GraphQL queries in GraphiQL.
Summary by CodeRabbit
New Features
Chores