max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
4 files changed,
+579 insertions,
-30 deletions
| @@ -1163,6 +1163,20 @@ | |||
| 1163 | 1163 | ||
| 1164 | 1164 | // ── Mailing lists (wiki: mnw-mailing-lists) ── | |
| 1165 | 1165 | ||
| 1166 | + | /// The legacy per-project list types map onto the unified kinds one-for-one. | |
| 1167 | + | /// Kept as a conversion rather than merging the two enums, because | |
| 1168 | + | /// `MailingListType` is pinned by a CHECK on the old table and will be dropped | |
| 1169 | + | /// with it rather than grown. | |
| 1170 | + | impl From<MailingListType> for ListKind { | |
| 1171 | + | fn from(t: MailingListType) -> Self { | |
| 1172 | + | match t { | |
| 1173 | + | MailingListType::Content => Self::Content, | |
| 1174 | + | MailingListType::Devlog => Self::Devlog, | |
| 1175 | + | MailingListType::Patches => Self::Patches, | |
| 1176 | + | } | |
| 1177 | + | } | |
| 1178 | + | } | |
| 1179 | + | ||
| 1166 | 1180 | /// What a list is attached to. `Platform` lists have no `scope_id`; every other | |
| 1167 | 1181 | /// scope requires one, and the database enforces the pairing. | |
| 1168 | 1182 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] |
| @@ -154,8 +154,115 @@ | |||
| 154 | 154 | Ok(moved) | |
| 155 | 155 | } | |
| 156 | 156 | ||
| 157 | + | /// The states that may receive mail. | |
| 158 | + | /// | |
| 159 | + | /// `imported` is here to preserve pre-migration behaviour, not because we hold | |
| 160 | + | /// evidence of consent for those rows. Before the unified tables, everyone in | |
| 161 | + | /// `mailing_list_subscribers` was mailed, and a refactor whose side effect is | |
| 162 | + | /// that some subscribers silently stop receiving mail is worse than one that | |
| 163 | + | /// changes nothing. Whether imported subscribers should keep receiving | |
| 164 | + | /// marketing is the open decision in GoingsOn 04a882b4; answering it "no" is | |
| 165 | + | /// this constant minus one entry, and the test below is what makes that a | |
| 166 | + | /// deliberate edit rather than a drift. | |
| 167 | + | const SENDABLE_STATES: &[&str] = &["confirmed", "imported"]; | |
| 168 | + | ||
| 169 | + | /// One deliverable recipient. | |
| 170 | + | #[derive(Debug, Clone, sqlx::FromRow)] | |
| 171 | + | pub struct Recipient { | |
| 172 | + | /// The subscription this delivery is against. Carried so the caller can | |
| 173 | + | /// mint a per-recipient unsubscribe link and, later, record the send. | |
| 174 | + | pub subscription_id: ListSubscriptionId, | |
| 175 | + | /// `None` for a bare address with no account behind it. | |
| 176 | + | pub user_id: Option<UserId>, | |
| 177 | + | pub email: String, | |
| 178 | + | pub display_name: Option<String>, | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | /// A list and everyone who may currently be mailed on it. | |
| 182 | + | #[derive(Debug, Clone)] | |
| 183 | + | pub struct Audience { | |
| 184 | + | pub list_id: ListId, | |
| 185 | + | /// Transactional list nobody may leave, so no unsubscribe footer is owed. | |
| 186 | + | /// The caller reads this rather than deciding per send, which is what stops | |
| 187 | + | /// a marketing send from quietly omitting the footer. | |
| 188 | + | pub required: bool, | |
| 189 | + | pub recipients: Vec<Recipient>, | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | /// Everyone who may be mailed on a list, and nobody who may not. | |
| 193 | + | /// | |
| 194 | + | /// This is the one place the delivery rules live. They were previously spread | |
| 195 | + | /// across each send's own query, which is why suppression was applied | |
| 196 | + | /// consistently (it sat in `send_email_inner`) and nothing else was. | |
| 197 | + | /// | |
| 198 | + | /// Applied here, in order: | |
| 199 | + | /// - the subscription state must be sendable (see [`SENDABLE_STATES`]); | |
| 200 | + | /// - the address must not be suppressed, which covers bounces and complaints; | |
| 201 | + | /// - an account subscriber must have a verified, unsuspended account. A bare | |
| 202 | + | /// address has no account to check, and excluding those was a real bug once | |
| 203 | + | /// (Run 21): an INNER JOIN meant imported subscribers were never mailed. | |
| 204 | + | /// | |
| 205 | + | /// Capped at 10,000, matching the query it replaces. | |
| 206 | + | #[tracing::instrument(skip_all)] | |
| 207 | + | pub async fn resolve_audience(pool: &PgPool, list_id: ListId) -> Result<Audience> { | |
| 208 | + | let required = sqlx::query_scalar::<_, bool>("SELECT required FROM lists WHERE id = $1") | |
| 209 | + | .bind(list_id) | |
| 210 | + | .fetch_one(pool) | |
| 211 | + | .await?; | |
| 212 | + | ||
| 213 | + | let recipients = sqlx::query_as::<_, Recipient>( | |
| 214 | + | r" | |
| 215 | + | SELECT ls.id AS subscription_id, u.id AS user_id, u.email, u.display_name | |
| 216 | + | FROM list_subscriptions ls | |
| 217 | + | JOIN users u ON u.id = ls.user_id | |
| 218 | + | WHERE ls.list_id = $1 | |
| 219 | + | AND ls.state = ANY($2) | |
| 220 | + | AND u.email_verified = true | |
| 221 | + | AND u.suspended_at IS NULL | |
| 222 | + | AND LOWER(u.email) NOT IN (SELECT LOWER(email) FROM email_suppressions) | |
| 223 | + | UNION ALL | |
| 224 | + | SELECT ls.id AS subscription_id, NULL::uuid AS user_id, ls.email, NULL AS display_name | |
| 225 | + | FROM list_subscriptions ls | |
| 226 | + | WHERE ls.list_id = $1 | |
| 227 | + | AND ls.state = ANY($2) | |
| 228 | + | AND ls.user_id IS NULL | |
| 229 | + | AND ls.email IS NOT NULL | |
| 230 | + | AND LOWER(ls.email) NOT IN (SELECT LOWER(email) FROM email_suppressions) | |
| 231 | + | LIMIT 10000 | |
| 232 | + | ", | |
| 233 | + | ) | |
| 234 | + | .bind(list_id) | |
| 235 | + | .bind(SENDABLE_STATES) | |
| 236 | + | .fetch_all(pool) | |
| 237 | + | .await?; | |
| 238 | + | ||
| 239 | + | Ok(Audience { | |
| 240 | + | list_id, | |
| 241 | + | required, | |
| 242 | + | recipients, | |
| 243 | + | }) | |
| 244 | + | } | |
| 245 | + | ||
| 246 | + | /// The unified list mirroring a legacy per-project list. | |
| 247 | + | /// | |
| 248 | + | /// Resolves through `mailing_lists` rather than storing a foreign key, so the | |
| 249 | + | /// old table needs no schema change during the migration and dropping it later | |
| 250 | + | /// leaves nothing dangling. | |
| 251 | + | #[tracing::instrument(skip_all)] | |
| 252 | + | pub async fn list_for_legacy(pool: &PgPool, mailing_list_id: uuid::Uuid) -> Result<Option<ListId>> { | |
| 253 | + | let id = sqlx::query_scalar::<_, ListId>( | |
| 254 | + | "SELECT l.id FROM mailing_lists ml \ | |
| 255 | + | JOIN lists l ON l.scope = 'project' AND l.scope_id = ml.project_id AND l.kind = ml.list_type \ | |
| 256 | + | WHERE ml.id = $1", | |
| 257 | + | ) | |
| 258 | + | .bind(mailing_list_id) | |
| 259 | + | .fetch_optional(pool) | |
| 260 | + | .await?; | |
| 261 | + | Ok(id) | |
| 262 | + | } | |
| 263 | + | ||
| 157 | 264 | /// Count subscriptions on a list in a given state. Exists for the backfill | |
| 158 | - | /// tests and the admin view; the send path gets its own resolver in step 3. | |
| 265 | + | /// tests and the admin view; the send path uses [`resolve_audience`]. | |
| 159 | 266 | #[tracing::instrument(skip_all)] | |
| 160 | 267 | pub async fn count_in_state( | |
| 161 | 268 | pool: &PgPool, | |
| @@ -172,6 +279,145 @@ | |||
| 172 | 279 | Ok(count) | |
| 173 | 280 | } | |
| 174 | 281 | ||
| 282 | + | // ── Mirroring the legacy tables ── | |
| 283 | + | // | |
| 284 | + | // `mailing_lists` / `mailing_list_subscribers` are still what the product | |
| 285 | + | // writes to, and `resolve_audience` is what sends now read. Every legacy write | |
| 286 | + | // therefore has to reach here, or a subscriber added after the migration is one | |
| 287 | + | // no send can see. | |
| 288 | + | // | |
| 289 | + | // These propagate their errors rather than logging and continuing. A subscribe | |
| 290 | + | // that does not reach the send path is a broken subscribe, and the failure | |
| 291 | + | // should be visible where it happened rather than at the next announcement. | |
| 292 | + | ||
| 293 | + | /// Mirror a legacy project list into `lists`. | |
| 294 | + | #[tracing::instrument(skip_all)] | |
| 295 | + | pub async fn mirror_legacy_list( | |
| 296 | + | pool: &PgPool, | |
| 297 | + | project_id: uuid::Uuid, | |
| 298 | + | kind: ListKind, | |
| 299 | + | title: &str, | |
| 300 | + | ) -> Result<()> { | |
| 301 | + | sqlx::query( | |
| 302 | + | "INSERT INTO lists (scope, scope_id, kind, title, required, owner_id) \ | |
| 303 | + | SELECT 'project', $1, $2, $3, FALSE, p.user_id FROM projects p WHERE p.id = $1 \ | |
| 304 | + | ON CONFLICT DO NOTHING", | |
| 305 | + | ) | |
| 306 | + | .bind(project_id) | |
| 307 | + | .bind(kind.to_string()) | |
| 308 | + | .bind(title) | |
| 309 | + | .execute(pool) | |
| 310 | + | .await?; | |
| 311 | + | Ok(()) | |
| 312 | + | } | |
| 313 | + | ||
| 314 | + | /// Mirror a legacy subscribe. | |
| 315 | + | /// | |
| 316 | + | /// A subscribe through the product is a real act, so it lands `confirmed` with | |
| 317 | + | /// an `opt_in` event, unlike the backfill's `imported`. `evidence` records what | |
| 318 | + | /// the person was doing at the time, which is the difference between consent we | |
| 319 | + | /// can show and consent we assert. | |
| 320 | + | #[tracing::instrument(skip_all)] | |
| 321 | + | pub async fn mirror_legacy_subscribe( | |
| 322 | + | pool: &PgPool, | |
| 323 | + | mailing_list_id: uuid::Uuid, | |
| 324 | + | subscriber: &Subscriber, | |
| 325 | + | state: SubscriptionState, | |
| 326 | + | source: SubscriptionSource, | |
| 327 | + | evidence: Option<&str>, | |
| 328 | + | ) -> Result<()> { | |
| 329 | + | let Some(list_id) = list_for_legacy(pool, mailing_list_id).await? else { | |
| 330 | + | // The legacy list predates its mirror. Create-list mirroring runs first | |
| 331 | + | // for anything made since the migration, so this means a list that was | |
| 332 | + | // never backfilled, which is a bug worth seeing rather than skipping. | |
| 333 | + | return Err(crate::error::AppError::Internal(anyhow::anyhow!( | |
| 334 | + | "legacy mailing list {mailing_list_id} has no unified list" | |
| 335 | + | ))); | |
| 336 | + | }; | |
| 337 | + | let event = match state { | |
| 338 | + | SubscriptionState::Imported => ConsentEvent::Import, | |
| 339 | + | _ => ConsentEvent::OptIn, | |
| 340 | + | }; | |
| 341 | + | subscribe(pool, list_id, subscriber, state, source, event, evidence).await?; | |
| 342 | + | Ok(()) | |
| 343 | + | } | |
| 344 | + | ||
| 345 | + | /// Mirror a legacy unsubscribe for an account subscriber. | |
| 346 | + | /// | |
| 347 | + | /// The most important mirror of the three: a missed unsubscribe means mailing | |
| 348 | + | /// somebody who asked us not to. | |
| 349 | + | #[tracing::instrument(skip_all)] | |
| 350 | + | pub async fn mirror_legacy_unsubscribe_user( | |
| 351 | + | pool: &PgPool, | |
| 352 | + | mailing_list_id: uuid::Uuid, | |
| 353 | + | user_id: UserId, | |
| 354 | + | ) -> Result<()> { | |
| 355 | + | let Some(list_id) = list_for_legacy(pool, mailing_list_id).await? else { | |
| 356 | + | return Ok(()); | |
| 357 | + | }; | |
| 358 | + | mark_unsubscribed(pool, list_id, &Subscriber::User(user_id)).await | |
| 359 | + | } | |
| 360 | + | ||
| 361 | + | /// Mirror a legacy unsubscribe for a bare address. | |
| 362 | + | #[tracing::instrument(skip_all)] | |
| 363 | + | pub async fn mirror_legacy_unsubscribe_email( | |
| 364 | + | pool: &PgPool, | |
| 365 | + | mailing_list_id: uuid::Uuid, | |
| 366 | + | email: &str, | |
| 367 | + | ) -> Result<()> { | |
| 368 | + | let Some(list_id) = list_for_legacy(pool, mailing_list_id).await? else { | |
| 369 | + | return Ok(()); | |
| 370 | + | }; | |
| 371 | + | mark_unsubscribed(pool, list_id, &Subscriber::Email(email.to_string())).await | |
| 372 | + | } | |
| 373 | + | ||
| 374 | + | /// Mirror an unsubscribe from every list on a project (the unfollow path). | |
| 375 | + | #[tracing::instrument(skip_all)] | |
| 376 | + | pub async fn mirror_legacy_unsubscribe_project( | |
| 377 | + | pool: &PgPool, | |
| 378 | + | project_id: uuid::Uuid, | |
| 379 | + | user_id: UserId, | |
| 380 | + | ) -> Result<()> { | |
| 381 | + | let ids = sqlx::query_scalar::<_, ListId>( | |
| 382 | + | "SELECT id FROM lists WHERE scope = 'project' AND scope_id = $1", | |
| 383 | + | ) | |
| 384 | + | .bind(project_id) | |
| 385 | + | .fetch_all(pool) | |
| 386 | + | .await?; | |
| 387 | + | for list_id in ids { | |
| 388 | + | mark_unsubscribed(pool, list_id, &Subscriber::User(user_id)).await?; | |
| 389 | + | } | |
| 390 | + | Ok(()) | |
| 391 | + | } | |
| 392 | + | ||
| 393 | + | /// Move a subscription to `unsubscribed` and append the opt-out, by identity | |
| 394 | + | /// rather than by subscription id. No-op when there is nothing subscribed. | |
| 395 | + | async fn mark_unsubscribed(pool: &PgPool, list_id: ListId, subscriber: &Subscriber) -> Result<()> { | |
| 396 | + | let existing = | |
| 397 | + | match subscriber { | |
| 398 | + | Subscriber::User(id) => { | |
| 399 | + | sqlx::query_scalar::<_, ListSubscriptionId>( | |
| 400 | + | "SELECT id FROM list_subscriptions WHERE list_id = $1 AND user_id = $2", | |
| 401 | + | ) | |
| 402 | + | .bind(list_id) | |
| 403 | + | .bind(id) | |
| 404 | + | .fetch_optional(pool) | |
| 405 | + | .await? | |
| 406 | + | } | |
| 407 | + | Subscriber::Email(addr) => sqlx::query_scalar::<_, ListSubscriptionId>( | |
| 408 | + | "SELECT id FROM list_subscriptions WHERE list_id = $1 AND LOWER(email) = LOWER($2)", | |
| 409 | + | ) | |
| 410 | + | .bind(list_id) | |
| 411 | + | .bind(addr) | |
| 412 | + | .fetch_optional(pool) | |
| 413 | + | .await?, | |
| 414 | + | }; | |
| 415 | + | if let Some(subscription_id) = existing { | |
| 416 | + | unsubscribe(pool, subscription_id, ConsentEvent::OptOut).await?; | |
| 417 | + | } | |
| 418 | + | Ok(()) | |
| 419 | + | } | |
| 420 | + | ||
| 175 | 421 | #[cfg(test)] | |
| 176 | 422 | mod tests { | |
| 177 | 423 | use super::*; | |
| @@ -191,6 +437,19 @@ | |||
| 191 | 437 | } | |
| 192 | 438 | } | |
| 193 | 439 | ||
| 440 | + | /// Which states receive mail is a policy, and an open one. Changing this | |
| 441 | + | /// set changes who gets email, so it should be an edit somebody made on | |
| 442 | + | /// purpose rather than a line that moved during a refactor. | |
| 443 | + | #[test] | |
| 444 | + | fn sendable_states_are_the_agreed_set() { | |
| 445 | + | assert_eq!( | |
| 446 | + | SENDABLE_STATES, | |
| 447 | + | &["confirmed", "imported"], | |
| 448 | + | "dropping 'imported' is the GoingsOn 04a882b4 decision; if that is \ | |
| 449 | + | what happened, update this test with it" | |
| 450 | + | ); | |
| 451 | + | } | |
| 452 | + | ||
| 194 | 453 | /// The strings are a database CHECK constraint, so a rename here that is | |
| 195 | 454 | /// not matched by a migration fails at insert rather than at compile time. | |
| 196 | 455 | #[test] |
| @@ -31,6 +31,11 @@ | |||
| 31 | 31 | .fetch_one(pool) | |
| 32 | 32 | .await?; | |
| 33 | 33 | ||
| 34 | + | // Mirror into the unified `lists` table, which is what sends read from now | |
| 35 | + | // (see db::lists). Errors propagate: a list that exists only in the legacy | |
| 36 | + | // table is one no send can reach. | |
| 37 | + | super::lists::mirror_legacy_list(pool, project_id.into(), list_type.into(), name).await?; | |
| 38 | + | ||
| 34 | 39 | Ok(row) | |
| 35 | 40 | } | |
| 36 | 41 | ||
| @@ -67,6 +72,16 @@ | |||
| 67 | 72 | .execute(pool) | |
| 68 | 73 | .await?; | |
| 69 | 74 | ||
| 75 | + | super::lists::mirror_legacy_subscribe( | |
| 76 | + | pool, | |
| 77 | + | list_id.into(), | |
| 78 | + | &super::lists::Subscriber::User(user_id), | |
| 79 | + | super::SubscriptionState::Confirmed, | |
| 80 | + | super::SubscriptionSource::ProjectPage, | |
| 81 | + | Some("Subscribed through the product (project page, follow, or purchase)."), | |
| 82 | + | ) | |
| 83 | + | .await?; | |
| 84 | + | ||
| 70 | 85 | Ok(()) | |
| 71 | 86 | } | |
| 72 | 87 | ||
| @@ -80,6 +95,8 @@ | |||
| 80 | 95 | .execute(pool) | |
| 81 | 96 | .await?; | |
| 82 | 97 | ||
| 98 | + | super::lists::mirror_legacy_unsubscribe_user(pool, list_id.into(), user_id).await?; | |
| 99 | + | ||
| 83 | 100 | Ok(result.rows_affected() > 0) | |
| 84 | 101 | } | |
| 85 | 102 | ||
| @@ -102,6 +119,8 @@ | |||
| 102 | 119 | .execute(pool) | |
| 103 | 120 | .await?; | |
| 104 | 121 | ||
| 122 | + | super::lists::mirror_legacy_unsubscribe_project(pool, project_id.into(), user_id).await?; | |
| 123 | + | ||
| 105 | 124 | Ok(result.rows_affected()) | |
| 106 | 125 | } | |
| 107 | 126 | ||
| @@ -115,42 +134,41 @@ | |||
| 115 | 134 | pub display_name: Option<String>, | |
| 116 | 135 | } | |
| 117 | 136 | ||
| 118 | - | /// Get all deliverable subscribers on a list, both MNW users AND email-only | |
| 119 | - | /// imported subscribers. Capped at 10,000. | |
| 137 | + | /// Get all deliverable subscribers on a list. | |
| 120 | 138 | /// | |
| 121 | - | /// User rows require a verified, non-suspended account; email-only rows | |
| 122 | - | /// (`user_id IS NULL`) come straight from the import. Both branches exclude | |
| 123 | - | /// suppressed addresses. Previously this INNER JOINed `users`, so imported | |
| 124 | - | /// email-only subscribers were silently never emailed (Run 21 data). | |
| 139 | + | /// An adapter over [`crate::db::lists::resolve_audience`], which is now the one | |
| 140 | + | /// place the delivery rules live. It used to hold its own copy of them, which | |
| 141 | + | /// is how the rules came to differ per send: suppression was applied | |
| 142 | + | /// everywhere because it sat in `send_email_inner`, and nothing else was. | |
| 143 | + | /// | |
| 144 | + | /// The shape is unchanged so the two announcement senders did not have to move | |
| 145 | + | /// in the same commit. They move to [`crate::db::lists::Audience`] in step 4, | |
| 146 | + | /// when the unsubscribe link changes form and this function goes away. | |
| 125 | 147 | #[tracing::instrument(skip_all)] | |
| 126 | 148 | pub async fn get_subscribers( | |
| 127 | 149 | pool: &PgPool, | |
| 128 | 150 | list_id: MailingListId, | |
| 129 | 151 | ) -> Result<Vec<MailingSubscriber>> { | |
| 130 | - | let rows = sqlx::query_as::<_, MailingSubscriber>( | |
| 131 | - | r" | |
| 132 | - | SELECT u.id AS user_id, u.email, u.display_name | |
| 133 | - | FROM mailing_list_subscribers s | |
| 134 | - | JOIN users u ON u.id = s.user_id | |
| 135 | - | WHERE s.list_id = $1 | |
| 136 | - | AND u.email_verified = true | |
| 137 | - | AND u.suspended_at IS NULL | |
| 138 | - | AND LOWER(u.email) NOT IN (SELECT LOWER(email) FROM email_suppressions) | |
| 139 | - | UNION ALL | |
| 140 | - | SELECT NULL::uuid AS user_id, s.email, NULL AS display_name | |
| 141 | - | FROM mailing_list_subscribers s | |
| 142 | - | WHERE s.list_id = $1 | |
| 143 | - | AND s.user_id IS NULL | |
| 144 | - | AND s.email IS NOT NULL | |
| 145 | - | AND LOWER(s.email) NOT IN (SELECT LOWER(email) FROM email_suppressions) | |
| 146 | - | LIMIT 10000 | |
| 147 | - | ", | |
| 148 | - | ) | |
| 149 | - | .bind(list_id) | |
| 150 | - | .fetch_all(pool) | |
| 151 | - | .await?; | |
| 152 | + | let Some(unified) = super::lists::list_for_legacy(pool, list_id.into()).await? else { | |
| 153 | + | // Every legacy list is mirrored, by the step-2 backfill or by | |
| 154 | + | // create_list. Missing means the mirror was skipped, and silently | |
| 155 | + | // sending to nobody would hide that until a creator asked why their | |
| 156 | + | // announcement never arrived. | |
| 157 | + | return Err(crate::error::AppError::Internal(anyhow::anyhow!( | |
| 158 | + | "legacy mailing list {list_id} has no unified list" | |
| 159 | + | ))); | |
| 160 | + | }; | |
| 152 | 161 | ||
| 153 | - | Ok(rows) | |
| 162 | + | let audience = super::lists::resolve_audience(pool, unified).await?; | |
| 163 | + | Ok(audience | |
| 164 | + | .recipients | |
| 165 | + | .into_iter() | |
| 166 | + | .map(|r| MailingSubscriber { | |
| 167 | + | user_id: r.user_id, | |
| 168 | + | email: r.email, | |
| 169 | + | display_name: r.display_name, | |
| 170 | + | }) | |
| 171 | + | .collect()) | |
| 154 | 172 | } | |
| 155 | 173 | ||
| 156 | 174 | /// Unsubscribe an email-only subscriber (no MNW account) from a list. Removes | |
| @@ -170,6 +188,8 @@ | |||
| 170 | 188 | .execute(pool) | |
| 171 | 189 | .await?; | |
| 172 | 190 | ||
| 191 | + | super::lists::mirror_legacy_unsubscribe_email(pool, list_id.into(), email).await?; | |
| 192 | + | ||
| 173 | 193 | Ok(result.rows_affected() > 0) | |
| 174 | 194 | } | |
| 175 | 195 | ||
| @@ -230,6 +250,22 @@ | |||
| 230 | 250 | .execute(pool) | |
| 231 | 251 | .await?; | |
| 232 | 252 | ||
| 253 | + | // Mirror as `imported`, not `confirmed`. This is a creator's CSV upload: | |
| 254 | + | // the addresses did not opt in here and we hold no evidence that they | |
| 255 | + | // opted in anywhere, so the state and the event say import rather than | |
| 256 | + | // manufacturing consent the row cannot support. | |
| 257 | + | for email in &lowered { | |
| 258 | + | super::lists::mirror_legacy_subscribe( | |
| 259 | + | pool, | |
| 260 | + | list_id.into(), | |
| 261 | + | &super::lists::Subscriber::Email(email.clone()), | |
| 262 | + | super::SubscriptionState::Imported, | |
| 263 | + | super::SubscriptionSource::Import, | |
| 264 | + | Some("Bulk import from a creator-supplied subscriber list."), | |
| 265 | + | ) | |
| 266 | + | .await?; | |
| 267 | + | } | |
| 268 | + | ||
| 233 | 269 | Ok(result.rows_affected()) | |
| 234 | 270 | } | |
| 235 | 271 |
| @@ -289,3 +289,243 @@ | |||
| 289 | 289 | "consent rows outlived the subscription they described" | |
| 290 | 290 | ); | |
| 291 | 291 | } | |
| 292 | + | ||
| 293 | + | // ── Step 3: the resolver, and the legacy writes that feed it ── | |
| 294 | + | // | |
| 295 | + | // Sends now read the unified tables, so a write that reaches only the legacy | |
| 296 | + | // tables is a subscriber no send can see, or worse, an unsubscribe no send | |
| 297 | + | // honours. These pin the mirroring that closes that gap. | |
| 298 | + | ||
| 299 | + | /// The resolver requires a verified, unsuspended account, matching the query it | |
| 300 | + | /// replaced. `signup` does not verify, so tests that expect delivery say so. | |
| 301 | + | async fn verify_email(h: &TestHarness, user: makenotwork::db::UserId) { | |
| 302 | + | sqlx::query("UPDATE users SET email_verified = true WHERE id = $1") | |
| 303 | + | .bind(user) | |
| 304 | + | .execute(&h.db) | |
| 305 | + | .await | |
| 306 | + | .expect("verify email"); | |
| 307 | + | } | |
| 308 | + | ||
| 309 | + | /// Create a project through the API and return its unified content list. | |
| 310 | + | async fn project_with_list(h: &mut TestHarness) -> (makenotwork::db::UserId, uuid::Uuid) { | |
| 311 | + | let creator_id = h.signup("mirror", "mirror@test.com", "password123").await; | |
| 312 | + | h.grant_creator(creator_id).await; | |
| 313 | + | h.client.post_form("/logout", "").await; | |
| 314 | + | h.login("mirror", "password123").await; | |
| 315 | + | let resp = h | |
| 316 | + | .client | |
| 317 | + | .post_form("/api/projects", "slug=mirrorproj&title=Mirror+Project") | |
| 318 | + | .await; | |
| 319 | + | assert!(resp.status.is_success(), "create project: {}", resp.text); | |
| 320 | + | let project: serde_json::Value = resp.json(); | |
| 321 | + | let project_id: uuid::Uuid = project["id"].as_str().unwrap().parse().unwrap(); | |
| 322 | + | (creator_id, project_id) | |
| 323 | + | } | |
| 324 | + | ||
| 325 | + | /// Creating a project mirrors its default lists, so an announcement has | |
| 326 | + | /// somewhere to resolve. Without this the send errors rather than silently | |
| 327 | + | /// mailing nobody. | |
| 328 | + | #[tokio::test] | |
| 329 | + | async fn creating_a_project_mirrors_its_lists() { | |
| 330 | + | let mut h = TestHarness::new().await; | |
| 331 | + | let (_creator, project_id) = project_with_list(&mut h).await; | |
| 332 | + | ||
| 333 | + | let content = lists::find_list( | |
| 334 | + | &h.db, | |
| 335 | + | ListScope::Project, | |
| 336 | + | Some(project_id), | |
| 337 | + | ListKind::Content, | |
| 338 | + | ) | |
| 339 | + | .await | |
| 340 | + | .expect("query"); | |
| 341 | + | assert!(content.is_some(), "content list was not mirrored"); | |
| 342 | + | ||
| 343 | + | let devlog = lists::find_list( | |
| 344 | + | &h.db, | |
| 345 | + | ListScope::Project, | |
| 346 | + | Some(project_id), | |
| 347 | + | ListKind::Devlog, | |
| 348 | + | ) | |
| 349 | + | .await | |
| 350 | + | .expect("query"); | |
| 351 | + | assert!(devlog.is_some(), "devlog list was not mirrored"); | |
| 352 | + | } | |
| 353 | + | ||
| 354 | + | /// A subscribe through the legacy path reaches the audience the resolver | |
| 355 | + | /// returns. This is the dual-write hazard: writes still go to the old tables, | |
| 356 | + | /// and sends now read the new ones. | |
| 357 | + | #[tokio::test] | |
| 358 | + | async fn a_legacy_subscribe_reaches_the_resolved_audience() { | |
| 359 | + | let mut h = TestHarness::new().await; | |
| 360 | + | let (_creator, project_id) = project_with_list(&mut h).await; | |
| 361 | + | let fan = h | |
| 362 | + | .signup("mirrorfan", "mirrorfan@test.com", "password123") | |
| 363 | + | .await; | |
| 364 | + | verify_email(&h, fan).await; | |
| 365 | + | ||
| 366 | + | let legacy = makenotwork::db::mailing_lists::get_list_by_project_and_type( | |
| 367 | + | &h.db, | |
| 368 | + | project_id.into(), | |
| 369 | + | makenotwork::db::MailingListType::Content, | |
| 370 | + | ) | |
| 371 | + | .await | |
| 372 | + | .unwrap() | |
| 373 | + | .expect("legacy list"); | |
| 374 | + | makenotwork::db::mailing_lists::subscribe(&h.db, legacy.id, fan) | |
| 375 | + | .await | |
| 376 | + | .expect("subscribe"); | |
| 377 | + | ||
| 378 | + | let unified = lists::find_list( | |
| 379 | + | &h.db, | |
| 380 | + | ListScope::Project, | |
| 381 | + | Some(project_id), | |
| 382 | + | ListKind::Content, | |
| 383 | + | ) | |
| 384 | + | .await | |
| 385 | + | .unwrap() | |
| 386 | + | .unwrap(); | |
| 387 | + | let audience = lists::resolve_audience(&h.db, unified) | |
| 388 | + | .await | |
| 389 | + | .expect("resolve"); | |
| 390 | + | assert!( | |
| 391 | + | audience.recipients.iter().any(|r| r.user_id == Some(fan)), | |
| 392 | + | "a subscriber added through the legacy path is invisible to sends" | |
| 393 | + | ); | |
| 394 | + | } | |
| 395 | + | ||
| 396 | + | /// The one that matters most: an unsubscribe through the legacy path must | |
| 397 | + | /// remove them from the audience. A missed mirror here means mailing somebody | |
| 398 | + | /// who asked us not to. | |
| 399 | + | #[tokio::test] | |
| 400 | + | async fn a_legacy_unsubscribe_removes_them_from_the_audience() { | |
| 401 | + | let mut h = TestHarness::new().await; | |
| 402 | + | let (_creator, project_id) = project_with_list(&mut h).await; | |
| 403 | + | let fan = h.signup("leaver", "leaver@test.com", "password123").await; | |
| 404 | + | verify_email(&h, fan).await; | |
| 405 | + | ||
| 406 | + | let legacy = makenotwork::db::mailing_lists::get_list_by_project_and_type( | |
| 407 | + | &h.db, | |
| 408 | + | project_id.into(), | |
| 409 | + | makenotwork::db::MailingListType::Content, | |
| 410 | + | ) | |
| 411 | + | .await | |
| 412 | + | .unwrap() | |
| 413 | + | .unwrap(); | |
| 414 | + | makenotwork::db::mailing_lists::subscribe(&h.db, legacy.id, fan) | |
| 415 | + | .await | |
| 416 | + | .unwrap(); | |
| 417 | + | ||
| 418 | + | let unified = lists::find_list( | |
| 419 | + | &h.db, | |
| 420 | + | ListScope::Project, | |
| 421 | + | Some(project_id), | |
| 422 | + | ListKind::Content, | |
| 423 | + | ) | |
| 424 | + | .await | |
| 425 | + | .unwrap() | |
| 426 | + | .unwrap(); | |
| 427 | + | ||
| 428 | + | // Present first, so this cannot pass by never having been subscribed. | |
| 429 | + | let before = lists::resolve_audience(&h.db, unified).await.unwrap(); | |
| 430 | + | assert!( | |
| 431 | + | before.recipients.iter().any(|r| r.user_id == Some(fan)), | |
| 432 | + | "test setup: the subscriber never reached the audience" | |
| 433 | + | ); | |
| 434 | + | ||
| 435 | + | makenotwork::db::mailing_lists::unsubscribe(&h.db, legacy.id, fan) | |
| 436 | + | .await | |
| 437 | + | .unwrap(); | |
| 438 | + | ||
| 439 | + | let audience = lists::resolve_audience(&h.db, unified).await.unwrap(); | |
| 440 | + | assert!( | |
| 441 | + | !audience.recipients.iter().any(|r| r.user_id == Some(fan)), | |
| 442 | + | "an unsubscribed user is still in the send audience" | |
| 443 | + | ); | |
| 444 | + | ||
| 445 | + | // And the opt-out is on the record, not just absent from the audience. | |
| 446 | + | let events: Vec<String> = sqlx::query_scalar( | |
| 447 | + | "SELECT ce.event FROM consent_events ce \ | |
| 448 | + | JOIN list_subscriptions ls ON ls.id = ce.subscription_id \ | |
| 449 | + | WHERE ls.user_id = $1 ORDER BY ce.at", | |
| 450 | + | ) | |
| 451 | + | .bind(fan) | |
| 452 | + | .fetch_all(&h.db) | |
| 453 | + | .await | |
| 454 | + | .unwrap(); | |
| 455 | + | assert!(events.contains(&"opt_out".to_string())); | |
| 456 | + | } | |
| 457 | + | ||
| 458 | + | /// A suppressed address never resolves, whatever its subscription says. | |
| 459 | + | /// Bounces and complaints are the one rule that was already applied | |
| 460 | + | /// consistently, and it stays that way. | |
| 461 | + | #[tokio::test] | |
| 462 | + | async fn suppressed_addresses_are_never_in_the_audience() { | |
| 463 | + | let h = TestHarness::new().await; | |
| 464 | + | let list = lists::find_list(&h.db, ListScope::Platform, None, ListKind::Marketing) | |
| 465 | + | .await | |
| 466 | + | .unwrap() | |
| 467 | + | .unwrap(); | |
| 468 | + | ||
| 469 | + | lists::subscribe( | |
| 470 | + | &h.db, | |
| 471 | + | list, | |
| 472 | + | &lists::Subscriber::Email("bounced@example.com".to_string()), | |
| 473 | + | SubscriptionState::Confirmed, | |
| 474 | + | SubscriptionSource::LandingForm, | |
| 475 | + | ConsentEvent::OptIn, | |
| 476 | + | None, | |
| 477 | + | ) | |
| 478 | + | .await | |
| 479 | + | .unwrap(); | |
| 480 | + | ||
| 481 | + | let before = lists::resolve_audience(&h.db, list).await.unwrap(); | |
| 482 | + | assert_eq!(before.recipients.len(), 1); | |
| 483 | + | ||
| 484 | + | sqlx::query("INSERT INTO email_suppressions (email, reason) VALUES ($1, 'bounce')") | |
| 485 | + | .bind("bounced@example.com") | |
| 486 | + | .execute(&h.db) | |
| 487 | + | .await | |
| 488 | + | .unwrap(); | |
| 489 | + | ||
| 490 | + | let after = lists::resolve_audience(&h.db, list).await.unwrap(); | |
| 491 | + | assert!( | |
| 492 | + | after.recipients.is_empty(), | |
| 493 | + | "a suppressed address resolved as deliverable" | |
| 494 | + | ); | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | /// An unsubscribed subscription is not sendable, and neither is a pending one: | |
| 498 | + | /// nothing may be mailed on the strength of a double opt-in that never | |
| 499 | + | /// completed. | |
| 500 | + | #[tokio::test] | |
| 501 | + | async fn unsendable_states_stay_out_of_the_audience() { | |
| 502 | + | let h = TestHarness::new().await; | |
| 503 | + | let list = lists::find_list(&h.db, ListScope::Platform, None, ListKind::Marketing) | |
| 504 | + | .await | |
| 505 | + | .unwrap() | |
| 506 | + | .unwrap(); | |
| 507 | + | ||
| 508 | + | for (addr, state) in [ | |
| 509 | + | ("pending@example.com", SubscriptionState::Pending), | |
| 510 | + | ("bounced2@example.com", SubscriptionState::Bounced), | |
| 511 | + | ] { | |
| 512 | + | lists::subscribe( | |
| 513 | + | &h.db, | |
| 514 | + | list, | |
| 515 | + | &lists::Subscriber::Email(addr.to_string()), | |
| 516 | + | state, | |
| 517 | + | SubscriptionSource::LandingForm, | |
| 518 | + | ConsentEvent::OptIn, | |
| 519 | + | None, | |
| 520 | + | ) | |
| 521 | + | .await | |
| 522 | + | .unwrap(); | |
| 523 | + | } | |
| 524 | + | ||
| 525 | + | let audience = lists::resolve_audience(&h.db, list).await.unwrap(); | |
| 526 | + | assert!( | |
| 527 | + | audience.recipients.is_empty(), | |
| 528 | + | "pending or bounced subscriptions resolved as deliverable: {:?}", | |
| 529 | + | audience.recipients | |
| 530 | + | ); | |
| 531 | + | } |