max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
12 files changed,
+242 insertions,
-26 deletions
| @@ -1214,6 +1214,9 @@ | |||
| 1214 | 1214 | Login, | |
| 1215 | 1215 | Status, | |
| 1216 | 1216 | Tip, | |
| 1217 | + | // Someone redeemed one of your invite codes. Added by migration 195, when | |
| 1218 | + | // that notice stopped being mail you could not turn off. | |
| 1219 | + | Invite, | |
| 1217 | 1220 | } | |
| 1218 | 1221 | ||
| 1219 | 1222 | impl_str_enum!(ListKind { | |
| @@ -1229,8 +1232,31 @@ | |||
| 1229 | 1232 | Login => "login", | |
| 1230 | 1233 | Status => "status", | |
| 1231 | 1234 | Tip => "tip", | |
| 1235 | + | Invite => "invite", | |
| 1232 | 1236 | }); | |
| 1233 | 1237 | ||
| 1238 | + | impl ListKind { | |
| 1239 | + | /// Every kind. Exists so a test can assert the enum against the | |
| 1240 | + | /// `lists_kind_check` constraint: a kind lives in two places, and adding it | |
| 1241 | + | /// to only one fails at INSERT on a deployed database rather than at | |
| 1242 | + | /// compile time here. | |
| 1243 | + | pub const ALL: &'static [ListKind] = &[ | |
| 1244 | + | ListKind::Content, | |
| 1245 | + | ListKind::Devlog, | |
| 1246 | + | ListKind::Patches, | |
| 1247 | + | ListKind::Releases, | |
| 1248 | + | ListKind::Issues, | |
| 1249 | + | ListKind::Announce, | |
| 1250 | + | ListKind::Marketing, | |
| 1251 | + | ListKind::Sale, | |
| 1252 | + | ListKind::Follower, | |
| 1253 | + | ListKind::Login, | |
| 1254 | + | ListKind::Status, | |
| 1255 | + | ListKind::Tip, | |
| 1256 | + | ListKind::Invite, | |
| 1257 | + | ]; | |
| 1258 | + | } | |
| 1259 | + | ||
| 1234 | 1260 | /// Where a subscription stands. | |
| 1235 | 1261 | /// | |
| 1236 | 1262 | /// `Imported` is its own state on purpose. Everything the step-2 backfill |
| @@ -520,7 +520,8 @@ | |||
| 520 | 520 | /// that keeps it unreachable. | |
| 521 | 521 | fn default_enabled(kind: &str) -> bool { | |
| 522 | 522 | // Status alerts are the one opt-in: they are platform operations noise, and | |
| 523 | - | // a new account has not asked for them. | |
| 523 | + | // a new account has not asked for them. Everything else, `invite` included | |
| 524 | + | // (migration 195, which has no column behind it), is on until turned off. | |
| 524 | 525 | kind != "status" | |
| 525 | 526 | } | |
| 526 | 527 | ||
| @@ -573,6 +574,7 @@ | |||
| 573 | 574 | pub issues: bool, | |
| 574 | 575 | pub status: bool, | |
| 575 | 576 | pub tip: bool, | |
| 577 | + | pub invite: bool, | |
| 576 | 578 | } | |
| 577 | 579 | ||
| 578 | 580 | #[tracing::instrument(skip_all)] | |
| @@ -601,6 +603,7 @@ | |||
| 601 | 603 | issues: enabled("issues"), | |
| 602 | 604 | status: enabled("status"), | |
| 603 | 605 | tip: enabled("tip"), | |
| 606 | + | invite: enabled("invite"), | |
| 604 | 607 | }) | |
| 605 | 608 | } | |
| 606 | 609 | ||
| @@ -852,6 +855,61 @@ | |||
| 852 | 855 | } | |
| 853 | 856 | } | |
| 854 | 857 | ||
| 858 | + | /// Every `ListKind` is accepted by the database. | |
| 859 | + | /// | |
| 860 | + | /// A kind lives in two places: this enum and the `lists_kind_check` | |
| 861 | + | /// constraint. Adding it to only the enum compiles, passes every unit test, | |
| 862 | + | /// and then fails at INSERT against a deployed database, which is a long | |
| 863 | + | /// way from the edit that caused it. So the constraint is read back here. | |
| 864 | + | /// | |
| 865 | + | /// Reads the last migration that redefines the constraint, since each one | |
| 866 | + | /// replaces the previous in full (186, then 195). | |
| 867 | + | #[test] | |
| 868 | + | fn every_kind_is_allowed_by_the_check_constraint() { | |
| 869 | + | let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("migrations"); | |
| 870 | + | let mut files: Vec<_> = std::fs::read_dir(&dir) | |
| 871 | + | .expect("migrations directory") | |
| 872 | + | .filter_map(|e| e.ok().map(|e| e.path())) | |
| 873 | + | .filter(|p| { | |
| 874 | + | std::fs::read_to_string(p).is_ok_and(|s| s.contains("lists_kind_check CHECK")) | |
| 875 | + | }) | |
| 876 | + | .collect(); | |
| 877 | + | files.sort(); | |
| 878 | + | let newest = files | |
| 879 | + | .last() | |
| 880 | + | .expect("some migration defines lists_kind_check"); | |
| 881 | + | let sql = std::fs::read_to_string(newest).expect("readable migration"); | |
| 882 | + | ||
| 883 | + | let clause = sql | |
| 884 | + | .split_once("lists_kind_check CHECK (kind IN (") | |
| 885 | + | .expect("the constraint has the expected shape") | |
| 886 | + | .1 | |
| 887 | + | .split_once("))") | |
| 888 | + | .expect("the constraint list is closed") | |
| 889 | + | .0; | |
| 890 | + | let allowed: Vec<&str> = clause | |
| 891 | + | .split(',') | |
| 892 | + | .map(|s| s.trim().trim_matches('\'').trim()) | |
| 893 | + | .filter(|s| !s.is_empty()) | |
| 894 | + | .collect(); | |
| 895 | + | ||
| 896 | + | for kind in ListKind::ALL { | |
| 897 | + | let s = kind.to_string(); | |
| 898 | + | assert!( | |
| 899 | + | allowed.contains(&s.as_str()), | |
| 900 | + | "ListKind::{kind:?} (\"{s}\") is not in the lists_kind_check constraint. \ | |
| 901 | + | Adding a kind takes a migration as well as an enum variant, or the first \ | |
| 902 | + | insert of one fails on a deployed database.", | |
| 903 | + | ); | |
| 904 | + | } | |
| 905 | + | assert_eq!( | |
| 906 | + | allowed.len(), | |
| 907 | + | ListKind::ALL.len(), | |
| 908 | + | "the constraint allows {allowed:?}, which is not the set ListKind names. A kind \ | |
| 909 | + | the database accepts but the enum cannot represent is unreachable from the code.", | |
| 910 | + | ); | |
| 911 | + | } | |
| 912 | + | ||
| 855 | 913 | /// Which states receive mail is a policy, and an open one. Changing this | |
| 856 | 914 | /// set changes who gets email, so it should be an edit somebody made on | |
| 857 | 915 | /// purpose rather than a line that moved during a refactor. |
| @@ -905,6 +905,7 @@ | |||
| 905 | 905 | pub login_notification_enabled: bool, | |
| 906 | 906 | pub notify_issues: bool, | |
| 907 | 907 | pub notify_status: bool, | |
| 908 | + | pub notify_invite: bool, | |
| 908 | 909 | } | |
| 909 | 910 | ||
| 910 | 911 | #[tracing::instrument(skip_all)] | |
| @@ -920,6 +921,7 @@ | |||
| 920 | 921 | login_notification_enabled, | |
| 921 | 922 | notify_issues, | |
| 922 | 923 | notify_status, | |
| 924 | + | notify_invite, | |
| 923 | 925 | } = prefs; | |
| 924 | 926 | ||
| 925 | 927 | for (kind, enabled) in [ | |
| @@ -929,6 +931,7 @@ | |||
| 929 | 931 | ("issues", notify_issues), | |
| 930 | 932 | ("status", notify_status), | |
| 931 | 933 | ("login", login_notification_enabled), | |
| 934 | + | ("invite", notify_invite), | |
| 932 | 935 | ] { | |
| 933 | 936 | crate::db::lists::sync_notification_subscription(pool, id, kind, enabled).await?; | |
| 934 | 937 | } | |
| @@ -959,23 +962,32 @@ | |||
| 959 | 962 | Ok(()) | |
| 960 | 963 | } | |
| 961 | 964 | ||
| 962 | - | /// Turn one notification off, by the legacy preference name the unsubscribe | |
| 963 | - | /// links carry. | |
| 965 | + | /// Turn one notification off, by the name the unsubscribe link carries. | |
| 964 | 966 | /// | |
| 965 | - | /// The names are the old column names because they are baked into signed URLs | |
| 966 | - | /// already sitting in inboxes. They map to list kinds here rather than being | |
| 967 | - | /// renamed, which would invalidate every link ever sent. | |
| 967 | + | /// For the seven original preferences that name is the old `users.notify_*` | |
| 968 | + | /// column, because those names are baked into signed URLs already sitting in | |
| 969 | + | /// inboxes; they map back to list kinds here rather than being renamed, which | |
| 970 | + | /// would invalidate every link ever sent. | |
| 971 | + | /// | |
| 972 | + | /// A kind with no legacy column (`invite`, migration 195) carries its kind name | |
| 973 | + | /// instead. Nothing older is in an inbox to be broken, so there is no column | |
| 974 | + | /// name to preserve and inventing one would be cargo cult. | |
| 968 | 975 | #[tracing::instrument(skip_all)] | |
| 969 | 976 | pub async fn disable_notification( | |
| 970 | 977 | pool: &PgPool, | |
| 971 | 978 | user_id: UserId, | |
| 972 | 979 | preference: &str, | |
| 973 | 980 | ) -> Result<bool> { | |
| 974 | - | let Some(kind) = crate::db::lists::NOTIFICATION_LISTS | |
| 981 | + | let legacy: Option<&str> = crate::db::lists::NOTIFICATION_LISTS | |
| 975 | 982 | .iter() | |
| 976 | 983 | .find(|(_, legacy)| *legacy == preference) | |
| 977 | - | .map(|(kind, _)| *kind) | |
| 978 | - | else { | |
| 984 | + | .map(|(kind, _)| *kind); | |
| 985 | + | let Some(kind) = legacy.or_else(|| { | |
| 986 | + | preference | |
| 987 | + | .parse::<crate::db::ListKind>() | |
| 988 | + | .ok() | |
| 989 | + | .map(|_| preference) | |
| 990 | + | }) else { | |
| 979 | 991 | return Ok(false); | |
| 980 | 992 | }; | |
| 981 | 993 | crate::db::lists::sync_notification_subscription(pool, user_id, kind, false).await?; |
| @@ -80,7 +80,6 @@ | |||
| 80 | 80 | ContentExport, | |
| 81 | 81 | CreatorDeparture, | |
| 82 | 82 | UsageLimit, | |
| 83 | - | InviteRedeemed, | |
| 84 | 83 | /// Mail to an operator address (support routing, webhook failures, monitor | |
| 85 | 84 | /// alerts). Not addressed to a user account at all, so there is no | |
| 86 | 85 | /// preference to consult; it is named rather than left as an unclassified | |
| @@ -113,7 +112,6 @@ | |||
| 113 | 112 | OperationalKind::ContentExport, | |
| 114 | 113 | OperationalKind::CreatorDeparture, | |
| 115 | 114 | OperationalKind::UsageLimit, | |
| 116 | - | OperationalKind::InviteRedeemed, | |
| 117 | 115 | OperationalKind::OperatorAlert, | |
| 118 | 116 | ]; | |
| 119 | 117 | ||
| @@ -139,7 +137,6 @@ | |||
| 139 | 137 | Self::ContentExport => "Content export", | |
| 140 | 138 | Self::CreatorDeparture => "A creator you bought from is leaving", | |
| 141 | 139 | Self::UsageLimit => "Usage limit warning", | |
| 142 | - | Self::InviteRedeemed => "Your invite was used", | |
| 143 | 140 | Self::OperatorAlert => "Operator alert", | |
| 144 | 141 | } | |
| 145 | 142 | } | |
| @@ -225,10 +222,6 @@ | |||
| 225 | 222 | "You are approaching or have reached a plan limit. Past it, requests are \ | |
| 226 | 223 | refused, so a silent limit would read as an outage." | |
| 227 | 224 | } | |
| 228 | - | Self::InviteRedeemed => { | |
| 229 | - | "Someone signed up with your invite code. Sent because invites are a finite \ | |
| 230 | - | thing you hold and spend." | |
| 231 | - | } | |
| 232 | 225 | Self::OperatorAlert => { | |
| 233 | 226 | "Sent to a Makenotwork operations address, not to a user account." | |
| 234 | 227 | } | |
| @@ -309,7 +302,6 @@ | |||
| 309 | 302 | "Content export", | |
| 310 | 303 | "A creator you bought from is leaving", | |
| 311 | 304 | "Usage limit warning", | |
| 312 | - | "Your invite was used", | |
| 313 | 305 | "Operator alert", | |
| 314 | 306 | ]; | |
| 315 | 307 | assert_eq!( | |
| @@ -349,11 +341,10 @@ | |||
| 349 | 341 | | OperationalKind::ContentExport | |
| 350 | 342 | | OperationalKind::CreatorDeparture | |
| 351 | 343 | | OperationalKind::UsageLimit | |
| 352 | - | | OperationalKind::InviteRedeemed | |
| 353 | 344 | | OperationalKind::OperatorAlert => k.justification(), | |
| 354 | 345 | }; | |
| 355 | 346 | } | |
| 356 | - | assert_eq!(OperationalKind::ALL.len(), 21); | |
| 347 | + | assert_eq!(OperationalKind::ALL.len(), 20); | |
| 357 | 348 | } | |
| 358 | 349 | ||
| 359 | 350 | /// The published guide page matches the enum. |
| @@ -16,6 +16,9 @@ | |||
| 16 | 16 | Status, | |
| 17 | 17 | MailingList, | |
| 18 | 18 | NotifyTip, | |
| 19 | + | /// Invite-code redemption notices. Carries the list kind rather than a | |
| 20 | + | /// `notify_*` name because it never had a column (migration 195). | |
| 21 | + | Invite, | |
| 19 | 22 | /// The landing page "notify me" list (`email_signups`). Email-keyed only: | |
| 20 | 23 | /// a signup carries no account, so the user-id form cannot represent it. | |
| 21 | 24 | Signup, | |
| @@ -33,6 +36,7 @@ | |||
| 33 | 36 | Self::Status => "status", | |
| 34 | 37 | Self::MailingList => "mailing_list", | |
| 35 | 38 | Self::NotifyTip => "notify_tip", | |
| 39 | + | Self::Invite => "invite", | |
| 36 | 40 | Self::Signup => "signup", | |
| 37 | 41 | }; | |
| 38 | 42 | f.write_str(s) | |
| @@ -53,6 +57,7 @@ | |||
| 53 | 57 | "status" => Ok(Self::Status), | |
| 54 | 58 | "mailing_list" => Ok(Self::MailingList), | |
| 55 | 59 | "notify_tip" => Ok(Self::NotifyTip), | |
| 60 | + | "invite" => Ok(Self::Invite), | |
| 56 | 61 | "signup" => Ok(Self::Signup), | |
| 57 | 62 | other => Err(format!("invalid UnsubscribeAction: {other}")), | |
| 58 | 63 | } |