Refactor: extract shared Payment Link helper from monthly + tournament checkout services

### Type Feature ### Lineage Standalone — discovered 2026-04-18 during QA review of PR #499 (monthly-fee Payment Links migration). Reviewer flagged `src/basketball_api/services/monthly_checkout.py` and `src/basketball_api/services/tournament_checkout.py` as ~95% structurally identical. ### Repo `forgejo_admin/basketball-api` ### User Story As a developer adding a third Payment Link flow (jersey, tryout, generic — any future migration from Sessions), I want one shared helper that takes the per-flow deltas as parameters, not two near-duplicate implementations I have to keep in sync by hand. ### Context PR #499 shipped `services/monthly_checkout.py::create_monthly_order_payment_link` as a direct analog of `services/tournament_checkout.py::create_tournament_order_payment_link` from PR #494. QA noted ~95% overlap. The two differ on: - Whether `setup_future_usage: "off_session"` is set (monthly yes, tournament no) - Amount source: `Order.amount_cents` (monthly, prorated) vs `product.price_cents` (tournament, flat) - Redirect vs direct embed (monthly keeps the `/checkout/first-payment?token=...` redirect) All three are parameterizable. A shared `create_order_payment_link(order, *, setup_future_usage=False, amount_source="order")` helper would replace both with thinner call-site wrappers. Lowers the cost of the next migration (jersey/tryout/generic if those ever need to inherit indefinite-lived URLs). ### File Targets Dev agent's decision — grep the two existing helpers and factor the commonality. Likely targets: - NEW: `src/basketball_api/services/payment_links.py` (shared helper) - EDIT: `src/basketball_api/services/monthly_checkout.py` (thin wrapper, ≤20 lines) - EDIT: `src/basketball_api/services/tournament_checkout.py` (thin wrapper, ≤20 lines) - NEW: `tests/test_payment_links_helper.py` (unit coverage for kwarg branches) - UNCHANGED: `src/basketball_api/routes/webhooks.py` (gate logic is already a single function; no refactor there) ### Scope 1. Extract common helper with the three deltas as kwargs. 2. Port both per-flow helpers to delegate. 3. Keep per-flow named entry points so call-site reads naturally. 4. Add unit tests for the shared helper's kwarg branches; existing per-flow tests continue to pass unchanged. ### Acceptance Criteria - [ ] `services/payment_links.py` (or equivalent) houses the shared helper - [ ] `services/monthly_checkout.py` and `services/tournament_checkout.py` each thin out to a wrapper (≤20 lines each) - [ ] All existing tests pass (including `tests/test_monthly_payment_link_real_stripe.py` and `tests/test_payment_link_real_stripe.py`) - [ ] Helper has its own test covering the kwarg branches (setup_future_usage on/off; amount_source order/product) - [ ] No behavior change — this is a pure refactor ### Test Expectations - [ ] Webhook roundtrip tests for both monthly + tournament continue to pass unchanged - [ ] `setup_future_usage: "off_session"` still asserted on the monthly path - [ ] Real-Stripe integration tests for both paths continue to pass ### Constraints - Do NOT change any call-site external behavior. - Do NOT introduce a generic `create_payment_link(**kwargs)` that accepts arbitrary Stripe params — keep the helper opinionated, with exactly the three named deltas as kwargs. - Do NOT merge the webhook gate logic — `webhooks.py` is already a single function with a category gate; no refactor needed there. ### Checklist - [ ] PR opened - [ ] Tests pass in CI - [ ] Reviewer confirms line-count reduction in monthly/tournament_checkout.py ### Related - `forgejo_admin/basketball-api #494` — tournament Payment Link migration - `forgejo_admin/basketball-api #498` — monthly Payment Link migration (PR #499) - QA review comment on PR #499 — DRY concern

forgejo_admin commented

2026-04-18 06:07:59 +00:00

Owner

Type

Feature

Lineage

Standalone — discovered 2026-04-18 during QA review of PR #499 (monthly-fee Payment Links migration). Reviewer flagged src/basketball_api/services/monthly_checkout.py and src/basketball_api/services/tournament_checkout.py as ~95% structurally identical.

Repo

forgejo_admin/basketball-api

User Story

As a developer adding a third Payment Link flow (jersey, tryout, generic — any future migration from Sessions), I want one shared helper that takes the per-flow deltas as parameters, not two near-duplicate implementations I have to keep in sync by hand.

Context

PR #499 shipped services/monthly_checkout.py::create_monthly_order_payment_link as a direct analog of services/tournament_checkout.py::create_tournament_order_payment_link from PR #494. QA noted ~95% overlap. The two differ on:

Whether setup_future_usage: "off_session" is set (monthly yes, tournament no)
Amount source: Order.amount_cents (monthly, prorated) vs product.price_cents (tournament, flat)
Redirect vs direct embed (monthly keeps the /checkout/first-payment?token=... redirect)

All three are parameterizable. A shared create_order_payment_link(order, *, setup_future_usage=False, amount_source="order") helper would replace both with thinner call-site wrappers. Lowers the cost of the next migration (jersey/tryout/generic if those ever need to inherit indefinite-lived URLs).

File Targets

Dev agent's decision — grep the two existing helpers and factor the commonality. Likely targets:

NEW: src/basketball_api/services/payment_links.py (shared helper)
EDIT: src/basketball_api/services/monthly_checkout.py (thin wrapper, ≤20 lines)
EDIT: src/basketball_api/services/tournament_checkout.py (thin wrapper, ≤20 lines)
NEW: tests/test_payment_links_helper.py (unit coverage for kwarg branches)
UNCHANGED: src/basketball_api/routes/webhooks.py (gate logic is already a single function; no refactor there)

Scope

Extract common helper with the three deltas as kwargs.
Port both per-flow helpers to delegate.
Keep per-flow named entry points so call-site reads naturally.
Add unit tests for the shared helper's kwarg branches; existing per-flow tests continue to pass unchanged.

Acceptance Criteria

services/payment_links.py (or equivalent) houses the shared helper
services/monthly_checkout.py and services/tournament_checkout.py each thin out to a wrapper (≤20 lines each)
All existing tests pass (including tests/test_monthly_payment_link_real_stripe.py and tests/test_payment_link_real_stripe.py)
Helper has its own test covering the kwarg branches (setup_future_usage on/off; amount_source order/product)
No behavior change — this is a pure refactor

Test Expectations

Webhook roundtrip tests for both monthly + tournament continue to pass unchanged
setup_future_usage: "off_session" still asserted on the monthly path
Real-Stripe integration tests for both paths continue to pass

Constraints

Do NOT change any call-site external behavior.
Do NOT introduce a generic create_payment_link(**kwargs) that accepts arbitrary Stripe params — keep the helper opinionated, with exactly the three named deltas as kwargs.
Do NOT merge the webhook gate logic — webhooks.py is already a single function with a category gate; no refactor needed there.

Checklist

PR opened
Tests pass in CI
Reviewer confirms line-count reduction in monthly/tournament_checkout.py

forgejo_admin/basketball-api #494 — tournament Payment Link migration
forgejo_admin/basketball-api #498 — monthly Payment Link migration (PR #499)
QA review comment on PR #499 — DRY concern

Rows
Columns

Refactor: extract shared Payment Link helper from monthly + tournament checkout services #502