forgejo_admin/basketball-api

Fork 1

feat: add skip_proration flag to first-payment endpoint and checkout route #459

Merged

forgejo_admin merged 1 commit from 458-skip-proration-flag into 369-first-payment-email-blast

2026-04-12 23:46:15 +00:00

forgejo_admin commented

2026-04-12 19:47:29 +00:00

Owner

Summary

Adds a skip_proration flag so custom-deal players (e.g. Jaxon Gerber $50/mo flat) get charged their exact monthly fee instead of the auto-prorated amount. Also adds the GET /checkout/first-payment route that was missing from this branch.

Changes

src/basketball_api/services/email.py — send_first_payment_email() accepts skip_proration kwarg; when true, uses flat monthly_fee, says "April fee" instead of "Prorated April fee", and appends &prorate=false to checkout URL
src/basketball_api/routes/admin.py — POST /admin/email/first-payment accepts skip_proration query param, forwards to send function
src/basketball_api/routes/checkout.py — New GET /first-payment endpoint: looks up player by contract_token, creates Stripe Checkout Session with setup_future_usage for card saving; respects prorate query param (default true)
tests/test_first_payment_email.py — 2 new tests: skip_proration=True uses flat fee, skip_proration=False still prorates
tests/test_first_payment_blast.py — 2 new blast tests (flag forwarding) + 3 new checkout route tests (prorated, non-prorated, invalid token)

Test Plan

pytest tests/test_first_payment_email.py tests/test_first_payment_blast.py -v
# 15 passed

Manual verification:

POST /admin/email/first-payment?test_email=X&skip_proration=true — email shows flat fee, checkout link has prorate=false
GET /checkout/first-payment?token=X&prorate=false — Stripe session charges monthly_fee * 100 cents
Default behavior unchanged (proration still applies when flag omitted)

Review Checklist

ruff format and ruff check pass
15 tests pass (7 email + 5 blast + 3 checkout)
No unrelated changes
Existing proration behavior unchanged when flag omitted

Forgejo issue: #458
Parent branch: 369-first-payment-email-blast

Closes #458

## Summary Adds a `skip_proration` flag so custom-deal players (e.g. Jaxon Gerber $50/mo flat) get charged their exact monthly fee instead of the auto-prorated amount. Also adds the `GET /checkout/first-payment` route that was missing from this branch. ## Changes - `src/basketball_api/services/email.py` — `send_first_payment_email()` accepts `skip_proration` kwarg; when true, uses flat `monthly_fee`, says "April fee" instead of "Prorated April fee", and appends `&prorate=false` to checkout URL - `src/basketball_api/routes/admin.py` — `POST /admin/email/first-payment` accepts `skip_proration` query param, forwards to send function - `src/basketball_api/routes/checkout.py` — New `GET /first-payment` endpoint: looks up player by `contract_token`, creates Stripe Checkout Session with `setup_future_usage` for card saving; respects `prorate` query param (default true) - `tests/test_first_payment_email.py` — 2 new tests: skip_proration=True uses flat fee, skip_proration=False still prorates - `tests/test_first_payment_blast.py` — 2 new blast tests (flag forwarding) + 3 new checkout route tests (prorated, non-prorated, invalid token) ## Test Plan ``` pytest tests/test_first_payment_email.py tests/test_first_payment_blast.py -v # 15 passed ``` Manual verification: - `POST /admin/email/first-payment?test_email=X&skip_proration=true` — email shows flat fee, checkout link has `prorate=false` - `GET /checkout/first-payment?token=X&prorate=false` — Stripe session charges `monthly_fee * 100` cents - Default behavior unchanged (proration still applies when flag omitted) ## Review Checklist - [x] ruff format and ruff check pass - [x] 15 tests pass (7 email + 5 blast + 3 checkout) - [x] No unrelated changes - [x] Existing proration behavior unchanged when flag omitted ## Related Notes - Forgejo issue: #458 - Parent branch: `369-first-payment-email-blast` Closes #458

forgejo_admin added 1 commit

2026-04-12 19:47:29 +00:00

feat: add skip_proration flag to first-payment endpoint and checkout route

ci/woodpecker/pr/woodpecker Pipeline failed

Details

462badee5a

Adds skip_proration param to POST /admin/email/first-payment and
send_first_payment_email() so custom-deal players get charged their
flat monthly fee instead of the prorated amount. Adds GET
/checkout/first-payment route that accepts prorate=false to charge
monthly_fee * 100 cents directly via Stripe.

Closes #458

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-04-12 19:48:13 +00:00

Author

Owner

QA Review — PR #459

Summary

Adds skip_proration flag to the first-payment blast endpoint and email function, plus a new GET /checkout/first-payment route. 5 files changed, 363 additions, 11 deletions. All changes are well-scoped to the issue spec.

Findings

No blockers found.

Minor observations (not blocking):

