[FEATURE] Enforce squad number immutability on PUT

[nanotaboada]

Problem

On PUT, `squad_number` is included in the request body but silently discarded
— the service always overwrites it with the path parameter value
(`player.squad_number = squad_number` in `services/player_service.py`).
Clients receive no feedback when the body value conflicts with the URL, which
is misleading: a request that appears well-formed is quietly modified
server-side with no indication that the body value was ignored.

Proposed Solution (minimal fix)

Add a mismatch guard at the top of `put_async` in `routes/player_route.py`,
before the existence lookup:

```python
if player_model.squad_number != squad_number:
raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST)
```

The path parameter is the authoritative source of identity on PUT. If the body
carries a different value the request is ambiguous — rejecting it with 400 is
the correct and explicit behaviour.

Alternatives Considered

Option A — Split into PlayerCreateModel / PlayerUpdateModel

Splitting the request model adds a third Pydantic model to the codebase.
The model itself does not need to know about the difference between POST and
PUT — that distinction belongs at the route layer, where HTTP concerns are
handled. A route-level check is simpler and keeps the model layer focused on
data shape, not operation semantics.

Option B — PlayerUpdateModel with Optional squad_number

Making `squad_number` optional on an update model is cleaner from a pure REST
perspective (the URL already conveys identity), but it silently accepts a
mismatched value from the client rather than rejecting it — which is the
original problem. This option trades one form of silent behaviour for another.

Option C — Pydantic model_validator with context

Pydantic v2 supports `model_validate(data, context=...)` for per-operation
validation rules. However, FastAPI validates `Body(...)` automatically and
does not expose a way to inject context into that pipeline without bypassing
framework conventions. The added complexity yields no benefit over a simple
route-level check.

Acceptance Criteria

• PUT returns 400 if `squad_number` in the body does not match the path parameter
• `PlayerRequestModel` remains unchanged (single model for POST and PUT)
• New test `test_request_put_player_squadnumber_mismatch_response_status_bad_request` covers the mismatch case
• All pre-commit checks pass (flake8, black, pytest, coverage ≥ 80%)

[coderabbitai]

🔗 Related PRs

#489 - refactor(tests): adopt action-oriented naming convention [merged]
#494 - Replace Postman collection with .rest file [merged]
#524 - refactor(api): use squad number for PUT and DELETE endpoints [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Create two new Pydantic models (PlayerCreateModel, PlayerUpdateModel) in the existing models module, following the established MainModel inheritance pattern
• Add explicit validation in the PUT route handler to return 400 when body squad_number differs from the path parameter, matching Go/NET reference behavior
• Update service layer type hints for consistency without changing business logic
• Add a dedicated test for the mismatch scenario while preserving existing test coverage

Design Choices

Design Choice 1: Should `PlayerUpdateModel` include or omit the `squad_number` field?

Options Considered:

1. Omit squad_number entirely — Cleaner model, but changes the API contract; existing clients sending squad_number would receive validation errors.
2. Keep squad_number as required and validate match — Maintains backwards compatibility, provides explicit mismatch feedback, aligns with Go/NET reference implementations that return 400 on mismatch.

Chosen Option: 2

Rationale: Keep squad_number as required in PlayerUpdateModel and validate it matches the path parameter, returning 400 on mismatch. This preserves the existing API contract while adding the validation behavior described in the ticket and matching reference implementations.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Create New Pydantic Models

Define the two new purpose-specific models to separate POST and PUT request validation, following existing model patterns.

Task 1: Create PlayerCreateModel

Add the new model for POST requests that includes all fields from the current PlayerRequestModel.

• Add PlayerCreateModel class in models/player_model.py inheriting from MainModel
• Include all existing fields: first_name, middle_name, last_name, date_of_birth, squad_number, position, abbr_position, team, league, starting11
• Maintain identical field types and defaults as current PlayerRequestModel

Task 2: Create PlayerUpdateModel

Add the new model for PUT requests that retains squad_number for validation purposes.

• Add PlayerUpdateModel class in models/player_model.py inheriting from MainModel
• Include all same fields as PlayerCreateModel (including squad_number as required)
• The squad_number field will be used for validation against the path parameter in the route layer

Task 3: Deprecate PlayerRequestModel

Remove or mark the original model as deprecated to prevent future misuse.

