# Season ↔ Tournament Drift Audit

**Date:** July 17, 2026
**Method:** Parallel review of every player-facing "twin" — code copied from the season flow to create the tournament flow (or vice versa) — hunting fixes that landed on one side only. Five review passes: cancellation forms, GroupController branches, RegistrationController branches, OrderCompleteSubscriber branches, notifications + admin GroupInvitationsForm. Key findings spot-verified against source.
**Companions:** `INVITATION_FLOW_ANALYSIS.md`, `ORDER_FLOW_EDGE_CASES.md` (findings there are not repeated).

---

## 🔴 Critical

### D1 — Season Commerce refund is off by 100x (money bug) — VERIFIED
`CancelRegistrationForm.php:526-527`:
```php
$amount_cents = (string) ($amount_dollars * 100);
$refund_amount = new Price($amount_cents, ...);
```
`commerce_price\Price` takes **dollars**, not cents — the tournament twin does it right (`TournamentCancelRegistrationForm.php:376`, no `*100`), as does `TournamentDepositRefundForm.php:220`. Practical behavior: refunds larger than 1/100 of the captured payment throw/decline (refund > capture), so **the season "Refund via Commerce" option is effectively broken** — and the dangerous case is a small partial refund: refund $0.50 on a $50 order → succeeds as a **$50 refund**. Do not use the season Commerce-refund option until fixed (credit option is unaffected). One-line fix; tournament side is the template.

**Confirmed in production July 17:** order 87 (Avi, $133, cancelled Jul 8) — payment shows "Refunded: $0.00", state Completed, no credits issued. The refund silently failed via this exact path; player was never notified. Remediated manually via the order's Payments tab (Commerce core refund).

**✅ IMPLEMENTED July 17, 2026** (uncommitted working-tree change, `CancelRegistrationForm.php`): dropped the `* 100` (Price takes dollars, matching the tournament forms); validation now rejects `$0` refunds; the refund-failure message now states explicitly that no money moved and the player was not notified, with manual-remediation instructions. Deploy: code only, `drush cr`. Test on LOCAL/TEST with a test-gateway order: commerce refund of a normal amount succeeds and payment state flips to Refunded; refund > captured amount shows the failure message.

