pal-e-docs

Review: Fix confirmation email skipped for existing Keycloak accounts + token=None in profile URL

review-891-2026-04-08-v2 Doc

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — discovered during registration flow validation (2026-04-07)
  • [x] Repo — forgejo_admin/basketball-api
  • [x] What Broke — two issues clearly described (email skip + token=None)
  • [x] Repro Steps — 4-step repro
  • [x] Expected Behavior — clear expected outcome
  • [x] Environment — production, basketball-api, promo registration path
  • [x] File Targets — 3 targets with specific line references
  • [x] Acceptance Criteria — 4 ACs covering both paths + regression
  • [x] Test Expectations — 3 unit tests + run command
  • [x] Constraints — credentials handling documented
  • [x] Checklist — PR, tests, no unrelated changes
  • [x] Related — westside-basketball

Traceability

  • [x] story:WS-S22 — "As a parent, I want to receive clear email communications with action links so that I never miss a deadline" (verified in project-westside-basketball user-stories section, Parent list)
  • [x] arch:email — email component
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-email for component email (tracked separately as discovered scope, not a blocker for this ticket)
  • [x] Forgejo issue — forgejo_admin/basketball-api#390, open

File Targets

  • [x] src/basketball_api/routes/register.py:~1402 — verified: if keycloak_credentials: guard at line 1402 wraps send_confirmation_email() call at line 1406. When create_account_for_parent() returns None for existing accounts, email is skipped.
  • [x] src/basketball_api/routes/webhooks.py:~347 — verified: identical if keycloak_credentials: guard at line 347 wraps send_confirmation_email() at line 356. Same bug in webhook path.
  • [x] src/basketball_api/services/registration.py:77-78 — verified: registration_token generation at lines 77-78, conditional on if not parent.registration_token. Runs for new parents; needs verification for promo re-registration where parent already exists with a token.
All targets are specific enough for an agent to act on without guessing.

Repo Placement

OK. Issue filed on basketball-api, all three file targets are in basketball-api. Single-repo fix. No multi-repo concerns.

Dependencies

  • [x] send_confirmation_email() already accepts credentials=None — parameter signature is credentials: Credentials | None = None at email.py:72. No upstream changes needed.
  • [x] No blocking board items — #891 is independent in backlog.
No unresolved dependencies.

Acceptance Criteria

  • AC #1: Promo path with existing Keycloak account sends email — testable via unit test mocking create_account_for_parent to return None
  • AC #2: Profile URL has valid token (not None) — testable by asserting registration_token is set before email construction
  • AC #3: Webhook path with existing Keycloak account sends email — testable via unit test (added in refinement)
  • AC #4: New Keycloak accounts still get credentials in email — regression test, testable
All ACs are specific and programmatically verifiable. Run command pytest tests/ -k test_register is valid.

Blast Radius

send_confirmation_email() is called in 3 locations:
  • register.py:1406 — promo path (THIS BUG, guarded by if keycloak_credentials)
  • webhooks.py:356 — webhook/card payment path (THIS BUG, same guard)
  • registration.py:126 — complete_registration_from_checkout path — already calls unconditionally, no bug here
The registration_token field is consumed by 7 route files (admin, jersey, tryouts, register, checkout, email). All are read-only consumers after token generation. No downstream risk from ensuring the token exists earlier.
Rollback is straightforward — revert single PR.

Decomposition Assessment

Apply three-thing limit and five-minute rule:
  • 2 file changes (register.py + webhooks.py) + 1 supporting verification (registration.py) — all in 1 repo. Under the 3-file threshold.
  • 4 ACs — under the 5 AC threshold.
  • Estimated agent time: ~3 minutes. Under the 5-minute rule.
  • No independent subtasks that need parallelization — changes are coupled.
No decomposition needed.

Recommendation

No action needed. Scope is solid after refinements. The missing arch-email note is tracked as discovered scope and is not a blocker for this ticket.