Move the mailer out of the mail router #455

Merged

johnsturgeon merged 1 commit from move-the-mailer-out-of-the-mail-router-so-jobs-can-send-too-453 into main

2026-09-02 19:17:38 +02:00

johnsturgeon commented

2026-09-02 18:59:41 +02:00

Owner

Move only, no behaviour change.

What moved

app/mailer.py now owns sending: the ConnectionConfig, a shared
_send(subject, recipients, template_name, body), and send_welcome_email.

app/routers/mail.py keeps its two routes and imports from it. v2.py imports
from app.mailer instead of app.routers.mail -- the router-importing-a-router
that motivated the issue.

send_welcome_email keeps its Request. It is sent from routes only, and
url_for is the right way to build its links while a request is in hand.

Left behind by the copy

Three things came along that are dead in a mailer and were removed:

EmailSchema -- an HTTP request-body model, belongs with the route that
parses it
router -- a second APIRouter with the same /mail prefix, never included
templates -- the router renders send_welcome.j2; the mailer renders no
pages

Their imports went with them. The router also had a stale ConnectionConfig
import and had lost the template_folder its Jinja2Templates referenced, so
it was failing at import until that came back with the right .parent.parent.

email_welcome.j2 never used the discord_img_url key the body passed it, so
that goes too.

Deliberately duplicated

template_folder is defined in both files, pointing at the same directory --
the mailer needs it for ConnectionConfig, the router for page rendering.
Having the router import it from the mailer would be worse than the two lines.

Follow-ups it sets up

#209 -- the picks-ready mail is now a function plus a job. Its template has
no links, so it needs no request and no base URL.
#454 -- the Discord invite URL is still hardcoded, now in mailer.py.

284 tests passing.

resolves #453

🤖 Generated with Claude Code

Move only, no behaviour change. ## What moved `app/mailer.py` now owns sending: the `ConnectionConfig`, a shared `_send(subject, recipients, template_name, body)`, and `send_welcome_email`. `app/routers/mail.py` keeps its two routes and imports from it. `v2.py` imports from `app.mailer` instead of `app.routers.mail` -- the router-importing-a-router that motivated the issue. `send_welcome_email` keeps its `Request`. It is sent from routes only, and `url_for` is the right way to build its links while a request is in hand. ## Left behind by the copy Three things came along that are dead in a mailer and were removed: - `EmailSchema` -- an HTTP request-body model, belongs with the route that parses it - `router` -- a second `APIRouter` with the same `/mail` prefix, never included - `templates` -- the router renders `send_welcome.j2`; the mailer renders no pages Their imports went with them. The router also had a stale `ConnectionConfig` import and had lost the `template_folder` its `Jinja2Templates` referenced, so it was failing at import until that came back with the right `.parent.parent`. `email_welcome.j2` never used the `discord_img_url` key the body passed it, so that goes too. ## Deliberately duplicated `template_folder` is defined in both files, pointing at the same directory -- the mailer needs it for `ConnectionConfig`, the router for page rendering. Having the router import it from the mailer would be worse than the two lines. ## Follow-ups it sets up - #209 -- the picks-ready mail is now a function plus a job. Its template has no links, so it needs no request and no base URL. - #454 -- the Discord invite URL is still hardcoded, now in `mailer.py`. 284 tests passing. resolves #453 🤖 Generated with [Claude Code](https://claude.com/claude-code)

johnsturgeon added 1 commit

2026-09-02 18:59:41 +02:00

Move the mailer out of the mail router

Claude Code Review / claude-review (pull_request) Successful in 38s

Details

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

Details

Tests / pytest (pull_request) Successful in 13s

Details

cda0f93198

Everything needed to send mail lived inside `app/routers/mail.py` -- the
connection config, the message assembly, the FastMail call -- so nothing but a
route could reach it. `v2.py` was already importing `send_welcome_email` out of
a router to get at it.

`app/mailer.py` now owns sending, and both routers import from it.
`send_welcome_email` keeps its `Request`: it is sent from routes only, and
`url_for` is the right way to build its links while one is in hand.

