fix: match paid player in token flow despite name mismatch #42

Merged
forgejo_admin merged 1 commit from fix/token-player-dedup into main 2026-03-10 06:18:53 +00:00

Summary

  • Fix "Payment Required" shown after parent completes registration via token link
  • Root cause: Stripe player name ("Creed Draney") != form player name ("Creed") creating a duplicate player with no paid registration

Changes

  • src/basketball_api/routes/register.py: When name match fails in token flow, fall back to matching the parent's single paid player. Updates name to form value. Siblings (multiple paid players) still use name matching.

Test Plan

  • Parent with token link fills form with different player name than Stripe → uses existing paid player, shows "Payment Received"
  • Parent with multiple paid players still uses name matching (no false dedup)
  • Walk-up flow (no token) unchanged

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive
  • Closes #20 — Phase 3b email blast (fixes the registration flow that token emails link to)
  • plan-2026-03-08-tryout-prep — Phase 3b
## Summary - Fix "Payment Required" shown after parent completes registration via token link - Root cause: Stripe player name ("Creed Draney") != form player name ("Creed") creating a duplicate player with no paid registration ## Changes - `src/basketball_api/routes/register.py`: When name match fails in token flow, fall back to matching the parent's single paid player. Updates name to form value. Siblings (multiple paid players) still use name matching. ## Test Plan - [ ] Parent with token link fills form with different player name than Stripe → uses existing paid player, shows "Payment Received" - [ ] Parent with multiple paid players still uses name matching (no false dedup) - [ ] Walk-up flow (no token) unchanged ## Review Checklist - [ ] Passed automated review-fix loop - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related - Closes #20 — Phase 3b email blast (fixes the registration flow that token emails link to) - `plan-2026-03-08-tryout-prep` — Phase 3b
fix: match paid player in token flow despite name mismatch
Some checks failed
ci/woodpecker/pr/woodpecker Pipeline failed
f3acb9ea28
When a parent arrives via token link (already paid via Stripe), the
form's player name often doesn't match the Stripe checkout name
(e.g. "Creed" vs "Creed Draney"). Previously this created a duplicate
player with no paid registration, showing "Payment Required" on the
confirmation page.

Now: if exact name match fails AND parent came via token AND has
exactly one paid player, use that player. Updates the name to what
the parent typed in the form. Handles the common case while still
supporting siblings (multiple paid players keep name-based matching).

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

PR #42 Review

BLOCKERS

None. The fix is logically correct and well-scoped.

NITS

  1. No automated test for the new fallback path. The existing test_register.py covers the token pre-fill flow and the name-match dedup, but there is no test for the specific scenario this PR fixes: token flow where the form player name differs from the Stripe player name, with a single paid player under the parent. A test like this would pin down the fix:

    • Seed a parent with a token, a player named "Creed Draney" with a paid registration.
    • POST /register with token=... and player_name="Creed".
    • Assert: only 1 player exists (not 2), player name updated to "Creed", confirmation shows "Payment Received."
    • Bonus: a sibling variant with 2 paid players that confirms name matching is still required.

    Not blocking given the time-sensitive nature, but this should be backfilled.

  2. Issue reference mismatch. The PR body says "Closes #20" but issue #20 is "Phase 3b email blast," not a player dedup bug. There does not appear to be a dedicated Forgejo issue for this dedup bug. Consider either creating one or correcting the reference so the issue link is accurate. This matters for audit trail.

  3. Branch naming. Branch is fix/token-player-dedup rather than the SOP convention of naming after an issue number (e.g., issue-XX/...). Related to point 2 above -- no dedicated issue exists to name it after.

SOP COMPLIANCE

  • Branch named after issue -- branch is fix/token-player-dedup, no corresponding issue number
  • PR body follows template -- Summary, Changes, Test Plan, Related all present
  • Related references plan slug -- plan-2026-03-08-tryout-prep referenced
  • No secrets committed -- single file change in routes/register.py, no credentials
  • No unnecessary file changes -- 1 file, 26 additions / 3 deletions, tightly scoped
  • Commit message is descriptive -- fix: match paid player in token flow despite name mismatch

Code Analysis

