pal-e-docs

Review: Bug: Generic checkout webhook doesn't sync jersey status to players

review-392-2026-03-26b Review

review ready
Project

Verdict: READY

Re-review after refinement. Previous review (review-392-2026-03-26) found 6 issues; all 6 have been addressed in the updated issue body. One minor observation noted below (JerseySize enum mismatch) but it does not block -- the agent can handle it during implementation.

Template Completeness

  • [x] Type -- "Bug"
  • [x] Lineage -- "standalone -- discovered during operations/data audit"
  • [x] Repo -- forgejo_admin/basketball-api
  • [x] What Broke -- detailed root cause with affected players, replaces User Story (appropriate for bug)
  • [x] Repro Steps -- 5-step reproduction path
  • [x] Expected Behavior -- clear statement
  • [x] Environment -- cluster, namespace, service version
  • [x] File Targets -- 5 targets with full repo-root paths and line references
  • [x] Product-to-JerseyOption Mapping -- table with IDs, names, enum values
  • [x] Test Expectations -- run command, existing test extension, 4 new test descriptions
  • [x] Acceptance Criteria -- 5 checkboxes, all verifiable
  • [x] Constraints -- idempotency, no-overwrite guard, no downtime
  • [x] Decisions -- legacy path deprecation, Baby Betty split, Creed duplicates
  • [x] Related -- #171, project, migration 013, westside-contracts

File Targets

  • [x] src/basketball_api/routes/webhooks.py -- verified: _handle_generic_order_completed() at line 137-169. Only sets order.status = OrderStatus.paid and stripe_payment_intent_id. No player field sync. Bug confirmed.
  • [x] src/basketball_api/routes/checkout.py -- verified: opt-out path at lines 141-160 creates Order with status=paid but does not touch player jersey fields. Bug confirmed.
  • [x] src/basketball_api/models.py -- verified: Product model at line 328, ProductCategory enum at line 114, JerseyOption at line 71, JerseySize at line 77. All exist as described.
  • [x] alembic/versions/023_backfill_player_jersey_from_orders.py -- NEW file. Latest migration is 022_merge_heads.py. Numbering is correct.
  • [x] tests/test_checkout.py -- verified: test_webhook_updates_order_to_paid at line 321. Currently only asserts order.status == OrderStatus.paid and stripe_payment_intent_id. Correct target for extension.

Repo Placement

OK. Issue filed on forgejo_admin/basketball-api. All file targets are within basketball-api. No cross-repo work needed -- westside-contracts (the frontend) is referenced as context only, not as a modification target.

Dependencies

  • #171 (Baby Betty contradictory state) -- exists on the board (item #393, backlog). Correctly split from this ticket. No blocking dependency in either direction.
  • No in_progress blockers -- the two in_progress items (Phase 14: Billing Tiers, Phase 15: Production Port) are unrelated to stripe-webhook/checkout code.
  • Migration 013 -- the migration that created the orders table. Read and confirmed it exists and seeds the 3 jersey products. The backfill migration (023) will need to reference these seeded product rows.

Acceptance Criteria

All 5 ACs are agent-verifiable:
  • AC1 (webhook syncs player fields): testable via the extended test_webhook_updates_order_to_paid + new test_webhook_syncs_jersey_fields_to_player.
  • AC2 (opt-out path syncs): testable via new test_opt_out_checkout_syncs_player_fields.
  • AC3 (alembic backfill migration): testable by running the migration and querying. The mapping table and custom_data extraction are documented.
  • AC4 (duplicate order guard): testable via new test_duplicate_order_prevention.
  • AC5 (all tests pass): testable via provided run command.
Test command is real and correct: python -m pytest tests/test_checkout.py tests/test_jersey.py -v. Both files exist.

Blast Radius

  • JerseySize enum mismatch (observation, not blocker): The product custom_fields options for top_size include S, M, L, XL (no prefix), but the JerseySize enum values are AS, AM, AL, AXL (adult prefix). The agent will need to handle this mapping gracefully -- either a lookup table or a try/except with fallback. The ticket's mapping section says "Map top_size to player.jersey_size" but doesn't call out this mismatch. This is implementable without ticket changes since the agent will discover it when writing the code and the existing legacy webhook (_handle_jersey_checkout_completed) already handles JerseySize via try/except at line 207-209.
  • Legacy jersey route still active: src/basketball_api/routes/jersey.py exists with its own checkout flow. Ticket correctly says "keep active, add deprecation warning" in Decisions. No conflict.
  • Webhook routing order is safe: Lines 256-262 check generic (order_id) first, then legacy (jersey_option). The fix only adds logic inside _handle_generic_order_completed and doesn't change routing. No risk of breaking legacy or tryout registration flows.
  • No similar bug in other services: The checkout/webhook pattern is basketball-api specific. No other repos in the platform have Stripe webhook handlers.

Recommendation

No action needed. Ticket is READY for agent execution. The JerseySize enum mismatch between product custom_fields options and the JerseySize enum is a real implementation detail but not a scoping gap -- the agent will encounter it naturally and can follow the existing try/except pattern from the legacy handler (webhooks.py:207-209).

Previous Review Issues -- Resolution Status

  • File Targets missing -- RESOLVED: full repo-root paths with line references added.
  • Test Expectations missing -- RESOLVED: run command, 1 extension, 4 new tests described.
  • AC3 vague -- RESOLVED: mapping table and custom_data JSONB extraction documented.
  • AC4 (Baby Betty) split -- RESOLVED: split to #171, on board as item #393.
  • AC6 (retire legacy) removed from ACs -- RESOLVED: moved to Decisions section.
  • Constraints missing -- RESOLVED: idempotency, no-overwrite guard, no downtime documented.