pal-e-docs

Review: settings.base_url still used in 7 locations after frontend_url migration

review-601-2026-03-28 Review

review block
Project

Verdict: BLOCK

The ticket's core assumption is wrong. settings.base_url and settings.frontend_url are two distinct config fields with different semantics. Replacing one with the other would break production features.

Template Completeness

Issue declares ### Type: Feature. Validated against template-issue-feature.
  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets (present but targets are wrong -- see below)
  • [x] Acceptance Criteria (present but criteria are wrong -- see below)
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related
All template sections are present. However, the board item labels say type:bug while the Forgejo issue says ### Type: Feature. Minor mismatch.

Traceability

  • [x] story:WS-S5 label -- westside core platform story
  • [x] arch:basketball-api label -- correct repo
  • [x] Forgejo issue -- forgejo_admin/basketball-api#215, open

File Targets

CRITICAL: The ticket's file targets are fundamentally wrong.
The config model at src/basketball_api/config.py has TWO separate fields:
  • base_url: str = "http://localhost:8000" (line 26) -- API server URL, used for Stripe callbacks, photo URL construction, coach invite links (all API routes)
  • frontend_url: str = "https://westsidekingsandqueens.tail5b443a.ts.net" (line 29) -- Frontend app URL, used for user-facing links in emails, password reset, etc.
There is NO compatibility alias or property. These are two independent fields serving different purposes.
The 7 settings.base_url references break down as:
src/ -- 3 occurrences (all CORRECT usage of base_url):
  • [x] src/basketball_api/routes/admin.py:208 -- builds coach invite URL /coach/onboard?token=... -- this is an API route, correctly uses API base URL
  • [x] src/basketball_api/routes/tryouts.py:511 -- builds absolute photo URL from relative path -- photos are served by the API server, correctly uses API base URL
  • [x] src/basketball_api/services/coach_onboarding.py:112 -- Stripe Connect return/refresh URLs (/coach/onboard/stripe-*) -- these are API routes, correctly uses API base URL
scripts/ -- 1 occurrence (possibly stale):
  • [ ] scripts/send_registration_emails.py:100 -- patches settings.base_url = args.base_url, but the email service (services/email.py) now uses settings.frontend_url for all links. This patch is dead code. ISSUE: Script should patch frontend_url instead, but this is a different bug than what the ticket describes.
tests/ -- 3 occurrences (correctly test base_url behavior):
  • [x] tests/test_tryouts.py:666 -- asserts photo URL uses settings.base_url, matches the production code it tests
  • [x] tests/test_keycloak_integration.py:170,268 -- sets mock_settings.base_url for test isolation, matches the production code it tests

Repo Placement

Correct -- issue is filed on basketball-api and all file targets are in basketball-api.

Dependencies

No blocking dependencies. Board item #206 (done) was the original base_url -> frontend_url fix for email/tryout links -- that work is complete. Board item #514 (done) was the validation that discovered this ticket. No items in in_progress touch basketball-api config.

Acceptance Criteria

INVALID. The acceptance criteria assume all settings.base_url references should become settings.frontend_url. Executing these criteria would:
  • Break Stripe Connect onboarding (return/refresh URLs would point to frontend instead of API)
  • Break photo URL construction (photos are served by the API, not the frontend)
  • Break coach invite URLs (API route, not frontend route)
The criterion "remove compatibility alias" is also invalid -- no alias exists. These are two independent fields.

Blast Radius

If executed as written, this ticket would break:
  • Coach onboarding flow (Stripe Connect + invite links)
  • Photo display on roster pages
  • The send_registration_emails.py script (already partially broken -- separate concern)
No similar pattern exists in sibling services -- this is basketball-api specific config.

Decomposition Assessment

Not applicable in current form -- the ticket should not be executed as written. If rescoped to the actual work (fix the stale script patch + optionally rename base_url to api_url for clarity), the scope would be 1-2 files, well under the 5-minute rule, no decomposition needed.

Recommendation

  • [SCOPE] Close or fundamentally rescope this issue. The premise is wrong: base_url is not a leftover from a rename. It is a separate config field (API server URL) that is correctly used in all 3 src/ locations. Replacing it with frontend_url would break Stripe Connect callbacks, photo URLs, and coach invites.
  • [BODY] If the ticket is kept, it should be rescoped to: (1) fix the dead settings.base_url patch in scripts/send_registration_emails.py to patch settings.frontend_url instead, and (2) optionally rename base_url to something less ambiguous like api_url or api_base_url to prevent future confusion.
  • [LABEL] Fix board item type label: board says type:bug but Forgejo issue says ### Type: Feature. Align to one or the other.