Correctness of the fallback logic (register.py lines 809-828):

  • The len(paid_players) == 1 guard correctly prevents false dedup when a parent has multiple paid siblings. With 2+ paid players and no name match, the code falls through and creates a new player -- correct behavior.
  • The if not existing_player and form_token: gate ensures this fallback ONLY fires in the token flow (paid parents clicking email links). Walk-up flow (no token) is completely unaffected.
  • After matching the paid player, existing_player.name = player_name on line 832 updates the Stripe-sourced name to the form value. Downstream, player.name is used for the confirmation page, so the parent sees the name they typed.
  • The log message on lines 823-828 correctly distinguishes form name (player_name) from Stripe name (existing_player.name) for debugging.

Edge case considered: Parent arrives via token, has 1 paid player, AND names happen to match (case-insensitive). The first query (line 799-806) catches this, so the fallback never fires. No double-match risk.

VERDICT: APPROVED

The fix is correct, minimal, and safe. The single-paid-player heuristic with the token-flow guard is the right approach for the known Stripe-vs-form name mismatch problem. The SOP gaps (no dedicated issue, branch not named after issue) and missing automated test are non-blocking given this is a time-sensitive bug fix for the March 13 tryout. Recommend backfilling the test and correcting the issue reference post-merge.

## PR #42 Review ### BLOCKERS None. The fix is logically correct and well-scoped. ### NITS 1. **No automated test for the new fallback path.** The existing `test_register.py` covers the token pre-fill flow and the name-match dedup, but there is no test for the specific scenario this PR fixes: token flow where the form player name differs from the Stripe player name, with a single paid player under the parent. A test like this would pin down the fix: - Seed a parent with a token, a player named "Creed Draney" with a paid registration. - POST `/register` with `token=...` and `player_name="Creed"`. - Assert: only 1 player exists (not 2), player name updated to "Creed", confirmation shows "Payment Received." - Bonus: a sibling variant with 2 paid players that confirms name matching is still required. Not blocking given the time-sensitive nature, but this should be backfilled. 2. **Issue reference mismatch.** The PR body says "Closes #20" but issue #20 is "Phase 3b email blast," not a player dedup bug. There does not appear to be a dedicated Forgejo issue for this dedup bug. Consider either creating one or correcting the reference so the issue link is accurate. This matters for audit trail. 3. **Branch naming.** Branch is `fix/token-player-dedup` rather than the SOP convention of naming after an issue number (e.g., `issue-XX/...`). Related to point 2 above -- no dedicated issue exists to name it after. ### SOP COMPLIANCE - [ ] Branch named after issue -- branch is `fix/token-player-dedup`, no corresponding issue number - [x] PR body follows template -- Summary, Changes, Test Plan, Related all present - [x] Related references plan slug -- `plan-2026-03-08-tryout-prep` referenced - [x] No secrets committed -- single file change in `routes/register.py`, no credentials - [x] No unnecessary file changes -- 1 file, 26 additions / 3 deletions, tightly scoped - [x] Commit message is descriptive -- `fix: match paid player in token flow despite name mismatch` ### Code Analysis **Correctness of the fallback logic** (`register.py` lines 809-828): - The `len(paid_players) == 1` guard correctly prevents false dedup when a parent has multiple paid siblings. With 2+ paid players and no name match, the code falls through and creates a new player -- correct behavior. - The `if not existing_player and form_token:` gate ensures this fallback ONLY fires in the token flow (paid parents clicking email links). Walk-up flow (no token) is completely unaffected. - After matching the paid player, `existing_player.name = player_name` on line 832 updates the Stripe-sourced name to the form value. Downstream, `player.name` is used for the confirmation page, so the parent sees the name they typed. - The log message on lines 823-828 correctly distinguishes form name (`player_name`) from Stripe name (`existing_player.name`) for debugging. **Edge case considered**: Parent arrives via token, has 1 paid player, AND names happen to match (case-insensitive). The first query (line 799-806) catches this, so the fallback never fires. No double-match risk. ### VERDICT: APPROVED The fix is correct, minimal, and safe. The single-paid-player heuristic with the token-flow guard is the right approach for the known Stripe-vs-form name mismatch problem. The SOP gaps (no dedicated issue, branch not named after issue) and missing automated test are non-blocking given this is a time-sensitive bug fix for the March 13 tryout. Recommend backfilling the test and correcting the issue reference post-merge.
forgejo_admin deleted branch fix/token-player-dedup 2026-03-10 06:18:53 +00:00
Sign in to join this conversation.
No description provided.