forgejo_admin/basketball-api

Fork 1

Bug: Generic checkout webhook doesn't sync jersey status back to players table #170

New issue

Closed

opened 2026-03-26 16:40:12 +00:00 by forgejo_admin · 3 comments

forgejo_admin commented

2026-03-26 16:40:12 +00:00

Owner

Type

Bug

Lineage

standalone — discovered during operations/data audit

Repo

forgejo_admin/basketball-api

What Broke

The generic checkout system (migration 013) introduced an orders table to replace denormalized players.jersey_* fields, but the Stripe webhook handler for the new system (_handle_generic_order_completed in webhooks.py) only updates orders.status = paid — it never syncs back to players.jersey_option, jersey_order_status, jersey_size, or jersey_number. Parents who order through the new /checkout/create-session path appear to have no jersey order when querying players.

Confirmed affected players:

Anaiyah Fesolai (id=96) — paid $130 Jersey + Warmup. orders.status = paid, players.jersey_order_status = none
Ayvah Apaisa (id=112) — opted out via new path. orders.status = paid, players.jersey_order_status = none

Root cause: Two parallel checkout systems (legacy jersey.py and generic checkout.py) both active, neither decommissioned. Webhook routing in webhooks.py:256-262 checks order_id first — if present, generic handler runs and returns, legacy handler (which DOES update players) is never reached.

Repro Steps

Parent visits jersey ordering page, goes through /checkout/create-session (new generic path)
Completes Stripe payment
Webhook fires checkout.session.completed with order_id in metadata
_handle_generic_order_completed sets orders.status = paid, returns
Query SELECT jersey_order_status FROM players WHERE name = 'Anaiyah Fesolai'; — returns none

Expected Behavior

After successful Stripe payment via either checkout path, players.jersey_option, jersey_order_status, jersey_size, and jersey_number should reflect the completed order. Admin queries against players should return accurate jersey status.

Environment

Cluster/namespace: prod / basketball-api
Service version/commit: current main
Related alerts: none (silent data corruption — no monitoring on this)

File Targets

src/basketball_api/routes/webhooks.py — _handle_generic_order_completed() (lines 137-169). Add player sync after order.status = paid.
src/basketball_api/routes/checkout.py — create_checkout_session() opt-out path (lines 140-160). Add player field sync on opt-out order creation.
src/basketball_api/models.py — Product model (line 328), ProductCategory enum (line 114). Used for jersey category detection.
alembic/versions/023_backfill_player_jersey_from_orders.py — NEW. One-time data migration to reconcile existing paid orders → player records.
tests/test_checkout.py — test_webhook_updates_order_to_paid() (line 321). Extend to assert player fields. Add new tests.

Product → JerseyOption Mapping

The webhook must map product.name to JerseyOption enum when product.category == 'jersey':

Product ID	Product Name	JerseyOption
1	Reversible Jersey	`reversible`
2	Jersey + Warmup Package	`jersey_warmup`
3	Opt Out	`opt_out`

order.custom_data JSONB contains sizing: {top_size: S, shorts_size: S, jersey_number: 1}. Map top_size → player.jersey_size, jersey_number → player.jersey_number.

Test Expectations

Run: cd ~/basketball-api && python -m pytest tests/test_checkout.py tests/test_jersey.py -v

Extend existing test:

test_webhook_updates_order_to_paid (test_checkout.py:321) — after asserting order.status == paid, also assert player.jersey_option, player.jersey_order_status == paid, player.jersey_size, player.jersey_number

New tests needed:

test_webhook_syncs_jersey_fields_to_player — generic order webhook for a jersey-category product updates player record
test_webhook_skips_player_sync_for_non_jersey_product — tournament/equipment orders don't touch player jersey fields
test_opt_out_checkout_syncs_player_fields — opt-out via /checkout/create-session sets player.jersey_option = opt_out and player.jersey_order_status = paid
test_duplicate_order_prevention — second checkout for same player+product returns existing pending order or errors

