Add player_ids filter to query_unsigned_contracts + blast endpoint #448

New issue

Open

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

forgejo_admin commented

2026-04-11 20:20:16 +00:00

Owner

Type

Feature

Lineage

Standalone — discovered during 2026-04-10 blast incident (basketball-api#445). The query_unsigned_contracts query in email_queries.py has only a test_email kwarg for filtering. There is no way to target a specific subset of players or exclude specific players from a blast without NULLing their contract_tokens directly (destructive, requires undo to restore).

Repo

forgejo_admin/basketball-api

User Story

As Ava (or any admin) running a blast
I want to specify exactly which players to include or exclude from a contract blast
So that I can send to "these 15 players" without over-sending or having to NULL tokens on players I want to exclude

Context

Current query_unsigned_contracts signature (src/basketball_api/services/email_queries.py:31):

def query_unsigned_contracts(
    db: Session, tenant: Tenant, *, test_email: str | None = None
) -> list[dict]:

The only filter is test_email, which is a post-query Python-level string compare (if parent.email != test_email: continue). No player_ids, exclude_player_ids, or parent_ids params.

The /admin/email/blast endpoint passes body.test_email through to the query function but doesn't accept any other filter params for the recipient set.

What this cost tonight: the 2026-04-10 blast intended for 15 players sent to 18 because the 3 Apaisa sisters had active contract_tokens and shared one parent email. Root cause in basketball-api#445.

File Targets

Files to modify:

src/basketball_api/services/email_queries.py — add player_ids: list[int] | None = None and exclude_player_ids: list[int] | None = None kwargs to query_unsigned_contracts (and optionally query_incomplete_profiles for consistency)
src/basketball_api/routes/admin.py — add player_ids and exclude_player_ids optional fields to BlastRequest Pydantic model, pass them through to the query function
tests/test_email_queries.py — new tests for both include and exclude semantics
tests/test_admin_email_blast.py — new integration test verifying blast endpoint honors the filters

Acceptance Criteria

query_unsigned_contracts(db, tenant, player_ids=[97, 202]) returns only those 2 player rows (if they match the other filters)
query_unsigned_contracts(db, tenant, exclude_player_ids=[111, 186, 112]) returns all offered players EXCEPT those 3
Both kwargs can be combined with test_email — if all three are provided, the intersection applies
POST /admin/email/blast accepts optional player_ids and exclude_player_ids arrays in the request body
When both player_ids and exclude_player_ids are empty/None, current behavior is preserved (backward-compatible)
Tests cover: include-only, exclude-only, both combined, both empty (backward-compat)

Test Expectations

Unit: test_query_unsigned_contracts_player_ids_include
Unit: test_query_unsigned_contracts_player_ids_exclude
Unit: test_query_unsigned_contracts_combined_filters
Integration: test_blast_endpoint_honors_player_ids

Constraints

Must not break existing test_email behavior
Filter at the SQL level via .filter(Player.id.in_(player_ids)) and .filter(~Player.id.in_(exclude_player_ids)) — do NOT filter in Python post-query (that's the slow, error-prone pattern)
Match the existing style of query_unsigned_contracts — psycopg + SQLAlchemy, no new dependencies
No breaking changes to email_queries.py::QUERY_REGISTRY

Checklist

PR opened against basketball-api main
Unit + integration tests pass
CI green
Manual test: blast with player_ids=[97, 202] returns exactly 2 recipients via test_email flow
Manual test: blast with exclude_player_ids=[111, 186, 112] excludes Apaisa sisters
No unrelated changes

westside-basketball — project this affects
basketball-api#445 — root cause incident this ticket addresses
feedback_blast_pool_verification.md — companion lesson

### Type Feature ### Lineage Standalone — discovered during 2026-04-10 blast incident (basketball-api#445). The `query_unsigned_contracts` query in `email_queries.py` has only a `test_email` kwarg for filtering. There is no way to target a specific subset of players or exclude specific players from a blast without NULLing their contract_tokens directly (destructive, requires undo to restore). ### Repo `forgejo_admin/basketball-api` ### User Story As Ava (or any admin) running a blast I want to specify exactly which players to include or exclude from a contract blast So that I can send to "these 15 players" without over-sending or having to NULL tokens on players I want to exclude ### Context Current `query_unsigned_contracts` signature (src/basketball_api/services/email_queries.py:31): ```python def query_unsigned_contracts( db: Session, tenant: Tenant, *, test_email: str | None = None ) -> list[dict]: ``` The only filter is `test_email`, which is a post-query Python-level string compare (`if parent.email != test_email: continue`). No `player_ids`, `exclude_player_ids`, or `parent_ids` params. The `/admin/email/blast` endpoint passes `body.test_email` through to the query function but doesn't accept any other filter params for the recipient set. **What this cost tonight**: the 2026-04-10 blast intended for 15 players sent to 18 because the 3 Apaisa sisters had active contract_tokens and shared one parent email. Root cause in basketball-api#445. ### File Targets Files to modify: - `src/basketball_api/services/email_queries.py` — add `player_ids: list[int] | None = None` and `exclude_player_ids: list[int] | None = None` kwargs to `query_unsigned_contracts` (and optionally `query_incomplete_profiles` for consistency) - `src/basketball_api/routes/admin.py` — add `player_ids` and `exclude_player_ids` optional fields to `BlastRequest` Pydantic model, pass them through to the query function - `tests/test_email_queries.py` — new tests for both include and exclude semantics - `tests/test_admin_email_blast.py` — new integration test verifying blast endpoint honors the filters ### Acceptance Criteria - [ ] `query_unsigned_contracts(db, tenant, player_ids=[97, 202])` returns only those 2 player rows (if they match the other filters) - [ ] `query_unsigned_contracts(db, tenant, exclude_player_ids=[111, 186, 112])` returns all offered players EXCEPT those 3 - [ ] Both kwargs can be combined with `test_email` — if all three are provided, the intersection applies - [ ] `POST /admin/email/blast` accepts optional `player_ids` and `exclude_player_ids` arrays in the request body - [ ] When both `player_ids` and `exclude_player_ids` are empty/None, current behavior is preserved (backward-compatible) - [ ] Tests cover: include-only, exclude-only, both combined, both empty (backward-compat) ### Test Expectations - [ ] Unit: `test_query_unsigned_contracts_player_ids_include` - [ ] Unit: `test_query_unsigned_contracts_player_ids_exclude` - [ ] Unit: `test_query_unsigned_contracts_combined_filters` - [ ] Integration: `test_blast_endpoint_honors_player_ids` ### Constraints - Must not break existing `test_email` behavior - Filter at the SQL level via `.filter(Player.id.in_(player_ids))` and `.filter(~Player.id.in_(exclude_player_ids))` — do NOT filter in Python post-query (that's the slow, error-prone pattern) - Match the existing style of `query_unsigned_contracts` — psycopg + SQLAlchemy, no new dependencies - No breaking changes to `email_queries.py::QUERY_REGISTRY` ### Checklist - [ ] PR opened against basketball-api main - [ ] Unit + integration tests pass - [ ] CI green - [ ] Manual test: blast with `player_ids=[97, 202]` returns exactly 2 recipients via test_email flow - [ ] Manual test: blast with `exclude_player_ids=[111, 186, 112]` excludes Apaisa sisters - [ ] No unrelated changes ### Related - `westside-basketball` — project this affects - basketball-api#445 — root cause incident this ticket addresses - `feedback_blast_pool_verification.md` — companion lesson