Twelve mails carried hardcoded English while the app itself has enforced HU/EN parity since E5.
A mail is the one user-facing surface nobody can re-render by switching the UI language: it arrives
once, in whatever language we picked, and stays that way. That made the epic's own headline mail —
"one mail, one click" — English for a Hungarian reader.
The text moved to a catalog (`mailtext.py`) with a `text(key, lang, **vars)` renderer, and the parity
rule is now a test rather than a habit: every key must carry both languages AND the same
`{placeholders}`. A translation that drops `{link}` sends a mail with no link in it; one that invents
a placeholder raises inside a background task nobody is watching. Both are silent in production.
Every `send_*` takes the language as its second argument, resolved by the REQUEST handler and never
inside the mail — these all run as background tasks, after the request's session is gone, which is
the same reason the approval mail computes its link in the handler.
Where the language comes from, in order: the account's own preference; failing that, the language the
person asked us in; failing that, English. The middle one is new — `invites.lang`, migration `0082` —
because the mails an access request produces go to an address that usually has no account yet, and
the signal does exist: the page their form was submitted from, or the locale Google reported for a
sign-in we refused. Captured rather than derived, for the third time in this epic (`google_email` in
S3, `preferences.language` in S6 taught the same lesson): a signal that exists only in one moment
cannot be backfilled later. So the admin is asked in exactly one place — the invitation to an address
they typed in themselves, where nobody has ever told us anything.
That send is now a dialog rather than a bare click, which is also where the "you already wrote to
them" warning belongs — it used to live only in a tooltip the admin may never hover, guarding the one
action whose failure mode is mailing a real person twice.
S6's registration `lang` field turned out to be dead on arrival: the backend accepted it, the
frontend never sent it, so every account has been storing nothing. The request builders now stamp the
UI language themselves — a signal that has to be passed in by every caller is one every caller can
forget, which is exactly how it shipped unset.
Admins are notified one mail per language group instead of one blast to everyone. They are people
with their own UI language; a single message would have to pick one of them and be wrong for the rest.
Also removes a duplicated `invited_at` column declaration on `Invite` (two identical definitions
landed in S6).
150 lines
5.3 KiB
Python
150 lines
5.3 KiB
Python
"""An undelivered email leaves a trace in the admins' inbox.
|
|
|
|
Sending is deliberately fail-soft — a missing SMTP setting or a rejecting relay logs and returns
|
|
False so nothing user-facing breaks. The cost of that used to be total silence: an instance whose
|
|
SMTP was never filled in swallowed verification links, password resets and access-request notices,
|
|
and the operator's first clue was somebody saying nothing arrived.
|
|
"""
|
|
|
|
import smtplib
|
|
|
|
import pytest
|
|
from sqlalchemy import select
|
|
|
|
from app import email as email_mod
|
|
from app.models import Notification
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _no_smtp_config(monkeypatch):
|
|
"""Every test here drives the real `_send`, so make sure it never reaches a socket."""
|
|
monkeypatch.setattr(
|
|
email_mod,
|
|
"_smtp",
|
|
lambda: {"host": "", "port": 587, "user": "", "password": "", "from": ""},
|
|
)
|
|
|
|
|
|
def _notifications(db, user_id: int) -> list[Notification]:
|
|
return list(
|
|
db.execute(
|
|
select(Notification)
|
|
.where(Notification.user_id == user_id, Notification.type == "email_undelivered")
|
|
.order_by(Notification.id)
|
|
)
|
|
.scalars()
|
|
.all()
|
|
)
|
|
|
|
|
|
def test_a_skipped_email_notifies_every_admin(db, make_user):
|
|
admin_a = make_user(email="admin-a@example.com", role="admin")
|
|
admin_b = make_user(email="admin-b@example.com", role="admin")
|
|
plain = make_user(email="plain@example.com")
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False # fail-soft, as before
|
|
|
|
db.expire_all()
|
|
for admin in (admin_a, admin_b):
|
|
(n,) = _notifications(db, admin.id)
|
|
assert n.data["reason"] == "not_configured"
|
|
assert n.data["subject"] == "Siftlode SMTP test"
|
|
assert n.data["recipients"] == ["someone@example.com"]
|
|
assert _notifications(db, plain.id) == [] # not an operator's problem to see
|
|
|
|
|
|
def test_the_notice_is_throttled_so_a_dead_relay_cannot_flood_the_inbox(db, make_user):
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
|
|
for _ in range(5):
|
|
email_mod.send_test("someone@example.com", "en")
|
|
|
|
db.expire_all()
|
|
assert len(_notifications(db, admin.id)) == 1
|
|
|
|
|
|
def test_a_send_failure_is_reported_separately_from_a_missing_config(db, make_user, monkeypatch):
|
|
# A configured-but-rejecting relay is a different operator problem from an empty config, and the
|
|
# inbox copy differs — so the reason has to survive to the notification.
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
monkeypatch.setattr(
|
|
email_mod,
|
|
"_smtp",
|
|
lambda: {
|
|
"host": "smtp.example.com",
|
|
"port": 587,
|
|
"user": "u",
|
|
"password": "p",
|
|
"from": "Siftlode <u@example.com>",
|
|
},
|
|
)
|
|
|
|
def _boom(*a, **kw):
|
|
raise smtplib.SMTPException("nope")
|
|
|
|
monkeypatch.setattr(email_mod.smtplib, "SMTP", _boom)
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False
|
|
|
|
db.expire_all()
|
|
(n,) = _notifications(db, admin.id)
|
|
assert n.data["reason"] == "send_failed"
|
|
|
|
|
|
def test_no_admins_means_no_notification_and_no_error(db, make_user):
|
|
# A fresh instance can have no admin yet (the wizard has not run). The alert must stay silent
|
|
# rather than blowing up inside the fail-soft path it is supposed to make visible.
|
|
plain = make_user(email="plain@example.com")
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False
|
|
|
|
db.expire_all()
|
|
assert _notifications(db, plain.id) == []
|
|
|
|
|
|
def test_a_drop_with_nobody_to_tell_does_not_burn_the_hour(db, make_user):
|
|
# The throttle window must only be spent on a notice that was actually written. Otherwise a drop
|
|
# during setup — before any admin exists — silences the next hour for the admin created a minute
|
|
# later, which is exactly the blind spot this alert exists to remove.
|
|
email_mod.send_test("someone@example.com", "en") # no admins yet
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False
|
|
|
|
db.expire_all()
|
|
assert len(_notifications(db, admin.id)) == 1
|
|
|
|
|
|
def test_a_failed_alert_does_not_burn_the_hour_either(db, make_user, monkeypatch):
|
|
admin = make_user(email="admin@example.com", role="admin")
|
|
boom = {"on": True}
|
|
|
|
def _maybe_boom(*a, **kw):
|
|
if boom["on"]:
|
|
raise RuntimeError("notification store is down")
|
|
from app.notifications import create_notification as real
|
|
|
|
return real(*a, **kw)
|
|
|
|
monkeypatch.setattr(email_mod, "create_notification", _maybe_boom)
|
|
email_mod.send_test("someone@example.com", "en") # the write blows up; nothing recorded
|
|
boom["on"] = False
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False
|
|
|
|
db.expire_all()
|
|
assert len(_notifications(db, admin.id)) == 1
|
|
|
|
|
|
def test_a_broken_alert_never_breaks_the_send(db, make_user, monkeypatch):
|
|
# The alert is the fallback path; if IT fails, the caller must still get the ordinary fail-soft
|
|
# False rather than an exception surfacing from a background task.
|
|
make_user(email="admin@example.com", role="admin")
|
|
|
|
def _boom(*a, **kw):
|
|
raise RuntimeError("notification store is down")
|
|
|
|
monkeypatch.setattr(email_mod, "create_notification", _boom)
|
|
|
|
assert email_mod.send_test("someone@example.com", "en") is False
|