email_log.player_id is NULL on /admin/email/blast sends (missing FK write) #447

New issue

Open

opened 2026-04-11 20:19:45 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-04-11 20:19:45 +00:00

Owner

Type

Bug

Lineage

Standalone — discovered 2026-04-10 during basketball-api#445 incident forensics. When querying email_log for the 3 wrong Apaisa emails, the player_id column was NULL on all 3 rows despite the query_unsigned_contracts function returning player data.

Repo

forgejo_admin/basketball-api

What Broke

The /admin/email/blast endpoint writes email_log rows without setting player_id, even when the query (e.g. query_unsigned_contracts) is player-scoped and returns a player per recipient.

Verified in prod:

SELECT id, recipient_email, email_type::text, sent_at, gmail_message_id, player_id
FROM email_log
WHERE recipient_email = 'apaisasandra@gmail.com'
ORDER BY sent_at DESC LIMIT 10;

Rows 548, 549, 550 — all contract_offer, all sent via /admin/email/blast on 2026-04-10 23:50 UTC — all have player_id = NULL. By contrast, older rows from other send paths (send_contract_signed_email, send_jersey_reminder_email, announcements) correctly have player_id populated.

The query_unsigned_contracts function returns dicts that include parent_id and enough data to derive player_id. The blast endpoint's send_templated_email() call passes parent_id to email_log but NOT player_id.

Repro Steps

Call POST /admin/email/blast with query=unsigned_contracts and any layout/data
After the send, query: SELECT player_id FROM email_log WHERE id = (SELECT MAX(id) FROM email_log);
Observe: player_id is NULL

Expected Behavior

When query_unsigned_contracts (or any player-scoped query) is the recipient source, each email_log row should have player_id set to the player whose contract triggered the email. This is:

Required for audit trail ("which player did this email reference?")
Required for the incident forensics we needed tonight (couldn't easily link email_log rows back to Apaisa players without joining through parent_id and guessing)
Consistent with how other send functions (send_contract_signed_email, etc.) populate the column

Environment

Cluster/namespace: prod (basketball-api)
Service version/commit: main at commit containing PR #426 merge (contract-offer endpoint landing — but this bug is in admin.py's /admin/email/blast handler, not the new endpoint)
Affected table: email_log (player_id column)
Impact: retrospective forensics harder for any blast-sent email; no user-facing breakage

Root Cause (likely)

query_unsigned_contracts returns dicts with parent_id (for FK) but the recipient dict doesn't have an explicit player_id key — player info is in player_name/team_name strings only. The blast endpoint's send_templated_email() call passes parent_id=recipient.get("parent_id") without a corresponding player_id=....

Expected Fix

Add player_id to the dict returned by query_unsigned_contracts (and any other player-scoped query in email_queries.py)
In /admin/email/blast handler in routes/admin.py, pass player_id=recipient.get("player_id") to send_templated_email()
send_templated_email() already accepts a player_id kwarg — verify it writes to email_log correctly

Acceptance Criteria

query_unsigned_contracts returns dicts with player_id key
/admin/email/blast passes player_id to send_templated_email()
New email_log rows from blast sends have player_id populated
Historical rows (548, 549, 550 etc) are NOT backfilled — just fix going forward
Test: integration test calls the blast endpoint, verifies the latest email_log row has non-NULL player_id

westside-basketball — project this affects
basketball-api#445 — the incident that surfaced this
basketball-api#TBD — player_ids filter on blast (companion ticket, same file targets)

### Type Bug ### Lineage Standalone — discovered 2026-04-10 during basketball-api#445 incident forensics. When querying email_log for the 3 wrong Apaisa emails, the `player_id` column was NULL on all 3 rows despite the `query_unsigned_contracts` function returning player data. ### Repo `forgejo_admin/basketball-api` ### What Broke The `/admin/email/blast` endpoint writes `email_log` rows without setting `player_id`, even when the query (e.g. `query_unsigned_contracts`) is player-scoped and returns a player per recipient. **Verified in prod**: ```sql SELECT id, recipient_email, email_type::text, sent_at, gmail_message_id, player_id FROM email_log WHERE recipient_email = 'apaisasandra@gmail.com' ORDER BY sent_at DESC LIMIT 10; ``` Rows 548, 549, 550 — all `contract_offer`, all sent via `/admin/email/blast` on 2026-04-10 23:50 UTC — all have `player_id = NULL`. By contrast, older rows from other send paths (`send_contract_signed_email`, `send_jersey_reminder_email`, announcements) correctly have `player_id` populated. The `query_unsigned_contracts` function returns dicts that include `parent_id` and enough data to derive `player_id`. The blast endpoint's `send_templated_email()` call passes `parent_id` to email_log but NOT `player_id`. ### Repro Steps 1. Call `POST /admin/email/blast` with `query=unsigned_contracts` and any layout/data 2. After the send, query: `SELECT player_id FROM email_log WHERE id = (SELECT MAX(id) FROM email_log);` 3. Observe: player_id is NULL ### Expected Behavior When `query_unsigned_contracts` (or any player-scoped query) is the recipient source, each email_log row should have `player_id` set to the player whose contract triggered the email. This is: - Required for audit trail ("which player did this email reference?") - Required for the incident forensics we needed tonight (couldn't easily link email_log rows back to Apaisa players without joining through parent_id and guessing) - Consistent with how other send functions (`send_contract_signed_email`, etc.) populate the column ### Environment - Cluster/namespace: prod (basketball-api) - Service version/commit: main at commit containing PR #426 merge (contract-offer endpoint landing — but this bug is in `admin.py`'s `/admin/email/blast` handler, not the new endpoint) - Affected table: `email_log` (player_id column) - Impact: retrospective forensics harder for any blast-sent email; no user-facing breakage ### Root Cause (likely) `query_unsigned_contracts` returns dicts with `parent_id` (for FK) but the recipient dict doesn't have an explicit `player_id` key — player info is in `player_name`/`team_name` strings only. The blast endpoint's `send_templated_email()` call passes `parent_id=recipient.get("parent_id")` without a corresponding `player_id=...`. ### Expected Fix 1. Add `player_id` to the dict returned by `query_unsigned_contracts` (and any other player-scoped query in `email_queries.py`) 2. In `/admin/email/blast` handler in `routes/admin.py`, pass `player_id=recipient.get("player_id")` to `send_templated_email()` 3. `send_templated_email()` already accepts a `player_id` kwarg — verify it writes to email_log correctly ### Acceptance Criteria - [ ] `query_unsigned_contracts` returns dicts with `player_id` key - [ ] `/admin/email/blast` passes `player_id` to `send_templated_email()` - [ ] New email_log rows from blast sends have player_id populated - [ ] Historical rows (548, 549, 550 etc) are NOT backfilled — just fix going forward - [ ] Test: integration test calls the blast endpoint, verifies the latest email_log row has non-NULL player_id ### Related - `westside-basketball` — project this affects - basketball-api#445 — the incident that surfaced this - basketball-api#TBD — `player_ids` filter on blast (companion ticket, same file targets)