ldraney/basketball-api

Fork 0

feat: add jersey fields to AdminPlayerItem response #250

Merged

forgejo_admin merged 1 commit from 248-admin-jersey-fields into main

2026-03-29 20:12:17 +00:00

forgejo_admin commented

2026-03-29 20:05:02 +00:00

Contributor

Summary

Expose jersey_option, jersey_size, jersey_number, and jersey_order_status in the GET /admin/players endpoint so the admin CRM view can display jersey selection data inline without a separate API call.

Changes

src/basketball_api/routes/admin.py — Added 4 fields to AdminPlayerItem Pydantic model and populated them from the Player ORM model in the list comprehension. Enum values are serialized as strings; nullable enums return null.
tests/test_admin_spa.py — Added jersey enum imports, set jersey data on the player1 fixture, added test_jersey_fields_populated covering both populated and default/null cases, updated test_response_fields and test_includes_player_details to assert the new fields.

Test Plan

pytest tests/test_admin_spa.py -v — 9/9 pass
Full suite: 690/690 pass
ruff format and ruff check clean

Review Checklist

Jersey enum fields (JerseyOption, JerseySize, JerseyOrderStatus) already exist on the Player model — no migration needed
jersey_number is a plain string field on Player — no enum conversion needed
Nullable enums serialize to null, jersey_order_status defaults to "none" via server_default
Rebased on latest main (includes is_public field from prior PR)

Closes #248

Forgejo issue: #248

## Summary Expose `jersey_option`, `jersey_size`, `jersey_number`, and `jersey_order_status` in the `GET /admin/players` endpoint so the admin CRM view can display jersey selection data inline without a separate API call. ## Changes - `src/basketball_api/routes/admin.py` — Added 4 fields to `AdminPlayerItem` Pydantic model and populated them from the Player ORM model in the list comprehension. Enum values are serialized as strings; nullable enums return `null`. - `tests/test_admin_spa.py` — Added jersey enum imports, set jersey data on the `player1` fixture, added `test_jersey_fields_populated` covering both populated and default/null cases, updated `test_response_fields` and `test_includes_player_details` to assert the new fields. ## Test Plan - `pytest tests/test_admin_spa.py -v` — 9/9 pass - Full suite: 690/690 pass - `ruff format` and `ruff check` clean ## Review Checklist - [x] Jersey enum fields (`JerseyOption`, `JerseySize`, `JerseyOrderStatus`) already exist on the Player model — no migration needed - [x] `jersey_number` is a plain string field on Player — no enum conversion needed - [x] Nullable enums serialize to `null`, `jersey_order_status` defaults to `"none"` via server_default - [x] Rebased on latest main (includes `is_public` field from prior PR) ## Related Notes - Closes #248 ## Related - Forgejo issue: #248

forgejo_admin added 1 commit

2026-03-29 20:05:02 +00:00

feat: add jersey fields to AdminPlayerItem response

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

4013269e49

Expose jersey_option, jersey_size, jersey_number, and jersey_order_status
in the GET /admin/players endpoint so the admin CRM view can display
jersey selection data without a separate API call.

Closes #248

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-29 20:05:50 +00:00

Author

Contributor

QA Review -- PR #250

Scope Check

Issue #248 requests 4 fields added to AdminPlayerItem: jersey_option, jersey_size, jersey_number, jersey_order_status. All 4 are present in the diff. No scope creep, no missing fields.

Model Review

jersey_option: str | None -- correct. JerseyOption is nullable on the Player model, serialized as .value string.
jersey_size: str | None -- correct. JerseySize is nullable on the Player model, serialized as .value string.
jersey_number: str | None -- correct. Plain String(2) column, passed through directly.
jersey_order_status: str -- correct. Non-nullable with server_default="none", serialized as .value string.

Serialization Review

Nullable enums use the player.jersey_option.value if player.jersey_option else None pattern, consistent with existing division field handling in the same function.
jersey_order_status always has a value (server_default), so .value is safe without a null guard.
jersey_number is a plain string, no enum conversion needed.

Test Review

test_jersey_fields_populated: Covers both the populated case (Jordan with all 4 fields set) and the null/default case (Alex with None/"none"). Good coverage.
test_includes_player_details: Extended with 4 new assertions for Jordan's jersey data.
test_response_fields: Expected fields list updated with all 4 new fields.
Fixture populated_db: player1 now has jersey data (reversible, YL, "23", paid), player2 has no jersey data (tests the default path).

SOP Compliance

Branch naming: 248-admin-jersey-fields -- correct format.
Ruff: format and lint clean.
Full suite: 690/690 pass.
No migration needed (fields already exist on Player model).
PR body has all required sections.

Findings

No issues found.

VERDICT: APPROVED

