ldraney/basketball-api

Fork 0

Migration 040: jersey_public_orders table #429

New issue

Closed

opened 2026-04-10 21:51:56 +00:00 by forgejo_admin · 1 comment

forgejo_admin commented

2026-04-10 21:51:56 +00:00

Contributor

Type

Feature

Lineage

Standalone — part of System B production rollout. Architecture in arch-jersey-intake. Blocks POST (#430) and GET (#432) endpoints. Revised 2026-04-10 (twice): first to add submitter_keycloak_sub for Keycloak-gated identity, then to fix stale migration number (014 → 031) and stale model path (models/ submodule → flat models.py).

Repo

forgejo_admin/basketball-api

User Story

As Marcus
I want a dedicated jersey_public_orders table tied to Keycloak user identity
So that submissions are stored with verified keycloak_sub values (not just self-declared email), reconciliation against existing parents/players is precise, and System B orders are clearly separated from System A and System C

Context

System B submissions are Keycloak-authenticated (see westside-landing#243). Every row has a verified sub claim from the JWT. email and player_name remain user-editable because (a) they may differ from the JWT-holder (parent submitting for child), (b) the form should be editable rather than locked to JWT identity.

Actual migration head verified 2026-04-10: 030_add_registration_type_to_registrations.py (revision 030, down_revision 029). Next available revision is 031. Do NOT use any lower number — migrations 014 through 030 already exist.

Actual model location verified 2026-04-10: all SQLAlchemy models live in a single flat file src/basketball_api/models.py (31 classes, including Parent and Player). There is no models/ subdirectory. The new JerseyPublicOrder class must be appended to models.py, not split into a submodule.

File Targets

Files to create:

alembic/versions/031_add_jersey_public_orders.py — Alembic migration, revision = "031", down_revision = "030"

Files to modify:

src/basketball_api/models.py — append a new JerseyPublicOrder SQLAlchemy class at the end of the file. Do NOT modify any existing class.

Files the agent should NOT touch:

Any existing migration in alembic/versions/ (013 through 030) — those are System C and other features, hands off
Any existing class in src/basketball_api/models.py — append-only
routes/checkout.py, routes/jersey.py — other systems

Schema (exact)

-- In upgrade() — FIRST ensure pgcrypto is available for gen_random_uuid()
CREATE EXTENSION IF NOT EXISTS pgcrypto;

CREATE TABLE jersey_public_orders (
    id                      UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    submitter_keycloak_sub  VARCHAR(255) NOT NULL,
    player_name             VARCHAR(200) NOT NULL,
    email                   VARCHAR(320) NOT NULL,
    team                    VARCHAR(100) NOT NULL,
    kq                      VARCHAR(10)  NOT NULL CHECK (kq IN ('kings','queens')),
    preferred_number_1      VARCHAR(2),
    preferred_number_2      VARCHAR(2),
    preferred_number_3      VARCHAR(2),
    top_size                VARCHAR(10) NOT NULL,
    short_size              VARCHAR(10) NOT NULL,
    tier                    VARCHAR(40) NOT NULL CHECK (tier IN ('90-reversible','130-reversible-shooter')),
    status                  VARCHAR(20) NOT NULL DEFAULT 'pending' CHECK (status IN ('pending','reviewed','fulfilled','cancelled')),
    linked_parent_id        INTEGER REFERENCES parents(id) ON DELETE SET NULL,
    linked_player_id        INTEGER REFERENCES players(id) ON DELETE SET NULL,
    reviewed_at             TIMESTAMP WITH TIME ZONE,
    reviewed_by             VARCHAR(200),
    submission_ip           INET,
    created_at              TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW()
);

CREATE INDEX idx_jpo_submitter_sub ON jersey_public_orders (submitter_keycloak_sub);
CREATE INDEX idx_jpo_email         ON jersey_public_orders (email);
CREATE INDEX idx_jpo_status        ON jersey_public_orders (status);
CREATE INDEX idx_jpo_created_at    ON jersey_public_orders (created_at DESC);

In downgrade(): DROP TABLE jersey_public_orders. Do NOT drop the pgcrypto extension — other tables may rely on it.

Acceptance Criteria

alembic upgrade head applies cleanly against a fresh DB from scratch (runs 001 through 031)
alembic downgrade -1 reverses 031 cleanly (head becomes 030)
alembic upgrade head re-applies 031 cleanly after the downgrade
pgcrypto extension is created if not present (idempotent)
All four indexes created
CHECK constraints reject invalid kq, tier, status values
submitter_keycloak_sub is NOT NULL
FKs to parents/players use ON DELETE SET NULL
JerseyPublicOrder class is importable from basketball_api.models
No existing class in models.py is modified (verify with git diff main -- src/basketball_api/models.py — only additions below the final pre-existing class)

Test Expectations

Unit test: upgrade head → downgrade -1 → upgrade head round-trip succeeds
Unit test: insert valid row, fetch, all fields round-trip
Unit test: insert with kq='kingdoms' → IntegrityError
Unit test: insert with NULL submitter_keycloak_sub → IntegrityError
Run command: pytest tests/ -k jersey_public_order

Constraints

Match existing Alembic migration style from alembic/versions/030_add_registration_type_to_registrations.py (op.create_table, op.create_index, etc.)
No new Python dependencies
email is NOT UNIQUE (same email can submit multiple orders)
submitter_keycloak_sub is NOT UNIQUE either (same Keycloak user can submit multiple orders)
Do NOT create a separate keycloak_user table — we store the sub claim as a string and Keycloak is the source of truth
Do NOT modify down_revision on any existing migration

Checklist

PR opened against basketball-api main
Migration applies in CI against test DB (runs full chain 001 → 031)
JerseyPublicOrder importable
git diff main -- src/basketball_api/models.py shows only additions
Tests pass

westside-basketball — project
story:WS-S31 — admin public jersey intake link
arch-jersey-intake — architecture doc
feedback_funnel_requires_auth.md — why every row has a verified Keycloak identity
Reference: alembic/versions/030_*.py — style guide, hands off

### Type Feature ### Lineage Standalone — part of System B production rollout. Architecture in `arch-jersey-intake`. Blocks POST (#430) and GET (#432) endpoints. **Revised 2026-04-10 (twice):** first to add `submitter_keycloak_sub` for Keycloak-gated identity, then to fix stale migration number (014 → 031) and stale model path (`models/` submodule → flat `models.py`). ### Repo `forgejo_admin/basketball-api` ### User Story As Marcus I want a dedicated `jersey_public_orders` table tied to Keycloak user identity So that submissions are stored with verified `keycloak_sub` values (not just self-declared email), reconciliation against existing parents/players is precise, and System B orders are clearly separated from System A and System C ### Context System B submissions are Keycloak-authenticated (see `westside-landing#243`). Every row has a verified `sub` claim from the JWT. `email` and `player_name` remain user-editable because (a) they may differ from the JWT-holder (parent submitting for child), (b) the form should be editable rather than locked to JWT identity. **Actual migration head verified 2026-04-10:** `030_add_registration_type_to_registrations.py` (revision `030`, down_revision `029`). Next available revision is **031**. Do NOT use any lower number — migrations 014 through 030 already exist. **Actual model location verified 2026-04-10:** all SQLAlchemy models live in a single flat file `src/basketball_api/models.py` (31 classes, including `Parent` and `Player`). There is no `models/` subdirectory. The new `JerseyPublicOrder` class must be **appended** to `models.py`, not split into a submodule. ### File Targets Files to create: - `alembic/versions/031_add_jersey_public_orders.py` — Alembic migration, `revision = "031"`, `down_revision = "030"` Files to modify: - `src/basketball_api/models.py` — **append** a new `JerseyPublicOrder` SQLAlchemy class at the end of the file. Do NOT modify any existing class. Files the agent should NOT touch: - Any existing migration in `alembic/versions/` (013 through 030) — those are System C and other features, hands off - Any existing class in `src/basketball_api/models.py` — append-only - `routes/checkout.py`, `routes/jersey.py` — other systems ### Schema (exact) ```sql -- In upgrade() — FIRST ensure pgcrypto is available for gen_random_uuid() CREATE EXTENSION IF NOT EXISTS pgcrypto; CREATE TABLE jersey_public_orders ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), submitter_keycloak_sub VARCHAR(255) NOT NULL, player_name VARCHAR(200) NOT NULL, email VARCHAR(320) NOT NULL, team VARCHAR(100) NOT NULL, kq VARCHAR(10) NOT NULL CHECK (kq IN ('kings','queens')), preferred_number_1 VARCHAR(2), preferred_number_2 VARCHAR(2), preferred_number_3 VARCHAR(2), top_size VARCHAR(10) NOT NULL, short_size VARCHAR(10) NOT NULL, tier VARCHAR(40) NOT NULL CHECK (tier IN ('90-reversible','130-reversible-shooter')), status VARCHAR(20) NOT NULL DEFAULT 'pending' CHECK (status IN ('pending','reviewed','fulfilled','cancelled')), linked_parent_id INTEGER REFERENCES parents(id) ON DELETE SET NULL, linked_player_id INTEGER REFERENCES players(id) ON DELETE SET NULL, reviewed_at TIMESTAMP WITH TIME ZONE, reviewed_by VARCHAR(200), submission_ip INET, created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW() ); CREATE INDEX idx_jpo_submitter_sub ON jersey_public_orders (submitter_keycloak_sub); CREATE INDEX idx_jpo_email ON jersey_public_orders (email); CREATE INDEX idx_jpo_status ON jersey_public_orders (status); CREATE INDEX idx_jpo_created_at ON jersey_public_orders (created_at DESC); ``` In `downgrade()`: `DROP TABLE jersey_public_orders`. Do NOT drop the `pgcrypto` extension — other tables may rely on it. ### Acceptance Criteria - [ ] `alembic upgrade head` applies cleanly against a fresh DB from scratch (runs 001 through 031) - [ ] `alembic downgrade -1` reverses 031 cleanly (head becomes 030) - [ ] `alembic upgrade head` re-applies 031 cleanly after the downgrade - [ ] `pgcrypto` extension is created if not present (idempotent) - [ ] All four indexes created - [ ] CHECK constraints reject invalid `kq`, `tier`, `status` values - [ ] `submitter_keycloak_sub` is NOT NULL - [ ] FKs to parents/players use `ON DELETE SET NULL` - [ ] `JerseyPublicOrder` class is importable from `basketball_api.models` - [ ] No existing class in `models.py` is modified (verify with `git diff main -- src/basketball_api/models.py` — only additions below the final pre-existing class) ### Test Expectations - [ ] Unit test: `upgrade head → downgrade -1 → upgrade head` round-trip succeeds - [ ] Unit test: insert valid row, fetch, all fields round-trip - [ ] Unit test: insert with `kq='kingdoms'` → IntegrityError - [ ] Unit test: insert with NULL `submitter_keycloak_sub` → IntegrityError - [ ] Run command: `pytest tests/ -k jersey_public_order` ### Constraints - Match existing Alembic migration style from `alembic/versions/030_add_registration_type_to_registrations.py` (op.create_table, op.create_index, etc.) - No new Python dependencies - `email` is NOT UNIQUE (same email can submit multiple orders) - `submitter_keycloak_sub` is NOT UNIQUE either (same Keycloak user can submit multiple orders) - Do NOT create a separate `keycloak_user` table — we store the `sub` claim as a string and Keycloak is the source of truth - Do NOT modify `down_revision` on any existing migration ### Checklist - [ ] PR opened against `basketball-api` main - [ ] Migration applies in CI against test DB (runs full chain 001 → 031) - [ ] `JerseyPublicOrder` importable - [ ] `git diff main -- src/basketball_api/models.py` shows only additions - [ ] Tests pass ### Related - `westside-basketball` — project - `story:WS-S31` — admin public jersey intake link - `arch-jersey-intake` — architecture doc - `feedback_funnel_requires_auth.md` — why every row has a verified Keycloak identity - Reference: `alembic/versions/030_*.py` — style guide, hands off

forgejo_admin referenced this issue

2026-04-10 22:05:01 +00:00

POST /api/jersey-public-orders — public submission endpoint #430

forgejo_admin referenced this issue

2026-04-10 22:05:46 +00:00

GET /api/jersey-public-orders — admin list endpoint #432

forgejo_admin commented

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

Author

Contributor

Scope Review: NEEDS_REFINEMENT

Review note: review-947-2026-04-10

Scope, schema, and traceability are solid, but two factual claims in the body are stale and will block dev if not fixed.

Blocking [BODY] fixes required:

Migration number is wrong. Current head is 030 (030_add_registration_type_to_registrations.py), not 013. Migrations 014-030 all exist. New file must be alembic/versions/031_add_jersey_public_orders.py with revision = "031", down_revision = "030".
Model path is wrong. There is no src/basketball_api/models/ directory. All SQLAlchemy models (including Parent, Player) live in a single flat file src/basketball_api/models.py. JerseyPublicOrder must be appended to that file, not placed in a new submodule. Remove the "or wherever project convention puts models" hedge.
Add op.execute("CREATE EXTENSION IF NOT EXISTS pgcrypto") to upgrade() before CREATE TABLE so gen_random_uuid() resolves on a fresh DB.
Rewrite the Context paragraph: "Migration 013 already exists... Next migration is 014" is false. State the real head (030) and next version (031).
Expand hands-off list: "Do not touch any existing migration 013-030. Do not modify any existing class in src/basketball_api/models.py — append only."

[SCOPE] discovered:

arch-jersey-intake note does not exist in pal-e-docs. The arch:jersey-intake label is set but there's no backing note. Not a blocker for T2 itself — queue as a separate scoping ticket so T3/T5 have a real arch doc.

Verified good:

story:WS-S31 exists in project-westside-basketball user-stories (Admin/Marcus)
FK types match: parents.id and players.id are both Integer — schema is correct
ON DELETE SET NULL correct for reconciliation semantics
No UNIQUE on email or submitter_sub — correct
CHECK constraints on kq/tier/status are sound
Zero existing references to jersey_public_order in basketball-api (additive-only, no blast radius)
System A and System C (migration 013) are not touched
AC covers upgrade/downgrade/upgrade round-trip
1 repo, 2 files, ~8 AC — passes 5-min rule, no decomposition needed

Fix the body, re-submit for re-review.

## Scope Review: NEEDS_REFINEMENT Review note: `review-947-2026-04-10` Scope, schema, and traceability are solid, but two factual claims in the body are stale and will block dev if not fixed. **Blocking [BODY] fixes required:** - Migration number is wrong. Current head is **030** (`030_add_registration_type_to_registrations.py`), not 013. Migrations 014-030 all exist. New file must be `alembic/versions/031_add_jersey_public_orders.py` with `revision = "031"`, `down_revision = "030"`. - Model path is wrong. There is no `src/basketball_api/models/` directory. All SQLAlchemy models (including `Parent`, `Player`) live in a single flat file `src/basketball_api/models.py`. `JerseyPublicOrder` must be **appended** to that file, not placed in a new submodule. Remove the "or wherever project convention puts models" hedge. - Add `op.execute("CREATE EXTENSION IF NOT EXISTS pgcrypto")` to `upgrade()` before `CREATE TABLE` so `gen_random_uuid()` resolves on a fresh DB. - Rewrite the Context paragraph: "Migration 013 already exists... Next migration is 014" is false. State the real head (030) and next version (031). - Expand hands-off list: "Do not touch any existing migration 013-030. Do not modify any existing class in `src/basketball_api/models.py` — append only." **[SCOPE] discovered:** - `arch-jersey-intake` note does not exist in pal-e-docs. The `arch:jersey-intake` label is set but there's no backing note. Not a blocker for T2 itself — queue as a separate scoping ticket so T3/T5 have a real arch doc. **Verified good:** - story:WS-S31 exists in `project-westside-basketball` user-stories (Admin/Marcus) - FK types match: `parents.id` and `players.id` are both `Integer` — schema is correct - `ON DELETE SET NULL` correct for reconciliation semantics - No UNIQUE on email or submitter_sub — correct - CHECK constraints on kq/tier/status are sound - Zero existing references to `jersey_public_order` in basketball-api (additive-only, no blast radius) - System A and System C (migration 013) are not touched - AC covers upgrade/downgrade/upgrade round-trip - 1 repo, 2 files, ~8 AC — passes 5-min rule, no decomposition needed Fix the body, re-submit for re-review.

forgejo_admin referenced this issue

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

GET /api/jersey-public-orders — admin list endpoint #432

forgejo_admin changed title from ~~Migration 014: jersey_public_orders table~~ to Migration 031: jersey_public_orders table

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

forgejo_admin referenced this issue

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

POST /api/jersey-public-orders — public submission endpoint #430

forgejo_admin referenced this issue from a commit

2026-04-10 22:20:57 +00:00

feat: migration 040 + JerseyPublicOrder model (#429)

forgejo_admin referenced this issue from a pull request that will close it,