Expand {{ standings }} token in admin emails to a styled table #483

Merged

johnsturgeon merged 1 commit from create-a-form-page-for-sending-out-the-standings-233 into main

2026-09-06 21:39:32 +02:00

johnsturgeon commented

2026-09-06 21:36:48 +02:00

Owner

What

Adds a  token to the admin custom-email composer. When present in the Markdown body, it expands to the current standings as an inline-styled HTML table that mirrors the /standings page — gray header, zebra rows, maroon names — styled inline so it survives email clients (no external CSS, no <style> block).

No UI change to the compose page beyond one new row in the Markdown cheat sheet documenting the token.

How

app/standings_email.py (new): standings_markdown(session) builds a Markdown pipe table from active players (same query/sort as /standings); style_email_table(html) injects inline styles into markdown-it's table output.
app/mailer.py: custom_email_body does the token substitution — it renders the standings fragment in isolation and styles it before the outer email template wraps everything in layout tables, so the styler never touches the presentation tables. send_custom_email forwards the pre-built markdown.
Preview (v2.py) and background send (send_admin_email.py) each build the standings lazily — only when a message uses the token — so a token-free batch never depends on a current week existing (which would raise between seasons).

Testing

tests/test_mailer.py: token present → styled <table>/<th>; token absent → body unchanged.
flake8 clean, pylint 10.00/10.
Note: the 7 test_admin_email_routes.py failures are pre-existing on main, unrelated to this change.

resolves #233

🤖 Generated with Claude Code

## What Adds a `` token to the admin custom-email composer. When present in the Markdown body, it expands to the current standings as an inline-styled HTML table that mirrors the `/standings` page — gray header, zebra rows, maroon names — styled inline so it survives email clients (no external CSS, no `<style>` block). No UI change to the compose page beyond one new row in the Markdown cheat sheet documenting the token. ## How - **`app/standings_email.py`** (new): `standings_markdown(session)` builds a Markdown pipe table from active players (same query/sort as `/standings`); `style_email_table(html)` injects inline styles into markdown-it's table output. - **`app/mailer.py`**: `custom_email_body` does the token substitution — it renders the standings fragment in isolation and styles it *before* the outer email template wraps everything in layout tables, so the styler never touches the presentation tables. `send_custom_email` forwards the pre-built markdown. - Preview (`v2.py`) and background send (`send_admin_email.py`) each build the standings **lazily — only when a message uses the token** — so a token-free batch never depends on a current week existing (which would raise between seasons). ## Testing - `tests/test_mailer.py`: token present → styled `<table>`/`<th>`; token absent → body unchanged. - flake8 clean, pylint 10.00/10. - Note: the 7 `test_admin_email_routes.py` failures are pre-existing on `main`, unrelated to this change. resolves #233 🤖 Generated with [Claude Code](https://claude.com/claude-code)

johnsturgeon added 1 commit

2026-09-06 21:36:48 +02:00

Expand {{ standings }} token in admin emails to a styled table

Claude Code Review / claude-review (pull_request) Successful in 2m14s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 30s

Details

Tests / pytest (pull_request) Successful in 15s

Details

Release on merge / release (pull_request) Successful in 0s

Details

d0d761153b

The admin custom-email composer now recognizes a `<!-- STANDINGS -->`
token in the Markdown body and replaces it with the current standings,
rendered as an inline-styled table that mirrors the /standings page
(gray header, zebra rows, maroon names) so it survives email clients.

Substitution lives in `custom_email_body`, the shared choke point for
both the preview and the background send, so the two always match. The
standings are built lazily -- only when a message actually uses the
token -- so a token-free batch never depends on a current week existing.

resolves #233

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

forgejo-actions commented

2026-09-06 21:39:02 +02:00

Summary

This PR adds a  token to admin custom emails that expands to an inline-styled standings table mirroring the /standings page, with the standings built lazily only when a message actually uses the token. The implementation is clean, mirrors the existing standings route closely, and the substitution logic is sound; I found no blocking issues, just a few minor points worth noting.

Worth fixing

app/standings_email.py:25 — a nickname containing | breaks the generated table. The row is assembled as an f-string of pipe-delimited fields, so any | in player.nick_name injects an extra column and corrupts markdown-it's table parsing for that row. nick_name is admin-controlled (set via v2.py:423/486), so this is low risk, but a one-line escape (player.nick_name.replace("|", "\\|")) would harden it. Relatedly, note that MarkdownIt("commonmark") runs with html: true, so raw HTML in a nickname passes through into the email unescaped — unlike the /standings page, where Jinja autoescapes {{ player.nick_name }}. Again low risk given admin-only input, but it's a behavioral difference worth being aware of.

