pal-e-docs

Review: Fix base_url to frontend_url in email/tryout links

review-206-2026-03-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] What Broke
  • [x] Repro Steps
  • [x] Expected Behavior
  • [x] Environment (partial -- no commit SHA, acceptable for a discovered-scope bug)
  • [x] File Targets
  • [x] Acceptance Criteria
  • [x] Related
  • [ ] Related -- missing project slug reference (should mention westside-basketball)

Traceability

  • [ ] story:X label -- missing. This is a bug fix for registration email links which maps to user story WS-S17 (player registration flow). Should be labeled story:WS-S17.
  • [ ] arch:X label -- missing. Affects basketball-api email service and tryouts route. Foundational bug fix, acceptable without arch label.
  • [x] Forgejo issue -- forgejo_admin/basketball-api#105, open

File Targets

  • [ ] src/basketball_api/services/email.py line 56 -- ISSUE: Line 56 is a blank line. The actual settings.base_url usage is at line 83: reg_url = f"{settings.base_url}/register?token={parent.registration_token}". This is genuinely user-facing and DOES need to change to frontend_url.
  • [ ] src/basketball_api/routes/tryouts.py line 509 -- ISSUE: This builds a photo_url pointing to an API-hosted resource (player photo served by basketball-api, likely MinIO-proxied). This should remain settings.base_url. Changing it to frontend_url would break photo loading since the frontend does not serve photos.
  • [x] src/basketball_api/routes/tryouts.py line 795 -- verified: reg_url = f"{settings.base_url}/register?token=...". This is a user-facing registration link rendered in a "Copy Link" button. Correctly identified as needing frontend_url.

Repo Placement

OK. All changes are in forgejo_admin/basketball-api. No cross-repo work needed. The frontend_url setting already exists in config.py (line 29) and is already used in 15+ other places across the codebase (jersey.py, register.py, password_reset.py, email.py lines 93/429/430/480/512/695/700/732/846/1151, checkout.py). PR #104 introduced this config value.

Dependencies

No blockers. All phase-11 items that this could depend on are in done column. PR #104 (which introduced frontend_url) is already merged. No items blocked by this ticket.

Acceptance Criteria

Criteria are testable but could be more specific:
  • "All user-facing URLs in emails use settings.frontend_url" -- good but vague. Should specify the exact 2 locations (email.py:83, tryouts.py:795).
  • "API-internal URLs still use settings.base_url" -- good guard rail.
  • Missing: explicit criterion for photo_url remaining unchanged (since ticket incorrectly targets it).
  • Test command pytest tests/ is real and runnable.

Blast Radius

Full settings.base_url audit across the codebase reveals 5 usages:
  • email.py:83 -- reg_url in confirmation email. Needs fix (user-facing).
  • tryouts.py:509 -- photo_url. Keep as-is (API resource).
  • tryouts.py:795 -- reg_url in admin roster "Copy Link." Needs fix (user-facing).
  • admin.py:203 -- coach invite URL. Keep as-is (coach onboarding is served by the API at /coach/onboard, confirmed no SvelteKit route exists).
  • coach_onboarding.py:112 -- Stripe refresh/return URLs. Keep as-is (Stripe callbacks hit the API server).
The ticket correctly identifies 2 of the 3 locations it lists, but incorrectly includes the photo_url. No other hidden usages need attention.

Decomposition

2 lines changed across 2 files in 1 repo, 2 acceptance criteria. Well within single-agent scope. No decomposition needed.

Recommendation

Before moving to next_up, update Forgejo issue #105:
  • Fix email.py line reference: Change "line 56" to "line 83" in the File Targets section.
  • Remove tryouts.py:509 from scope: The photo_url correctly uses base_url because photos are served by the API. Only tryouts.py:795 (reg_url) needs the fix.
  • Add story label: Add story:WS-S17 to the board item labels (registration flow user story).
  • Optional: Add project reference westside-basketball to the Related section.