fix: BASKETBALL_BASE_URL + coach Stripe interstitial #41

Merged

forgejo_admin merged 6 commits from fix/coach-ux-polish into main 2026-03-10 06:00:58 +00:00

Conversation 1 Commits 6 Files changed 4 +87 -12

forgejo_admin commented 2026-03-10 05:41:26 +00:00

Owner

Copy link

Summary

• Fix missing BASKETBALL_BASE_URL causing Stripe Connect HTTPS error in live mode
• Add interstitial page before Stripe redirect explaining payment setup to coaches

Changes

• k8s/deployment.yaml: Add BASKETBALL_BASE_URL=https://basketball-api.tail5b443a.ts.net env var
• src/basketball_api/routes/coach.py: Replace immediate Stripe redirect with branded interstitial page explaining what to expect

Test Plan

• Coach signup redirects to interstitial, then to Stripe (not direct redirect)
• Stripe Connect onboarding completes without HTTPS error
• Interstitial shows on all three paths: signup, onboard POST, onboard GET retry

Review Checklist

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

Related

• Fixes #21 — roster + tryout management (partial: deployment config fix)
• plan-2026-03-08-tryout-prep — Phase 3b-iv: Coach onboarding verification

## Summary - Fix missing BASKETBALL_BASE_URL causing Stripe Connect HTTPS error in live mode - Add interstitial page before Stripe redirect explaining payment setup to coaches ## Changes - `k8s/deployment.yaml`: Add `BASKETBALL_BASE_URL=https://basketball-api.tail5b443a.ts.net` env var - `src/basketball_api/routes/coach.py`: Replace immediate Stripe redirect with branded interstitial page explaining what to expect ## Test Plan - [ ] Coach signup redirects to interstitial, then to Stripe (not direct redirect) - [ ] Stripe Connect onboarding completes without HTTPS error - [ ] Interstitial shows on all three paths: signup, onboard POST, onboard GET retry ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Fixes #21 — roster + tryout management (partial: deployment config fix) - `plan-2026-03-08-tryout-prep` — Phase 3b-iv: Coach onboarding verification

forgejo_admin added 1 commit 2026-03-10 05:41:26 +00:00

fix: add BASKETBALL_BASE_URL to k8s + Stripe interstitial page ... 971c221be0

- Add BASKETBALL_BASE_URL env var to deployment.yaml (was missing,
  causing Stripe Connect to use http://localhost:8000 callback URLs
  which Stripe rejects in live mode)
- Add interstitial page before Stripe Connect redirect explaining
  what the coach should expect (payment setup, ~5 min, create
  Individual/Sole Proprietor if no Stripe account)

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

forgejo_admin commented 2026-03-10 05:42:52 +00:00

Author

Owner

Copy link

PR #41 Review

BLOCKERS

1. Four existing tests will break due to redirect-to-interstitial change.

The PR replaces RedirectResponse(url=onboarding_url, status_code=303) with _render_stripe_interstitial(coach, onboarding_url) in three code paths (signup_submit, onboard_page, accept_agreement). However, the existing tests still assert status_code == 303 and check resp.headers["location"]. These tests will now receive a 200 with HTML body instead:

• tests/test_coach.py TestOnboardPage.test_already_signed_no_stripe_creates_account (line 140-141) -- expects 303 redirect
• tests/test_coach.py TestAcceptAgreement.test_signs_and_redirects_to_stripe (line 173-174) -- expects 303 redirect
• tests/test_coach_signup.py TestSignupSubmit.test_creates_coach_and_redirects_to_stripe (line 92-93) -- expects 303 redirect
• tests/test_coach_signup.py TestSignupDuplicateEmail.test_unsigned_coach_gets_updated (line 198-199) -- expects 303 redirect

These tests need to be updated to assert status_code == 200 and check for interstitial content (e.g., "Payment Setup" or "Continue to Payment Setup" in resp.text) instead of checking redirect headers.

NITS

1. The interstitial heading says "Agreement Signed!" but the onboard_page GET path (line 165) also renders this interstitial for coaches who signed previously but have no Stripe account -- they are retrying Stripe setup, not freshly signing. The heading could be slightly misleading on that path. Minor, non-blocking.

2. No new tests were added for the interstitial rendering itself. A test verifying that _render_stripe_interstitial returns HTML containing the expected content (coach name, onboarding URL, "Continue to Payment Setup") would be good to have. Non-blocking for a hotfix.

SOP COMPLIANCE

• Branch named after issue number -- Branch is fix/coach-ux-polish, not named after an issue number. The PR references #21 but #21 is "roster + tryout numbers" which does not match this PR's scope. No dedicated issue exists for this fix.
• PR body follows template -- Has Summary, Changes, Test Plan, Related sections.
• Related references plan slug -- References plan-2026-03-08-tryout-prep.
• No secrets committed -- k8s deployment only references secret refs and a public Tailscale hostname. No API keys or credentials in the diff.
• Tests updated -- Existing tests not updated to match the behavioral change (redirect -> interstitial). CI will fail.
• No unnecessary file changes -- Two files changed, both directly related to the fix.
• Commit messages are descriptive -- Title clearly describes both changes.

VERDICT: NOT APPROVED

The code changes themselves are correct and well-scoped: the missing BASKETBALL_BASE_URL env var was causing Stripe Connect to build http://localhost:8000 callback URLs (from the default in config.py line 21), and the interstitial is a good UX improvement with proper escape() on user input. However, 4 existing tests will break because they assert 303 redirects on code paths that now return 200 HTML. These tests must be updated before merge to avoid a red CI pipeline.

