Fix: fresh-pending reuse shows stale Stripe amount when fee changed #479

Closed
opened 2026-04-14 02:28:59 +00:00 by forgejo_admin · 1 comment

Type

Bug

Lineage

Regression from forgejo_admin/basketball-api#473 (first-payment 409 fix). The "reuse existing Stripe session for fresh pending orders" branch blindly reuses the old session amount, causing a bait-and-switch when the admin adjusts a player's monthly_fee between clicks.

Repo

forgejo_admin/basketball-api

What Broke

Email shows one amount; Stripe checkout shows a different amount.

Reproduced end-to-end Apr 13:

  1. Test Queens Player monthly_fee=$200 → email sent showing "Pay $165 Now"
  2. Parent clicks → creates pending order #78 at $165, Stripe session at $165
  3. Admin changes fee to $4 → email resent showing "Pay $5 Now"
  4. Parent clicks the new "$5" link → endpoint sees fresh pending <30min, redirects to old $165 Stripe session
  5. Parent sees $165 on Stripe despite the email promising $5

Repro Steps

  1. Set Player.monthly_fee = 200 for any signed player
  2. Hit GET /checkout/first-payment?token={player.contract_token} → creates pending order at $165
  3. UPDATE players SET monthly_fee = 4 WHERE id = ... via DB (simulates admin fee change)
  4. Hit the same endpoint within 30 min
  5. Observe: 307 redirect to the old Stripe session URL showing $165, not $5

Expected Behavior

When the parent clicks and a fresh pending order exists but its amount no longer matches the currently-computed prorated amount, the old order/session is canceled and a new one is created at the correct amount. Email amount must always equal Stripe checkout amount.

Environment

  • Cluster/namespace: prod / basketball-api
  • Service version/commit: 44aef6d (main)
  • Stripe Checkout Sessions are immutable after creation — fixing this requires canceling the old session and creating a new one; Stripe has no "update amount" API

File Targets

Files to modify:

  • src/basketball_api/routes/checkout.py — lines 360-380 (first_payment_checkout). In the fresh-pending branch, add an amount comparison: if existing_order.amount_cents != amount_cents, cancel and fall through to create new.
  • tests/test_first_payment.py — add a test asserting the fee-change-mid-flight case produces a new order at the new amount.

Files to NOT touch:

  • src/basketball_api/services/email.py — email rendering is correct, uses fresh fee
  • Anything else

Acceptance Criteria

  • When a fresh pending order exists with an amount matching the currently-computed amount, still redirect to the existing session (preserves idempotency for repeat clicks).
  • When a fresh pending order exists with a different amount from the currently-computed amount, cancel it and create a new session at the correct amount.
  • When a stale pending order exists (>30 min), behavior unchanged (cancel + new).
  • When a paid order exists, still return 409 (unchanged).
  • Email amount always equals Stripe checkout amount at click time.

Test Expectations

  • New test: test_first_payment_fresh_pending_amount_changed — pending at $165, fee drops to $4, new click creates new order at $5 and redirects to the new Stripe session (not the old $165 one)
  • Existing test_first_payment_fresh_pending_returns_redirect still passes (same amount = reuse)
  • Existing test_first_payment_stale_pending_replaced still passes
  • Existing test_first_payment_paid_still_blocked still passes
  • Run command: pytest tests/test_first_payment.py -v
  • westside-basketball — project this affects
  • forgejo_admin/basketball-api#473 — the fix that introduced this regression
