Skip to main content

max / makenotwork

Read notification preferences from subscriptions, not columns Step 5b of wiki [[mnw-mailing-lists]], the half step 5 deferred. Every send decision now asks db::lists::may_notify instead of reading a users.notify_* column, so the subscription is what decides and the preferences page, the unsubscribe link and the settings screen all move the same thing. THE TASK SAID NINE READS. THERE WERE THIRTEEN. The four I missed when enumerating them during step 5: routes/pages/email_actions/links.rs:204, :243 magic-link login alert routes/api/users/library.rs:133 free-claim sale notice routes/stripe/checkout/item.rs:287 free-claim sale notice My step-5 grep filtered out email_actions and a few other paths as noise, which is exactly the failure this step exists to prevent: a missed read keeps mailing somebody who opted out. Found by re-running the sweep with no exclusions once the known nine were done, and there is now a sweep in the commit history showing zero remaining. may_notify falls back to a documented default when a subscription is missing rather than refusing, because the failure mode of a bug here should be mail somebody expected rather than silence they cannot diagnose. Status alerts default off, everything else on, matching the column defaults they replace. The fallback should be unreachable: the 186 backfill covered the accounts that existed and the 187 trigger covers every one since, and a test asserts no account is missing a row whichever path created it. The status-alert query now joins subscriptions instead of filtering on notify_status. THE COLUMNS STAY, and so does the preferences page's write-back to them. The task suggested dropping the write-back here; keeping it one more deploy is what makes this commit revertible without a code change, which on a read flip is worth more than one redundant UPDATE. The columns and the write-back go together in the follow-up, once this has had a deploy to prove itself.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-06 04:46 UTC
Signed with PGP, not checked
Commit: 11516ff4d12afb106366f28228bd0a7477b38f51
Parent: 9558d4c
10 files changed, +267 insertions, -17 deletions
M server/src/db/lists.rs +48
@@ -516,6 +516,54 @@
516 516 .map(|(_, col)| *col)
517 517 }
518 518
519 + /// What a preference is when nobody has said otherwise.
520 + ///
521 + /// Matches the `users` column defaults, which is where these lived until the
522 + /// reads moved. Only reached if a subscription row is missing, which the 186
523 + /// backfill and the 187 trigger between them should make impossible; the
524 + /// fallback exists so a missing row degrades to the documented default rather
525 + /// than to silence. `notification_rows_exist_for_every_account` is the test
526 + /// that keeps it unreachable.
527 + fn default_enabled(kind: &str) -> bool {
528 + // Status alerts are the one opt-in: they are platform operations noise, and
529 + // a new account has not asked for them.
530 + kind != "status"
531 + }
532 +
533 + /// May this user be sent this kind of notification?
534 + ///
535 + /// The single read for every account notification. Each of these used to be a
536 + /// `users.notify_*` column consulted at the send site, which is why the rules
537 + /// could differ per site and why opting out was all-or-nothing.
538 + ///
539 + /// A missing subscription falls back to [`default_enabled`] rather than
540 + /// refusing: the failure mode of a bug here should be mail somebody expected,
541 + /// not silence they cannot diagnose.
542 + #[tracing::instrument(skip_all)]
543 + pub async fn may_notify(pool: &PgPool, user_id: UserId, kind: ListKind) -> Result<bool> {
544 + let kind = kind.to_string();
545 + let state = sqlx::query_scalar::<_, String>(
546 + "SELECT ls.state FROM list_subscriptions ls \
547 + JOIN lists l ON l.id = ls.list_id \
548 + WHERE l.scope = 'platform' AND l.kind = $1 AND ls.user_id = $2",
549 + )
550 + .bind(&kind)
551 + .bind(user_id)
552 + .fetch_optional(pool)
553 + .await?;
554 +
555 + Ok(match state {
556 + Some(s) => SENDABLE_STATES.contains(&s.as_str()),
557 + None => {
558 + tracing::warn!(
559 + user_id = %user_id, kind = %kind,
560 + "no notification subscription row; falling back to the default"
561 + );
562 + default_enabled(&kind)
563 + }
564 + })
565 + }
566 +
519 567 /// Point a user's notification subscription at `enabled`, appending the consent
520 568 /// event that goes with it. Called after the column is written.
521 569 #[tracing::instrument(skip_all)]
M server/src/db/users.rs +5 -3
@@ -1046,9 +1046,11 @@
1046 1046 pub async fn get_status_alert_subscribers(pool: &PgPool) -> Result<Vec<StatusAlertSubscriber>> {
1047 1047 const STATUS_SUBSCRIBER_CAP: i64 = 10_000;
1048 1048 let rows = sqlx::query_as::<_, StatusAlertSubscriber>(
1049 - "SELECT id, email, display_name FROM users \
1050 - WHERE notify_status = true AND deactivated_at IS NULL \
1051 - ORDER BY id LIMIT $1",
1049 + "SELECT u.id, u.email, u.display_name FROM users u \
1050 + JOIN list_subscriptions ls ON ls.user_id = u.id \
1051 + JOIN lists l ON l.id = ls.list_id AND l.scope = 'platform' AND l.kind = 'status' \
1052 + WHERE ls.state IN ('confirmed', 'imported') AND u.deactivated_at IS NULL \
1053 + ORDER BY u.id LIMIT $1",
1052 1054 )
1053 1055 .bind(STATUS_SUBSCRIBER_CAP)
1054 1056 .fetch_all(pool)
M server/src/routes/auth.rs +9 -3
@@ -262,7 +262,9 @@
262 262 session
263 263 .insert(
264 264 "pending_2fa_notify_enabled",
265 - user.login_notification_enabled,
265 + db::lists::may_notify(&db, user.id, db::ListKind::Login)
266 + .await
267 + .unwrap_or(true),
266 268 )
267 269 .await
268 270 .context("session insert")?;
@@ -312,7 +314,9 @@
312 314 let user_id = user.id;
313 315 let notify_email = user.email.clone();
314 316 let notify_name = user.display_name.clone();
315 - let notify_enabled = user.login_notification_enabled;
317 + let notify_enabled = db::lists::may_notify(&db, user.id, db::ListKind::Login)
318 + .await
319 + .unwrap_or(true);
316 320
317 321 let session_user = SessionUser::from_db_user(user, &db, config.admin_user_id).await;
318 322
@@ -584,7 +588,9 @@
584 588 let passkey_user_id = user.id;
585 589 let notify_email = user.email.clone();
586 590 let notify_name = user.display_name.clone();
587 - let notify_enabled = user.login_notification_enabled;
591 + let notify_enabled = db::lists::may_notify(&db, user.id, db::ListKind::Login)
592 + .await
593 + .unwrap_or(true);
588 594 let session_user = SessionUser::from_db_user(user, &db, config.admin_user_id).await;
589 595
590 596 login_user(&session, session_user).await?;
M server/tests/workflows/lists.rs +156
@@ -1048,3 +1048,159 @@
1048 1048 .unwrap();
1049 1049 assert_eq!(title, "renamed: issues");
1050 1050 }
1051 +
1052 + // ── Step 5b: the notification reads ──
1053 +
1054 + /// The invariant every read now rests on. `may_notify` falls back to a default
1055 + /// when a subscription is missing, and that fallback should be unreachable: the
1056 + /// 186 backfill covered the accounts that existed and the 187 trigger covers
1057 + /// every one created since, whichever path created it.
1058 + #[tokio::test]
1059 + async fn notification_rows_exist_for_every_account() {
1060 + let mut h = TestHarness::new().await;
1061 + h.signup("viahandler", "viahandler@test.com", "password123")
1062 + .await;
1063 + // A direct insert, the path the seed flow and the harness itself use.
1064 + sqlx::query(
1065 + "INSERT INTO users (username, email, password_hash, email_verified) \
1066 + VALUES ('viasql', 'viasql@test.com', 'x', true)",
1067 + )
1068 + .execute(&h.db)
1069 + .await
1070 + .unwrap();
1071 +
1072 + let missing: i64 = sqlx::query_scalar(
1073 + "SELECT COUNT(*) FROM users u \
1074 + CROSS JOIN lists l \
1075 + WHERE l.scope = 'platform' \
1076 + AND l.kind IN ('sale','follower','releases','issues','status','tip','login') \
1077 + AND NOT EXISTS ( \
1078 + SELECT 1 FROM list_subscriptions ls \
1079 + WHERE ls.list_id = l.id AND ls.user_id = u.id)",
1080 + )
1081 + .fetch_one(&h.db)
1082 + .await
1083 + .unwrap();
1084 + assert_eq!(
1085 + missing, 0,
1086 + "accounts are missing notification subscriptions"
1087 + );
1088 + }
1089 +
1090 + /// Status alerts are the one opt-in default, and the read has to preserve that.
1091 + /// Everything else defaults to on.
1092 + #[tokio::test]
1093 + async fn notification_defaults_survive_the_move() {
1094 + let mut h = TestHarness::new().await;
1095 + let user = h
1096 + .signup("defaults", "defaults@test.com", "password123")
1097 + .await;
1098 +
1099 + for kind in [
1100 + ListKind::Sale,
1101 + ListKind::Follower,
1102 + ListKind::Releases,
1103 + ListKind::Issues,
1104 + ListKind::Tip,
1105 + ListKind::Login,
1106 + ] {
1107 + assert!(
1108 + lists::may_notify(&h.db, user, kind).await.unwrap(),
1109 + "{kind} should default on"
1110 + );
1111 + }
1112 + assert!(
1113 + !lists::may_notify(&h.db, user, ListKind::Status)
1114 + .await
1115 + .unwrap(),
1116 + "status alerts should default off"
1117 + );
1118 + }
1119 +
1120 + /// The read follows the subscription, which is the whole point of the move.
1121 + #[tokio::test]
1122 + async fn may_notify_follows_the_subscription() {
1123 + let mut h = TestHarness::new().await;
1124 + let user = h.signup("follows", "follows@test.com", "password123").await;
1125 + assert!(
1126 + lists::may_notify(&h.db, user, ListKind::Sale)
1127 + .await
1128 + .unwrap()
1129 + );
1130 +
1131 + let sub = notification_subscription(&h, user, ListKind::Sale).await;
1132 + lists::unsubscribe(&h.db, sub, ConsentEvent::OptOut)
1133 + .await
1134 + .unwrap();
1135 +
1136 + assert!(
1137 + !lists::may_notify(&h.db, user, ListKind::Sale)
1138 + .await
1139 + .unwrap(),
1140 + "a send would still fire for somebody who opted out"
1141 + );
1142 + }
1143 +
1144 + /// Unsubscribing on the preferences page reaches the read. Before 5b this
1145 + /// worked only because the page also wrote the column; now it is the
1146 + /// subscription itself doing the work.
1147 + #[tokio::test]
1148 + async fn the_preferences_page_now_drives_the_read_directly() {
1149 + let mut h = TestHarness::new().await;
1150 + let user = h.signup("viapage", "viapage@test.com", "password123").await;
1151 + let sub = notification_subscription(&h, user, ListKind::Sale).await;
1152 +
1153 + let resp = h
1154 + .client
1155 + .post_form(&prefs_url(sub), "List-Unsubscribe=One-Click")
1156 + .await;
1157 + assert_eq!(resp.status, 200);
1158 +
1159 + assert!(
1160 + !lists::may_notify(&h.db, user, ListKind::Sale)
1161 + .await
1162 + .unwrap(),
1163 + "the page unsubscribed them but the send would still fire"
1164 + );
1165 + }
1166 +
1167 + /// Status alerts are drawn from subscriptions now, not the column.
1168 + #[tokio::test]
1169 + async fn status_alert_recipients_come_from_subscriptions() {
1170 + let mut h = TestHarness::new().await;
1171 + let user = h
1172 + .signup("statusfan", "statusfan@test.com", "password123")
1173 + .await;
1174 +
1175 + let before = makenotwork::db::users::get_status_alert_subscribers(&h.db)
1176 + .await
1177 + .unwrap();
1178 + assert!(
1179 + !before.iter().any(|s| s.id == user),
1180 + "status alerts default off"
1181 + );
1182 +
1183 + let list = lists::find_list(&h.db, ListScope::Platform, None, ListKind::Status)
1184 + .await
1185 + .unwrap()
1186 + .unwrap();
1187 + lists::subscribe(
1188 + &h.db,
1189 + list,
1190 + &lists::Subscriber::User(user),
1191 + SubscriptionState::Confirmed,
1192 + SubscriptionSource::Admin,
1193 + ConsentEvent::OptIn,
1194 + None,
1195 + )
1196 + .await
1197 + .unwrap();
1198 +
1199 + let after = makenotwork::db::users::get_status_alert_subscribers(&h.db)
1200 + .await
1201 + .unwrap();
1202 + assert!(
1203 + after.iter().any(|s| s.id == user),
1204 + "opting in did not reach the status-alert query"
1205 + );
1206 + }
M server/src/routes/api/follows.rs +6 -2
@@ -56,7 +56,9 @@
56 56 FollowTargetType::User => {
57 57 if let Ok(Some(target_user)) =
58 58 db::users::get_user_by_id(&db, UserId::from(target_id)).await
59 - && target_user.notify_follower
59 + && db::lists::may_notify(&db, target_user.id, db::ListKind::Follower)
60 + .await
61 + .unwrap_or(true)
60 62 {
61 63 let follower_user = db::users::get_user_by_id(&db, user.id).await.ok().flatten();
62 64 let follower_username = follower_user
@@ -92,7 +94,9 @@
92 94 if let Ok(Some(project)) =
93 95 db::projects::get_project_by_id(&db, db::ProjectId::from(target_id)).await
94 96 && let Ok(Some(owner)) = db::users::get_user_by_id(&db, project.user_id).await
95 - && owner.notify_follower
97 + && db::lists::may_notify(&db, owner.id, db::ListKind::Follower)
98 + .await
99 + .unwrap_or(true)
96 100 {
97 101 let follower_user = db::users::get_user_by_id(&db, user.id).await.ok().flatten();
98 102 let follower_username = follower_user
M server/src/routes/postmark/issues.rs +8 -2
@@ -248,7 +248,10 @@
248 248 )
249 249 .await
250 250 .unwrap_or(false);
251 - if sender.id != owner_user.id && owner_user.notify_issues && !owner_muted {
251 + let owner_wants_issues = db::lists::may_notify(&db, owner_user.id, db::ListKind::Issues)
252 + .await
253 + .unwrap_or(true);
254 + if sender.id != owner_user.id && owner_wants_issues && !owner_muted {
252 255 let email_client = email.clone();
253 256 let host_url = config.host_url.clone();
254 257 let signing_secret = config.signing_secret.clone();
@@ -480,7 +483,10 @@
480 483 .await
481 484 .unwrap_or_default();
482 485 for user in users {
483 - if !user.notify_issues {
486 + if !db::lists::may_notify(&db, user.id, db::ListKind::Issues)
487 + .await
488 + .unwrap_or(true)
489 + {
484 490 continue;
485 491 }
486 492 // Same per-repo mute for issue participants.
M server/src/routes/api/users/library.rs +4 -1
M server/src/routes/pages/email_actions/links.rs +6 -2
M server/src/routes/stripe/checkout/item.rs +4 -1
M server/src/routes/stripe/webhook/checkout_helpers.rs +21 -3