## QA Review -- PR #250 ### Scope Check - Issue #248 requests 4 fields added to `AdminPlayerItem`: `jersey_option`, `jersey_size`, `jersey_number`, `jersey_order_status`. All 4 are present in the diff. No scope creep, no missing fields. ### Model Review - `jersey_option: str | None` -- correct. `JerseyOption` is nullable on the Player model, serialized as `.value` string. - `jersey_size: str | None` -- correct. `JerseySize` is nullable on the Player model, serialized as `.value` string. - `jersey_number: str | None` -- correct. Plain `String(2)` column, passed through directly. - `jersey_order_status: str` -- correct. Non-nullable with `server_default="none"`, serialized as `.value` string. ### Serialization Review - Nullable enums use the `player.jersey_option.value if player.jersey_option else None` pattern, consistent with existing `division` field handling in the same function. - `jersey_order_status` always has a value (server_default), so `.value` is safe without a null guard. - `jersey_number` is a plain string, no enum conversion needed. ### Test Review - `test_jersey_fields_populated`: Covers both the populated case (Jordan with all 4 fields set) and the null/default case (Alex with `None`/`"none"`). Good coverage. - `test_includes_player_details`: Extended with 4 new assertions for Jordan's jersey data. - `test_response_fields`: Expected fields list updated with all 4 new fields. - Fixture `populated_db`: `player1` now has jersey data (`reversible`, `YL`, `"23"`, `paid`), `player2` has no jersey data (tests the default path). ### SOP Compliance - Branch naming: `248-admin-jersey-fields` -- correct format. - Ruff: format and lint clean. - Full suite: 690/690 pass. - No migration needed (fields already exist on Player model). - PR body has all required sections. ### Findings No issues found. **VERDICT: APPROVED**

forgejo_admin commented

2026-03-29 20:07:15 +00:00

Author

Contributor

PR #250 Review

DOMAIN REVIEW

Tech stack: Python / FastAPI / SQLAlchemy / Pydantic

Model alignment verified:

jersey_option on Player is Mapped[JerseyOption | None] (nullable enum) -- Pydantic field str | None is correct. Serialization uses .value if player.jersey_option else None, matching the existing division pattern on line 352.
jersey_size on Player is Mapped[JerseySize | None] (nullable enum) -- same pattern, correct.
jersey_number on Player is Mapped[str | None] (plain string) -- passed directly without .value, correct.
jersey_order_status on Player is Mapped[JerseyOrderStatus] (non-nullable, server_default="none") -- Pydantic field str (non-nullable) is correct. Uses .value directly (no null guard needed), matching the subscription_status / contract_status pattern.

Enum values confirmed: JerseyOption (reversible/jersey_warmup/opt_out), JerseySize (YS/YM/YL/YXL/AS/AM/AL/AXL), JerseyOrderStatus (none/pending/paid/shipped) -- all serialize as strings via .value.

PEP compliance: Type hints use str | None (PEP 604), consistent with the rest of the model. No issues.

Ruff: PR body states ruff format and ruff check clean.

BLOCKERS

None.

NITS

None. The change is minimal, follows existing patterns exactly, and the test coverage is thorough.

SOP COMPLIANCE

Branch named after issue: 248-admin-jersey-fields
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related
Related references issue #248 with "Closes #248"
Tests exist: test_jersey_fields_populated covers both populated and null/default cases; test_includes_player_details and test_response_fields updated for new fields
No secrets committed
No unrelated changes -- exactly 2 files modified (admin.py, test_admin_spa.py), +50/-0 lines
Scope matches ticket: 4 jersey fields added to AdminPlayerItem, no extras

PROCESS OBSERVATIONS

Clean, scoped change. The 50-line additive footprint matches the ticket estimate exactly. Test coverage includes both the happy path (player with jersey data) and the default path (player without jersey data returning null/none), which is the right pattern for nullable fields. No deployment risk -- additive-only response model change, fully backward compatible for any existing consumers.

VERDICT: APPROVED

## PR #250 Review ### DOMAIN REVIEW **Tech stack**: Python / FastAPI / SQLAlchemy / Pydantic **Model alignment verified**: - `jersey_option` on Player is `Mapped[JerseyOption | None]` (nullable enum) -- Pydantic field `str | None` is correct. Serialization uses `.value if player.jersey_option else None`, matching the existing `division` pattern on line 352. - `jersey_size` on Player is `Mapped[JerseySize | None]` (nullable enum) -- same pattern, correct. - `jersey_number` on Player is `Mapped[str | None]` (plain string) -- passed directly without `.value`, correct. - `jersey_order_status` on Player is `Mapped[JerseyOrderStatus]` (non-nullable, `server_default="none"`) -- Pydantic field `str` (non-nullable) is correct. Uses `.value` directly (no null guard needed), matching the `subscription_status` / `contract_status` pattern. **Enum values confirmed**: `JerseyOption` (reversible/jersey_warmup/opt_out), `JerseySize` (YS/YM/YL/YXL/AS/AM/AL/AXL), `JerseyOrderStatus` (none/pending/paid/shipped) -- all serialize as strings via `.value`. **PEP compliance**: Type hints use `str | None` (PEP 604), consistent with the rest of the model. No issues. **Ruff**: PR body states `ruff format` and `ruff check` clean. ### BLOCKERS None. ### NITS None. The change is minimal, follows existing patterns exactly, and the test coverage is thorough. ### SOP COMPLIANCE - [x] Branch named after issue: `248-admin-jersey-fields` - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related - [x] Related references issue #248 with "Closes #248" - [x] Tests exist: `test_jersey_fields_populated` covers both populated and null/default cases; `test_includes_player_details` and `test_response_fields` updated for new fields - [x] No secrets committed - [x] No unrelated changes -- exactly 2 files modified (`admin.py`, `test_admin_spa.py`), +50/-0 lines - [x] Scope matches ticket: 4 jersey fields added to `AdminPlayerItem`, no extras ### PROCESS OBSERVATIONS Clean, scoped change. The 50-line additive footprint matches the ticket estimate exactly. Test coverage includes both the happy path (player with jersey data) and the default path (player without jersey data returning null/none), which is the right pattern for nullable fields. No deployment risk -- additive-only response model change, fully backward compatible for any existing consumers. ### VERDICT: APPROVED