### Type Bug ### Lineage Regression from `forgejo_admin/basketball-api#473` (first-payment 409 fix). The "reuse existing Stripe session for fresh pending orders" branch blindly reuses the old session amount, causing a bait-and-switch when the admin adjusts a player's `monthly_fee` between clicks. ### Repo `forgejo_admin/basketball-api` ### What Broke Email shows one amount; Stripe checkout shows a different amount. Reproduced end-to-end Apr 13: 1. Test Queens Player `monthly_fee=$200` → email sent showing "Pay $165 Now" 2. Parent clicks → creates pending order #78 at $165, Stripe session at $165 3. Admin changes fee to $4 → email resent showing "Pay $5 Now" 4. Parent clicks the new "$5" link → endpoint sees fresh pending <30min, **redirects to old $165 Stripe session** 5. Parent sees $165 on Stripe despite the email promising $5 ### Repro Steps 1. Set `Player.monthly_fee = 200` for any signed player 2. Hit `GET /checkout/first-payment?token={player.contract_token}` → creates pending order at $165 3. `UPDATE players SET monthly_fee = 4 WHERE id = ...` via DB (simulates admin fee change) 4. Hit the same endpoint within 30 min 5. Observe: 307 redirect to the old Stripe session URL showing $165, not $5 ### Expected Behavior When the parent clicks and a fresh pending order exists but its amount no longer matches the currently-computed prorated amount, the old order/session is canceled and a new one is created at the correct amount. Email amount must always equal Stripe checkout amount. ### Environment - Cluster/namespace: prod / `basketball-api` - Service version/commit: `44aef6d` (main) - Stripe Checkout Sessions are immutable after creation — fixing this requires canceling the old session and creating a new one; Stripe has no "update amount" API ### File Targets Files to modify: - `src/basketball_api/routes/checkout.py` — lines 360-380 (`first_payment_checkout`). In the fresh-pending branch, add an amount comparison: if `existing_order.amount_cents != amount_cents`, cancel and fall through to create new. - `tests/test_first_payment.py` — add a test asserting the fee-change-mid-flight case produces a new order at the new amount. Files to NOT touch: - `src/basketball_api/services/email.py` — email rendering is correct, uses fresh fee - Anything else ### Acceptance Criteria - [ ] When a fresh pending order exists with an amount matching the currently-computed amount, still redirect to the existing session (preserves idempotency for repeat clicks). - [ ] When a fresh pending order exists with a different amount from the currently-computed amount, cancel it and create a new session at the correct amount. - [ ] When a stale pending order exists (>30 min), behavior unchanged (cancel + new). - [ ] When a paid order exists, still return 409 (unchanged). - [ ] Email amount always equals Stripe checkout amount at click time. ### Test Expectations - [ ] New test: `test_first_payment_fresh_pending_amount_changed` — pending at $165, fee drops to $4, new click creates new order at $5 and redirects to the new Stripe session (not the old $165 one) - [ ] Existing `test_first_payment_fresh_pending_returns_redirect` still passes (same amount = reuse) - [ ] Existing `test_first_payment_stale_pending_replaced` still passes - [ ] Existing `test_first_payment_paid_still_blocked` still passes - Run command: `pytest tests/test_first_payment.py -v` ### Related - `westside-basketball` — project this affects - `forgejo_admin/basketball-api#473` — the fix that introduced this regression
Author
Owner

Scope Review: READY

Review note: review-1006-2026-04-13

Scope is tight and verified against origin/main. File targets exist, line references are close (actual fresh-pending branch is 358-374 vs ticket's 360-380 — same region). Existing tests referenced in AC all present. Pattern is isolated to first_payment_checkout only — no blast radius to jersey/tournament flows. Traceability: story:WS-S11 verified, arch:basketball-api labeled (arch note itself is missing as a pre-existing gap — tracked separately, does not block this fix). No decomposition needed (2 files, 1 repo, 5 AC, ~5 LOC + 1 test).

Ready to move backlog → todo.

## Scope Review: READY Review note: `review-1006-2026-04-13` Scope is tight and verified against `origin/main`. File targets exist, line references are close (actual fresh-pending branch is 358-374 vs ticket's 360-380 — same region). Existing tests referenced in AC all present. Pattern is isolated to `first_payment_checkout` only — no blast radius to jersey/tournament flows. Traceability: story:WS-S11 verified, arch:basketball-api labeled (arch note itself is missing as a pre-existing gap — tracked separately, does not block this fix). No decomposition needed (2 files, 1 repo, 5 AC, ~5 LOC + 1 test). Ready to move backlog → todo.
forgejo_admin 2026-04-14 02:34:54 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/basketball-api#479
No description provided.