Skip to main content

max / makenotwork

Name the class of email you cannot opt out of, in code and to the user Whether an email respected a preference was decided by the caller: sixteen db::lists::may_notify calls across seven route modules, and about forty send_* methods with no way to tell which were meant to be gated. So the gate could be forgotten by a new handler, the non-optional set was not enumerable without grepping every route, and nobody had told users it existed. The notification settings page reads as exhaustive and never was. Every send now goes through EmailClient::dispatch, which takes an EmailClass. Operational(kind) cannot be turned off and carries its own justification sentence; Optional(list) is checked by the send path itself; ListAudience names the third thing the code actually had, a recipient resolved from a mailing-list subscription where the subscription is the consent and may_notify would gate a project list on an unrelated account toggle. A new email cannot be written without stating which it is. The settings page and a new guide page render the operational set from the enum, so the prose cannot drift from the behaviour. Two tests hold it: the variant list is asserted against a checked-in expected set, so growing the cannot-opt-out category fails the build, and a seal fails the build if may_notify is called anywhere outside the send path again. Three classification calls worth knowing: - New-device sign-in stays Optional. The task's expected operational set named it, but ListKind::Login is a live preference the settings page offers, and taking an opt-out away from users who already have it needs deciding on its own terms. This also deletes the pending_2fa_notify_enabled session round trip: the preference was read before 2FA and carried through the session, and is now read once at send time. - The onboarding drip becomes Optional(Marketing). It had no gate at all, so a user who turned off marketing mail still got it. Behaviour change, deliberate. - Invite-redeemed is Operational for now because no list kind covers it. It is the weakest member of that set and wants its own preference; filed separately. EmailTransport loses send_email_with_unsub, which dispatch no longer reaches: the headers-and-unsub path produces identical output for an empty header slice.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-07 01:26 UTC
Signed with PGP, not checked
Commit: eae9061fd768405bb039c3c0c6141a5419ba8d5e
Parent: 11d9765
28 files changed, +1249 insertions, -228 deletions
@@ -185,6 +185,10 @@
185 185 name = "export-openapi"
186 186 path = "src/bin/export-openapi.rs"
187 187
188 + [[bin]]
189 + name = "export-operational-mail-doc"
190 + path = "src/bin/export-operational-mail-doc.rs"
191 +
188 192 [build-dependencies]
189 193 # Emits static/geometry.css (makeover-geometry) and static/layout.css
190 194 # (makeover-webview). The same generator GO and BB run; only the output paths
@@ -825,8 +825,9 @@
825 825
826 826 /// Send a new-device login notification if the user has other active sessions.
827 827 ///
828 - /// Fire-and-forget, spawns a background task. Only sends if the user has opted in
829 - /// and has more than one active session (meaning this is a new device).
828 + /// Fire-and-forget, spawns a background task. Only sends if the user has more
829 + /// than one active session (meaning this is a new device). Whether they want the
830 + /// notification at all is the send path's question, not this helper's.
830 831 #[allow(clippy::too_many_arguments)]
831 832 pub async fn maybe_send_login_notification(
832 833 db: &sqlx::PgPool,
@@ -836,12 +837,8 @@
836 837 user_id: UserId,
837 838 email: &str,
838 839 display_name: Option<&str>,
839 - enabled: bool,
840 840 headers: &HeaderMap,
841 841 ) {
842 - if !enabled {
843 - return;
844 - }
845 842 let session_count = match db::sessions::count_user_sessions(db, user_id).await {
846 843 Ok(n) => n,
847 844 Err(e) => {
@@ -876,6 +873,7 @@
876 873 bg.spawn("login notification", async move {
877 874 if let Err(e) = email_client
878 875 .send_new_login_notification(
876 + user_id,
879 877 &email,
880 878 display_name.as_deref(),
881 879 user_agent.as_deref(),
@@ -326,6 +326,7 @@
326 326 );
327 327 if let Err(e) = email_client
328 328 .send_status_notification(
329 + sub.id,
329 330 &sub.email,
330 331 sub.display_name.as_deref(),
331 332 &current_status,
@@ -8822,6 +8822,23 @@
8822 8822 margin-bottom: var(--gap-peer);
8823 8823 }
8824 8824
8825 + /* The cannot-opt-out list, rendered from email::OperationalKind. Reads as
8826 + reference material rather than as more settings: no controls, and the
8827 + justification sits under its label rather than beside it. */
8828 + .operational-mail {
8829 + margin: var(--gap-peer) 0 0 0;
8830 + }
8831 + .operational-mail dt {
8832 + font-weight: bold;
8833 + font-size: var(--text-note);
8834 + margin-top: var(--gap-peer);
8835 + }
8836 + .operational-mail dd {
8837 + margin: 0;
8838 + opacity: 0.8;
8839 + font-size: var(--text-note);
8840 + }
8841 +
8825 8842 /* ===========================================
8826 8843 PROJECT CODE TAB (replaces inline styles in partials/tabs/project_code.html)
8827 8844 =========================================== */
@@ -1,14 +1,24 @@
1 1 //! Email service for sending transactional emails via Postmark.
2 2 //!
3 + //! - `class`, what kind of mail a send is and whether it can be turned off
3 4 //! - `templates`, email composition methods (one per email type)
4 5 //! - `tokens`, HMAC-signed URL generation/verification for email actions
6 + //!
7 + //! Every template method reaches the transport through [`EmailClient::dispatch`],
8 + //! which takes an [`EmailClass`] and an [`Audience`]. That is the whole point of
9 + //! the shape: a new email cannot be written without saying which class it is,
10 + //! and an `Optional` one is preference-checked by the send path rather than by
11 + //! whoever remembered to call `may_notify` at the call site.
5 12
13 + mod class;
6 14 mod templates;
7 15 mod tokens;
16 + pub use class::{EmailClass, OperationalKind, operational_mail_doc};
8 17 pub use tokens::*;
9 18
10 19 use std::sync::Arc;
11 20
21 + use crate::db::{ListKind, UserId};
12 22 use crate::error::{AppError, Result};
13 23
14 24 /// Format an optional display name as a greeting suffix: " Alice" or "".
@@ -102,15 +112,6 @@
102 112 /// Send a plain email.
103 113 async fn send_email(&self, to: &str, subject: &str, body: &str) -> Result<()>;
104 114
105 - /// Send an email with an optional unsubscribe link.
106 - async fn send_email_with_unsub(
107 - &self,
108 - to: &str,
109 - subject: &str,
110 - body: &str,
111 - unsub_url: Option<&str>,
112 - ) -> Result<()>;
113 -
114 115 /// Send an email with extra headers and an optional unsubscribe link.
115 116 async fn send_email_with_headers_and_unsub(
116 117 &self,
@@ -199,23 +200,153 @@
199 200 while set.join_next().await.is_some() {}
200 201 }
201 202
203 + /// Who a message is going to, and what is known about their consent.
204 + ///
205 + /// The distinction is load-bearing rather than cosmetic: an `Optional` message
206 + /// can only be preference-checked against a user id, so pairing one with an
207 + /// [`Audience::Address`] is a bug the dispatcher refuses rather than a silent
208 + /// bypass of the gate.
209 + #[derive(Debug, Clone, Copy)]
210 + pub enum Audience<'a> {
211 + /// Someone with an account, so their notification preferences can be read.
212 + User(UserId, &'a str),
213 + /// A bare address with no account behind it: a guest buyer, an imported
214 + /// list subscriber, an operations mailbox.
215 + Address(&'a str),
216 + }
217 +
218 + impl Audience<'_> {
219 + fn email(&self) -> &str {
220 + match self {
221 + Audience::User(_, email) | Audience::Address(email) => email,
222 + }
223 + }
224 + }
225 +
226 + /// Per-message delivery options: the parts that vary between senders but are
227 + /// not the class, the audience, the subject or the body.
228 + #[derive(Default)]
229 + pub(crate) struct Delivery<'a> {
230 + /// One-click unsubscribe link. Required by every `Optional` and
231 + /// `ListAudience` send; see `dispatch`.
232 + pub unsub_url: Option<&'a str>,
233 + /// Extra headers (issue threading: Reply-To, Message-ID, In-Reply-To).
234 + pub headers: &'a [(&'a str, String)],
235 + /// Send on Postmark's broadcast stream rather than the transactional one.
236 + pub broadcast: bool,
237 + }
238 +
202 239 /// Email client for sending emails
203 240 #[derive(Clone)]
204 241 pub struct EmailClient {
205 242 transport: Arc<dyn EmailTransport>,
243 + /// Read for the `Optional` preference check. `None` in tests and in the
244 + /// dev-mode client, where `dispatch` sends rather than silently dropping:
245 + /// a test asserting an email was composed should not fail because there was
246 + /// no database to ask.
247 + pool: Option<sqlx::PgPool>,
206 248 }
207 249
208 250 impl EmailClient {
209 251 /// Create a new email client with Postmark transport.
210 252 pub fn new(config: EmailConfig, pool: Option<sqlx::PgPool>) -> Self {
211 253 EmailClient {
212 - transport: Arc::new(PostmarkTransport::new(config, pool)),
254 + transport: Arc::new(PostmarkTransport::new(config, pool.clone())),
255 + pool,
213 256 }
214 257 }
215 258
216 259 /// Create an email client with a custom transport (for testing).
217 260 pub fn with_transport(transport: Arc<dyn EmailTransport>) -> Self {
218 - EmailClient { transport }
261 + EmailClient {
262 + transport,
263 + pool: None,
264 + }
265 + }
266 +
267 + /// Attach a pool to a custom-transport client so `Optional` sends are
268 + /// preference-checked against a real database in integration tests.
269 + #[must_use]
270 + pub fn with_pool(mut self, pool: sqlx::PgPool) -> Self {
271 + self.pool = Some(pool);
272 + self
273 + }
274 +
275 + /// The single path from a template method to the transport.
276 + ///
277 + /// Every `send_*` goes through here, which is what makes the class
278 + /// mandatory: there is no way to compose an email and put it on the wire
279 + /// without naming one. For [`EmailClass::Optional`] the preference check
280 + /// happens here, so the sixteen `may_notify` calls that used to be spread
281 + /// across seven route modules collapse into this one.
282 + ///
283 + /// A failed preference lookup sends. That matches what every one of those
284 + /// call sites did (`.unwrap_or(true)`) and it is the right default: a
285 + /// database blip should not silently swallow a creator's sale notification.
286 + pub(crate) async fn dispatch(
287 + &self,
288 + class: EmailClass,
289 + audience: Audience<'_>,
290 + subject: &str,
291 + body: &str,
292 + delivery: Delivery<'_>,
293 + ) -> Result<()> {
294 + if let EmailClass::Optional(kind) = class
295 + && !self.wants(audience, kind).await?
296 + {
297 + tracing::debug!(
298 + list_kind = %kind,
299 + "email suppressed by the recipient's notification preference"
300 + );
301 + return Ok(());
302 + }
303 +
304 + let to = audience.email();
305 + if delivery.broadcast {
306 + self.transport
307 + .send_email_broadcast_with_unsub(to, subject, body, delivery.unsub_url)
308 + .await
309 + } else if delivery.headers.is_empty() && delivery.unsub_url.is_none() {
310 + self.transport.send_email(to, subject, body).await
311 + } else {
312 + self.transport
313 + .send_email_with_headers_and_unsub(
314 + to,
315 + subject,
316 + body,
317 + delivery.headers,
318 + delivery.unsub_url,
319 + )
320 + .await
321 + }
322 + }
323 +
324 + /// Whether an `Optional` message may go to this audience.
325 + async fn wants(&self, audience: Audience<'_>, kind: ListKind) -> Result<bool> {
326 + match audience {
327 + Audience::User(user_id, _) => {
328 + let Some(pool) = self.pool.as_ref() else {
329 + return Ok(true);
330 + };
331 + Ok(crate::db::lists::may_notify(pool, user_id, kind)
332 + .await
333 + .unwrap_or(true))
334 + }
335 + Audience::Address(email) => {
336 + // An opt-outable message aimed at an address with no account
337 + // cannot be preference-checked. Sending anyway would be the
338 + // silent bypass this module exists to remove, so refuse and let
339 + // it surface as an error rather than as unstoppable mail.
340 + tracing::error!(
341 + recipient = %email,
342 + list_kind = %kind,
343 + "optional-class email addressed to a bare address; no preference to check"
344 + );
345 + Err(AppError::Internal(anyhow::anyhow!(
346 + "optional-class email ({kind}) requires Audience::User, got Audience::Address"
347 + )))
348 + }
349 + }
219 350 }
220 351 }
221 352
@@ -397,17 +528,6 @@
397 528 self.send_email_inner(to, subject, body, &[], None).await
398 529 }
399 530
400 - async fn send_email_with_unsub(
401 - &self,
402 - to: &str,
403 - subject: &str,
404 - body: &str,
405 - unsub_url: Option<&str>,
406 - ) -> Result<()> {
407 - self.send_with_unsub_inner(to, subject, body, unsub_url, None)
408 - .await
409 - }
410 -
411 531 async fn send_email_with_headers_and_unsub(
412 532 &self,
413 533 to: &str,
@@ -479,6 +599,54 @@
479 599 }
480 600 }
481 601
602 + /// Seal for the notification-preference gate.
603 + ///
604 + /// `may_notify` used to be called from sixteen places across seven route
605 + /// modules, which is how the gate became forgettable: a new email was
606 + /// opt-outable only if whoever wrote the handler remembered, and nothing failed
607 + /// when they did not. The fix moves the check into [`EmailClient::dispatch`],
608 + /// where declaring an [`EmailClass`] is mandatory. This test fails the build if
609 + /// any file outside `db/lists.rs` (its definition) and this module (its one
610 + /// caller) references it again, so re-inlining the check at a call site is not
611 + /// something you can do quietly.
612 + ///
613 + /// `notification_prefs` is deliberately not sealed: it reads the same
614 + /// preferences to render the settings form, which is a different job from
615 + /// gating a send.
616 + #[cfg(test)]
617 + mod notification_gate_seal_guard {
618 + use std::path::Path;
619 +
620 + #[test]
621 + fn may_notify_called_only_from_the_send_path() {
622 + let src_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
623 + let allowed = [
624 + Path::new(env!("CARGO_MANIFEST_DIR")).join("src/db/lists.rs"),
625 + Path::new(env!("CARGO_MANIFEST_DIR")).join("src/email/mod.rs"),
626 + Path::new(env!("CARGO_MANIFEST_DIR")).join("src/email/class.rs"),
627 + ];
628 + let mut offenders = Vec::new();
629 + super::broadcast_cap_seal_guard::walk(&src_dir, &mut |path, contents| {
630 + if allowed.iter().any(|a| a == path) {
631 + return;
632 + }
633 + for (i, line) in contents.lines().enumerate() {
634 + let code = line.trim();
635 + if code.contains("may_notify") && !code.starts_with("//") {
636 + offenders.push(format!("{}:{}: {}", path.display(), i + 1, code));
637 + }
638 + }
639 + });
640 + assert!(
641 + offenders.is_empty(),
642 + "notification-preference seal violated. Do not check may_notify at a call site: \
643 + give the email an EmailClass::Optional(kind) and let EmailClient::dispatch do it, \
644 + so the gate cannot be forgotten by the next handler. Offending lines:\n{}",
645 + offenders.join("\n")
646 + );
647 + }
648 + }
649 +
482 650 /// Seal for the broadcast recipient cap.
483 651 ///
484 652 /// The cap was a copy-pasted `if count > BROADCAST_MAX_RECIPIENTS` check that had
@@ -518,7 +686,7 @@
518 686 );
519 687 }
520 688
521 - fn walk(dir: &Path, f: &mut impl FnMut(&Path, &str)) {
689 + pub(super) fn walk(dir: &Path, f: &mut impl FnMut(&Path, &str)) {
522 690 let Ok(entries) = std::fs::read_dir(dir) else {
523 691 return;
524 692 };