ldraney/basketball-api

Fork 0

feat: payment links table + outstanding balance emails #524

Open

ldraney wants to merge 0 commits from feat/payment-links-outstanding-balance into main

ldraney commented

2026-05-18 12:32:45 +00:00

Owner

Summary

Add payment_links table (migration 048) for unified per-player Stripe Payment Link tracking
Add POST /email/outstanding-balance admin endpoint and outstanding_balances blast query
Remove April-specific proration from monthly checkout and first-payment email
Include backfill script (already run — 98 rows in prod)

Changes

alembic/versions/048_add_payment_links_table.py: new migration — creates payment_links table, adds outstanding_balance to emailtype enum
src/basketball_api/models.py: PaymentLink model, PaymentLinkStatus enum, outstanding_balance email type
src/basketball_api/routes/admin.py: new outstanding balance endpoint, subscription status filter on first-payment
src/basketball_api/routes/checkout.py: remove proration, charge flat monthly_fee
src/basketball_api/services/email.py: send_outstanding_balance_email, clean up first-payment copy
src/basketball_api/services/email_queries.py: query_outstanding_balances for blast system
scripts/backfill_payment_links.py: backfill script with dry-run mode

Test Plan

Migration 048 already applied in prod — table has 98 rows
Backfill script already executed successfully
Pod running 1/1, healthz 200
Verify /email/outstanding-balance endpoint with test_email param
Verify monthly checkout no longer prorates

Review Checklist

No secrets committed
No unnecessary file changes
Commit messages are descriptive
Passed automated review-fix loop

Closes #523
project-westside-basketball — parent project

## Summary - Add `payment_links` table (migration 048) for unified per-player Stripe Payment Link tracking - Add `POST /email/outstanding-balance` admin endpoint and `outstanding_balances` blast query - Remove April-specific proration from monthly checkout and first-payment email - Include backfill script (already run — 98 rows in prod) ## Changes - `alembic/versions/048_add_payment_links_table.py`: new migration — creates `payment_links` table, adds `outstanding_balance` to emailtype enum - `src/basketball_api/models.py`: `PaymentLink` model, `PaymentLinkStatus` enum, `outstanding_balance` email type - `src/basketball_api/routes/admin.py`: new outstanding balance endpoint, subscription status filter on first-payment - `src/basketball_api/routes/checkout.py`: remove proration, charge flat monthly_fee - `src/basketball_api/services/email.py`: `send_outstanding_balance_email`, clean up first-payment copy - `src/basketball_api/services/email_queries.py`: `query_outstanding_balances` for blast system - `scripts/backfill_payment_links.py`: backfill script with dry-run mode ## Test Plan - [x] Migration 048 already applied in prod — table has 98 rows - [x] Backfill script already executed successfully - [x] Pod running 1/1, healthz 200 - [ ] Verify `/email/outstanding-balance` endpoint with `test_email` param - [ ] Verify monthly checkout no longer prorates ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive - [ ] Passed automated review-fix loop ## Related Notes - Closes #523 - `project-westside-basketball` — parent project

ldraney added 1 commit

2026-05-18 12:32:45 +00:00

feat: payment links table, outstanding balance emails, and monthly billing cleanup

ci/woodpecker/pr/woodpecker Pipeline failed

Details

464b66fa59

Add payment_links table (migration 048) to track per-player Stripe Payment
Links for unified billing. Add outstanding balance email endpoint and query
for the blast system. Remove April-specific proration from monthly checkout
and first-payment email copy. Includes backfill script for existing players.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-05-18 12:36:55 +00:00

Author

Owner

PR #524 Review

Scope: Adds payment_links table (migration 048), PaymentLink model, outstanding-balance email endpoint + blast query, backfill script, and monthly billing cleanup. Already deployed to prod as hotfix; migration applied with 98 rows.

DOMAIN REVIEW

Model layer — Clean. Proper FK indexes, unique constraint on (player_id, product_id, tenant_id). Enum additions consistent with existing patterns.

Migration 048 — Correct create_table, proper FK/indexes. ALTER TYPE emailtype ADD VALUE IF NOT EXISTS is the right Postgres pattern. Downgrade can't remove the enum value (Postgres limitation) — acceptable.

Admin route — Auth-gated, groups by parent, error aggregation follows established pattern. test_email filter works correctly.

Email service — Properly escapes all user-supplied data. Uses _brand_wrapper consistently. Logs to email_log. Plain-text fallback included.