### D2 — Admin-accepting a tournament invitation crashes mid-transaction — VERIFIED
`GroupInvitationsForm.php:1279` queries `loadByProperties(['tournament' => $context_id, ...])`, but the Invitation entity **has no `tournament` field** (only `season`, `group_id`, `team` — `Invitation.php:83-112`). Entity query on an undefined field throws. The call happens *after* the registration was already mutated and saved (`acceptInvitation()` → line ~1238), so the admin gets an error page with half-committed state: player joined the team, invitation left `pending`, competing invites never declined. Correct pattern exists in `GroupController::acceptTeamInvitation()` (filter by `isTeamInvite()` + team's tournament). Season branch uses the real `season` field and works.

### D3 — Tournament order completion never re-checks that the tournament is still open
Season completion hard-blocks and flags the order if the season went inactive/hidden mid-checkout (`OrderCompleteSubscriber.php:364`, Finding F). The tournament branch has **no check of `active`, `registration_visible`, or `status`** — a player checking out while an admin cancels the tournament still gets a `paid` registration, and the `create` branch still spawns a Team entity + team-name taxonomy term for a dead tournament. Money captured, no flag, no admin surface. Port the season guard (+ flag write) to `createTournamentRegistration()`.

---

## 🟠 High

### D4 — Co-captains can see manager controls but every remove fails
`manage()` treats captain OR co-captain as manager (`isTeamLeader()`, GroupController:404) and renders Remove buttons — but `removeMember()` gates on the season rule only (`invited_by` empty, line 1465). A co-captain always has `invited_by` set → **"Only the group manager can remove members"** on every click. Meanwhile co-captain *invite* works. Fix: tournament branch of `removeMember()` should use `isTeamLeader()`.

### D5 — Tournament cancellation notifies nobody
Season cancel emails the player in all three refund branches (`sendRegistrationCancelled`, CancelRegistrationForm:392-435). Tournament cancel (`TournamentCancelRegistrationForm.php:238-327`) sets status, processes the refund, messages the **admin's screen only** — the player is silently dropped (and possibly silently refunded). Also cross-check with ORDER_FLOW S1: even the season email is preference-gated.

### D6 — Tournament cancel leaves the player on the team roster
Neither cancel form removes the player from `Team.players` or clears `group_id`/`invited_by` — but the module already has the correct helper doing exactly this: `TournamentTeamManager::unassignPlayerFromTeam()` (TournamentTeamManager.php:470-503). The cancel form just doesn't call it. Ghost roster entries inflate `isFull()` and block real joins. (Season half of this is INVITATION doc E4.)

### D7 — Admin accept/decline desyncs `Team.players` from registrations
`GroupInvitationsForm::submitForm()` sets/clears `registration.team` (lines ~926-936) but never edits the Team entity's `players` list — the player-facing paths keep both in sync. Admin-declined players remain ghost roster members; admin-accepted players are missing from the roster. Tournament-only (seasons have no Team entity).

---

## 🟡 Medium

- **D8 — `removeMember()` invitation cleanup missing on BOTH branches** (extends the Myk bug): the tournament branch also leaves the accepted invitation, and the duplicate-invite check then blocks the captain from ever re-inviting the removed player. `leaveGroup()` (lines 1640-1652) is the correct pattern to copy into both branches.
- **D9 — Season list shows inactive seasons.** Season list query filters `registration_visible` only (RegistrationController:176-179); tournament list also excludes dead statuses. A season with `active=FALSE, visible=TRUE` renders a live Register button that dead-ends at `addSeasonToCart`'s active guard.
- **D10 — Tournament eligibility is enforced only at add-to-cart.** Seasons compute `ineligible` and hide the card; `getTournamentState` does no age check, so underage players see a live card and get rejected after clicking. Also: `checkUserAge`'s override branch is gated to `$entity->getEntityTypeId() === 'season'` (line ~1245) — an age-override granted for a tournament would not be honored.
- **D11 — Gender is checked for display, not action (season-internal).** `addSeasonToCart` re-checks age but not the mens35 gender rule — direct URL lets a wrong-gender user into checkout for a hidden season. Display-guard vs action-guard gap.
- **D12 — Tournament close date has no fallback.** Tournament "closed" state derives only from `registration_close`; if unset, registration stays open forever. Seasons route through `Season::isRegistrationOpen()`. (Tournament entity has its own `isRegistrationOpen()` — the controller just doesn't use it.)
- **D13 — Season group counts include cancelled registrations.** `getGroupSize()` and `manage()`'s roster query have no status filter (myRegistrations remembered the filter; capacity paths didn't). Tournament avoids this by deriving counts from the curated `players` field. Add `status NOT IN (cancelled, expired)` to the season queries.
- **D14 — Nudge redirect never adapted for tournaments.** `nudge()` looks up the registration by `season` only (GroupController:1102-1107) — NULL for team invites, so tournament nudges land on My Registrations instead of the team page. `deleteInvitation()` (1133-1159) is the already-fixed pattern to copy. Email itself sends fine (subject to the invitee_uid gate).
- **D15 — Admin form's paid-gating differs by type.** `buildMembersTable()` requires a `paid` season registration before offering accept, but applies no status filter for tournaments — an unpaid/cancelled tournament registrant can be attached to a team by an admin.

## ⚪ Low / policy decisions (flagged, possibly by design)

- **D16 — Tournament players have no self-leave path** (`leaveGroup` explicitly blocks tournaments) and co-captain removal is broken (D4) → a player who wants off a team has no path but the captain. Season members self-serve.
- **D17 — No tournament equivalent of `groups_locked`.** Seasons have a full roster-freeze lifecycle enforced across invite/accept/remove/leave; tournaments have none. If the board expects to freeze tournament rosters before scheduling, the mechanism doesn't exist.
- **D18 — Season cancel records no `cancellation_date`** (tournament does — port it); season refund validation lets `$0` through (tournament requires > 0); season redirect trusts a hidden field the tournament form defensively re-derives.
- **D19 — `invitation_status` field written only on season registrations** — model difference, flag in case shared UI ever counts on it.
- **D20 — Admin accept/decline sends no notifications on either side** (player paths do). Admin-vs-player drift rather than season-vs-tournament, noted for completeness.

## Verified symmetric (no action)
Token branch of `available()`, pending-invitations banner, decline flows, `deleteInvitation`, `sendPlayerJoined/Declined/Removed` wiring in GroupController, order-completion dedupe/status/cache handling, capacity-check philosophy (season = player cap, tournament = team cap; `isFull` vs `isFullIncludingPending` used correctly per invited/uninvited join), tournament having no credit option (CreditsPane documented as season-only), waitlist/override being season-only features.

---

## Suggested sequencing

1. **D1 today** (one line; until then, don't use season "Refund via Commerce").
2. **D2, D3, D4** — small, well-bounded fixes; D3 reuses the season guard verbatim.
3. **D5–D8** as a "cancellation & roster hygiene" batch — overlaps heavily with the invitation PR's scope (removeMember, cleanup), so consider folding into or sequencing right after it.
4. D9–D15 as a checklist for a cleanup session; D16/D17 are board-policy questions to decide before next tournament season.

## Process note

Every serious finding across all three documents is the same failure mode: **twin code paths that drifted**. Two cheap guards for the future: (a) when fixing a bug in either flow, grep for the twin (`Tournament*` ↔ season equivalent) before closing the ticket; (b) longer term, the shared logic (invitation lifecycle, cancellation cleanup, refund math) wants to move into services both flows call — `TournamentTeamManager::unassignPlayerFromTeam()` shows the pattern, it's just not consistently used.