Nits

N+1 query pattern in app/standings_email.py:29-31. wins_for_week, losses_for_week, and bonus_for_week each independently call picks_for_week, so every player costs three separate queries for the "Last W/L/B" columns. This exactly matches the existing /standings page (standings.j2:56-58), so it's consistent with current conventions rather than a regression — but since this now also runs inside the background drain_batch send loop, it's worth flagging that a large active-player list multiplies the query count per email batch. Both call sites run these sync queries on the event loop, which is the established pattern in this codebase.
Test coverage gaps. The new tests (tests/test_mailer.py) cover the token-present/token-absent substitution well, but a couple of paths are untested: style_email_table's zebra striping across multiple rows (the test fixture has a single data row), and the lazy-build/caching behavior in drain_batch (send_admin_email.py:15-24), including that a token-free batch never calls standings_markdown. The description notes standings_markdown is DB-dependent and hard to unit test, which is fair; a small test asserting drain_batch doesn't invoke it when no job carries the token would be cheap insurance.
app/mailer.py:88-92 renders the body twice on the token path (md.render(body_markdown) is effectively computed once here, fine) — actually only the branch structure is slightly redundant. Purely cosmetic; the current form is clear enough.

Overall this looks good and matches the /standings layout, zebra rows (i % 2 == 0 → highlighted, same as row_with_style), and sort order faithfully. The lazy-build design correctly avoids depending on a current week for token-free batches.

## Summary This PR adds a `` token to admin custom emails that expands to an inline-styled standings table mirroring the `/standings` page, with the standings built lazily only when a message actually uses the token. The implementation is clean, mirrors the existing standings route closely, and the substitution logic is sound; I found no blocking issues, just a few minor points worth noting. ## Worth fixing - **`app/standings_email.py:25` — a nickname containing `|` breaks the generated table.** The row is assembled as an f-string of pipe-delimited fields, so any `|` in `player.nick_name` injects an extra column and corrupts markdown-it's table parsing for that row. `nick_name` is admin-controlled (set via `v2.py:423`/`486`), so this is low risk, but a one-line escape (`player.nick_name.replace("|", "\\|")`) would harden it. Relatedly, note that `MarkdownIt("commonmark")` runs with `html: true`, so raw HTML in a nickname passes through into the email unescaped — unlike the `/standings` page, where Jinja autoescapes `{{ player.nick_name }}`. Again low risk given admin-only input, but it's a behavioral difference worth being aware of. ## Nits - **N+1 query pattern in `app/standings_email.py:29-31`.** `wins_for_week`, `losses_for_week`, and `bonus_for_week` each independently call `picks_for_week`, so every player costs three separate queries for the "Last W/L/B" columns. This exactly matches the existing `/standings` page (`standings.j2:56-58`), so it's consistent with current conventions rather than a regression — but since this now also runs inside the background `drain_batch` send loop, it's worth flagging that a large active-player list multiplies the query count per email batch. Both call sites run these sync queries on the event loop, which is the established pattern in this codebase. - **Test coverage gaps.** The new tests (`tests/test_mailer.py`) cover the token-present/token-absent substitution well, but a couple of paths are untested: `style_email_table`'s zebra striping across multiple rows (the test fixture has a single data row), and the lazy-build/caching behavior in `drain_batch` (`send_admin_email.py:15-24`), including that a token-free batch never calls `standings_markdown`. The description notes `standings_markdown` is DB-dependent and hard to unit test, which is fair; a small test asserting `drain_batch` doesn't invoke it when no job carries the token would be cheap insurance. - **`app/mailer.py:88-92` renders the body twice on the token path** (`md.render(body_markdown)` is effectively computed once here, fine) — actually only the branch structure is slightly redundant. Purely cosmetic; the current form is clear enough. Overall this looks good and matches the `/standings` layout, zebra rows (`i % 2 == 0` → highlighted, same as `row_with_style`), and sort order faithfully. The lazy-build design correctly avoids depending on a current week for token-free batches.

johnsturgeon merged commit b9ea727551 into main

2026-09-06 21:39:32 +02:00

johnsturgeon deleted branch create-a-form-page-for-sending-out-the-standings-233

2026-09-06 21:39:32 +02:00

johnsturgeon referenced this pull request from a commit

2026-09-06 21:39:33 +02:00

Merge pull request 'Expand {{ standings }} token in admin emails to a styled table' (#483) from create-a-form-page-for-sending-out-the-standings-233 into main