Merge pull request #548 from nanotaboada/feat/rest-polish-530

feat(api): REST polish — cache fix, Location header, 409 detail (#530)

Author: nanotaboada

.claude/commands/precommit.md

@@ -1,8 +1,12 @@
+# Pre-commit checklist
+
 Run the pre-commit checklist for this project:
 
-1. Remind me to update `CHANGELOG.md` `[Unreleased]` section (Added / Changed / Fixed / Removed) — I must do this manually.
+1. Update `CHANGELOG.md` `[Unreleased]` section — add an entry under the
+   appropriate subsection (Added / Changed / Fixed / Removed) describing the
+   changes made, referencing the issue number.
 2. Run `uv run flake8 .` — must pass.
 3. Run `uv run black --check .` — must pass (run `uv run black .` to auto-fix).
 4. Run `uv run pytest --cov=./ --cov-report=term` — all tests must pass, coverage must be ≥80%.
 
-Run steps 2–4, report the results clearly, then propose a branch name and commit message for my approval using the format `type(scope): description (#issue)` (max 80 chars; types: `feat` `fix` `chore` `docs` `test` `refactor` `ci` `perf`). Do not create the branch or commit until I explicitly confirm.
+Run steps 1–4, report the results clearly, then propose a branch name and commit message for my approval using the format `type(scope): description (#issue)` (max 80 chars; types: `feat` `fix` `chore` `docs` `test` `refactor` `ci` `perf`). Do not create the branch or commit until I explicitly confirm.

.claude/commands/prerelease.md

@@ -0,0 +1,84 @@
+# Pre-release checklist
+
+Run the pre-release checklist for this project.
+
+**Working style**: propose before acting at every step — do not commit, push,
+open PRs, or create tags until I explicitly confirm.
+
+---
+
+## Phase 1 — Determine next release
+
+1. **Verify working tree**: Confirm we are on `master` with a clean working
+   tree. If behind the remote, propose `git pull` and wait for confirmation
+   before pulling.
+
+2. **Detect current release and propose next**: Read `CHANGELOG.md` for the
+   coach naming convention (A–Z table) and the most recent release heading.
+   Run `git tag --sort=-v:refname` to confirm the latest tag. Then:
+
+   - **Next codename**: next letter in the A–Z sequence after the current one.
+     Use lowercase with no spaces for the tag (e.g. `eriksson`);
+     Title Case for the CHANGELOG heading (e.g. `Eriksson`).
+   - **Version bump** — infer from `[Unreleased]`:
+
+     | Condition | Bump |
+     |---|---|
+     | Any entry marked BREAKING | MAJOR |
+     | Entries under Added | MINOR |
+     | Only Changed / Fixed / Removed | PATCH |
+
+   - If `[Unreleased]` has no entries, stop and warn — there is nothing to release.
+
+   Present a summary before proceeding:
+
+   ```text
+   Current:  v2.1.0-delbosque
+   Proposed: v2.2.0-eriksson  (MINOR — new features in Added)
+   Branch:   release/v2.2.0-eriksson
+   Tag:      v2.2.0-eriksson
+   ```
+
+---
+
+## Phase 2 — Prepare release branch
+
+3. **Create release branch**: `release/vX.Y.Z-{codename}`.
+
+4. **Update CHANGELOG.md**: Move all content under `[Unreleased]` into a new
+   versioned heading `[X.Y.Z - Codename] - {today's date}`. Replace
+   `[Unreleased]` with a fresh empty template:
+
+   ```markdown
+   ## [Unreleased]
+
+   ### Added
+
+   ### Changed
+
+   ### Fixed
+
+   ### Removed
+   ```
+
+5. **Propose commit**: `docs(changelog): release vX.Y.Z Codename`
+
+6. **After confirmation**: commit. Then run steps 2–4 of `/precommit` (linting,
+   formatting, tests — the CHANGELOG step is already handled). Push the branch
+   and open a PR into `master` only once all checks pass.
+
+---
+
+## Phase 3 — Tag and release
+
+7. **Stop and wait** for confirmation that:
+   - All CI checks have passed
+   - CodeRabbit review comments have been addressed
+   - The PR has been merged into `master`
+
+8. **Pull `master`**, then propose the annotated tag:
+   - Tag name: `vX.Y.Z-{codename}`
+   - Message:  `Release X.Y.Z - Codename`
+
+9. **After confirmation**: create and push the tag. The CD pipeline triggers
+   automatically.

CHANGELOG.md

@@ -42,6 +42,24 @@ This project uses famous football coaches as release codenames, following an A-Z
 
 ## [Unreleased]
 
+### Added
+
+### Changed
+
+- `GET /players/` cache check changed from `if not players` to
+  `if players is None` so that an empty collection is cached correctly
+  instead of triggering a DB fetch on every request (#530)
+- `POST /players/` 409 response now includes a human-readable `detail`
+  message: "A Player with this squad number already exists." (#530)
+
+### Fixed
+
+- `POST /players/` 201 response now includes a `Location` header
+  pointing to the created resource at
+  `/players/squadnumber/{squad_number}` per RFC 7231 §7.1.2 (#530)
+
+### Removed
+
 ---
 
 ## [2.1.0 - Del Bosque] - 2026-03-31

routes/player_route.py

@@ -49,7 +49,8 @@
 async def post_async(
     player_model: Annotated[PlayerRequestModel, Body(...)],
     async_session: Annotated[AsyncSession, Depends(generate_async_session)],
-):
+    response: Response,
+) -> PlayerResponseModel:
     """
     Endpoint to create a new player.
 
@@ -68,14 +69,18 @@ async def post_async(
         async_session, player_model.squad_number
     )
     if existing:
-        raise HTTPException(status_code=status.HTTP_409_CONFLICT)
+        raise HTTPException(
+            status_code=status.HTTP_409_CONFLICT,
+            detail="A Player with this squad number already exists.",
+        )
     player = await player_service.create_async(async_session, player_model)
     if player is None:  # pragma: no cover
         raise HTTPException(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
             detail="Failed to create the Player due to a database error.",
         )
     await simple_memory_cache.clear(CACHE_KEY)
+    response.headers["Location"] = f"/players/squadnumber/{player.squad_number}"
     return player
 
 
@@ -92,7 +97,7 @@ async def post_async(
 async def get_all_async(
     response: Response,
     async_session: Annotated[AsyncSession, Depends(generate_async_session)],
-):
+) -> List[PlayerResponseModel]:
     """
     Endpoint to retrieve all players.
 
@@ -104,7 +109,7 @@ async def get_all_async(
     """
     players = await simple_memory_cache.get(CACHE_KEY)
     response.headers["X-Cache"] = "HIT"
-    if not players:
+    if players is None:
         players = await player_service.retrieve_all_async(async_session)
         await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
         response.headers["X-Cache"] = "MISS"

tests/test_main.py

@@ -178,6 +178,18 @@ def test_request_post_player_body_existing_response_status_conflict(client):
     assert response.status_code == 409
 
 
+def test_request_post_player_body_existing_response_body_detail(client):
+    """POST /players/ with existing player returns 409 with detail message"""
+    # Arrange
+    player = existing_player()
+    # Act
+    response = client.post(PATH, json=player.__dict__)
+    # Assert
+    assert (
+        response.json()["detail"] == "A Player with this squad number already exists."
+    )
+
+
 def test_request_post_player_body_nonexistent_response_status_created(client):
     """POST /players/ with nonexistent player returns 201 Created with a valid UUID"""
     # Arrange
@@ -275,3 +287,21 @@ def test_request_delete_player_squadnumber_existing_response_status_no_content(c
     response = client.delete(PATH + "squadnumber/" + str(player.squad_number))
     # Assert
     assert response.status_code == 204
+
+
+def test_request_post_player_body_nonexistent_response_header_location(client):
+    """POST /players/ with nonexistent player returns 201 with Location header"""
+    # Arrange
+    player = nonexistent_player()
+    try:
+        # Act
+        response = client.post(PATH, json=player.__dict__)
+        # Assert
+        assert response.status_code == 201
+        assert "Location" in response.headers
+        assert (
+            response.headers["Location"]
+            == f"/players/squadnumber/{player.squad_number}"
+        )
+    finally:
+        client.delete(PATH + "squadnumber/" + str(player.squad_number))