# Code Quality / Performance Backlog

**Source:** Extracted July 5, 2026 from `CC_Soccer_Code_Review_2026_04_02.docx`
(now in `archive/`). Contains only the items from that review that are **not**
tracked elsewhere. Security-critical findings from the April review were
superseded by the June 13 and July 3, 2026 security reviews and are NOT
repeated here (see "Resolved / tracked elsewhere" at the bottom).

None of these are launch-blocking. The site is live; these are for the
off-season / next development cycle.

---

## Security-adjacent (do first when picking this list up)

### B-1. XSS via `#markup` HTML concatenation in checkout panes — HIGH
**Files:** `CompletionPane.php`, `AgreementsPane.php`, `TournamentController.php`, `AdminController.php`
Checkout panes build HTML strings by concatenation and inject them into
`#markup`. CompletionPane constructs full HTML cards with variable
interpolation; AgreementsPane renders admin-configured waiver text as raw
markup without sanitization; TournamentController builds HTML `<select>`
elements as raw strings.
**Fix:** Replace `#markup` concatenation with proper render arrays. Use
`#type => 'processed_text'` for admin-configured HTML.

### B-2. Audit 139+ instances of `accessCheck(FALSE)` — HIGH
Entity queries throughout the module bypass Drupal's access system. Many are
justified (admin operations), but unaudited bypasses could leak data if
permission boundaries shift.
**Fix:** Audit each instance; change to `accessCheck(TRUE)` where possible;
add an inline justification comment where the bypass is intentional.

### B-3. Input validation on AJAX endpoints — MEDIUM
AJAX endpoints extract values from JSON bodies without type validation (team
IDs / game IDs not checked numeric; position fields not validated against
home/away enums).
**Fix:** `is_numeric()`, `in_array()` for enums, explicit casts in each AJAX
controller method.

---

## Performance

### B-4. N+1 queries in skill calculation and team tables — HIGH
**Files:** `Team.php` (`calculateSkillLevelFromPlayers()`), `TournamentController.php`, `GroupPane.php`
Loads all players then queries registrations per player; tournament tables
load captains and calculate skill per team in loops. ~300+ queries for 20
teams of 16. Fine at ~250 players, will matter if league grows.
**Fix:** Batch-load related entities before loops (single query with IN
conditions); pre-calculate/cache skill levels.

### B-5. Missing database indexes — MEDIUM
**Files:** `WaitlistManagerService`, `PlayerAdminController`
Queries sort/filter on season + status + created and do leading-wildcard LIKE
searches on names without confirmed indexes.
**Fix:** Add composite indexes on frequently queried combinations.

### B-6. Aggressive cache disabling — MEDIUM
**Files:** `AdminController.php` and others
Multiple render arrays set `#cache max-age 0`; `Registration.php` invalidates
cache on every save regardless of what changed.
**Fix:** Proper cache tags/contexts instead of max-age 0; invalidate only on
relevant field changes. (Note: the July 5 series-rules cache-tag fix is the
pattern to follow.)

---

## Architecture

### B-7. No automated test coverage — HIGH
No tests exist for payment processing, credit calculations, registration, or
notification delivery — the financial flows. Biggest single risk on this list.
**Fix:** Start with kernel tests for `OrderCompleteSubscriber` (credits,
idempotency flag, season-full flags) and `CreditManagerService`.

### B-8. Missing transaction boundaries — MEDIUM
**Files:** `CreditManagerService` (credit splitting), bulk roster assignments, schedule generation
Multi-step operations don't use DB transactions; a mid-operation failure
leaves inconsistent state.
**Fix:** Wrap in `$this->database->startTransaction()` with rollback on
exception.

### B-9. Missing cascade-delete handling — MEDIUM
Deleting a Season or Tournament leaves orphaned Games, Teams, and
Registrations.
**Fix:** Implement `hook_entity_predelete()` cleanup or block deletion while
children exist.

### B-10. Incomplete service layer / stub services — MEDIUM
`SeasonRegistrationService` and `TournamentRegistrationService` are stubs
registered in `services.yml`. Runtime error risk if anything depends on them.
**Fix:** Complete or remove.

### B-11. Generic entity access handlers — LOW
All 11 custom entities use the generic `EntityAccessControlHandler`.
Registration and Override deserve custom access logic.

### B-12. DI inconsistencies — LOW
Mix of proper constructor injection and static `\Drupal::` calls. Standardize
on injection (blocks unit testing, see B-7).

---

## Code quality (opportunistic — fix when touching the file)

- **B-13.** Fat controllers: `TournamentController.php` (1,093 lines),
  `GroupController.php`, `AdminController.php` — extract business logic to
  services. (Overlaps checklist item 31, team handling refactor.)
- **B-14.** Duplicate code: `Registration.php` duplicate return (~line 269);
  Tournament vs Season controller patterns could share a base class.
- **B-15.** Magic strings for statuses ('pending', 'active', 'paid') — create
  a StatusConstants class; add type declarations; use strict comparisons.
- **B-16.** Inconsistent date formatting — standardize on the
  `date.formatter` service with central format constants.
- **B-17.** Hardcoded values: store ID 1 in `CommerceProductService`, cents
  conversion 100, 48*60*60 — name them.
- **B-18.** Missing error handling in JS `fetch()` calls, notably
  `passkey-autotrigger.js`.
- **B-19.** Inline styles in PHP form builders (e.g. AgreementsPane) — move to
  CSS classes.

---

## Resolved / tracked elsewhere (do not re-do)

| April finding | Where it's handled |
|---|---|
| 1.1 CSRF on AJAX endpoints | Fixed March 16 (`fetchWithCsrf()`, 8 JS files); missed form routes fixed July 3 (items A/D) |
| 1.4 Access checks on read endpoints | Fixed March 16 (`hasPermission()` in 8 controllers) |
| 1.6 GroupPane session token handling | Fixed June 23–24 (token validated against current user, self-clears on masquerade) |
| 2.3 Exception messages exposed to users | Open, tracked as July 3 review item K |
| Inconsistent escaping | Open, tracked as July 3 review item K |
| SQL injection in ReportController raw queries | Verified safe July 3 ("SQL injection — none"; all sites parameterized + escapeLike) |
| 5.1 CSS consolidation (30+ files) | Checklist item 30 |
| Fat controller / team refactor overlap | Checklist items 29–31 |