refactor(api): use squad number for PUT and DELETE endpoints#524
Merged
nanotaboada merged 2 commits intomasterfrom Mar 17, 2026
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughThe pull request refactors PUT and DELETE player endpoints to use Squad Number (natural key) instead of UUID (surrogate key). Service methods are renamed to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
Possibly related issues
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #524 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 110 111 +1
=========================================
+ Hits 110 111 +1
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
routes/player_route.py (1)
185-189: UseAnnotated[...]for FastAPI params/dependencies to resolve the Sonar gate issue.Line [229] is already flagged by Sonar; applying
AnnotatedtoPath/Body/Dependsin these handlers will make typing explicit and consistent.♻️ Suggested refactor
-from typing import List +from typing import Annotated, List @@ async def put_async( - squad_number: int = Path(..., title="The Squad Number of the Player"), - player_model: PlayerRequestModel = Body(...), - async_session: AsyncSession = Depends(generate_async_session), + squad_number: Annotated[ + int, Path(..., title="The Squad Number of the Player") + ], + player_model: Annotated[PlayerRequestModel, Body(...)], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ): @@ async def delete_async( - squad_number: int = Path(..., title="The Squad Number of the Player"), - async_session: AsyncSession = Depends(generate_async_session), + squad_number: Annotated[ + int, Path(..., title="The Squad Number of the Player") + ], + async_session: Annotated[AsyncSession, Depends(generate_async_session)], ):Also applies to: 228-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/player_route.py` around lines 185 - 189, The FastAPI handler put_async currently uses Path, Body and Depends directly which triggers the Sonar typing gate; update the parameter annotations to use typing.Annotated for the FastAPI metadata so types are explicit (e.g., annotate squad_number as Annotated[int, Path(...)] and player_model as Annotated[PlayerRequestModel, Body(...)] and async_session as Annotated[AsyncSession, Depends(generate_async_session)]), keep the same default values and imports, and ensure typing.Annotated is imported where put_async is defined so the function signature uses Annotated for Path/Body/Depends.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/player_service.py`:
- Around line 120-122: The update_by_squad_number_async function currently uses
player_model.squad_number which allows the request body to re-key the resource;
change this to enforce path/body consistency by either (a) validating that
player_model.squad_number equals the path param squad_number and
returning/raising a 400 if they differ, or (b) ignoring the body value and
overriding player_model.squad_number = squad_number before performing the
update; update the logic in update_by_squad_number_async (and the same pattern
in the nearby block handling lines ~135-143) to use the path param as the
canonical key and reference only squad_number (path) for lookups and uniqueness
checks.
- Around line 174-176: The code calls await async_session.delete(player) without
checking if player is None and performs the delete outside the try/except;
update the delete path (the block using retrieve_by_squad_number_async and
async_session.delete) to first check if player is None and return False if so,
then move the await async_session.delete(player) call inside the try/except
(same pattern used in update_by_squad_number_async) so any exceptions are caught
and the function returns False on failure; reference
retrieve_by_squad_number_async, the local variable player, and
async_session.delete when making the change.
---
Nitpick comments:
In `@routes/player_route.py`:
- Around line 185-189: The FastAPI handler put_async currently uses Path, Body
and Depends directly which triggers the Sonar typing gate; update the parameter
annotations to use typing.Annotated for the FastAPI metadata so types are
explicit (e.g., annotate squad_number as Annotated[int, Path(...)] and
player_model as Annotated[PlayerRequestModel, Body(...)] and async_session as
Annotated[AsyncSession, Depends(generate_async_session)]), keep the same default
values and imports, and ensure typing.Annotated is imported where put_async is
defined so the function signature uses Annotated for Path/Body/Depends.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9b0fbf2-70b7-4297-96a8-e2e84669f42f
📒 Files selected for processing (9)
.claude/settings.local.json.github/copilot-instructions.md.markdownlint.jsonCHANGELOG.mdREADME.mdrest/players.restroutes/player_route.pyservices/player_service.pytests/test_main.py
BREAKING CHANGE: PUT /players/{player_id}
→ PUT /players/squadnumber/{squad_number}
BREAKING CHANGE: DELETE /players/{player_id}
→ DELETE /players/squadnumber/{squad_number}
Co-authored-by: Claude <noreply@anthropic.com>
nanotaboada
force-pushed
the
refactor/use-squad-number-for-mutation-endpoints
branch
from
8997f6e to
39cf3bf
Compare
- Use Annotated for FastAPI Path/Body/Depends across all route handlers - Define SQUAD_NUMBER_TITLE constant to avoid literal duplication - Enforce path param as canonical key in update_by_squad_number_async - Add None guard and move delete inside try/except in delete_by_squad_number_async Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
nanotaboada
force-pushed
the
refactor/use-squad-number-for-mutation-endpoints
branch
from
39cf3bf to
12eaff0
Compare
|
nanotaboada
deleted the
refactor/use-squad-number-for-mutation-endpoints
branch
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.
Closes #521
This change is
Summary by CodeRabbit
Release Notes
Breaking Changes
/players/squadnumber/{squad_number}endpoints.Documentation
Configuration