From 985890646369ef128150ecdcc8a964f588284a0e Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 20 Oct 2022 10:52:34 -0600 Subject: [PATCH] Return an optional response when creating users via invites If the user already exists, we return none. This will allow the web frontend to avoid reporting a "join alpha" user event but also not error. Co-Authored-By: Max Brunsfeld Co-Authored-By: Joseph Lyons --- crates/collab/src/api.rs | 14 ++++++++++---- crates/collab/src/db.rs | 15 ++++++--------- crates/collab/src/db_tests.rs | 32 +++++++++++++++++++------------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 5d28fdf668..4f97d93824 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -156,7 +156,7 @@ async fn create_user( Json(params): Json, Extension(app): Extension>, Extension(rpc_server): Extension>, -) -> Result> { +) -> Result>> { let user = NewUserParams { github_login: params.github_login, github_user_id: params.github_user_id, @@ -165,7 +165,8 @@ async fn create_user( // Creating a user via the normal signup process let result = if let Some(email_confirmation_code) = params.email_confirmation_code { - app.db + if let Some(result) = app + .db .create_user_from_invite( &Invite { email_address: params.email_address, @@ -174,6 +175,11 @@ async fn create_user( user, ) .await? + { + result + } else { + return Ok(Json(None)); + } } // Creating a user as an admin else if params.admin { @@ -200,11 +206,11 @@ async fn create_user( .await? .ok_or_else(|| anyhow!("couldn't find the user we just created"))?; - Ok(Json(CreateUserResponse { + Ok(Json(Some(CreateUserResponse { user, metrics_id: result.metrics_id, signup_device_id: result.signup_device_id, - })) + }))) } #[derive(Deserialize)] diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 9b3dca1f2c..b8c09d9afb 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -51,7 +51,7 @@ pub trait Db: Send + Sync { &self, invite: &Invite, user: NewUserParams, - ) -> Result; + ) -> Result>; /// Registers a new project for the given user. async fn register_project(&self, host_user_id: UserId) -> Result; @@ -482,7 +482,7 @@ impl Db for PostgresDb { &self, invite: &Invite, user: NewUserParams, - ) -> Result { + ) -> Result> { let mut tx = self.pool.begin().await?; let (signup_id, existing_user_id, inviting_user_id, signup_device_id): ( @@ -506,10 +506,7 @@ impl Db for PostgresDb { .ok_or_else(|| Error::Http(StatusCode::NOT_FOUND, "no such invite".to_string()))?; if existing_user_id.is_some() { - Err(Error::Http( - StatusCode::UNPROCESSABLE_ENTITY, - "invitation already redeemed".to_string(), - ))?; + return Ok(None); } let (user_id, metrics_id): (UserId, String) = sqlx::query_as( @@ -576,12 +573,12 @@ impl Db for PostgresDb { } tx.commit().await?; - Ok(NewUserResult { + Ok(Some(NewUserResult { user_id, metrics_id, inviting_user_id, signup_device_id, - }) + })) } // invite codes @@ -1958,7 +1955,7 @@ mod test { &self, _invite: &Invite, _user: NewUserParams, - ) -> Result { + ) -> Result> { unimplemented!() } diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index d5ef045e66..ff5e05dd5d 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -852,6 +852,7 @@ async fn test_invite_codes() { }, ) .await + .unwrap() .unwrap(); let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); assert_eq!(invite_count, 1); @@ -897,6 +898,7 @@ async fn test_invite_codes() { }, ) .await + .unwrap() .unwrap(); let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); assert_eq!(invite_count, 0); @@ -954,6 +956,7 @@ async fn test_invite_codes() { ) .await .unwrap() + .unwrap() .user_id; let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap(); @@ -1099,6 +1102,7 @@ async fn test_signups() { }, ) .await + .unwrap() .unwrap(); let user = db.get_user_by_id(user_id).await.unwrap().unwrap(); assert!(inviting_user_id.is_none()); @@ -1108,19 +1112,21 @@ async fn test_signups() { assert_eq!(signup_device_id.unwrap(), "device_id_0"); // 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(), - }, - NewUserParams { - github_login: "some-other-github_account".into(), - github_user_id: 1, - invite_count: 5, - }, - ) - .await - .unwrap_err(); + assert!(db + .create_user_from_invite( + &Invite { + 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(), + github_user_id: 1, + invite_count: 5, + }, + ) + .await + .unwrap() + .is_none()); // cannot redeem a signup with the wrong confirmation code. db.create_user_from_invite(