Skip to main content

max / makenotwork

Drop the seven notify_* columns Step 5c of wiki [[mnw-mailing-lists]], the tail of the notification migration. 5b moved all thirteen send decisions onto db::lists::may_notify; these columns have been dead weight since, kept one release so that read flip stayed revertible. THE TRIGGER MOVES IN THE SAME MIGRATION. 187's seed function read NEW.notify_* to give each new account its subscriptions, so dropping the columns without rewriting it breaks the next signup. They cannot be separate migrations: there is a moment between them where one is true. The defaults now live in the function and in db::lists::default_enabled, and nowhere else. Gone with them: set_notification, the preferences page's write-back, and the column half of update_notification_preferences and update_tip_preferences. tips_enabled stays a column, because it is a capability (whether a creator accepts tips at all) rather than a notification preference, despite arriving on the same form. The legacy preference NAMES survive in NOTIFICATION_LISTS. disable_notification still accepts "notify_sale" and friends, because unsubscribe links already sitting in inboxes carry those strings in their signed URLs, and renaming them would invalidate every link ever sent. They map to list kinds now instead of to columns. The account settings tab reads a NotificationPrefs struct built from subscriptions in one query, rather than seven fields off the user row. FOUR TESTS OUTSIDE THE LIST SUITE ASSERTED ON THE COLUMNS and only the full suite caught them: the filtered run I used while working passed clean. Two forced notify_sale on as setup, which the signup default already does; two read the column back. All four now go through subscriptions.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-06 13:53 UTC
Signed with PGP, not checked
Commit: c089397dd4ae7dd2227558821d82e25e6250eb01
Parent: 11516ff
15 files changed, +263 insertions, -284 deletions
M server/src/db/lists.rs +56 -16
@@ -479,25 +479,19 @@
479 479
480 480 // ── Account notification preferences ──
481 481 //
482 - // Seven bool columns on `users` and seven platform lists describe the same
483 - // thing. Reads still use the columns (each one is a send decision, and moving
484 - // nine of those in the same release as the data would give two suspects when
485 - // something stops arriving), so every write has to keep both sides in step, in
486 - // both directions:
482 + // Subscriptions are the whole record. Seven bool columns on `users` held these
483 + // until migration 189; every read moved in 5b and the columns went in 5c, so
484 + // there is one place a preference lives and one place it is read from.
487 485 //
488 - // settings screen / unsubscribe email link -> column, then subscription
489 - // preferences page -> subscription, then column
490 - //
491 - // The second direction is what makes the preferences page truthful. Without it
492 - // a user could unsubscribe from "Sales" there and keep receiving sale
493 - // notifications, because the send still reads the column. A page that says it
494 - // changed something and did not is worse than one that never offered.
486 + // What is left of the columns is their names, in NOTIFICATION_LISTS below:
487 + // unsubscribe links already sent carry them in signed URLs.
495 488
496 - /// Platform lists that mirror a `users.notify_*` column, paired with it.
489 + /// Platform notification lists, paired with the legacy preference name.
497 490 ///
498 - /// The column name is the one `disable_notification` already accepts, so the
499 - /// two agree by construction rather than by a second list somebody has to
500 - /// remember to update.
491 + /// The second element was the `users.notify_*` column until migration 189
492 + /// dropped them. It survives because unsubscribe links already sitting in
493 + /// inboxes carry those names in their signed URLs, and renaming them would
494 + /// invalidate every link ever sent. `disable_notification` maps them back.
501 495 pub const NOTIFICATION_LISTS: &[(&str, &str)] = &[
502 496 ("sale", "notify_sale"),
503 497 ("follower", "notify_follower"),
@@ -564,6 +558,52 @@
564 558 })
565 559 }
566 560
561 + /// Every notification preference for one user, for the settings screen.
562 + ///
563 + /// One query rather than seven `may_notify` calls, because the account tab
564 + /// renders all of them at once. Missing rows fall back to the same defaults
565 + /// `may_notify` uses, so the two cannot disagree about an account the trigger
566 + /// somehow missed.
567 + #[derive(Debug, Clone)]
568 + pub struct NotificationPrefs {
569 + pub sale: bool,
570 + pub follower: bool,
571 + pub release: bool,
572 + pub login: bool,
573 + pub issues: bool,
574 + pub status: bool,
575 + pub tip: bool,
576 + }
577 +
578 + #[tracing::instrument(skip_all)]
579 + pub async fn notification_prefs(pool: &PgPool, user_id: UserId) -> Result<NotificationPrefs> {
580 + let rows = sqlx::query_as::<_, (String, String)>(
581 + "SELECT l.kind, ls.state FROM list_subscriptions ls \
582 + JOIN lists l ON l.id = ls.list_id \
583 + WHERE l.scope = 'platform' AND ls.user_id = $1",
584 + )
585 + .bind(user_id)
586 + .fetch_all(pool)
587 + .await?;
588 +
589 + let enabled = |kind: &str| {
590 + rows.iter().find(|(k, _)| k == kind).map_or_else(
591 + || default_enabled(kind),
592 + |(_, state)| SENDABLE_STATES.contains(&state.as_str()),
593 + )
594 + };
595 +
596 + Ok(NotificationPrefs {
597 + sale: enabled("sale"),
598 + follower: enabled("follower"),
599 + release: enabled("releases"),
600 + login: enabled("login"),
601 + issues: enabled("issues"),
602 + status: enabled("status"),
603 + tip: enabled("tip"),
604 + })
605 + }
606 +
567 607 /// Point a user's notification subscription at `enabled`, appending the consent
568 608 /// event that goes with it. Called after the column is written.
569 609 #[tracing::instrument(skip_all)]
M server/src/db/users.rs +27 -116
@@ -849,7 +849,10 @@
849 849 }
850 850
851 851 /// Update a user's email notification preferences.
852 - /// Email notification toggles for a user, one field per `users` column.
852 + ///
853 + /// Writes subscriptions only. The `users.notify_*` columns these used to set
854 + /// are gone (migration 189); `db::lists` is the single record, and the
855 + /// preferences page and the unsubscribe links write the same rows.
853 856 #[derive(Debug, Clone, Copy)]
854 857 pub struct NotificationPreferences {
855 858 pub notify_sale: bool,
@@ -874,32 +877,7 @@
874 877 notify_issues,
875 878 notify_status,
876 879 } = prefs;
877 - sqlx::query(
878 - r"
879 - UPDATE users
880 - SET notify_sale = $2,
881 - notify_follower = $3,
882 - notify_release = $4,
883 - login_notification_enabled = $5,
884 - notify_issues = $6,
885 - notify_status = $7,
886 - updated_at = NOW()
887 - WHERE id = $1
888 - ",
889 - )
890 - .bind(id)
891 - .bind(notify_sale)
892 - .bind(notify_follower)
893 - .bind(notify_release)
894 - .bind(login_notification_enabled)
895 - .bind(notify_issues)
896 - .bind(notify_status)
897 - .execute(pool)
898 - .await?;
899 880
900 - // Keep the subscription side in step. Reads still use the columns above,
901 - // so this is the mirror rather than the source, but the preferences page
902 - // and any consent question are answered from it.
903 881 for (kind, enabled) in [
904 882 ("sale", notify_sale),
905 883 ("follower", notify_follower),
@@ -914,7 +892,12 @@
914 892 Ok(())
915 893 }
916 894
917 - /// Update a user's tip preferences (tips_enabled toggle and notification).
895 + /// Update tip settings.
896 + ///
897 + /// `tips_enabled` is a capability (whether the creator accepts tips at all) and
898 + /// stays a column. `notify_tip` is a notification preference and now lives in
899 + /// subscriptions with the other six, so the two are written to different
900 + /// places despite arriving from the same form.
918 901 #[tracing::instrument(skip_all)]
919 902 pub async fn update_tip_preferences(
920 903 pool: &PgPool,
@@ -922,109 +905,37 @@
922 905 tips_enabled: bool,
923 906 notify_tip: bool,
924 907 ) -> Result<()> {
925 - sqlx::query(
926 - r"
927 - UPDATE users
928 - SET tips_enabled = $2,
929 - notify_tip = $3,
930 - updated_at = NOW()
931 - WHERE id = $1
932 - ",
933 - )
934 - .bind(id)
935 - .bind(tips_enabled)
936 - .bind(notify_tip)
937 - .execute(pool)
938 - .await?;
908 + sqlx::query("UPDATE users SET tips_enabled = $2, updated_at = NOW() WHERE id = $1")
909 + .bind(id)
910 + .bind(tips_enabled)
911 + .execute(pool)
912 + .await?;
939 913
914 + crate::db::lists::sync_notification_subscription(pool, id, "tip", notify_tip).await?;
940 915 Ok(())
941 916 }
942 917
943 - /// Disable a single notification preference by column name.
918 + /// Turn one notification off, by the legacy preference name the unsubscribe
919 + /// links carry.
944 920 ///
945 - /// Used by the email unsubscribe handler. Only accepts known column names
946 - /// to prevent SQL injection.
921 + /// The names are the old column names because they are baked into signed URLs
922 + /// already sitting in inboxes. They map to list kinds here rather than being
923 + /// renamed, which would invalidate every link ever sent.
947 924 #[tracing::instrument(skip_all)]
948 925 pub async fn disable_notification(
949 926 pool: &PgPool,
950 927 user_id: UserId,
951 928 preference: &str,
952 929 ) -> Result<bool> {
953 - let sql = match preference {
954 - "notify_sale" => "UPDATE users SET notify_sale = false, updated_at = NOW() WHERE id = $1",
955 - "notify_follower" => {
956 - "UPDATE users SET notify_follower = false, updated_at = NOW() WHERE id = $1"
957 - }
958 - "notify_release" => {
959 - "UPDATE users SET notify_release = false, updated_at = NOW() WHERE id = $1"
960 - }
961 - "login_notification_enabled" => {
962 - "UPDATE users SET login_notification_enabled = false, updated_at = NOW() WHERE id = $1"
963 - }
964 - "notify_issues" => {
965 - "UPDATE users SET notify_issues = false, updated_at = NOW() WHERE id = $1"
966 - }
967 - "notify_tip" => "UPDATE users SET notify_tip = false, updated_at = NOW() WHERE id = $1",
968 - "notify_status" => {
969 - "UPDATE users SET notify_status = false, updated_at = NOW() WHERE id = $1"
970 - }
971 - _ => return Ok(false),
972 - };
973 - let result = sqlx::query(sql).bind(user_id).execute(pool).await?;
974 -
975 - // Same mirror, for the single-preference path the email links use.
976 - if let Some(kind) = crate::db::lists::NOTIFICATION_LISTS
930 + let Some(kind) = crate::db::lists::NOTIFICATION_LISTS
977 931 .iter()
978 - .find(|(_, col)| *col == preference)
932 + .find(|(_, legacy)| *legacy == preference)
979 933 .map(|(kind, _)| *kind)
980 - {
981 - crate::db::lists::sync_notification_subscription(pool, user_id, kind, false).await?;
982 - }
983 -
984 - Ok(result.rows_affected() > 0)
985 - }
986 -
987 - /// Set one notification column, either way.
988 - ///
989 - /// The write-back half of the preferences page: reads still use these columns,
990 - /// so a subscription change has to land here to have any effect.
991 - ///
992 - /// Deliberately does NOT mirror into subscriptions, unlike
993 - /// [`disable_notification`]. This function is called *by* the subscription
994 - /// side, and mirroring back would loop and append a duplicate consent event
995 - /// for a change already recorded.
996 - ///
997 - /// `column` comes from `lists::NOTIFICATION_LISTS`, never from a request, and
998 - /// is matched against a fixed set anyway so it cannot reach the query as text.
999 - #[tracing::instrument(skip_all)]
1000 - pub async fn set_notification(
1001 - pool: &PgPool,
1002 - user_id: UserId,
1003 - column: &str,
1004 - enabled: bool,
1005 - ) -> Result<bool> {
1006 - let sql = match column {
1007 - "notify_sale" => "UPDATE users SET notify_sale = $2, updated_at = NOW() WHERE id = $1",
1008 - "notify_follower" => {
1009 - "UPDATE users SET notify_follower = $2, updated_at = NOW() WHERE id = $1"
1010 - }
1011 - "notify_release" => {
1012 - "UPDATE users SET notify_release = $2, updated_at = NOW() WHERE id = $1"
1013 - }
1014 - "login_notification_enabled" => {
1015 - "UPDATE users SET login_notification_enabled = $2, updated_at = NOW() WHERE id = $1"
1016 - }
1017 - "notify_issues" => "UPDATE users SET notify_issues = $2, updated_at = NOW() WHERE id = $1",
1018 - "notify_tip" => "UPDATE users SET notify_tip = $2, updated_at = NOW() WHERE id = $1",
1019 - "notify_status" => "UPDATE users SET notify_status = $2, updated_at = NOW() WHERE id = $1",
1020 - _ => return Ok(false),
934 + else {
935 + return Ok(false);
1021 936 };
1022 - let result = sqlx::query(sql)
1023 - .bind(user_id)
1024 - .bind(enabled)
1025 - .execute(pool)
1026 - .await?;
1027 - Ok(result.rows_affected() > 0)
937 + crate::db::lists::sync_notification_subscription(pool, user_id, kind, false).await?;
938 + Ok(true)
1028 939 }
1029 940
1030 941 /// A user who opted into platform status notifications.
M server/src/templates/partials.rs +4
@@ -220,6 +220,10 @@
220 220 #[template(path = "partials/tabs/user_account.html")]
221 221 pub struct UserAccountTabTemplate {
222 222 pub user: User,
223 + /// Notification toggles, read from subscriptions rather than off `user`.
224 + /// The columns they used to live on were dropped in migration 189; the
225 + /// subscription is the record now.
226 + pub notifications: crate::db::lists::NotificationPrefs,
223 227 pub sessions: Vec<DbUserSession>,
224 228 pub current_session_id: Option<UserSessionId>,
225 229 /// Whether this user has creator access (controls creator-specific prefs).
M server/src/types/conversions.rs -7
@@ -64,14 +64,7 @@
64 64 stripe_payouts_enabled: u.stripe_payouts_enabled,
65 65 stripe_charges_enabled: u.stripe_charges_enabled,
66 66 stripe_tax_enabled: u.stripe_tax_enabled,
67 - notify_sale: u.notify_sale,
68 - notify_follower: u.notify_follower,
69 - notify_release: u.notify_release,
70 - login_notification_enabled: u.login_notification_enabled,
71 - notify_issues: u.notify_issues,
72 - notify_status: u.notify_status,
73 67 tips_enabled: u.tips_enabled,
74 - notify_tip: u.notify_tip,
75 68 }
76 69 }
77 70 }
M server/src/types/mod.rs -7
@@ -87,14 +87,7 @@
87 87 stripe_payouts_enabled: payouts,
88 88 stripe_charges_enabled: false,
89 89 stripe_tax_enabled: false,
90 - notify_sale: true,
91 - notify_follower: true,
92 - notify_release: true,
93 - login_notification_enabled: true,
94 - notify_issues: true,
95 - notify_status: false,
96 90 tips_enabled: false,
97 - notify_tip: true,
98 91 }
99 92 }
100 93
M server/src/types/user.rs -7
@@ -20,15 +20,8 @@
20 20 pub stripe_charges_enabled: bool,
21 21 pub stripe_tax_enabled: bool,
22 22 // Notification preferences
23 - pub notify_sale: bool,
24 - pub notify_follower: bool,
25 - pub notify_release: bool,
26 - pub login_notification_enabled: bool,
27 - pub notify_issues: bool,
28 - pub notify_status: bool,
29 23 // Tips
30 24 pub tips_enabled: bool,
31 - pub notify_tip: bool,
32 25 }
33 26
34 27 impl User {
M server/tests/workflows/failure_paths_checkout.rs -6
M server/tests/workflows/git_issues.rs +16 -22
M server/tests/workflows/lists.rs +94 -42
M server/tests/workflows/mock_payment_flows.rs -5
M server/src/db/models/user.rs -21
M server/templates/partials/tabs/user_account.html +7 -7
M server/src/routes/pages/email_actions/account.rs -28
M server/src/routes/pages/dashboard/tabs/user/mod.rs +3
A server/migrations/189_drop_notify_columns.sql +56