Acceptance Criteria

_handle_generic_order_completed syncs player.jersey_option, jersey_order_status, jersey_size, jersey_number when product.category == jersey
checkout.py opt-out path sets player jersey fields (same as legacy jersey.py opt-out does)
Alembic migration 023_backfill_player_jersey_from_orders reconciles all existing orders.status = paid + product.category = jersey rows → player records
Duplicate order guard: /checkout/create-session checks for existing pending order for same player+product before creating new one
All new and extended tests pass

Constraints

Alembic migration MUST be idempotent (safe to re-run if it fails partway)
Migration must handle the case where player already has jersey fields set (legacy path) — don't overwrite with stale order data. Use WHERE players.jersey_order_status = 'none' guard.
No downtime — webhook fix and migration can be deployed together

Decisions

Legacy path (/jersey/checkout): Keep active for now. Add a deprecation logger.warning on each call. Full retirement is a separate future ticket.
Baby Betty contradictory state: Split to #171. Manual data fix, not part of this code change.
Creed Draney duplicate orders: The duplicate prevention guard in this ticket prevents future duplicates. Existing test data duplicates are low priority — no cleanup needed.

#171 — Baby Betty contradictory state (split from this ticket)
project-westside-basketball — project this affects
alembic/versions/013_generic_checkout_system.py — migration that introduced orders table
westside-contracts — frontend SvelteKit app that drives the checkout flow

