refactor(api): address review feedback on route and service layer (#521)

- 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>

Author: 3 people

routes/player_route.py

@@ -19,7 +19,7 @@
 - DELETE /players/squadnumber/{squad_number} : Delete an existing Player.
 """
 
-from typing import List
+from typing import Annotated, List
 from uuid import UUID
 from fastapi import APIRouter, Body, Depends, HTTPException, status, Path, Response
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -34,6 +34,7 @@
 
 CACHE_KEY = "players"
 CACHE_TTL = 600  # 10 minutes
+SQUAD_NUMBER_TITLE = "The Squad Number of the Player"
 
 # POST -------------------------------------------------------------------------
 
@@ -46,8 +47,8 @@
     tags=["Players"],
 )
 async def post_async(
-    player_model: PlayerRequestModel = Body(...),
-    async_session: AsyncSession = Depends(generate_async_session),
+    player_model: Annotated[PlayerRequestModel, Body(...)],
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to create a new player.
@@ -89,7 +90,8 @@ async def post_async(
     tags=["Players"],
 )
 async def get_all_async(
-    response: Response, async_session: AsyncSession = Depends(generate_async_session)
+    response: Response,
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to retrieve all players.
@@ -117,8 +119,8 @@ async def get_all_async(
     tags=["Players"],
 )
 async def get_by_id_async(
-    player_id: UUID = Path(..., title="The UUID of the Player"),
-    async_session: AsyncSession = Depends(generate_async_session),
+    player_id: Annotated[UUID, Path(..., title="The UUID of the Player")],
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to retrieve a Player by its UUID.
@@ -148,8 +150,8 @@ async def get_by_id_async(
     tags=["Players"],
 )
 async def get_by_squad_number_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=SQUAD_NUMBER_TITLE)],
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to retrieve a Player by its Squad Number.
@@ -183,9 +185,9 @@ async def get_by_squad_number_async(
     tags=["Players"],
 )
 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=SQUAD_NUMBER_TITLE)],
+    player_model: Annotated[PlayerRequestModel, Body(...)],
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to entirely update an existing Player.
@@ -226,8 +228,8 @@ async def put_async(
     tags=["Players"],
 )
 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=SQUAD_NUMBER_TITLE)],
+    async_session: Annotated[AsyncSession, Depends(generate_async_session)],
 ):
     """
     Endpoint to delete an existing Player.

services/player_service.py

@@ -140,7 +140,7 @@ async def update_by_squad_number_async(
     player.middle_name = player_model.middle_name
     player.last_name = player_model.last_name
     player.date_of_birth = player_model.date_of_birth
-    player.squad_number = player_model.squad_number
+    player.squad_number = squad_number
     player.position = player_model.position
     player.abbr_position = player_model.abbr_position
     player.team = player_model.team
@@ -172,8 +172,11 @@ async def delete_by_squad_number_async(
         True if the Player was deleted successfully, False otherwise.
     """
     player = await retrieve_by_squad_number_async(async_session, squad_number)
-    await async_session.delete(player)
+    if player is None:  # pragma: no cover
+        logger.error("Player not found for delete: squad_number=%s", squad_number)
+        return False
     try:
+        await async_session.delete(player)
         await async_session.commit()
         return True
     except SQLAlchemyError as error:  # pragma: no cover