## PR #41 Review ### BLOCKERS **1. Four existing tests will break due to redirect-to-interstitial change.** The PR replaces `RedirectResponse(url=onboarding_url, status_code=303)` with `_render_stripe_interstitial(coach, onboarding_url)` in three code paths (signup_submit, onboard_page, accept_agreement). However, the existing tests still assert `status_code == 303` and check `resp.headers["location"]`. These tests will now receive a 200 with HTML body instead: - `tests/test_coach.py` `TestOnboardPage.test_already_signed_no_stripe_creates_account` (line 140-141) -- expects 303 redirect - `tests/test_coach.py` `TestAcceptAgreement.test_signs_and_redirects_to_stripe` (line 173-174) -- expects 303 redirect - `tests/test_coach_signup.py` `TestSignupSubmit.test_creates_coach_and_redirects_to_stripe` (line 92-93) -- expects 303 redirect - `tests/test_coach_signup.py` `TestSignupDuplicateEmail.test_unsigned_coach_gets_updated` (line 198-199) -- expects 303 redirect These tests need to be updated to assert `status_code == 200` and check for interstitial content (e.g., `"Payment Setup"` or `"Continue to Payment Setup"` in `resp.text`) instead of checking redirect headers. ### NITS 1. The interstitial heading says "Agreement Signed!" but the `onboard_page` GET path (line 165) also renders this interstitial for coaches who signed previously but have no Stripe account -- they are retrying Stripe setup, not freshly signing. The heading could be slightly misleading on that path. Minor, non-blocking. 2. No new tests were added for the interstitial rendering itself. A test verifying that `_render_stripe_interstitial` returns HTML containing the expected content (coach name, onboarding URL, "Continue to Payment Setup") would be good to have. Non-blocking for a hotfix. ### SOP COMPLIANCE - [ ] **Branch named after issue number** -- Branch is `fix/coach-ux-polish`, not named after an issue number. The PR references #21 but #21 is "roster + tryout numbers" which does not match this PR's scope. No dedicated issue exists for this fix. - [x] **PR body follows template** -- Has Summary, Changes, Test Plan, Related sections. - [x] **Related references plan slug** -- References `plan-2026-03-08-tryout-prep`. - [x] **No secrets committed** -- k8s deployment only references secret refs and a public Tailscale hostname. No API keys or credentials in the diff. - [ ] **Tests updated** -- Existing tests not updated to match the behavioral change (redirect -> interstitial). CI will fail. - [x] **No unnecessary file changes** -- Two files changed, both directly related to the fix. - [x] **Commit messages are descriptive** -- Title clearly describes both changes. ### VERDICT: NOT APPROVED The code changes themselves are correct and well-scoped: the missing `BASKETBALL_BASE_URL` env var was causing Stripe Connect to build `http://localhost:8000` callback URLs (from the default in `config.py` line 21), and the interstitial is a good UX improvement with proper `escape()` on user input. However, **4 existing tests will break** because they assert 303 redirects on code paths that now return 200 HTML. These tests must be updated before merge to avoid a red CI pipeline.

forgejo_admin added 1 commit 2026-03-10 05:44:50 +00:00

fix: update tests for Stripe interstitial page (200 not 303) ... 7c48091c3a

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

forgejo_admin added 1 commit 2026-03-10 05:47:19 +00:00

feat: add Stripe onboarding guidance to coach interstitial ... 88f7a69331

Tell coaches exactly what to select: Individual/Sole Proprietor,
Recreational Camps industry, skip website, bring banking info.

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

forgejo_admin added 1 commit 2026-03-10 05:50:00 +00:00

feat: add 1099 tax reminder to coach interstitial ... a36011f6d4

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

forgejo_admin added 1 commit 2026-03-10 05:57:04 +00:00

fix: add Westside website URL to coach Stripe guidance ... 011adb1f46

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

forgejo_admin added 1 commit 2026-03-10 06:00:51 +00:00

feat: open Stripe onboarding in new tab so instructions stay visible ... c09470e149

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

forgejo_admin merged commit 1123753bb6 into main 2026-03-10 06:00:58 +00:00

forgejo_admin deleted branch fix/coach-ux-polish 2026-03-10 06:00:58 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-10 06:01:00 +00:00

fix: BASKETBALL_BASE_URL + coach Stripe interstitial (#41)

forgejo_admin referenced this pull request from a commit 2026-03-10 06:31:38 +00:00

chore: bump image to 7166559 (dedup fix + coach interstitial + retry)

forgejo_admin referenced this pull request 2026-03-10 06:31:56 +00:00

chore: bump image to 7166559 (dedup fix + coach interstitial + retry) #43

forgejo_admin referenced this pull request 2026-03-10 06:32:44 +00:00

chore: bump image to 7166559 (dedup fix + coach interstitial + retry) #43

forgejo_admin referenced this pull request 2026-03-11 00:31:15 +00:00

Phase 3c: Registration form improvements (photo required, division field, coach interstitial fix) #44

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

  

status:approved

QA passed, awaiting merge approval

  

status:in-progress

Dev agent is actively working

  

status:needs-fix

QA found issues, back to dev

  

status:qa

PR submitted, awaiting QA review

  

type:bug

Bug fix

  

type:devops

Infrastructure/CI/config work

  

type:feature

New functionality

No labels

domain:backend

domain:devops

domain:frontend

status:approved

status:in-progress

status:needs-fix

status:qa

type:bug

type:devops

type:feature

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!41

No description provided.