### Type Bug ### Lineage standalone — discovered during operations/data audit ### Repo `forgejo_admin/basketball-api` ### What Broke The generic checkout system (migration 013) introduced an `orders` table to replace denormalized `players.jersey_*` fields, but the Stripe webhook handler for the new system (`_handle_generic_order_completed` in `webhooks.py`) only updates `orders.status = paid` — it never syncs back to `players.jersey_option`, `jersey_order_status`, `jersey_size`, or `jersey_number`. Parents who order through the new `/checkout/create-session` path appear to have no jersey order when querying `players`. **Confirmed affected players:** - **Anaiyah Fesolai** (id=96) — paid $130 Jersey + Warmup. `orders.status = paid`, `players.jersey_order_status = none` - **Ayvah Apaisa** (id=112) — opted out via new path. `orders.status = paid`, `players.jersey_order_status = none` **Root cause:** Two parallel checkout systems (legacy `jersey.py` and generic `checkout.py`) both active, neither decommissioned. Webhook routing in `webhooks.py:256-262` checks `order_id` first — if present, generic handler runs and returns, legacy handler (which DOES update players) is never reached. ### Repro Steps 1. Parent visits jersey ordering page, goes through `/checkout/create-session` (new generic path) 2. Completes Stripe payment 3. Webhook fires `checkout.session.completed` with `order_id` in metadata 4. `_handle_generic_order_completed` sets `orders.status = paid`, returns 5. Query `SELECT jersey_order_status FROM players WHERE name = 'Anaiyah Fesolai';` — returns `none` ### Expected Behavior After successful Stripe payment via either checkout path, `players.jersey_option`, `jersey_order_status`, `jersey_size`, and `jersey_number` should reflect the completed order. Admin queries against `players` should return accurate jersey status. ### Environment - Cluster/namespace: prod / `basketball-api` - Service version/commit: current main - Related alerts: none (silent data corruption — no monitoring on this) ### File Targets - `src/basketball_api/routes/webhooks.py` — `_handle_generic_order_completed()` (lines 137-169). Add player sync after `order.status = paid`. - `src/basketball_api/routes/checkout.py` — `create_checkout_session()` opt-out path (lines 140-160). Add player field sync on opt-out order creation. - `src/basketball_api/models.py` — `Product` model (line 328), `ProductCategory` enum (line 114). Used for jersey category detection. - `alembic/versions/023_backfill_player_jersey_from_orders.py` — NEW. One-time data migration to reconcile existing paid orders → player records. - `tests/test_checkout.py` — `test_webhook_updates_order_to_paid()` (line 321). Extend to assert player fields. Add new tests. ### Product → JerseyOption Mapping The webhook must map `product.name` to `JerseyOption` enum when `product.category == 'jersey'`: | Product ID | Product Name | JerseyOption | |------------|-------------|-------------| | 1 | Reversible Jersey | `reversible` | | 2 | Jersey + Warmup Package | `jersey_warmup` | | 3 | Opt Out | `opt_out` | `order.custom_data` JSONB contains sizing: `{top_size: S, shorts_size: S, jersey_number: 1}`. Map `top_size` → `player.jersey_size`, `jersey_number` → `player.jersey_number`. ### Test Expectations Run: `cd ~/basketball-api && python -m pytest tests/test_checkout.py tests/test_jersey.py -v` **Extend existing test:** - `test_webhook_updates_order_to_paid` (test_checkout.py:321) — after asserting `order.status == paid`, also assert `player.jersey_option`, `player.jersey_order_status == paid`, `player.jersey_size`, `player.jersey_number` **New tests needed:** - `test_webhook_syncs_jersey_fields_to_player` — generic order webhook for a jersey-category product updates player record - `test_webhook_skips_player_sync_for_non_jersey_product` — tournament/equipment orders don't touch player jersey fields - `test_opt_out_checkout_syncs_player_fields` — opt-out via `/checkout/create-session` sets `player.jersey_option = opt_out` and `player.jersey_order_status = paid` - `test_duplicate_order_prevention` — second checkout for same player+product returns existing pending order or errors ### Acceptance Criteria - [ ] `_handle_generic_order_completed` syncs `player.jersey_option`, `jersey_order_status`, `jersey_size`, `jersey_number` when `product.category == jersey` - [ ] `checkout.py` opt-out path sets player jersey fields (same as legacy `jersey.py` opt-out does) - [ ] Alembic migration `023_backfill_player_jersey_from_orders` reconciles all existing `orders.status = paid` + `product.category = jersey` rows → player records - [ ] Duplicate order guard: `/checkout/create-session` checks for existing pending order for same player+product before creating new one - [ ] All new and extended tests pass ### Constraints - Alembic migration MUST be idempotent (safe to re-run if it fails partway) - Migration must handle the case where player already has jersey fields set (legacy path) — don't overwrite with stale order data. Use `WHERE players.jersey_order_status = 'none'` guard. - No downtime — webhook fix and migration can be deployed together ### Decisions - **Legacy path (`/jersey/checkout`)**: Keep active for now. Add a deprecation `logger.warning` on each call. Full retirement is a separate future ticket. - **Baby Betty contradictory state**: Split to #171. Manual data fix, not part of this code change. - **Creed Draney duplicate orders**: The duplicate prevention guard in this ticket prevents future duplicates. Existing test data duplicates are low priority — no cleanup needed. ### Related - `#171` — Baby Betty contradictory state (split from this ticket) - `project-westside-basketball` — project this affects - `alembic/versions/013_generic_checkout_system.py` — migration that introduced orders table - `westside-contracts` — frontend SvelteKit app that drives the checkout flow

forgejo_admin commented

2026-03-26 16:44:30 +00:00

Author

Owner

Scope Review: NEEDS_REFINEMENT

Review note: review-392-2026-03-26

Root cause analysis and code references are accurate — all file targets verified against the codebase. The bug is real and well-documented.

Six issues prevent READY status:

Missing File Targets section — files mentioned inline but not in template-required format with full repo-root paths (e.g. src/basketball_api/routes/webhooks.py, not webhooks.py)
Missing Test Expectations section — no test commands or descriptions of what to write/extend
AC3 ambiguous — "one-time data migration" needs mechanism specified (alembic vs. script) and how to derive jersey fields from orders.custom_data JSONB
AC4 not agent-verifiable — "Baby Betty contradictory state" is manual ops work, should be a separate ticket
AC6 not agent-verifiable — "Decision: retire legacy path" is an architectural decision, not a code acceptance criterion
Missing Constraints section — data migration safety (dry-run, backup, rollback) not specified

