feat: Queens pink palette for girls first-payment emails #415

Merged

forgejo_admin merged 1 commit from 413-queens-pink-email into main

2026-04-10 05:49:57 +00:00

forgejo_admin commented

2026-04-10 05:32:47 +00:00

Owner

Summary

Girls division players now receive first-payment emails with the Queens pink accent (#e91e8c) instead of Kings red (#d42026). Boys division and all other email types remain unchanged.

Changes

src/basketball_api/brand.py — added COLOR_QUEENS_PINK and COLOR_QUEENS_PINK_HOVER constants after the existing Kings reds.
src/basketball_api/services/email.py:
- Added _BRAND_QUEENS_PINK = "#e91e8c" next to _BRAND_RED.
- _brand_wrapper() now takes an optional accent_color kwarg (defaults to _BRAND_RED); the 3px header bottom border uses it. All 10 existing callers are unaffected thanks to the default value.
- send_first_payment_email() selects accent based on player.division: Queens pink for Division.girls, brand red otherwise. The accent is threaded through the h3 color, CTA button background, player-name  highlight, and the _brand_wrapper call.
- Added Division to the existing basketball_api.models import block.
tests/test_first_payment_email.py — two new tests asserting the correct hex color appears (and the other does not) for girls and boys division players.

Backward Compatibility

The 10 other _brand_wrapper() callers (jersey-reminder, profile-reminder, roster-export, tryout-announcement, contract-signed, etc.) are unchanged — they continue to use _BRAND_RED via the default parameter.

Review Checklist

ruff format clean
ruff check clean
New tests added for both girls and boys division branching
Backward-compatible default on _brand_wrapper(accent_color=...)
Only send_first_payment_email modified; no other email builders touched
Full test suite run; new tests pass; remaining 8 failures verified pre-existing on main

Test Plan

ruff format and ruff check on all modified files — clean
pytest tests/test_first_payment_email.py -v — 7/7 passed (including both new tests)
Full suite: 911 passed, 8 pre-existing failures verified unrelated (test_jersey_reminder, test_reconciliation, test_templated_email — same failures exist on main before this branch's changes)
Manual QA: trigger first-payment email for a girls-division player and verify header border + CTA + player-name highlight render pink

Brand tokens: src/basketball_api/brand.py
Email service: src/basketball_api/services/email.py
Division enum: src/basketball_api/models.py (Division.boys, Division.girls)

Closes #413

## Summary Girls division players now receive first-payment emails with the Queens pink accent (`#e91e8c`) instead of Kings red (`#d42026`). Boys division and all other email types remain unchanged. ## Changes - `src/basketball_api/brand.py` — added `COLOR_QUEENS_PINK` and `COLOR_QUEENS_PINK_HOVER` constants after the existing Kings reds. - `src/basketball_api/services/email.py`: - Added `_BRAND_QUEENS_PINK = "#e91e8c"` next to `_BRAND_RED`. - `_brand_wrapper()` now takes an optional `accent_color` kwarg (defaults to `_BRAND_RED`); the 3px header bottom border uses it. All 10 existing callers are unaffected thanks to the default value. - `send_first_payment_email()` selects accent based on `player.division`: Queens pink for `Division.girls`, brand red otherwise. The accent is threaded through the h3 color, CTA button background, player-name `` highlight, and the `_brand_wrapper` call. - Added `Division` to the existing `basketball_api.models` import block. - `tests/test_first_payment_email.py` — two new tests asserting the correct hex color appears (and the other does not) for girls and boys division players. ## Backward Compatibility The 10 other `_brand_wrapper()` callers (jersey-reminder, profile-reminder, roster-export, tryout-announcement, contract-signed, etc.) are unchanged — they continue to use `_BRAND_RED` via the default parameter. ## Review Checklist - [x] `ruff format` clean - [x] `ruff check` clean - [x] New tests added for both girls and boys division branching - [x] Backward-compatible default on `_brand_wrapper(accent_color=...)` - [x] Only `send_first_payment_email` modified; no other email builders touched - [x] Full test suite run; new tests pass; remaining 8 failures verified pre-existing on `main` ## Test Plan - [x] `ruff format` and `ruff check` on all modified files — clean - [x] `pytest tests/test_first_payment_email.py -v` — 7/7 passed (including both new tests) - [x] Full suite: 911 passed, 8 pre-existing failures verified unrelated (test_jersey_reminder, test_reconciliation, test_templated_email — same failures exist on `main` before this branch's changes) - [ ] Manual QA: trigger first-payment email for a girls-division player and verify header border + CTA + player-name highlight render pink ## Related Notes - Brand tokens: `src/basketball_api/brand.py` - Email service: `src/basketball_api/services/email.py` - Division enum: `src/basketball_api/models.py` (`Division.boys`, `Division.girls`) ## Related - Closes #413

forgejo_admin added 1 commit

2026-04-10 05:32:47 +00:00

feat: Queens pink palette for girls division first-payment emails

ci/woodpecker/pr/woodpecker Pipeline failed

Details

2ca086182a

Adds Queens pink accent color (#e91e8c) for first-payment emails sent
to girls division players, while keeping Kings red (#d42026) for boys.
The _brand_wrapper helper now accepts an optional accent_color kwarg,
defaulting to red so all 10 other callers are unaffected.

Closes #413

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-04-10 05:38:27 +00:00

Author

Owner

PR #415 Review

DOMAIN REVIEW

Stack: Python / SQLAlchemy / email rendering (basketball-api). Applied Python/PEP + OWASP + SQLAlchemy lens.

Diff surface: 3 files, +62/-6. Changes are tightly scoped:

brand.py: two new color token constants added after existing Kings reds. Matches acceptance criteria exactly (#e91e8c / #f23d9e).
services/email.py: _BRAND_QUEENS_PINK constant, _brand_wrapper() grows optional accent_color: str = _BRAND_RED kwarg (backward compatible default), header border now uses accent_color, and send_first_payment_email() threads the accent through h3, CTA button, player-name , and the wrapper call. Division added to the existing basketball_api.models import block.
tests/test_first_payment_email.py: two new tests — girls asserts #e91e8c present and #d42026 absent; boys asserts the inverse.

Caller safety (critical check): Verified against local main. The seven other _brand_wrapper(...) call sites (lines 508, 635, 830, 1071, 1294, 1398, plus the def) are NOT modified by the diff — only line 963 (first_payment) gains the accent_color=accent kwarg. The new parameter defaults to _BRAND_RED, so every untouched caller renders identically. No unintended visual regressions for jersey-reminder, profile-reminder, roster-export, tryout-announcement, contract-signed, etc. (Note: PR body says "10 other callers" but the actual count in source is 7. Cosmetic PR-body inaccuracy, not a code issue.)

None-division safety: Player.division is Mapped[Division | None] (nullable). The expression player.division == Division.girls evaluates to False when division is None, cleanly falling through to _BRAND_RED. No crash, no implicit coercion. Kings players (boys or null) correctly get red/black.

Circular import: Division is added to the already-existing from basketball_api.models import (...) block. Since models is already imported by services/email.py, there is zero new import surface — no circular risk.

Color propagation audit in send_first_payment_email: accent flows to (1) h3 color, (2) CTA background, (3)  player-name highlight, (4) wrapper header border. I checked the rest of the function for stray _BRAND_RED references in the first-payment builder — none remain inside this function's body. No other email builder function was touched by the diff.

PEP / Ruff: Signature change is formatted across lines cleanly, type hint on the new kwarg is correct (str), default is a module-level constant (not a mutable default). PR body confirms ruff format and ruff check clean.

Test quality: Both new tests use the existing parent_and_player fixture, mock get_gmail_client, mutate player.division, and do positive+negative hex assertions. Hex-in-HTML assertion is reasonable since the colors appear inline via f-strings. Covers both branches of the new conditional. Good coverage for a visual branding change.

BLOCKERS

None.

NITS

PR body overstates caller count as 10; actual is 7 in source. Fix in the PR description if you want, not blocking.
Consider a tiny helper _accent_for_division(division) -> str if Queens pink expands to other email types later — not needed for this PR since only one caller branches on division today. YAGNI holds.
The negative assertion assert "#d42026" not in html in the girls test is strict and correct given the current template, but will become fragile if anyone ever hardcodes another red somewhere in the wrapper footer. Acceptable for now; the strictness is actually a feature that will catch regressions.

SOP COMPLIANCE

Branch named after issue (413-queens-pink-email)
PR body has Summary / Changes / Test Plan / Related
Closes #413 referenced
No secrets committed
Tests added for new behavior (both branches)
Scope is tight — no unrelated changes
ruff format / ruff check clean per PR body
Full suite reported: 911 passed, 8 pre-existing failures verified unrelated to this branch

PROCESS OBSERVATIONS

Textbook small, reviewable change. High DF impact (fast to land), near-zero CFR risk (default-parameter refactor, single caller changed, two-branch test coverage). Documentation-ready: the constants live in brand.py where the convention says brand tokens live, so future Queens surfaces (web, contracts, jerseys) can import COLOR_QUEENS_PINK directly. Good foundation if girls-division branding expands beyond first-payment emails.

VERDICT: APPROVED

## PR #415 Review ### DOMAIN REVIEW Stack: Python / SQLAlchemy / email rendering (basketball-api). Applied Python/PEP + OWASP + SQLAlchemy lens. **Diff surface:** 3 files, +62/-6. Changes are tightly scoped: - `brand.py`: two new color token constants added after existing Kings reds. Matches acceptance criteria exactly (`#e91e8c` / `#f23d9e`). - `services/email.py`: `_BRAND_QUEENS_PINK` constant, `_brand_wrapper()` grows optional `accent_color: str = _BRAND_RED` kwarg (backward compatible default), header border now uses `accent_color`, and `send_first_payment_email()` threads the accent through h3, CTA button, player-name ``, and the wrapper call. `Division` added to the existing `basketball_api.models` import block. - `tests/test_first_payment_email.py`: two new tests — girls asserts `#e91e8c` present and `#d42026` absent; boys asserts the inverse. **Caller safety (critical check):** Verified against local `main`. The seven other `_brand_wrapper(...)` call sites (lines 508, 635, 830, 1071, 1294, 1398, plus the def) are NOT modified by the diff — only line 963 (first_payment) gains the `accent_color=accent` kwarg. The new parameter defaults to `_BRAND_RED`, so every untouched caller renders identically. No unintended visual regressions for jersey-reminder, profile-reminder, roster-export, tryout-announcement, contract-signed, etc. (Note: PR body says "10 other callers" but the actual count in source is 7. Cosmetic PR-body inaccuracy, not a code issue.) **None-division safety:** `Player.division` is `Mapped[Division | None]` (nullable). The expression `player.division == Division.girls` evaluates to `False` when `division is None`, cleanly falling through to `_BRAND_RED`. No crash, no implicit coercion. Kings players (boys or null) correctly get red/black. **Circular import:** `Division` is added to the already-existing `from basketball_api.models import (...)` block. Since `models` is already imported by `services/email.py`, there is zero new import surface — no circular risk. **Color propagation audit in `send_first_payment_email`:** accent flows to (1) h3 color, (2) CTA background, (3) `` player-name highlight, (4) wrapper header border. I checked the rest of the function for stray `_BRAND_RED` references in the first-payment builder — none remain inside this function's body. No other email builder function was touched by the diff. **PEP / Ruff:** Signature change is formatted across lines cleanly, type hint on the new kwarg is correct (`str`), default is a module-level constant (not a mutable default). PR body confirms `ruff format` and `ruff check` clean. **Test quality:** Both new tests use the existing `parent_and_player` fixture, mock `get_gmail_client`, mutate `player.division`, and do positive+negative hex assertions. Hex-in-HTML assertion is reasonable since the colors appear inline via f-strings. Covers both branches of the new conditional. Good coverage for a visual branding change. ### BLOCKERS None. ### NITS - PR body overstates caller count as 10; actual is 7 in source. Fix in the PR description if you want, not blocking. - Consider a tiny helper `_accent_for_division(division) -> str` if Queens pink expands to other email types later — not needed for this PR since only one caller branches on division today. YAGNI holds. - The negative assertion `assert "#d42026" not in html` in the girls test is strict and correct given the current template, but will become fragile if anyone ever hardcodes another red somewhere in the wrapper footer. Acceptable for now; the strictness is actually a feature that will catch regressions. ### SOP COMPLIANCE - [x] Branch named after issue (`413-queens-pink-email`) - [x] PR body has Summary / Changes / Test Plan / Related - [x] Closes #413 referenced - [x] No secrets committed - [x] Tests added for new behavior (both branches) - [x] Scope is tight — no unrelated changes - [x] `ruff format` / `ruff check` clean per PR body - [x] Full suite reported: 911 passed, 8 pre-existing failures verified unrelated to this branch ### PROCESS OBSERVATIONS Textbook small, reviewable change. High DF impact (fast to land), near-zero CFR risk (default-parameter refactor, single caller changed, two-branch test coverage). Documentation-ready: the constants live in `brand.py` where the convention says brand tokens live, so future Queens surfaces (web, contracts, jerseys) can import `COLOR_QUEENS_PINK` directly. Good foundation if girls-division branding expands beyond first-payment emails. ### VERDICT: APPROVED