• Remove PlayerRequestModel from models/player_model.py if not used elsewhere
• Alternatively, retain with a deprecation comment if needed for backwards compatibility
• Update module exports if applicable

🤖 Prompt for AI agents

Implement two new purpose-specific Pydantic models in `models/player_model.py`,
replacing the shared `PlayerRequestModel`.

- Add `PlayerCreateModel` class inheriting from `MainModel` with all existing
fields: `first_name`, `middle_name`, `last_name`, `date_of_birth`,
`squad_number`, `position`, `abbr_position`, `team`, `league`, `starting11` —
maintaining identical field types and defaults as the current
`PlayerRequestModel`
- Add `PlayerUpdateModel` class inheriting from `MainModel` with the same full
set of fields, including `squad_number` as a required field (to be validated
against the path parameter in the route layer)
- Remove `PlayerRequestModel` from `models/player_model.py` if it is not used
anywhere else in the codebase; otherwise retain it with a deprecation comment
- Update module exports if applicable

Phase 2: Update Route and Service Layers

Integrate the new models into the API handlers and add the mismatch validation logic.

Task 1: Update POST Route Handler

Switch the POST endpoint to use PlayerCreateModel.

• In routes/player_route.py, update the post_async function signature to accept PlayerCreateModel instead of PlayerRequestModel
• Update the Body(...) type annotation accordingly
• No logic changes needed—behavior remains the same

Task 2: Update PUT Route Handler with Mismatch Validation

Switch the PUT endpoint to use PlayerUpdateModel and add explicit squad number validation.

• In routes/player_route.py, update the put_async function signature to accept PlayerUpdateModel
• Add validation check at the start of the handler: if player_model.squad_number != squad_number (path param), raise HTTPException(status_code=400) with a clear error message
• Position this check before the existence lookup for fail-fast behavior
• Maintain existing 404 and 204 response logic

Task 3: Update Service Layer Type Hints

Align service function signatures with the new model types.

• In services/player_service.py, update create_async parameter type hint from PlayerRequestModel to PlayerCreateModel
• Update update_by_squad_number_async parameter type hint to PlayerUpdateModel
• Add necessary imports for the new model classes
• No logic changes required—the model_dump() and field access patterns remain valid

🤖 Prompt for AI agents

Update `routes/player_route.py` and `services/player_service.py` to use the new
models and introduce squad number mismatch validation on PUT.

**`routes/player_route.py`:**
- Update the `post_async` function signature to accept `PlayerCreateModel`
(replacing `PlayerRequestModel`) in the `Body(...)` type annotation
- Update the `put_async` function signature to accept `PlayerUpdateModel`
- At the start of `put_async`, before any existence lookup, add a fail-fast
validation check: if `player_model.squad_number != squad_number` (path
parameter), raise `HTTPException(status_code=400)` with a clear error message
- Preserve existing 404 and 204 response logic

**`services/player_service.py`:**
- Update the `create_async` parameter type hint from `PlayerRequestModel` to
`PlayerCreateModel`
- Update the `update_by_squad_number_async` parameter type hint to
`PlayerUpdateModel`
- Add necessary imports for both new model classes
- No business logic changes are required — `model_dump()` and field access
patterns remain valid

Phase 3: Update Tests

Ensure existing tests pass with the model changes and add coverage for the new mismatch behavior.

Task 1: Update Existing POST and PUT Tests

Verify existing tests work with the new models and adjust if needed.

• Review tests in tests/test_main.py for POST and PUT endpoints
• Existing tests should continue to pass since the request body structure is unchanged
• Confirm stubs in tests/player_stub.py produce valid payloads for both model types

Task 2: Add Mismatch Validation Test

Add a new test case to verify the 400 response when squad numbers don't match on PUT.

• Add test test_request_put_player_squadnumber_mismatch_response_status_bad_request (or similar name) in tests/test_main.py
• Test should send a PUT request where URL path contains one squad number (e.g., 23) but body contains a different value (e.g., 99)
• Assert response status code is 400
• Consider adding a stub or modifying an existing stub's squad_number to create the mismatch scenario

Task 3: Verify Pre-commit Checks Pass

Ensure all code quality gates are satisfied.

• Run flake8 for linting compliance
• Run black for formatting
• Run pytest to confirm all tests pass
• Verify coverage remains ≥ 80%

