Skip to content

Commit fc081b6

Browse files
authored
Merge pull request #524 from nanotaboada/refactor/use-squad-number-for-mutation-endpoints
refactor(api): use squad number for PUT and DELETE endpoints
2 parents 8056837 + 12eaff0 commit fc081b6

File tree

10 files changed

+129
-93
lines changed

10 files changed

+129
-93
lines changed

.claude/settings.json

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
11
{
2-
"model": "claude-sonnet-4-6"
2+
"model": "claude-sonnet-4-6",
3+
"permissions": {
4+
"allow": [
5+
"Bash(gh issue:*)",
6+
"Bash(gh auth:*)",
7+
"Bash(uv run:*)",
8+
"Bash(source .venv/bin/activate)",
9+
"WebFetch(domain:github.com)",
10+
"WebFetch(domain:api.github.com)"
11+
]
12+
}
313
}

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ tests/ — pytest integration tests
4040
- **Async**: All routes and service functions must be `async def`; use `AsyncSession` (never `Session`); use `aiosqlite` (never `sqlite3`); use SQLAlchemy 2.0 `select()` (never `session.query()`)
4141
- **API contract**: camelCase JSON via Pydantic `alias_generator=to_camel`; Python internals stay snake_case
4242
- **Models**: `PlayerRequestModel` (no `id`, used for POST/PUT) and `PlayerResponseModel` (includes `id: UUID`, used for GET/POST responses); never use the removed `PlayerModel`
43-
- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for all CRUD operations. UUID v4 for API-created records; UUID v5 (deterministic) for migration-seeded records. `squad_number` is the natural key — human-readable, domain-meaningful, preferred lookup for external consumers
43+
- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for GET by id only. UUID v4 for API-created records; UUID v5 (deterministic) for migration-seeded records. `squad_number` is the natural key — human-readable, domain-meaningful, used for all mutation endpoints (PUT, DELETE) and preferred for all external consumers
4444
- **Caching**: cache key `"players"` (hardcoded); clear on POST/PUT/DELETE; `X-Cache` header (HIT/MISS)
4545
- **Errors**: Catch specific exceptions with rollback in services; Pydantic validation returns 422 (not 400)
4646
- **Logging**: `logging` module only; never `print()`

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ dmypy.json
160160
# Cython debug symbols
161161
cython_debug/
162162

163+
# Claude Code
164+
.claude/settings.local.json
165+
163166
# PyCharm
164167
# JetBrains specific template is maintained in a separate JetBrains.gitignore that can
165168
# be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore

.markdownlint.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"MD013": false,
3+
"MD024": {
4+
"siblings_only": true
5+
}
6+
}

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ This project uses famous football coaches as release codenames, following an A-Z
4242

4343
## [Unreleased]
4444

