feat: tournament product creation + per-player Stripe checkout links

forgejo_admin commented

2026-04-12 20:13:45 +00:00

Owner

Summary

Adds admin endpoints to create tournaments with team-specific pricing and generate per-player Stripe checkout URLs. First consumer: Utah Invitational (17U Elite $55, 17U Select $65, 16U Elite $55).

Changes

src/basketball_api/models.py — Added Tournament and TournamentProduct models with tenant FK, unique constraint on (tournament_id, team_id)
alembic/versions/039_add_tournament_tables.py — Migration creating tournaments and tournament_products tables with indexes
src/basketball_api/routes/admin.py — Added POST /admin/tournaments (create with team-specific products), GET /admin/tournaments/{id}, and POST /admin/tournaments/{id}/checkout-links (generate per-player Stripe sessions)
src/basketball_api/routes/checkout.py — Extracted create_player_checkout_session() helper for admin-driven checkout flow
tests/test_tournament.py — 11 tests: creation, validation, 404s, checkout links with Stripe mocks, duplicate order handling, model linkage

Test Plan

pytest tests/test_tournament.py -v — 11/11 pass
Full suite: 912 passed, 5 errors (all pre-existing Postgres connection drops, not code failures)
Verify: tournament products have category='tournament', type='one_time'
Verify: checkout URLs include tournament_name + team_name in Stripe metadata
Verify: duplicate orders return error entries, not duplicate sessions

Review Checklist

ruff format + ruff check pass
11 new tests pass
Full suite regression clean (912 passed)
Migration 039 follows revision chain (revises 038)
No changes to existing checkout endpoint or email services
Stripe key from settings, not hardcoded