Backfill script — Two-phase (monthly links + pending orders). Dry-run by default — good safety pattern.

Checkout — Removes April-specific proration, charges flat monthly_fee. Aligns with Payment Link strategy.

BLOCKERS

1. No test coverage for new functionality.

New endpoint, email function, query function, backfill script, and model — all untested. Per standard criteria this is a blocker.

Mitigation context: This is deployed to prod as a hotfix with 98 rows of real data. Main is out of sync with prod until this merges.

NITS

backfill_payment_links.py:96 — product_type="subscription" should use ProductType.subscription for type consistency
backfill_payment_links.py:255 — Query.get() deprecated in SQLAlchemy 2.0, use db.get(Product, order.product_id)
email_queries.py duplicates grouped-by-parent HTML construction from email.py — DRY concern for later
checkout.py — _calculate_prorated_cents appears to be dead code after this change
Migration 048 downgrade doesn't remove outstanding_balance from emailtype (Postgres limitation) — worth a comment

VERDICT: NOT APPROVED

Reason: Zero test coverage on new functionality.

Recommended path: Given this is already deployed and the migration is applied, the pragmatic approach is to merge to unblock main, then immediately open a follow-up issue for test coverage. The code quality itself is solid — this is a process call, not a quality call.

## PR #524 Review **Scope**: Adds `payment_links` table (migration 048), `PaymentLink` model, outstanding-balance email endpoint + blast query, backfill script, and monthly billing cleanup. Already deployed to prod as hotfix; migration applied with 98 rows. --- ### DOMAIN REVIEW **Model layer** — Clean. Proper FK indexes, unique constraint on `(player_id, product_id, tenant_id)`. Enum additions consistent with existing patterns. **Migration 048** — Correct `create_table`, proper FK/indexes. `ALTER TYPE emailtype ADD VALUE IF NOT EXISTS` is the right Postgres pattern. Downgrade can't remove the enum value (Postgres limitation) — acceptable. **Admin route** — Auth-gated, groups by parent, error aggregation follows established pattern. `test_email` filter works correctly. **Email service** — Properly escapes all user-supplied data. Uses `_brand_wrapper` consistently. Logs to `email_log`. Plain-text fallback included. **Backfill script** — Two-phase (monthly links + pending orders). Dry-run by default — good safety pattern. **Checkout** — Removes April-specific proration, charges flat monthly_fee. Aligns with Payment Link strategy. --- ### BLOCKERS **1. No test coverage for new functionality.** New endpoint, email function, query function, backfill script, and model — all untested. Per standard criteria this is a blocker. **Mitigation context**: This is deployed to prod as a hotfix with 98 rows of real data. Main is out of sync with prod until this merges. --- ### NITS 1. `backfill_payment_links.py:96` — `product_type="subscription"` should use `ProductType.subscription` for type consistency 2. `backfill_payment_links.py:255` — `Query.get()` deprecated in SQLAlchemy 2.0, use `db.get(Product, order.product_id)` 3. `email_queries.py` duplicates grouped-by-parent HTML construction from `email.py` — DRY concern for later 4. `checkout.py` — `_calculate_prorated_cents` appears to be dead code after this change 5. Migration 048 downgrade doesn't remove `outstanding_balance` from emailtype (Postgres limitation) — worth a comment --- ### VERDICT: NOT APPROVED **Reason**: Zero test coverage on new functionality. **Recommended path**: Given this is already deployed and the migration is applied, the pragmatic approach is to merge to unblock main, then immediately open a follow-up issue for test coverage. The code quality itself is solid — this is a process call, not a quality call.

forgejo_admin referenced this pull request from a commit

2026-05-18 12:57:10 +00:00

feat: payment links table + outstanding balance emails (#524)

ci/woodpecker/pr/woodpecker Pipeline failed

Details

This pull request has changes conflicting with the target branch.

alembic/versions/048_add_payment_links_table.py
scripts/backfill_payment_links.py

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/payment-links-outstanding-balance:feat/payment-links-outstanding-balance

git switch feat/payment-links-outstanding-balance

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff feat/payment-links-outstanding-balance

git switch feat/payment-links-outstanding-balance

git rebase main

git switch main

git merge --ff-only feat/payment-links-outstanding-balance

git switch feat/payment-links-outstanding-balance

git rebase main

git switch main

git merge --no-ff feat/payment-links-outstanding-balance

git switch main

git merge --squash feat/payment-links-outstanding-balance