🤖 Prompt for AI agents

Update `tests/test_main.py` and `tests/player_stub.py` to reflect the model
changes and add a new test for the squad number mismatch scenario on PUT.

- Review existing POST and PUT tests in `tests/test_main.py` — they should
continue to pass since the request body structure is unchanged; make any minor
adjustments if needed
- Confirm stubs in `tests/player_stub.py` produce valid payloads compatible with
both `PlayerCreateModel` and `PlayerUpdateModel`
- Add a new test named
`test_request_put_player_squadnumber_mismatch_response_status_bad_request` (or
similar) in `tests/test_main.py` that:
- Sends a PUT request with a squad number in the URL path (e.g., 23) and a
**different** squad number in the request body (e.g., 99)
  - Asserts the response status code is 400
- Add or modify a stub in `tests/player_stub.py` as needed to produce the
mismatched payload
- After implementing, verify: flake8 linting passes, black formatting passes,
all pytest tests pass, and coverage remains ≥ 80%

Research

This is a FastAPI-based Python web API with a layered architecture: Pydantic models in models/player_model.py, SQLAlchemy ORM in schemas/player_schema.py, route handlers in routes/player_route.py, and async service functions in services/player_service.py. All Pydantic models inherit from MainModel which applies camelCase aliasing. The current PlayerRequestModel is shared by both POST and PUT endpoints, with the PUT handler silently ignoring the body's squad_number in favor of the URL path parameter. Tests live in tests/test_main.py using stubs from tests/player_stub.py.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement two new purpose-specific Pydantic models in `models/player_model.py`,
replacing the shared `PlayerRequestModel`.

- Add `PlayerCreateModel` class inheriting from `MainModel` with all existing
fields: `first_name`, `middle_name`, `last_name`, `date_of_birth`,
`squad_number`, `position`, `abbr_position`, `team`, `league`, `starting11` —
maintaining identical field types and defaults as the current
`PlayerRequestModel`
- Add `PlayerUpdateModel` class inheriting from `MainModel` with the same full
set of fields, including `squad_number` as a required field (to be validated
against the path parameter in the route layer)
- Remove `PlayerRequestModel` from `models/player_model.py` if it is not used
anywhere else in the codebase; otherwise retain it with a deprecation comment
- Update module exports if applicable
===============================================================================

Task: 2

Update `routes/player_route.py` and `services/player_service.py` to use the new
models and introduce squad number mismatch validation on PUT.

**`routes/player_route.py`:**
- Update the `post_async` function signature to accept `PlayerCreateModel`
(replacing `PlayerRequestModel`) in the `Body(...)` type annotation
- Update the `put_async` function signature to accept `PlayerUpdateModel`
- At the start of `put_async`, before any existence lookup, add a fail-fast
validation check: if `player_model.squad_number != squad_number` (path
parameter), raise `HTTPException(status_code=400)` with a clear error message
- Preserve existing 404 and 204 response logic

**`services/player_service.py`:**
- Update the `create_async` parameter type hint from `PlayerRequestModel` to
`PlayerCreateModel`
- Update the `update_by_squad_number_async` parameter type hint to
`PlayerUpdateModel`
- Add necessary imports for both new model classes
- No business logic changes are required — `model_dump()` and field access
patterns remain valid
===============================================================================

Task: 3

Update `tests/test_main.py` and `tests/player_stub.py` to reflect the model
changes and add a new test for the squad number mismatch scenario on PUT.

- Review existing POST and PUT tests in `tests/test_main.py` — they should
continue to pass since the request body structure is unchanged; make any minor
adjustments if needed
- Confirm stubs in `tests/player_stub.py` produce valid payloads compatible with
both `PlayerCreateModel` and `PlayerUpdateModel`
- Add a new test named
`test_request_put_player_squadnumber_mismatch_response_status_bad_request` (or
similar) in `tests/test_main.py` that:
- Sends a PUT request with a squad number in the URL path (e.g., 23) and a
**different** squad number in the request body (e.g., 99)
  - Asserts the response status code is 400
- Add or modify a stub in `tests/player_stub.py` as needed to produce the
mismatched payload
- After implementing, verify: flake8 linting passes, black formatting passes,
all pytest tests pass, and coverage remains ≥ 80%

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!