Generic blast system (#456) provides {{checkout_url}} placeholder consumer
story:WS-S33 — tournament billing user story

Closes #457

## Summary Adds admin endpoints to create tournaments with team-specific pricing and generate per-player Stripe checkout URLs. First consumer: Utah Invitational (17U Elite $55, 17U Select $65, 16U Elite $55). ## Changes - `src/basketball_api/models.py` — Added `Tournament` and `TournamentProduct` models with tenant FK, unique constraint on (tournament_id, team_id) - `alembic/versions/039_add_tournament_tables.py` — Migration creating `tournaments` and `tournament_products` tables with indexes - `src/basketball_api/routes/admin.py` — Added `POST /admin/tournaments` (create with team-specific products), `GET /admin/tournaments/{id}`, and `POST /admin/tournaments/{id}/checkout-links` (generate per-player Stripe sessions) - `src/basketball_api/routes/checkout.py` — Extracted `create_player_checkout_session()` helper for admin-driven checkout flow - `tests/test_tournament.py` — 11 tests: creation, validation, 404s, checkout links with Stripe mocks, duplicate order handling, model linkage ## Test Plan - `pytest tests/test_tournament.py -v` — 11/11 pass - Full suite: 912 passed, 5 errors (all pre-existing Postgres connection drops, not code failures) - Verify: tournament products have `category='tournament'`, `type='one_time'` - Verify: checkout URLs include tournament_name + team_name in Stripe metadata - Verify: duplicate orders return error entries, not duplicate sessions ## Review Checklist - [x] ruff format + ruff check pass - [x] 11 new tests pass - [x] Full suite regression clean (912 passed) - [x] Migration 039 follows revision chain (revises 038) - [x] No changes to existing checkout endpoint or email services - [x] Stripe key from settings, not hardcoded ## Related Notes - Generic blast system (#456) provides `{{checkout_url}}` placeholder consumer - `story:WS-S33` — tournament billing user story Closes #457

forgejo_admin added 1 commit

2026-04-12 20:13:45 +00:00

feat: add tournament product creation and per-player Stripe checkout links

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

bca32a5ed1

Adds admin endpoints to create tournaments with team-specific pricing and
generate per-player Stripe checkout URLs for billing tournament entry fees.

- Tournament and TournamentProduct models with migration 039
- POST /admin/tournaments creates tournament + N products in one call
- GET /admin/tournaments/{id} retrieves tournament with products
- POST /admin/tournaments/{id}/checkout-links generates per-player URLs
- Extracted create_player_checkout_session helper from checkout.py
- 11 tests covering creation, retrieval, links, duplicates, and models

Closes #457

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

forgejo_admin commented

2026-04-12 20:15:52 +00:00

Author

Owner

PR #460 Review

DOMAIN REVIEW

Stack: Python / FastAPI / SQLAlchemy / Alembic / Stripe / Pydantic

Models (src/basketball_api/models.py): Tournament and TournamentProduct are well-structured. Tournament has id, name, event_date, description, tenant_id -- matches spec. TournamentProduct links tournament to product with team_id FK and unique constraint on (tournament_id, team_id) -- prevents duplicate team entries per tournament. Cascade deletes are correct. Relationships bidirectional with back_populates.

Migration (alembic/versions/039_add_tournament_tables.py): Schema matches models exactly. Indexes on all FK columns. Unique constraint matches model __table_args__. Downgrade drops tables in correct dependency order.

Routes (src/basketball_api/routes/admin.py):

POST /admin/tournaments validates teams exist (404 for missing), validates non-empty teams list (422), parses event_date with error handling (422), creates tournament + products in single transaction. All correct.
GET /admin/tournaments/{id} returns 404 for missing tournament. Lazy-loads tournament_products via relationship. Clean.
POST /admin/tournaments/{id}/checkout-links iterates team players, checks for existing paid/pending orders (idempotent skip with error message), calls extracted helper. 404 for missing tournament, 400 for no products configured.

Checkout helper (src/basketball_api/routes/checkout.py): create_player_checkout_session extracts the duplicate-check, customer creation, order creation, and Stripe session creation pattern. Uses _SUCCESS_URL / _CANCEL_URL from module-level settings (not hardcoded). Stripe API key configured via module-level import stripe with global config from settings. The helper raises HTTPException(409) for duplicates, which the caller catches gracefully.

DRY observation: The helper duplicates logic from the existing create_checkout_session endpoint (lines 149-253). Both do: duplicate order check, customer create-or-reuse, order creation, Stripe session creation. Ideally the existing endpoint would be refactored to call the helper too. However, this is additive code that does not modify the existing endpoint, and the duplication is in checkout logic (not auth/security), so this is a nit, not a blocker.

Tests (tests/test_tournament.py): 11 tests across 4 test classes:

TestCreateTournament: happy path (201, products created with correct category/type), empty teams (422), invalid team ID (404), no date (201 with null)
TestGetTournament: happy path (200), not found (404)
TestCheckoutLinks: happy path with Stripe mocks (verifies metadata includes tournament_id/name/team_name), not found (404), duplicate order skipped (errors list populated, other players still get links)
TestTournamentModel: direct ORM creation and relationship linkage

Auth is properly overridden via require_admin dependency in test fixtures.

BLOCKERS

1. Migration number collision (039 vs remote 044)

The PR creates alembic/versions/039_add_tournament_tables.py with revision = "039" and down_revision = "038". You stated remote main has migrations up to 044. This migration will fail to apply -- Alembic will not find revision "038" in the chain since the head is already at "044". The migration must be rebased to 045 (with down_revision = "044").

This is a hard blocker: the migration will not run on production.

NITS

DRY: checkout session creation -- create_player_checkout_session largely duplicates the Stripe session flow in create_checkout_session (lines 149-253 of checkout.py). Consider refactoring the existing endpoint to call the helper, reducing two copies of customer-create + order-create + session-create to one. Not blocking since both paths work independently and the existing endpoint is untouched.
Double duplicate-order check -- generate_checkout_links checks for existing orders before calling create_player_checkout_session, which also checks. The outer check handles the graceful skip (errors list), while the inner check is a safety net that raises 409. This is defensively correct but could be simplified by having only one check point.
event_date as string in request model -- CreateTournamentRequest.event_date is str | None with manual date.fromisoformat() parsing. Using datetime.date | None directly in the Pydantic model would let FastAPI handle validation automatically and remove the manual parse block.
Response model created_at as string -- TournamentResponse.created_at is str with manual .isoformat(). Using datetime type with Pydantic's JSON serialization would be cleaner.
N+1 query potential in get_tournament -- Iterating tournament.tournament_products then accessing tp.team.name and tp.product.price_cents triggers lazy loads per item. For small tournament rosters this is fine; for scale, consider joinedload or selectinload on the query.

SOP COMPLIANCE

Branch named after issue: 457-tournament-checkout-links
PR body has Summary, Changes, Test Plan, Related sections
Related references story story:WS-S33 and linked issue #456
Migration number valid on main -- FAIL: 039 collides with existing 040-044 on remote
No secrets committed
No unrelated file changes (5 files, all tournament-related)
Tests exist: 11 tests covering happy path + error paths

PROCESS OBSERVATIONS

Change failure risk: HIGH due to migration collision. If merged as-is, Alembic upgrade will fail on deploy, requiring manual intervention or a hotfix. This must be fixed before merge.
Deployment frequency: Clean feature addition, well-scoped. Once migration is rebased, this is low-risk to deploy.
Test quality: Strong coverage. Stripe is properly mocked. Duplicate order handling is explicitly tested. The test file is self-contained with clean fixtures.

VERDICT: NOT APPROVED

Single blocker: migration 039 collides with existing migrations on main (remote head is 044). Rebase the migration to 045 with down_revision = "044", then this PR is ready to merge.

## PR #460 Review ### DOMAIN REVIEW **Stack**: Python / FastAPI / SQLAlchemy / Alembic / Stripe / Pydantic **Models** (`src/basketball_api/models.py`): `Tournament` and `TournamentProduct` are well-structured. `Tournament` has id, name, event_date, description, tenant_id -- matches spec. `TournamentProduct` links tournament to product with team_id FK and unique constraint on `(tournament_id, team_id)` -- prevents duplicate team entries per tournament. Cascade deletes are correct. Relationships bidirectional with `back_populates`. **Migration** (`alembic/versions/039_add_tournament_tables.py`): Schema matches models exactly. Indexes on all FK columns. Unique constraint matches model `__table_args__`. Downgrade drops tables in correct dependency order. **Routes** (`src/basketball_api/routes/admin.py`): - `POST /admin/tournaments` validates teams exist (404 for missing), validates non-empty teams list (422), parses event_date with error handling (422), creates tournament + products in single transaction. All correct. - `GET /admin/tournaments/{id}` returns 404 for missing tournament. Lazy-loads tournament_products via relationship. Clean. - `POST /admin/tournaments/{id}/checkout-links` iterates team players, checks for existing paid/pending orders (idempotent skip with error message), calls extracted helper. 404 for missing tournament, 400 for no products configured. **Checkout helper** (`src/basketball_api/routes/checkout.py`): `create_player_checkout_session` extracts the duplicate-check, customer creation, order creation, and Stripe session creation pattern. Uses `_SUCCESS_URL` / `_CANCEL_URL` from module-level settings (not hardcoded). Stripe API key configured via module-level `import stripe` with global config from settings. The helper raises `HTTPException(409)` for duplicates, which the caller catches gracefully. **DRY observation**: The helper duplicates logic from the existing `create_checkout_session` endpoint (lines 149-253). Both do: duplicate order check, customer create-or-reuse, order creation, Stripe session creation. Ideally the existing endpoint would be refactored to call the helper too. However, this is additive code that does not modify the existing endpoint, and the duplication is in checkout logic (not auth/security), so this is a nit, not a blocker. **Tests** (`tests/test_tournament.py`): 11 tests across 4 test classes: - `TestCreateTournament`: happy path (201, products created with correct category/type), empty teams (422), invalid team ID (404), no date (201 with null) - `TestGetTournament`: happy path (200), not found (404) - `TestCheckoutLinks`: happy path with Stripe mocks (verifies metadata includes tournament_id/name/team_name), not found (404), duplicate order skipped (errors list populated, other players still get links) - `TestTournamentModel`: direct ORM creation and relationship linkage Auth is properly overridden via `require_admin` dependency in test fixtures. ### BLOCKERS **1. Migration number collision (039 vs remote 044)** The PR creates `alembic/versions/039_add_tournament_tables.py` with `revision = "039"` and `down_revision = "038"`. You stated remote main has migrations up to 044. This migration will fail to apply -- Alembic will not find revision "038" in the chain since the head is already at "044". The migration must be rebased to `045` (with `down_revision = "044"`). This is a hard blocker: the migration will not run on production. ### NITS 1. **DRY: checkout session creation** -- `create_player_checkout_session` largely duplicates the Stripe session flow in `create_checkout_session` (lines 149-253 of checkout.py). Consider refactoring the existing endpoint to call the helper, reducing two copies of customer-create + order-create + session-create to one. Not blocking since both paths work independently and the existing endpoint is untouched. 2. **Double duplicate-order check** -- `generate_checkout_links` checks for existing orders before calling `create_player_checkout_session`, which also checks. The outer check handles the graceful skip (errors list), while the inner check is a safety net that raises 409. This is defensively correct but could be simplified by having only one check point. 3. **`event_date` as string in request model** -- `CreateTournamentRequest.event_date` is `str | None` with manual `date.fromisoformat()` parsing. Using `datetime.date | None` directly in the Pydantic model would let FastAPI handle validation automatically and remove the manual parse block. 4. **Response model `created_at` as string** -- `TournamentResponse.created_at` is `str` with manual `.isoformat()`. Using `datetime` type with Pydantic's JSON serialization would be cleaner. 5. **N+1 query potential in `get_tournament`** -- Iterating `tournament.tournament_products` then accessing `tp.team.name` and `tp.product.price_cents` triggers lazy loads per item. For small tournament rosters this is fine; for scale, consider `joinedload` or `selectinload` on the query. ### SOP COMPLIANCE - [x] Branch named after issue: `457-tournament-checkout-links` - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references story `story:WS-S33` and linked issue #456 - [ ] Migration number valid on main -- **FAIL: 039 collides with existing 040-044 on remote** - [x] No secrets committed - [x] No unrelated file changes (5 files, all tournament-related) - [x] Tests exist: 11 tests covering happy path + error paths ### PROCESS OBSERVATIONS - **Change failure risk**: HIGH due to migration collision. If merged as-is, Alembic upgrade will fail on deploy, requiring manual intervention or a hotfix. This must be fixed before merge. - **Deployment frequency**: Clean feature addition, well-scoped. Once migration is rebased, this is low-risk to deploy. - **Test quality**: Strong coverage. Stripe is properly mocked. Duplicate order handling is explicitly tested. The test file is self-contained with clean fixtures. ### VERDICT: NOT APPROVED Single blocker: migration 039 collides with existing migrations on main (remote head is 044). Rebase the migration to 045 with `down_revision = "044"`, then this PR is ready to merge.

forgejo_admin force-pushed 457-tournament-checkout-links from bca32a5ed1

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

to d3b334eb4c

ci/woodpecker/pr/woodpecker Pipeline failed

Details

2026-04-12 20:19:38 +00:00

Compare