From d922118525f7f41835696757b262e5f5d01c121a Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:33:24 -0300 Subject: [PATCH 1/3] =?UTF-8?q?feat(api):=20REST=20polish=20=E2=80=94=20ca?= =?UTF-8?q?che=20fix,=20Location=20header,=20409=20detail=20(#530)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix GET /players/ cache check: `if not players` → `if players is None` so empty collections are cached correctly - Add Location header to POST /players/ 201 response pointing to /players/squadnumber/{squad_number} per RFC 7231 §7.1.2 - Add detail message to POST /players/ 409 response - Add return type annotations to modified route handlers - Add [Unreleased] subsections to CHANGELOG.md Co-authored-by: Claude Sonnet 4.6 --- CHANGELOG.md | 18 ++++++++++++++++++ routes/player_route.py | 13 +++++++++---- tests/test_main.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a03323..3cff2ff 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/routes/player_route.py b/routes/player_route.py index 631e511..dc69344 100644 --- a/routes/player_route.py +++ b/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,7 +69,10 @@ 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( @@ -76,6 +80,7 @@ async def post_async( 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" diff --git a/tests/test_main.py b/tests/test_main.py index dafbc79..fc39406 100644 --- a/tests/test_main.py +++ b/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)) From ce0b6874b37d3ff4553d8081a2e2de5d48cc6cf0 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:33:28 -0300 Subject: [PATCH 2/3] chore(commands): add /prerelease, update /precommit CHANGELOG step Co-authored-by: Claude Sonnet 4.6 --- .claude/commands/precommit.md | 6 ++- .claude/commands/prerelease.md | 80 ++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 .claude/commands/prerelease.md diff --git a/.claude/commands/precommit.md b/.claude/commands/precommit.md index 88d5468..1ddfc96 100644 --- a/.claude/commands/precommit.md +++ b/.claude/commands/precommit.md @@ -1,8 +1,10 @@ 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. diff --git a/.claude/commands/prerelease.md b/.claude/commands/prerelease.md new file mode 100644 index 0000000..a7c38d4 --- /dev/null +++ b/.claude/commands/prerelease.md @@ -0,0 +1,80 @@ +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. Pull latest if behind remote. + +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: + ``` + 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. From 9d1fd90e913b2998c99356bb163c9fa3c8f0887c Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:35:47 -0300 Subject: [PATCH 3/3] chore(commands): fix markdownlint violations in /prerelease - MD001: demote Phase headings from H3 to H2 (no heading level skip) - MD031: add blank line before fenced code block - MD040: add language specifier to unlabelled code fence - Step 1: require explicit confirmation before git pull Co-authored-by: Claude Sonnet 4.6 --- .claude/commands/precommit.md | 2 ++ .claude/commands/prerelease.md | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.claude/commands/precommit.md b/.claude/commands/precommit.md index 1ddfc96..8c656a9 100644 --- a/.claude/commands/precommit.md +++ b/.claude/commands/precommit.md @@ -1,3 +1,5 @@ +# Pre-commit checklist + Run the pre-commit checklist for this project: 1. Update `CHANGELOG.md` `[Unreleased]` section — add an entry under the diff --git a/.claude/commands/prerelease.md b/.claude/commands/prerelease.md index a7c38d4..ff4d5a1 100644 --- a/.claude/commands/prerelease.md +++ b/.claude/commands/prerelease.md @@ -1,3 +1,5 @@ +# Pre-release checklist + Run the pre-release checklist for this project. **Working style**: propose before acting at every step — do not commit, push, @@ -5,10 +7,11 @@ open PRs, or create tags until I explicitly confirm. --- -### Phase 1 — Determine next release +## Phase 1 — Determine next release 1. **Verify working tree**: Confirm we are on `master` with a clean working - tree. Pull latest if behind remote. + 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. @@ -28,7 +31,8 @@ open PRs, or create tags until I explicitly confirm. - 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 @@ -37,7 +41,7 @@ open PRs, or create tags until I explicitly confirm. --- -### Phase 2 — Prepare release branch +## Phase 2 — Prepare release branch 3. **Create release branch**: `release/vX.Y.Z-{codename}`. @@ -65,7 +69,7 @@ open PRs, or create tags until I explicitly confirm. --- -### Phase 3 — Tag and release +## Phase 3 — Tag and release 7. **Stop and wait** for confirmation that: - All CI checks have passed