From 3a4e802093f12306ecd1e7ff8585d5a5606b80a8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 Oct 2022 15:20:23 -0700 Subject: [PATCH 1/2] Include waitlist entries w/ unknown platform when summarizing and sending invites --- crates/collab/src/api.rs | 4 ++-- crates/collab/src/db.rs | 40 +++++++++++++++++++++++++++-------- crates/collab/src/db_tests.rs | 29 ++++++++++++++++--------- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 08dfa91ba98df..2d09bf3aeaf28 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{Invite, NewUserParams, ProjectId, Signup, User, UserId, WaitlistSummary}, + db::{Invite, NewUserParams, ProjectId, Signup, UnsentInvite, User, UserId, WaitlistSummary}, rpc::{self, ResultExt}, AppState, Error, Result, }; @@ -471,7 +471,7 @@ pub struct GetUnsentInvitesParams { async fn get_unsent_invites( Query(params): Query, Extension(app): Extension>, -) -> Result>> { +) -> Result>> { Ok(Json(app.db.get_unsent_invites(params.count).await?)) } diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 4f49c117fd3da..f241202b22def 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -45,7 +45,7 @@ pub trait Db: Send + Sync { async fn create_signup(&self, signup: Signup) -> Result<()>; async fn get_waitlist_summary(&self) -> Result; - async fn get_unsent_invites(&self, count: usize) -> Result>; + async fn get_unsent_invites(&self, count: usize) -> Result>; async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; async fn create_user_from_invite( &self, @@ -428,7 +428,8 @@ impl Db for PostgresDb { COUNT(*) as count, COALESCE(SUM(CASE WHEN platform_linux THEN 1 ELSE 0 END), 0) as linux_count, COALESCE(SUM(CASE WHEN platform_mac THEN 1 ELSE 0 END), 0) as mac_count, - COALESCE(SUM(CASE WHEN platform_windows THEN 1 ELSE 0 END), 0) as windows_count + COALESCE(SUM(CASE WHEN platform_windows THEN 1 ELSE 0 END), 0) as windows_count, + COALESCE(SUM(CASE WHEN platform_unknown THEN 1 ELSE 0 END), 0) as unknown_count FROM ( SELECT * FROM signups @@ -441,21 +442,33 @@ impl Db for PostgresDb { .await?) } - async fn get_unsent_invites(&self, count: usize) -> Result> { - Ok(sqlx::query_as( + async fn get_unsent_invites(&self, count: usize) -> Result> { + let rows: Vec<(String, String, bool)> = sqlx::query_as( " SELECT - email_address, email_confirmation_code + email_address, email_confirmation_code, platform_unknown FROM signups WHERE NOT email_confirmation_sent AND - platform_mac + (platform_mac OR platform_unknown) LIMIT $1 ", ) .bind(count as i32) .fetch_all(&self.pool) - .await?) + .await?; + Ok(rows + .into_iter() + .map( + |(email_address, email_confirmation_code, platform_unknown)| UnsentInvite { + invite: Invite { + email_address, + email_confirmation_code, + }, + platform_unknown, + }, + ) + .collect()) } async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()> { @@ -1720,14 +1733,23 @@ pub struct WaitlistSummary { pub mac_count: i64, #[sqlx(default)] pub windows_count: i64, + #[sqlx(default)] + pub unknown_count: i64, } -#[derive(FromRow, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Clone, FromRow, PartialEq, Debug, Serialize, Deserialize)] pub struct Invite { pub email_address: String, pub email_confirmation_code: String, } +#[derive(Serialize, Debug, PartialEq)] +pub struct UnsentInvite { + #[serde(flatten)] + pub invite: Invite, + pub platform_unknown: bool, +} + #[derive(Debug, Serialize, Deserialize)] pub struct NewUserParams { pub github_login: String, @@ -1943,7 +1965,7 @@ mod test { unimplemented!() } - async fn get_unsent_invites(&self, _count: usize) -> Result> { + async fn get_unsent_invites(&self, _count: usize) -> Result> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 477dcd4ab8126..b48a45018b174 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -1022,6 +1022,7 @@ async fn test_signups() { mac_count: 8, linux_count: 4, windows_count: 2, + unknown_count: 0, } ); @@ -1029,7 +1030,7 @@ async fn test_signups() { let signups_batch1 = db.get_unsent_invites(3).await.unwrap(); let addresses = signups_batch1 .iter() - .map(|s| &s.email_address) + .map(|s| &s.invite.email_address) .collect::>(); assert_eq!( addresses, @@ -1040,8 +1041,8 @@ async fn test_signups() { ] ); assert_ne!( - signups_batch1[0].email_confirmation_code, - signups_batch1[1].email_confirmation_code + signups_batch1[0].invite.email_confirmation_code, + signups_batch1[1].invite.email_confirmation_code ); // the waitlist isn't updated until we record that the emails @@ -1051,11 +1052,18 @@ async fn test_signups() { // once the emails go out, we can retrieve the next batch // of signups. - db.record_sent_invites(&signups_batch1).await.unwrap(); + db.record_sent_invites( + &signups_batch1 + .iter() + .map(|i| i.invite.clone()) + .collect::>(), + ) + .await + .unwrap(); let signups_batch2 = db.get_unsent_invites(3).await.unwrap(); let addresses = signups_batch2 .iter() - .map(|s| &s.email_address) + .map(|s| &s.invite.email_address) .collect::>(); assert_eq!( addresses, @@ -1074,6 +1082,7 @@ async fn test_signups() { mac_count: 5, linux_count: 2, windows_count: 1, + unknown_count: 0, } ); @@ -1087,8 +1096,8 @@ async fn test_signups() { } = db .create_user_from_invite( &Invite { - email_address: signups_batch1[0].email_address.clone(), - email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + email_address: signups_batch1[0].invite.email_address.clone(), + email_confirmation_code: signups_batch1[0].invite.email_confirmation_code.clone(), }, NewUserParams { github_login: "person-0".into(), @@ -1108,8 +1117,8 @@ async fn test_signups() { // cannot redeem the same signup again. db.create_user_from_invite( &Invite { - email_address: signups_batch1[0].email_address.clone(), - email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), + email_address: signups_batch1[0].invite.email_address.clone(), + email_confirmation_code: signups_batch1[0].invite.email_confirmation_code.clone(), }, NewUserParams { github_login: "some-other-github_account".into(), @@ -1123,7 +1132,7 @@ async fn test_signups() { // cannot redeem a signup with the wrong confirmation code. db.create_user_from_invite( &Invite { - email_address: signups_batch1[1].email_address.clone(), + email_address: signups_batch1[1].invite.email_address.clone(), email_confirmation_code: "the-wrong-code".to_string(), }, NewUserParams { From b541ac313c1d174ab873e2237ea4aced038e1d34 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 Oct 2022 16:13:38 -0700 Subject: [PATCH 2/2] Revert unnecessary logic for fetching invites' platform_unknown flag --- crates/collab/src/api.rs | 4 ++-- crates/collab/src/db.rs | 33 +++++++-------------------------- crates/collab/src/db_tests.rs | 27 ++++++++++----------------- 3 files changed, 19 insertions(+), 45 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 2d09bf3aeaf28..08dfa91ba98df 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -1,6 +1,6 @@ use crate::{ auth, - db::{Invite, NewUserParams, ProjectId, Signup, UnsentInvite, User, UserId, WaitlistSummary}, + db::{Invite, NewUserParams, ProjectId, Signup, User, UserId, WaitlistSummary}, rpc::{self, ResultExt}, AppState, Error, Result, }; @@ -471,7 +471,7 @@ pub struct GetUnsentInvitesParams { async fn get_unsent_invites( Query(params): Query, Extension(app): Extension>, -) -> Result>> { +) -> Result>> { Ok(Json(app.db.get_unsent_invites(params.count).await?)) } diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index f241202b22def..9b3dca1f2cfeb 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -45,7 +45,7 @@ pub trait Db: Send + Sync { async fn create_signup(&self, signup: Signup) -> Result<()>; async fn get_waitlist_summary(&self) -> Result; - async fn get_unsent_invites(&self, count: usize) -> Result>; + async fn get_unsent_invites(&self, count: usize) -> Result>; async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()>; async fn create_user_from_invite( &self, @@ -442,11 +442,11 @@ impl Db for PostgresDb { .await?) } - async fn get_unsent_invites(&self, count: usize) -> Result> { - let rows: Vec<(String, String, bool)> = sqlx::query_as( + async fn get_unsent_invites(&self, count: usize) -> Result> { + Ok(sqlx::query_as( " SELECT - email_address, email_confirmation_code, platform_unknown + email_address, email_confirmation_code FROM signups WHERE NOT email_confirmation_sent AND @@ -456,19 +456,7 @@ impl Db for PostgresDb { ) .bind(count as i32) .fetch_all(&self.pool) - .await?; - Ok(rows - .into_iter() - .map( - |(email_address, email_confirmation_code, platform_unknown)| UnsentInvite { - invite: Invite { - email_address, - email_confirmation_code, - }, - platform_unknown, - }, - ) - .collect()) + .await?) } async fn record_sent_invites(&self, invites: &[Invite]) -> Result<()> { @@ -1737,19 +1725,12 @@ pub struct WaitlistSummary { pub unknown_count: i64, } -#[derive(Clone, FromRow, PartialEq, Debug, Serialize, Deserialize)] +#[derive(FromRow, PartialEq, Debug, Serialize, Deserialize)] pub struct Invite { pub email_address: String, pub email_confirmation_code: String, } -#[derive(Serialize, Debug, PartialEq)] -pub struct UnsentInvite { - #[serde(flatten)] - pub invite: Invite, - pub platform_unknown: bool, -} - #[derive(Debug, Serialize, Deserialize)] pub struct NewUserParams { pub github_login: String, @@ -1965,7 +1946,7 @@ mod test { unimplemented!() } - async fn get_unsent_invites(&self, _count: usize) -> Result> { + async fn get_unsent_invites(&self, _count: usize) -> Result> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index b48a45018b174..d5ef045e667fe 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -1030,7 +1030,7 @@ async fn test_signups() { let signups_batch1 = db.get_unsent_invites(3).await.unwrap(); let addresses = signups_batch1 .iter() - .map(|s| &s.invite.email_address) + .map(|s| &s.email_address) .collect::>(); assert_eq!( addresses, @@ -1041,8 +1041,8 @@ async fn test_signups() { ] ); assert_ne!( - signups_batch1[0].invite.email_confirmation_code, - signups_batch1[1].invite.email_confirmation_code + signups_batch1[0].email_confirmation_code, + signups_batch1[1].email_confirmation_code ); // the waitlist isn't updated until we record that the emails @@ -1052,18 +1052,11 @@ async fn test_signups() { // once the emails go out, we can retrieve the next batch // of signups. - db.record_sent_invites( - &signups_batch1 - .iter() - .map(|i| i.invite.clone()) - .collect::>(), - ) - .await - .unwrap(); + db.record_sent_invites(&signups_batch1).await.unwrap(); let signups_batch2 = db.get_unsent_invites(3).await.unwrap(); let addresses = signups_batch2 .iter() - .map(|s| &s.invite.email_address) + .map(|s| &s.email_address) .collect::>(); assert_eq!( addresses, @@ -1096,8 +1089,8 @@ async fn test_signups() { } = db .create_user_from_invite( &Invite { - email_address: signups_batch1[0].invite.email_address.clone(), - email_confirmation_code: signups_batch1[0].invite.email_confirmation_code.clone(), + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), }, NewUserParams { github_login: "person-0".into(), @@ -1117,8 +1110,8 @@ async fn test_signups() { // cannot redeem the same signup again. db.create_user_from_invite( &Invite { - email_address: signups_batch1[0].invite.email_address.clone(), - email_confirmation_code: signups_batch1[0].invite.email_confirmation_code.clone(), + email_address: signups_batch1[0].email_address.clone(), + email_confirmation_code: signups_batch1[0].email_confirmation_code.clone(), }, NewUserParams { github_login: "some-other-github_account".into(), @@ -1132,7 +1125,7 @@ async fn test_signups() { // cannot redeem a signup with the wrong confirmation code. db.create_user_from_invite( &Invite { - email_address: signups_batch1[1].invite.email_address.clone(), + email_address: signups_batch1[1].email_address.clone(), email_confirmation_code: "the-wrong-code".to_string(), }, NewUserParams {