Merge pull request #1782 from zed-industries/idempotent-redemption

Max Brunsfeld created 3 years ago

Return an optional response when creating users via invites

Change summary

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(-)

Detailed changes

crates/collab/src/api.rs

@@ -156,7 +156,7 @@ async fn create_user(
     Json(params): Json<CreateUserParams>,
     Extension(app): Extension<Arc<AppState>>,
     Extension(rpc_server): Extension<Arc<rpc::Server>>,
-) -> Result<Json<CreateUserResponse>> {
+) -> Result<Json<Option<CreateUserResponse>>> {
     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)]

crates/collab/src/db.rs

@@ -51,7 +51,7 @@ pub trait Db: Send + Sync {
         &self,
         invite: &Invite,
         user: NewUserParams,
-    ) -> Result<NewUserResult>;
+    ) -> Result<Option<NewUserResult>>;
 
     /// Registers a new project for the given user.
     async fn register_project(&self, host_user_id: UserId) -> Result<ProjectId>;
@@ -482,7 +482,7 @@ impl Db for PostgresDb {
         &self,
         invite: &Invite,
         user: NewUserParams,
-    ) -> Result<NewUserResult> {
+    ) -> Result<Option<NewUserResult>> {
         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<NewUserResult> {
+        ) -> Result<Option<NewUserResult>> {
             unimplemented!()
         }
 

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(