Review: #498 Migrate monthly-fee flow to Payment Links (analog of #494)

Verdict: NEEDS_REFINEMENT

Template Completeness

• [x] Type — Feature
• [x] Lineage — accurate standalone framing, correct provenance off #494 and #490
• [x] Repo — forgejo_admin/basketball-api
• [x] User Story — parent-framed
• [x] Context — identifies the real per-parent amount_cents differentiator vs tournament
• [x] File Targets — "dev agent's decision" (acceptable per feedback_tickets_not_solution_specs)
• [x] Acceptance Criteria
• [x] Test Expectations — real-Stripe integration test mandated, #490 lesson cited
• [x] Constraints
• [x] Checklist
• [x] Related links — #494, #486, #493, feedback memory

Traceability

• [x] story:WS-S11 label — exists on project-westside-basketball Admin section ("As an admin, I want to create Stripe checkout sessions for jerseys/fees so that payments are automated"). Fit is reasonable though the issue's user story is parent-framed. Acceptable: WS-S11 is the closest existing story and covers the payment-mint machinery regardless of frame.
• [x] arch:dataflow-westside-basketball label — arch-dataflow-westside-basketball note exists in pal-e-docs
• [x] Forgejo issue — forgejo_admin/basketball-api#498, open

File Targets

• [x] src/basketball_api/routes/checkout.py:305 — first_payment_checkout (GET /checkout/first-payment?token=...) — the sole monthly-fee mint path. Reads player.monthly_fee, computes prorated amount_cents, creates Stripe Session with payment_intent_data.setup_future_usage = "off_session".
• [x] src/basketball_api/services/email.py:1021 — send_first_payment_email embeds {base_url}/checkout/first-payment?token=... as the email-facing URL. The email currently points at our redirect, not at Stripe directly. Migrating to Payment Links will change this — the email should embed the durable https://buy.stripe.com/... Payment Link URL persisted on Order.stripe_checkout_url.
• [x] Reference pattern at src/basketball_api/services/tournament_checkout.py on origin/main (post-#494).

Repo Placement

Dependencies

• Not blocked by #497 (monthly recovery). Per #497's own Constraints block: "Blocked on the sibling architecture migration ticket IF we decide monthly-recovery should mint Payment Links — OR we accept 24h TTL for the recovery window." #497 is the consumer, #498 is the enabler. Confirmed standalone.
• Depends on #494 being merged (it is — commit 4a4b88a on origin/main). Helper file services/tournament_checkout.py provides the dogfooded structural pattern (Price → PaymentLink → deactivate-on-webhook).

Acceptance Criteria

• [x] "URLs remain clickable indefinitely" — testable
• [x] "order_id metadata present" — testable against mocked webhook
• [x] "Payment Link deactivated programmatically" — testable. However, the dev agent must widen the webhook deactivation gate. Today webhooks.py:246-267 (origin/main) deactivates only when product.category == ProductCategory.tournament. Monthly orders will not trigger deactivation under the existing gate. The issue body says "same pattern as #494" but doesn't explicitly call out that the gate must be widened — dev agent may miss it and ship a working-but-never-deactivating implementation.
• [x] "Per-parent amount_cents flows through" — testable, explicitly preserves scholarships/prorations
• [ ] Missing AC: setup_future_usage: "off_session" must still fire so the card is saved for recurring monthly billing. The current Session call at checkout.py:423 passes payment_intent_data={"setup_future_usage": "off_session"}. Stripe Payment Links accept payment_intent_data.setup_future_usage, but #494's implementation does not set it (tournament doesn't need card save). A dev agent copying #494 verbatim will drop this behavior silently, breaking all downstream recurring charges. Test coverage at tests/test_first_payment.py::test_setup_future_usage_passed_to_stripe will fail loudly, which is good, but the AC should name this explicitly so it's not treated as a regression to fix but as a required carry-over.

Test Expectations

• [x] Real-Stripe integration test mandated — matches #494's pattern (tests/test_payment_link_real_stripe.py on origin/main)
• [x] Mock-based webhook match coverage — present
• [x] Deactivation-on-payment asserted — present
• [x] #490 lesson referenced correctly: "Mock-only coverage missed the 24h cap that broke #490" — accurate. PR #490 had comprehensive mocked tests (8-site parametrized test) but did not catch the 24h Stripe-side cap because mocks accepted any expires_at.

Blast Radius

• Webhook handler category gate (webhooks.py:250): As noted above, the existing deactivation hook is tournament-gated. Widening to include monthly is correct; the dev agent should NOT widen it for jersey/tryout/generic (those remain Sessions and self-expire).
• Email template (services/email.py:1041-1043): Monthly-fee email currently embeds our redirect endpoint URL. Dev agent must decide whether to (a) keep the redirect (and make first_payment_checkout return a persisted Payment Link URL) or (b) embed the Stripe Payment Link URL directly. Option (a) is closer to the existing shape and preserves our ability to re-check contract status on click; option (b) is simpler and truer to the "no expiry" promise. Issue body is silent on this — see [BODY] recommendation.
• Stale-pending reuse path (checkout.py:358-377): Current code "reuses fresh pending Session" within a 30-min stale threshold. With Payment Links there is no expiry and no reuse gymnastics — one Payment Link per Order, forever (until webhook-deactivated). The dev agent will need to simplify this block.
• Non-monthly flows: Jersey (jersey.py:291), admin tryout regen (admin.py:2005), register.py tryout paths (1378, 1423), generic checkout (checkout.py:247, 553) — all 24h-safe for their latency profile. Constraint in issue body is correct.

Decomposition Assessment

Recommendation

• [BODY] Add AC for setup_future_usage preservation: add an acceptance-criteria bullet: "Card is saved for future recurring billing — existing payment_intent_data.setup_future_usage = 'off_session' behavior (checkout.py:423) is preserved on the Payment Link. Regression test at tests/test_first_payment.py::test_setup_future_usage_passed_to_stripe must continue to pass."
• [BODY] Name the webhook gate widening explicitly: in the Acceptance Criteria or Constraints, add: "Webhook deactivation gate in routes/webhooks.py must be widened from ProductCategory.tournament-only to include ProductCategory.monthly. Do not remove the gate entirely — jersey/tryout/generic must remain untouched."
• [BODY] Answer the email-URL architectural question: add a Context or Constraints sentence clarifying whether the monthly-fee email should (a) continue to embed {base_url}/checkout/first-payment?token=... with first_payment_checkout returning the persisted Payment Link URL, or (b) embed the https://buy.stripe.com/... URL directly. Recommended: (a) — preserves contract-status re-check on click and matches the minimum-diff philosophy. Dev agent should not have to guess.