Private function import (_calculate_prorated_fee): Imported from services/email.py into routes/checkout.py with a leading underscore. Functionally correct — it's internal to the package — but could be renamed to drop the underscore if it's now part of the module's public interface. Not blocking since this is an existing pattern.
Checkout route is unauthenticated: GET /first-payment uses token-based auth via contract_token query param (no JWT/session). This matches the email-link flow pattern used elsewhere in the codebase (contract signing). The token is a UUID generated at contract creation, so it provides adequate security for this use case.
Test coverage is solid: 8 new tests cover skip_proration on the email function (both true/false), blast endpoint flag forwarding (both true/false), checkout route with proration, without proration, and invalid token. All 15 tests pass.
Acceptance criteria verification:
- skip_proration=true on blast → email shows flat fee
- skip_proration=true → checkout URL includes &prorate=false
- prorate=false on checkout → Stripe session uses monthly_fee * 100
- Default behavior unchanged (proration applies when flag omitted)
- EmailLog entry created for both prorated and non-prorated sends

VERDICT: APPROVED

## QA Review — PR #459 ### Summary Adds `skip_proration` flag to the first-payment blast endpoint and email function, plus a new `GET /checkout/first-payment` route. 5 files changed, 363 additions, 11 deletions. All changes are well-scoped to the issue spec. ### Findings **No blockers found.** Minor observations (not blocking): 1. **Private function import** (`_calculate_prorated_fee`): Imported from `services/email.py` into `routes/checkout.py` with a leading underscore. Functionally correct — it's internal to the package — but could be renamed to drop the underscore if it's now part of the module's public interface. Not blocking since this is an existing pattern. 2. **Checkout route is unauthenticated**: `GET /first-payment` uses token-based auth via `contract_token` query param (no JWT/session). This matches the email-link flow pattern used elsewhere in the codebase (contract signing). The token is a UUID generated at contract creation, so it provides adequate security for this use case. 3. **Test coverage is solid**: 8 new tests cover skip_proration on the email function (both true/false), blast endpoint flag forwarding (both true/false), checkout route with proration, without proration, and invalid token. All 15 tests pass. 4. **Acceptance criteria verification**: - [x] `skip_proration=true` on blast → email shows flat fee - [x] `skip_proration=true` → checkout URL includes `&prorate=false` - [x] `prorate=false` on checkout → Stripe session uses `monthly_fee * 100` - [x] Default behavior unchanged (proration applies when flag omitted) - [x] EmailLog entry created for both prorated and non-prorated sends ### VERDICT: APPROVED

forgejo_admin commented

2026-04-12 20:06:17 +00:00

Author

Owner

PR #459 Review

DOMAIN REVIEW

Stack: Python / FastAPI / SQLAlchemy / Stripe / Gmail SDK

Architecture: Clean feature addition. The skip_proration flag threads correctly from admin route -> email service -> checkout URL query param -> checkout route. The checkout route follows the existing token-based public endpoint pattern (no auth dependency, token as identifier), consistent with create_checkout_session.

Stripe integration: setup_future_usage="off_session" for card saving is correct for the use case. Customer create-or-reuse pattern is sound. The db.commit() after saving stripe_customer_id is appropriate.

Email service: The conditional HTML/plain-text generation is readable. The f-string ternary pattern for toggling proration explanation paragraphs works, though it produces trailing whitespace in the skip_proration case (empty string concatenated). Not a blocker.

BLOCKERS

None found. Test coverage is solid (7 tests across both files covering happy path, edge cases, flag forwarding, and 404 handling).

NITS

Hardcoded month name "April" -- fee_label in both email.py and checkout.py hardcodes "April fee" / "Prorated April fee". This works for the immediate use case but will require a code change for May onward. Consider extracting to a parameter or using datetime.now().strftime("%B"). Low priority since this is clearly a one-shot first-payment flow for the current cohort.
_calculate_prorated_fee called with raw player.monthly_fee (nullable)
In checkout.py line ~312 (diff context):
```
monthly_fee = player.monthly_fee or 200
if prorate:
    charge_cents = _calculate_prorated_fee(player.monthly_fee) * 100
```
The prorated path passes player.monthly_fee (which could be None) rather than the already-defaulted monthly_fee local. The email service (email.py) has the same pattern. This is presumably safe if _calculate_prorated_fee handles None internally (it existed before this PR), but it is inconsistent with the non-prorated path which uses the defaulted local. Worth a consistency pass in a follow-up.
charge_cents type -- _calculate_prorated_fee returns a dollar amount (int), and * 100 converts to cents. If _calculate_prorated_fee ever returns a float (e.g., for partial-day proration), charge_cents would be a float passed to Stripe's unit_amount, which expects an int. Stripe would reject it. A defensive int(...) wrap would be safer: charge_cents = int(_calculate_prorated_fee(player.monthly_fee) * 100).
Plain text double newline -- When skip_proration=True, full_fee_line is "" and the plain body gets f"{full_fee_line}\n" which produces a bare \n (extra blank line). Cosmetic only.
async mismatch -- The existing create_checkout_session is declared async def (line 104 in main). The new first_payment_checkout is def (sync). Both call stripe.* synchronously. Not a bug (FastAPI handles sync endpoints in a threadpool), but inconsistent. Match the existing pattern for consistency.