## Scope Review: NEEDS_REFINEMENT Review note: `review-392-2026-03-26` Root cause analysis and code references are accurate — all file targets verified against the codebase. The bug is real and well-documented. Six issues prevent READY status: - **Missing File Targets section** — files mentioned inline but not in template-required format with full repo-root paths (e.g. `src/basketball_api/routes/webhooks.py`, not `webhooks.py`) - **Missing Test Expectations section** — no test commands or descriptions of what to write/extend - **AC3 ambiguous** — "one-time data migration" needs mechanism specified (alembic vs. script) and how to derive jersey fields from `orders.custom_data` JSONB - **AC4 not agent-verifiable** — "Baby Betty contradictory state" is manual ops work, should be a separate ticket - **AC6 not agent-verifiable** — "Decision: retire legacy path" is an architectural decision, not a code acceptance criterion - **Missing Constraints section** — data migration safety (dry-run, backup, rollback) not specified

forgejo_admin commented

2026-03-26 17:06:00 +00:00

Author

Owner

Refinement (post review-392-2026-03-26)

Addressing 6 review findings. Updating issue body with refined scope below.

Decisions made

AC4 (Baby Betty) → split to separate ticket. Manual data fix is ops, not code.
AC6 (retire legacy path) → moved to Decisions section. Architectural decision, not an AC. Recommend keeping legacy path for now but adding deprecation log warning — retirement is a separate future ticket.
Data migration mechanism → alembic migration (not standalone script). It's a one-time schema-adjacent fix that belongs in the migration chain.

Refined issue body follows in next edit.

## Refinement (post review-392-2026-03-26) Addressing 6 review findings. Updating issue body with refined scope below. ### Decisions made - **AC4 (Baby Betty)** → split to separate ticket. Manual data fix is ops, not code. - **AC6 (retire legacy path)** → moved to Decisions section. Architectural decision, not an AC. Recommend keeping legacy path for now but adding deprecation log warning — retirement is a separate future ticket. - **Data migration mechanism** → alembic migration (not standalone script). It's a one-time schema-adjacent fix that belongs in the migration chain. ### Refined issue body follows in next edit.

forgejo_admin referenced this issue

2026-03-26 17:06:19 +00:00

Bug: Baby Betty has contradictory jersey state across legacy and generic checkout #171

forgejo_admin commented

2026-03-26 17:10:38 +00:00

Author

Owner

Scope Review: READY

Review note: review-392-2026-03-26b
Re-review pass: all 6 issues from previous review (review-392-2026-03-26) have been addressed. All file targets verified against codebase, all ACs are agent-testable, template complete.

Observation (not a blocker): Product custom_fields options for top_size include S, M, L, XL but JerseySize enum uses AS, AM, AL, AXL. Agent can handle this during implementation using the existing try/except pattern from the legacy handler (webhooks.py:207-209).

## Scope Review: READY Review note: `review-392-2026-03-26b` Re-review pass: all 6 issues from previous review (review-392-2026-03-26) have been addressed. All file targets verified against codebase, all ACs are agent-testable, template complete. **Observation (not a blocker):** Product `custom_fields` options for `top_size` include `S, M, L, XL` but `JerseySize` enum uses `AS, AM, AL, AXL`. Agent can handle this during implementation using the existing try/except pattern from the legacy handler (webhooks.py:207-209).

forgejo_admin referenced this issue from a commit

2026-03-26 22:20:44 +00:00

fix: sync jersey fields to player record from generic checkout webhook

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

2026-03-26 22:21:38 +00:00

fix: sync jersey fields to player record from generic checkout webhook #172

forgejo_admin referenced this issue from a commit

2026-03-26 22:22:45 +00:00

fix: add deprecation warning to legacy /jersey/checkout endpoint

forgejo_admin referenced this issue

2026-03-26 22:23:04 +00:00

fix: sync jersey fields to player record from generic checkout webhook #172