45+
### Changed
46+
47+
- **BREAKING**: `PUT /players/{player_id}` replaced by `PUT /players/squadnumber/{squad_number}` — mutation endpoints now use Squad Number (natural key) instead of UUID (surrogate key), consistent with `GET /players/squadnumber/{squad_number}` (#521)
48+
- **BREAKING**: `DELETE /players/{player_id}` replaced by `DELETE /players/squadnumber/{squad_number}` — same rationale as above (#521)
49+
- `update_async` and `delete_async` (UUID-based) replaced by `update_by_squad_number_async` and `delete_by_squad_number_async` in `services/player_service.py` (#521)
50+
4551
---
4652

4753
## [1.1.0 - Bielsa] - 2026-03-02

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ Interactive API documentation is available via Swagger UI at `http://localhost:9
184184
- `GET /players/{player_id}` — Get player by UUID (surrogate key)
185185
- `GET /players/squadnumber/{squad_number}` — Get player by squad number (natural key)
186186
- `POST /players/` — Create a new player
187-
- `PUT /players/{player_id}` — Update an existing player
188-
- `DELETE /players/{player_id}` — Remove a player
187+
- `PUT /players/squadnumber/{squad_number}` — Update an existing player
188+
- `DELETE /players/squadnumber/{squad_number}` — Remove a player
189189
- `GET /health` — Health check
190190

191191
### HTTP Requests

rest/players.rest

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
### https://marketplace.visualstudio.com/items?itemName=humao.rest-client
66
###
77
### Key design note:
8-
### squad_number = natural key (domain-meaningful, preferred for lookups)
9-
### id (UUID) = surrogate key (internal, opaque, used for CRUD operations)
8+
### squad_number = natural key (domain-meaningful, used for all CRUD operations)
9+
### id (UUID) = surrogate key (internal, opaque, used for GET by id only)
1010

1111
@baseUrl = http://localhost:9000
1212

@@ -68,12 +68,12 @@ GET {{baseUrl}}/players/squadnumber/10
6868
Accept: application/json
6969

7070
# ------------------------------------------------------------------------------
71-
# PUT /players/{player_id} — Update
72-
# Emiliano Martínez (squad 23): UUID v5, seeded by seed_001.
71+
# PUT /players/squadnumber/{squad_number} — Update
72+
# Emiliano Martínez (squad 23): seeded by seed_001.
7373
# ------------------------------------------------------------------------------
7474

75-
### PUT /players/{player_id} — Update an existing Player
76-
PUT {{baseUrl}}/players/b04965e6-a9bb-591f-8f8a-1adcb2c8dc39
75+
### PUT /players/squadnumber/{squad_number} — Update an existing Player
76+
PUT {{baseUrl}}/players/squadnumber/23
7777
Content-Type: application/json
7878

7979
{
@@ -90,15 +90,9 @@ Content-Type: application/json
9090
}
9191

9292
# ------------------------------------------------------------------------------
93-
# DELETE /players/{player_id} — Delete
94-
# Thiago Almada (squad 16): created by POST above. Since the UUID is generated
95-
# at runtime, retrieve it first via squad number, then substitute it below.
93+
# DELETE /players/squadnumber/{squad_number} — Delete
94+
# Thiago Almada (squad 16): created by POST above.
9695
# ------------------------------------------------------------------------------
9796

98-
### Step 1 — Look up Almada's UUID by squad number
99-
GET {{baseUrl}}/players/squadnumber/16
100-
Accept: application/json
101-
102-
### Step 2 — Delete Almada using the UUID returned above
103-
# Replace {player_id} with the id field from the response above.
104-
DELETE {{baseUrl}}/players/{player_id}
97+
### DELETE /players/squadnumber/{squad_number} — Delete an existing Player
98+
DELETE {{baseUrl}}/players/squadnumber/16

routes/player_route.py

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
(surrogate key, internal).
1616
- GET /players/squadnumber/{squad_number} : Retrieve Player by Squad Number
1717
(natural key, domain).
18-
- PUT /players/{player_id} : Update an existing Player.
19-
- DELETE /players/{player_id} : Delete an existing Player.
18+
- PUT /players/squadnumber/{squad_number} : Update an existing Player.
19+
- DELETE /players/squadnumber/{squad_number} : Delete an existing Player.
2020
"""
2121

22-
from typing import List
22+
from typing import Annotated, List
2323
from uuid import UUID
2424
from fastapi import APIRouter, Body, Depends, HTTPException, status, Path, Response
2525
from sqlalchemy.ext.asyncio import AsyncSession
@@ -34,6 +34,7 @@
3434

3535
CACHE_KEY = "players"
3636
CACHE_TTL = 600 # 10 minutes
37+
SQUAD_NUMBER_TITLE = "The Squad Number of the Player"
3738

3839
# POST -------------------------------------------------------------------------
3940

@@ -46,8 +47,8 @@
4647
tags=["Players"],
4748
)
4849
async def post_async(
49-
player_model: PlayerRequestModel = Body(...),
50-
async_session: AsyncSession = Depends(generate_async_session),
50+
player_model: Annotated[PlayerRequestModel, Body(...)],
51+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
5152
):
5253
"""
5354
Endpoint to create a new player.
@@ -89,7 +90,8 @@ async def post_async(
8990
tags=["Players"],
9091
)
9192
async def get_all_async(
92-
response: Response, async_session: AsyncSession = Depends(generate_async_session)
93+
response: Response,
94+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
9395
):
9496
"""
9597
Endpoint to retrieve all players.
@@ -117,8 +119,8 @@ async def get_all_async(
117119
tags=["Players"],
118120
)
119121
async def get_by_id_async(
120-
player_id: UUID = Path(..., title="The UUID of the Player"),
121-
async_session: AsyncSession = Depends(generate_async_session),
122+
player_id: Annotated[UUID, Path(..., title="The UUID of the Player")],
123+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
122124
):
123125
"""
124126
Endpoint to retrieve a Player by its UUID.
@@ -148,8 +150,8 @@ async def get_by_id_async(
148150
tags=["Players"],
149151
)
150152
async def get_by_squad_number_async(
151-
squad_number: int = Path(..., title="The Squad Number of the Player"),
152-
async_session: AsyncSession = Depends(generate_async_session),
153+
squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)],
154+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
153155
):
154156
"""
155157
Endpoint to retrieve a Player by its Squad Number.
@@ -177,33 +179,37 @@ async def get_by_squad_number_async(
177179

178180

179181
@api_router.put(
180-
"/players/{player_id}",
182+
"/players/squadnumber/{squad_number}",
181183
status_code=status.HTTP_204_NO_CONTENT,
182184
summary="Updates an existing Player",
183185
tags=["Players"],
184186
)
185187
async def put_async(
186-
player_id: UUID = Path(..., title="The UUID of the Player"),
187-
player_model: PlayerRequestModel = Body(...),
188-
async_session: AsyncSession = Depends(generate_async_session),
188+
squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)],
189+
player_model: Annotated[PlayerRequestModel, Body(...)],
190+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
189191
):
190192
"""
191193
Endpoint to entirely update an existing Player.
192194
193195
Args:
194-
player_id (UUID): The UUID of the Player to update.
196+
squad_number (int): The Squad Number of the Player to update.
195197
player_model (PlayerRequestModel): The Pydantic model representing the Player
196198
to update.
197199
async_session (AsyncSession): The async version of a SQLAlchemy ORM session.
198200
199201
Raises:
200-
HTTPException: HTTP 404 Not Found error if the Player with the specified UUID
201-
does not exist.
202+
HTTPException: HTTP 404 Not Found error if the Player with the specified Squad
203+
Number does not exist.
202204
"""
203-
player = await player_service.retrieve_by_id_async(async_session, player_id)
205+
player = await player_service.retrieve_by_squad_number_async(
206+
async_session, squad_number
207+
)
204208
if not player:
205209
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
206-
updated = await player_service.update_async(async_session, player_id, player_model)
210+
updated = await player_service.update_by_squad_number_async(
211+
async_session, squad_number, player_model
212+
)
207213
if not updated: # pragma: no cover
208214
raise HTTPException(
209215
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
@@ -216,30 +222,34 @@ async def put_async(
216222

217223

218224
@api_router.delete(
219-
"/players/{player_id}",
225+
"/players/squadnumber/{squad_number}",
220226
status_code=status.HTTP_204_NO_CONTENT,
221227
summary="Deletes an existing Player",
222228
tags=["Players"],
223229
)
224230
async def delete_async(
225-
player_id: UUID = Path(..., title="The UUID of the Player"),
226-
async_session: AsyncSession = Depends(generate_async_session),
231+
squad_number: Annotated[int, Path(..., title=SQUAD_NUMBER_TITLE)],
232+
async_session: Annotated[AsyncSession, Depends(generate_async_session)],
227233
):
228234
"""
229235
Endpoint to delete an existing Player.
230236
231237
Args:
232-
player_id (UUID): The UUID of the Player to delete.
238+
squad_number (int): The Squad Number of the Player to delete.
233239
async_session (AsyncSession): The async version of a SQLAlchemy ORM session.
234240
235241
Raises:
236-
HTTPException: HTTP 404 Not Found error if the Player with the specified UUID
237-
does not exist.
242+
HTTPException: HTTP 404 Not Found error if the Player with the specified Squad
243+
Number does not exist.
238244
"""
239-
player = await player_service.retrieve_by_id_async(async_session, player_id)
245+
player = await player_service.retrieve_by_squad_number_async(
246+
async_session, squad_number
247+
)
240248
if not player:
241249
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
242-
deleted = await player_service.delete_async(async_session, player_id)
250+
deleted = await player_service.delete_by_squad_number_async(
251+
async_session, squad_number
252+
)
243253
if not deleted: # pragma: no cover
244254
raise HTTPException(
245255
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,

services/player_service.py

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22
Async CRUD operations for Player entities using SQLAlchemy ORM.
33
44
Functions:
5-
- create_async : Add a new Player to the database.
6-
- retrieve_all_async : Fetch all Player records.
7-
- retrieve_by_id_async : Fetch a Player by its UUID
8-
(surrogate key, internal).
9-
- retrieve_by_squad_number_async : Fetch a Player by its Squad Number
10-
(natural key, domain).
11-
- update_async : Fully update an existing Player.
12-
- delete_async : Remove a Player from the database.
5+
- create_async : Add a new Player to the database.
6+
- retrieve_all_async : Fetch all Player records.
7+
- retrieve_by_id_async : Fetch a Player by its UUID
8+
(surrogate key, internal).
9+
- retrieve_by_squad_number_async : Fetch a Player by its Squad Number
10+
(natural key, domain).
11+
- update_by_squad_number_async : Fully update a Player by Squad Number.
12+
- delete_by_squad_number_async : Remove a Player by Squad Number.
1313
1414
Handles SQLAlchemy exceptions with transaction rollback and logs errors.
1515
"""
@@ -117,30 +117,30 @@ async def retrieve_by_squad_number_async(
117117
# Update -----------------------------------------------------------------------
118118

119119

120-
async def update_async(
121-
async_session: AsyncSession, player_id: UUID, player_model: PlayerRequestModel
120+
async def update_by_squad_number_async(
121+
async_session: AsyncSession, squad_number: int, player_model: PlayerRequestModel
122122
) -> bool:
123123
"""
124-
Updates (entirely) an existing Player in the database.
124+
Updates (entirely) an existing Player identified by Squad Number.
125125
126126
Args:
127127
async_session (AsyncSession): The async version of a SQLAlchemy ORM session.
128-
player_id (UUID): The UUID of the Player to update.
128+
squad_number (int): The Squad Number of the Player to update.
129129
player_model (PlayerRequestModel): The Pydantic model representing the Player
130130
to update.
131131
132132
Returns:
133133
True if the Player was updated successfully, False otherwise.
134134
"""
135-
player = await async_session.get(Player, player_id)
135+
player = await retrieve_by_squad_number_async(async_session, squad_number)
136136
if player is None: # pragma: no cover
137-
logger.error("Player not found for update: %s", player_id)
137+
logger.error("Player not found for update: squad_number=%s", squad_number)
138138
return False
139139
player.first_name = player_model.first_name
140140
player.middle_name = player_model.middle_name
141141
player.last_name = player_model.last_name
142142
player.date_of_birth = player_model.date_of_birth
143-
player.squad_number = player_model.squad_number
143+
player.squad_number = squad_number
144144
player.position = player_model.position
145145
player.abbr_position = player_model.abbr_position
146146
player.team = player_model.team
@@ -158,20 +158,25 @@ async def update_async(
158158
# Delete -----------------------------------------------------------------------
159159

160160

161-
async def delete_async(async_session: AsyncSession, player_id: UUID) -> bool:
161+
async def delete_by_squad_number_async(
162+
async_session: AsyncSession, squad_number: int
163+
) -> bool:
162164
"""
163-
Deletes an existing Player from the database.
165+
Deletes an existing Player identified by Squad Number from the database.
164166
165167
Args:
166168
async_session (AsyncSession): The async version of a SQLAlchemy ORM session.
167-
player_id (UUID): The UUID of the Player to delete.
169+
squad_number (int): The Squad Number of the Player to delete.
168170
169171
Returns:
170172
True if the Player was deleted successfully, False otherwise.
171173
"""
172-
player = await async_session.get(Player, player_id)
173-
await async_session.delete(player)
174+
player = await retrieve_by_squad_number_async(async_session, squad_number)
175+
if player is None: # pragma: no cover
176+
logger.error("Player not found for delete: squad_number=%s", squad_number)
177+
return False
174178
try:
179+
await async_session.delete(player)
175180
await async_session.commit()
176181
return True
177182
except SQLAlchemyError as error: # pragma: no cover

0 commit comments

Comments
 (0)