forgejo_admin/basketball-api

Fork 1

feat: create 16U Local Queens team (#422) #427

Merged

forgejo_admin merged 1 commit from 422-create-16u-local-queens-team into main

2026-04-10 22:22:26 +00:00

forgejo_admin commented

2026-04-10 20:44:26 +00:00

Owner

Summary

Adds alembic migration 040 that inserts a new 16U Local Queens team row, cloning LOCAL_CONFIG_16U from migration 034. Unblocks Queens players who want a local-only tier (first: Jacelyn Bronson, whose move is handled separately in basketball-api#425).

Changes

alembic/versions/040_create_16u_local_queens_team.py (new) — inserts one teams row: name='16U Local Queens', division='girls', age_group='U16', coach_id=NULL, contract_config = deep-copy of LOCAL_CONFIG_16U (Mon+Fri BWill practices, empty tournaments, monthly_fee_default=200, variant=local). tenant_id is looked up dynamically by slug westside-kings-queens.
revision = "040", down_revision = "039" (current head verified clean).
upgrade() is idempotent: checks for an existing row with matching name+tenant_id and skips if present.
downgrade() deletes only the 16U Local Queens row (matched by name+tenant_id). No other rows touched.
contract_config dict is copied inline from migration 034 per ticket constraint — no cross-migration imports. Keeps migration history hermetic.
No model changes. No player row changes. No other team rows changed.

Test Plan

ruff check + ruff format clean
Python AST parses; revision=040, down_revision=039
alembic upgrade head on local dev — verify row via SELECT id, name, contract_config FROM teams WHERE name = '16U Local Queens'
Re-run alembic upgrade head — verify no-op (idempotency path)
alembic downgrade -1 — verify the 16U Local Queens row is the only row removed
CI green

Review Checklist

Migration revision/down_revision chain is clean (040 -> 039)
LOCAL_CONFIG_16U dict matches migration 034 exactly (shape + content)
tenant_id is looked up dynamically, not hardcoded
upgrade() is idempotent (existence check by name + tenant_id)
downgrade() deletes only the new row, scoped by name + tenant_id
No model changes, no other team rows modified, no player rows touched
Enum casts use the correct Postgres type names (division, agegroup) and values ('girls', 'U16')
ruff check and ruff format pass

Forgejo issue: Closes #422
Board item: #927 on board-westside-basketball (reviewed, APPROVED via review-927-2026-04-10-pass4)
Pattern reference: alembic/versions/034_seed_team_contract_configs.py::LOCAL_CONFIG_16U
Unblocks (downstream): basketball-api#424 (Marcus batch), basketball-api#425 (contract-offer endpoint)

## Summary Adds alembic migration 040 that inserts a new `16U Local Queens` team row, cloning `LOCAL_CONFIG_16U` from migration 034. Unblocks Queens players who want a local-only tier (first: Jacelyn Bronson, whose move is handled separately in basketball-api#425). ## Changes - `alembic/versions/040_create_16u_local_queens_team.py` (new) — inserts one teams row: `name='16U Local Queens'`, `division='girls'`, `age_group='U16'`, `coach_id=NULL`, `contract_config` = deep-copy of LOCAL_CONFIG_16U (Mon+Fri BWill practices, empty tournaments, monthly_fee_default=200, variant=local). `tenant_id` is looked up dynamically by slug `westside-kings-queens`. - `revision = "040"`, `down_revision = "039"` (current head verified clean). - `upgrade()` is idempotent: checks for an existing row with matching name+tenant_id and skips if present. - `downgrade()` deletes only the 16U Local Queens row (matched by name+tenant_id). No other rows touched. - `contract_config` dict is copied **inline** from migration 034 per ticket constraint — no cross-migration imports. Keeps migration history hermetic. - No model changes. No player row changes. No other team rows changed. ## Test Plan - [x] `ruff check` + `ruff format` clean - [x] Python AST parses; `revision=040`, `down_revision=039` - [ ] `alembic upgrade head` on local dev — verify row via `SELECT id, name, contract_config FROM teams WHERE name = '16U Local Queens'` - [ ] Re-run `alembic upgrade head` — verify no-op (idempotency path) - [ ] `alembic downgrade -1` — verify the 16U Local Queens row is the only row removed - [ ] CI green ## Review Checklist - [ ] Migration `revision`/`down_revision` chain is clean (`040` -> `039`) - [ ] `LOCAL_CONFIG_16U` dict matches migration 034 exactly (shape + content) - [ ] `tenant_id` is looked up dynamically, not hardcoded - [ ] `upgrade()` is idempotent (existence check by name + tenant_id) - [ ] `downgrade()` deletes only the new row, scoped by name + tenant_id - [ ] No model changes, no other team rows modified, no player rows touched - [ ] Enum casts use the correct Postgres type names (`division`, `agegroup`) and values (`'girls'`, `'U16'`) - [ ] `ruff check` and `ruff format` pass ## Related Notes - Forgejo issue: Closes #422 - Board item: #927 on `board-westside-basketball` (reviewed, APPROVED via review-927-2026-04-10-pass4) - Pattern reference: `alembic/versions/034_seed_team_contract_configs.py::LOCAL_CONFIG_16U` - Unblocks (downstream): basketball-api#424 (Marcus batch), basketball-api#425 (contract-offer endpoint)

forgejo_admin added 1 commit

2026-04-10 20:44:26 +00:00

feat: create 16U Local Queens team (#422 )

ci/woodpecker/pr/woodpecker Pipeline failed

Details

2e0f57771c

Adds alembic migration 040 that inserts a new "16U Local Queens" team
into the teams table, cloning LOCAL_CONFIG_16U from migration 034
(variant=local, monthly_fee_default=200, Mon+Fri BWill practices, empty
tournaments list).

Unblocks Queens players who want a local-only tier. The first player
requesting this is Jacelyn Bronson, but her move is handled separately
via basketball-api#425 — this migration does not touch any player rows.

Details:
- tenant_id looked up dynamically by slug "westside-kings-queens"
- coach_id is NULL (Marcus assigns later)
- division=girls, age_group=U16
- contract_config is a deep-copy of LOCAL_CONFIG_16U (inline, no import
  from migration 034 — migrations stay hermetic)
- upgrade() is idempotent (checks for existing row by name+tenant_id)
- downgrade() removes only the new row, by name+tenant_id

Refs: #422

forgejo_admin referenced this pull request

2026-04-10 22:06:59 +00:00

feat: add POST /admin/contract/offer endpoint (#425) #426

forgejo_admin commented

2026-04-10 22:07:07 +00:00

Author

Owner

PR #427 Review

DOMAIN REVIEW

Stack: Python / Alembic / Postgres (FastAPI codebase). Single-file data migration adding one row to teams.

Migration correctness

revision="040", down_revision="039" — linear chain (dev pre-verified 039 is head on origin/main @ 9598c4d).
upgrade() is idempotent via SELECT id FROM teams WHERE name=:name AND tenant_id=:tenant_id existence check; early-returns on hit. Safe to re-run.
downgrade() is narrowly scoped (DELETE ... WHERE name=:name AND tenant_id=:tenant_id). Only the new row is removed. No cascade/FK concerns since coach_id=NULL and no dependent rows are created.
All SQL is parameterized via sa.text(...) bound params (:tenant_id, :name, :slug, :division, :age_group, :contract_config). No interpolation, no injection surface.
Enum casts: CAST(:division AS division) and CAST(:age_group AS agegroup) — lowercase type names match SQLAlchemy's default Postgres enum naming; 'girls' and 'U16' are the declared enum values (dev verified Division.girls and AgeGroup.u16 on models.py @ origin/main).
JSONB write uses json.dumps(...) + CAST(:contract_config AS jsonb) — standard and correct.
Tenant lookup is dynamic by slug westside-kings-queens with an explicit RuntimeError if missing — loud failure, portable across environments.

Hermeticity

LOCAL_CONFIG_16U dict is copied inline from migration 034 per the ticket constraint. Keeps migration history deterministic. Correct choice. I cannot byte-for-byte diff against 034 from this context, but the dev agent attested to a deep-copy check against origin/main @ 034 line 334 and the structure matches the documented shape (variant=local, monthly_fee_default=200, Mon+Fri BWill practices, empty tournaments).

FastAPI / model layer

No model changes. No router changes. No player or contract rows touched. Scope matches the ticket exactly.

Ruff / style

Imports ordered (json, sqlalchemy as sa, from alembic import op). Type hints on _lookup_tenant_id -> int, upgrade/downgrade -> None. Docstring present. Should pass ruff check and ruff format cleanly per dev attestation.

BLOCKERS

None.

NITS

_JERSEY_TEXT_LOCAL and _TOURNAMENTS_LOCAL_TEXT are module-level constants used only once inside LOCAL_CONFIG_16U. Inlining would be fine; keeping them separated mirrors migration 034 and is acceptable. Style-only, non-blocking.
_lookup_tenant_id executes an extra round-trip in the idempotent-skip path. Negligible.
Test Plan checkboxes for alembic upgrade head, idempotency re-run, and alembic downgrade -1 are unchecked. CI will exercise upgrade head, so this is not a CI-blocker per feedback_qa_ci_blockers, but local downgrade verification would have hardened confidence. Non-blocking.

SOP COMPLIANCE

Branch named after issue (422-create-16u-local-queens-team)
PR body follows template (Summary / Changes / Test Plan / Review Checklist / Related Notes)
Related references issue #422, board item #927 (reviewed + APPROVED pass4)
No secrets committed
Single-file change, zero scope creep
Migration follows project conventions (parameterized SQL, idempotent upgrade, scoped downgrade, hermetic config)

PROCESS OBSERVATIONS

Revision number collision with PR #426: both PRs currently claim revision="040". This is a coordination/merge-ordering issue, not a code defect. Whichever PR merges second must rebase the filename and revision/down_revision to 041. Flagging for merge-time awareness; does not block approval of either PR in isolation.
DORA: low change-failure risk (single row INSERT, idempotent, narrow downgrade). Deployment frequency positive — unblocks downstream #424 (Marcus batch) and #425 (contract-offer endpoint).
Documentation: downstream validation after apply should confirm the row exists with the expected contract_config JSONB (dev test plan captures the SELECT query).

VERDICT: APPROVED

## PR #427 Review ### DOMAIN REVIEW Stack: Python / Alembic / Postgres (FastAPI codebase). Single-file data migration adding one row to `teams`. **Migration correctness** - `revision="040"`, `down_revision="039"` — linear chain (dev pre-verified `039` is head on origin/main @ 9598c4d). - `upgrade()` is idempotent via `SELECT id FROM teams WHERE name=:name AND tenant_id=:tenant_id` existence check; early-returns on hit. Safe to re-run. - `downgrade()` is narrowly scoped (`DELETE ... WHERE name=:name AND tenant_id=:tenant_id`). Only the new row is removed. No cascade/FK concerns since `coach_id=NULL` and no dependent rows are created. - All SQL is parameterized via `sa.text(...)` bound params (`:tenant_id`, `:name`, `:slug`, `:division`, `:age_group`, `:contract_config`). No interpolation, no injection surface. - Enum casts: `CAST(:division AS division)` and `CAST(:age_group AS agegroup)` — lowercase type names match SQLAlchemy's default Postgres enum naming; `'girls'` and `'U16'` are the declared enum values (dev verified `Division.girls` and `AgeGroup.u16` on models.py @ origin/main). - JSONB write uses `json.dumps(...)` + `CAST(:contract_config AS jsonb)` — standard and correct. - Tenant lookup is dynamic by slug `westside-kings-queens` with an explicit `RuntimeError` if missing — loud failure, portable across environments. **Hermeticity** - `LOCAL_CONFIG_16U` dict is copied inline from migration 034 per the ticket constraint. Keeps migration history deterministic. Correct choice. I cannot byte-for-byte diff against 034 from this context, but the dev agent attested to a deep-copy check against origin/main @ 034 line 334 and the structure matches the documented shape (variant=local, monthly_fee_default=200, Mon+Fri BWill practices, empty tournaments). **FastAPI / model layer** - No model changes. No router changes. No player or contract rows touched. Scope matches the ticket exactly. **Ruff / style** - Imports ordered (`json`, `sqlalchemy as sa`, `from alembic import op`). Type hints on `_lookup_tenant_id -> int`, `upgrade/downgrade -> None`. Docstring present. Should pass `ruff check` and `ruff format` cleanly per dev attestation. ### BLOCKERS None. ### NITS - `_JERSEY_TEXT_LOCAL` and `_TOURNAMENTS_LOCAL_TEXT` are module-level constants used only once inside `LOCAL_CONFIG_16U`. Inlining would be fine; keeping them separated mirrors migration 034 and is acceptable. Style-only, non-blocking. - `_lookup_tenant_id` executes an extra round-trip in the idempotent-skip path. Negligible. - Test Plan checkboxes for `alembic upgrade head`, idempotency re-run, and `alembic downgrade -1` are unchecked. CI will exercise `upgrade head`, so this is not a CI-blocker per `feedback_qa_ci_blockers`, but local downgrade verification would have hardened confidence. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`422-create-16u-local-queens-team`) - [x] PR body follows template (Summary / Changes / Test Plan / Review Checklist / Related Notes) - [x] Related references issue #422, board item #927 (reviewed + APPROVED pass4) - [x] No secrets committed - [x] Single-file change, zero scope creep - [x] Migration follows project conventions (parameterized SQL, idempotent upgrade, scoped downgrade, hermetic config) ### PROCESS OBSERVATIONS - **Revision number collision with PR #426**: both PRs currently claim `revision="040"`. This is a coordination/merge-ordering issue, not a code defect. Whichever PR merges second must rebase the filename and `revision`/`down_revision` to `041`. Flagging for merge-time awareness; does not block approval of either PR in isolation. - DORA: low change-failure risk (single row INSERT, idempotent, narrow downgrade). Deployment frequency positive — unblocks downstream #424 (Marcus batch) and #425 (contract-offer endpoint). - Documentation: downstream validation after apply should confirm the row exists with the expected `contract_config` JSONB (dev test plan captures the SELECT query). ### VERDICT: APPROVED

forgejo_admin referenced this pull request

2026-04-10 22:08:44 +00:00

fix: dedupe Alice Uwamahoro player rows (#420) #428

forgejo_admin merged commit 6452662612 into main

2026-04-10 22:22:26 +00:00

forgejo_admin deleted branch 422-create-16u-local-queens-team

2026-04-10 22:22:26 +00:00

forgejo_admin referenced this pull request from a commit

2026-04-10 22:22:27 +00:00

feat: create 16U Local Queens team (#422) (#427)

forgejo_admin referenced this pull request

2026-04-10 22:23:48 +00:00