SOP COMPLIANCE

Branch named after issue: 458-skip-proration-flag references issue #458
PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present
Related references parent issue #458
Related does not reference a plan slug (none identified for this work -- acceptable for a feature branch off another feature branch)
No secrets committed
No scope creep -- all changes directly serve the skip_proration feature
Tests exist: 7 new tests (2 email unit, 2 blast integration, 3 checkout route)

PROCESS OBSERVATIONS

PR targets 369-first-payment-email-blast (not main), which is appropriate for a stacked feature branch. The merge path is 458 -> 369 -> main.
Test coverage is thorough: both flag states tested at every layer (email service, admin blast route, checkout route), plus invalid token 404.
The _make_unauthenticated_client helper in tests is well-structured for testing public endpoints without auth overhead.

VERDICT: APPROVED

## PR #459 Review ### DOMAIN REVIEW **Stack**: Python / FastAPI / SQLAlchemy / Stripe / Gmail SDK **Architecture**: Clean feature addition. The `skip_proration` flag threads correctly from admin route -> email service -> checkout URL query param -> checkout route. The checkout route follows the existing token-based public endpoint pattern (no auth dependency, token as identifier), consistent with `create_checkout_session`. **Stripe integration**: `setup_future_usage="off_session"` for card saving is correct for the use case. Customer create-or-reuse pattern is sound. The `db.commit()` after saving `stripe_customer_id` is appropriate. **Email service**: The conditional HTML/plain-text generation is readable. The f-string ternary pattern for toggling proration explanation paragraphs works, though it produces trailing whitespace in the `skip_proration` case (empty string concatenated). Not a blocker. ### BLOCKERS None found. Test coverage is solid (7 tests across both files covering happy path, edge cases, flag forwarding, and 404 handling). ### NITS 1. **Hardcoded month name "April"** -- `fee_label` in both `email.py` and `checkout.py` hardcodes "April fee" / "Prorated April fee". This works for the immediate use case but will require a code change for May onward. Consider extracting to a parameter or using `datetime.now().strftime("%B")`. Low priority since this is clearly a one-shot first-payment flow for the current cohort. 2. **`_calculate_prorated_fee` called with raw `player.monthly_fee` (nullable)** In `checkout.py` line ~312 (diff context): ```python monthly_fee = player.monthly_fee or 200 if prorate: charge_cents = _calculate_prorated_fee(player.monthly_fee) * 100 ``` The prorated path passes `player.monthly_fee` (which could be `None`) rather than the already-defaulted `monthly_fee` local. The email service (`email.py`) has the same pattern. This is presumably safe if `_calculate_prorated_fee` handles `None` internally (it existed before this PR), but it is inconsistent with the non-prorated path which uses the defaulted local. Worth a consistency pass in a follow-up. 3. **`charge_cents` type** -- `_calculate_prorated_fee` returns a dollar amount (int), and `* 100` converts to cents. If `_calculate_prorated_fee` ever returns a float (e.g., for partial-day proration), `charge_cents` would be a float passed to Stripe's `unit_amount`, which expects an int. Stripe would reject it. A defensive `int(...)` wrap would be safer: `charge_cents = int(_calculate_prorated_fee(player.monthly_fee) * 100)`. 4. **Plain text double newline** -- When `skip_proration=True`, `full_fee_line` is `""` and the plain body gets `f"{full_fee_line}\n"` which produces a bare `\n` (extra blank line). Cosmetic only. 5. **`async` mismatch** -- The existing `create_checkout_session` is declared `async def` (line 104 in main). The new `first_payment_checkout` is `def` (sync). Both call `stripe.*` synchronously. Not a bug (FastAPI handles sync endpoints in a threadpool), but inconsistent. Match the existing pattern for consistency. ### SOP COMPLIANCE - [x] Branch named after issue: `458-skip-proration-flag` references issue #458 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present - [x] Related references parent issue #458 - [ ] Related does not reference a plan slug (none identified for this work -- acceptable for a feature branch off another feature branch) - [x] No secrets committed - [x] No scope creep -- all changes directly serve the skip_proration feature - [x] Tests exist: 7 new tests (2 email unit, 2 blast integration, 3 checkout route) ### PROCESS OBSERVATIONS - PR targets `369-first-payment-email-blast` (not `main`), which is appropriate for a stacked feature branch. The merge path is 458 -> 369 -> main. - Test coverage is thorough: both flag states tested at every layer (email service, admin blast route, checkout route), plus invalid token 404. - The `_make_unauthenticated_client` helper in tests is well-structured for testing public endpoints without auth overhead. ### VERDICT: APPROVED