Move only. The picks-page-ready mail is #209 and is now a new function plus a
job rather than a refactor tangled with a feature.

`email_welcome.j2` never used the `discord_img_url` key the body passed it, so
it goes here.

resolves #453

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

forgejo-actions commented

2026-09-02 19:00:20 +02:00

This is a clean move-only refactor. I've verified the two template_folder paths resolve to the same app/templates directory (mailer.py uses .parent, the router uses .parent.parent), the removed discord_img_url key is genuinely unused by any template, and both remaining importers (app/routers/mail.py:9, app/routers/v2.py:29) now pull send_welcome_email from app.mailer. One stale comment is worth a touch-up.

Move-only refactor extracting the mailer (ConnectionConfig, _send, send_welcome_email) into app/mailer.py, with the router and v2.py importing from it and dead HTTP/template artifacts left behind. The behaviour is preserved and the change is correct; the only thing I'd flag is a comment that now points at the wrong file.

Nits

tests/conftest.py:84 — the comment "app/routers/mail.py builds a fastapi-mail ConnectionConfig at module scope" is now stale: that ConnectionConfig moved to app/mailer.py. The reasoning (pydantic rejects a MAIL_FROM without an @) still holds, so the placeholder is still needed — just update the file reference so the next reader can find it.
app/mailer.py:53-56 — the body dict is typed dict[str, str] on _send, which is fine, but request.url_for(...) returns a URL that's explicitly str()-wrapped here while admin_email/discord_invite_link are plain strings — consistent, no action needed, just confirming the str() calls are load-bearing for the type and the template.

Nothing blocking. The removal of EmailSchema/router/templates from the mailer and the discord_img_url key are all confirmed dead, and the version bump (2.22.1) follows the per-PR convention in CLAUDE.md.

This is a clean move-only refactor. I've verified the two `template_folder` paths resolve to the same `app/templates` directory (`mailer.py` uses `.parent`, the router uses `.parent.parent`), the removed `discord_img_url` key is genuinely unused by any template, and both remaining importers (`app/routers/mail.py:9`, `app/routers/v2.py:29`) now pull `send_welcome_email` from `app.mailer`. One stale comment is worth a touch-up. --- Move-only refactor extracting the mailer (`ConnectionConfig`, `_send`, `send_welcome_email`) into `app/mailer.py`, with the router and `v2.py` importing from it and dead HTTP/template artifacts left behind. The behaviour is preserved and the change is correct; the only thing I'd flag is a comment that now points at the wrong file. ### Nits - `tests/conftest.py:84` — the comment "`app/routers/mail.py` builds a fastapi-mail ConnectionConfig at module scope" is now stale: that `ConnectionConfig` moved to `app/mailer.py`. The reasoning (pydantic rejects a `MAIL_FROM` without an `@`) still holds, so the placeholder is still needed — just update the file reference so the next reader can find it. - `app/mailer.py:53-56` — the `body` dict is typed `dict[str, str]` on `_send`, which is fine, but `request.url_for(...)` returns a `URL` that's explicitly `str()`-wrapped here while `admin_email`/`discord_invite_link` are plain strings — consistent, no action needed, just confirming the `str()` calls are load-bearing for the type and the template. Nothing blocking. The removal of `EmailSchema`/`router`/`templates` from the mailer and the `discord_img_url` key are all confirmed dead, and the version bump (`2.22.1`) follows the per-PR convention in CLAUDE.md.

johnsturgeon merged commit 6cb32d24c4 into main

2026-09-02 19:17:38 +02:00

johnsturgeon deleted branch move-the-mailer-out-of-the-mail-router-so-jobs-can-send-too-453

2026-09-02 19:17:38 +02:00

johnsturgeon referenced this pull request from a commit

2026-09-02 19:17:39 +02:00

Merge pull request 'Move the mailer out of the mail router' (#455) from move-the-mailer-out-of-the-mail-router-so-jobs-can-send-too-453 into main