Make device_id optional on signups table

Max Brunsfeld and Joseph Lyons created

This way, signup won't fail if for some reason, the
user's client-side JS doesn't provide an amplitude
device id.

Co-authored-by: Joseph Lyons <joseph@zed.dev>

Change summary

crates/collab/migrations/20220913211150_create_signups.up.sql |  2 
crates/collab/src/api.rs                                      | 13 
crates/collab/src/db.rs                                       | 46 +++-
crates/collab/src/db_tests.rs                                 | 45 +++-
4 files changed, 75 insertions(+), 31 deletions(-)

Detailed changes

crates/collab/migrations/20220913211150_create_signups.up.sql 🔗

@@ -4,7 +4,7 @@ CREATE TABLE IF NOT EXISTS "signups" (
     "email_confirmation_code" VARCHAR(64) NOT NULL,
     "email_confirmation_sent" BOOLEAN NOT NULL,
     "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
-    "device_id" VARCHAR NOT NULL,
+    "device_id" VARCHAR,
     "user_id" INTEGER REFERENCES users (id) ON DELETE CASCADE,
     "inviting_user_id" INTEGER REFERENCES users (id) ON DELETE SET NULL,
 

crates/collab/src/api.rs 🔗

@@ -157,9 +157,9 @@ async fn create_user(
                 user,
             )
             .await?;
-        user_id = result.0;
-        signup_device_id = Some(result.2);
-        if let Some(inviter_id) = result.1 {
+        user_id = result.user_id;
+        signup_device_id = result.signup_device_id;
+        if let Some(inviter_id) = result.inviting_user_id {
             rpc_server
                 .invite_code_redeemed(inviter_id, user_id)
                 .await
@@ -425,6 +425,7 @@ async fn get_waitlist_summary(
 pub struct CreateInviteFromCodeParams {
     invite_code: String,
     email_address: String,
+    device_id: Option<String>,
 }
 
 async fn create_invite_from_code(
@@ -433,7 +434,11 @@ async fn create_invite_from_code(
 ) -> Result<Json<Invite>> {
     Ok(Json(
         app.db
-            .create_invite_from_code(&params.invite_code, &params.email_address)
+            .create_invite_from_code(
+                &params.invite_code,
+                &params.email_address,
+                params.device_id.as_deref(),
+            )
             .await?,
     ))
 }

crates/collab/src/db.rs 🔗

@@ -35,7 +35,12 @@ pub trait Db: Send + Sync {
     async fn set_invite_count_for_user(&self, id: UserId, count: u32) -> Result<()>;
     async fn get_invite_code_for_user(&self, id: UserId) -> Result<Option<(String, u32)>>;
     async fn get_user_for_invite_code(&self, code: &str) -> Result<User>;
-    async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result<Invite>;
+    async fn create_invite_from_code(
+        &self,
+        code: &str,
+        email_address: &str,
+        device_id: Option<&str>,
+    ) -> Result<Invite>;
 
     async fn create_signup(&self, signup: Signup) -> Result<()>;
     async fn get_waitlist_summary(&self) -> Result<WaitlistSummary>;
@@ -45,7 +50,7 @@ pub trait Db: Send + Sync {
         &self,
         invite: &Invite,
         user: NewUserParams,
-    ) -> Result<(UserId, Option<UserId>, String)>;
+    ) -> Result<NewUserResult>;
 
     /// Registers a new project for the given user.
     async fn register_project(&self, host_user_id: UserId) -> Result<ProjectId>;
@@ -458,14 +463,14 @@ impl Db for PostgresDb {
         &self,
         invite: &Invite,
         user: NewUserParams,
-    ) -> Result<(UserId, Option<UserId>, String)> {
+    ) -> Result<NewUserResult> {
         let mut tx = self.pool.begin().await?;
 
-        let (signup_id, existing_user_id, inviting_user_id, device_id): (
+        let (signup_id, existing_user_id, inviting_user_id, signup_device_id): (
             i32,
             Option<UserId>,
             Option<UserId>,
-            String,
+            Option<String>,
         ) = sqlx::query_as(
             "
             SELECT id, user_id, inviting_user_id, device_id
@@ -552,7 +557,11 @@ impl Db for PostgresDb {
         }
 
         tx.commit().await?;
-        Ok((user_id, inviting_user_id, device_id))
+        Ok(NewUserResult {
+            user_id,
+            inviting_user_id,
+            signup_device_id,
+        })
     }
 
     // invite codes
@@ -625,7 +634,12 @@ impl Db for PostgresDb {
         })
     }
 
-    async fn create_invite_from_code(&self, code: &str, email_address: &str) -> Result<Invite> {
+    async fn create_invite_from_code(
+        &self,
+        code: &str,
+        email_address: &str,
+        device_id: Option<&str>,
+    ) -> Result<Invite> {
         let mut tx = self.pool.begin().await?;
 
         let existing_user: Option<UserId> = sqlx::query_scalar(
@@ -679,10 +693,11 @@ impl Db for PostgresDb {
                 platform_linux,
                 platform_mac,
                 platform_windows,
-                platform_unknown
+                platform_unknown,
+                device_id
             )
             VALUES
-                ($1, $2, 'f', $3, 'f', 'f', 'f', 't')
+                ($1, $2, 'f', $3, 'f', 'f', 'f', 't', $4)
             ON CONFLICT (email_address)
             DO UPDATE SET
                 inviting_user_id = excluded.inviting_user_id
@@ -692,6 +707,7 @@ impl Db for PostgresDb {
         .bind(&email_address)
         .bind(&random_email_confirmation_code())
         .bind(&inviter_id)
+        .bind(&device_id)
         .fetch_one(&mut tx)
         .await?;
 
@@ -1675,7 +1691,7 @@ pub struct Signup {
     pub platform_linux: bool,
     pub editor_features: Vec<String>,
     pub programming_languages: Vec<String>,
-    pub device_id: String,
+    pub device_id: Option<String>,
 }
 
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, FromRow)]
@@ -1703,6 +1719,13 @@ pub struct NewUserParams {
     pub invite_count: i32,
 }
 
+#[derive(Debug)]
+pub struct NewUserResult {
+    pub user_id: UserId,
+    pub inviting_user_id: Option<UserId>,
+    pub signup_device_id: Option<String>,
+}
+
 fn random_invite_code() -> String {
     nanoid::nanoid!(16)
 }
@@ -1905,7 +1928,7 @@ mod test {
             &self,
             _invite: &Invite,
             _user: NewUserParams,
-        ) -> Result<(UserId, Option<UserId>, String)> {
+        ) -> Result<NewUserResult> {
             unimplemented!()
         }
 
@@ -1928,6 +1951,7 @@ mod test {
             &self,
             _code: &str,
             _email_address: &str,
+            _device_id: Option<&str>,
         ) -> Result<Invite> {
             unimplemented!()
         }

crates/collab/src/db_tests.rs 🔗

@@ -954,10 +954,14 @@ async fn test_invite_codes() {
 
     // User 2 redeems the invite code and becomes a contact of user 1.
     let user2_invite = db
-        .create_invite_from_code(&invite_code, "u2@example.com")
+        .create_invite_from_code(&invite_code, "u2@example.com", Some("user-2-device-id"))
         .await
         .unwrap();
-    let (user2, inviter, _) = db
+    let NewUserResult {
+        user_id: user2,
+        inviting_user_id,
+        signup_device_id,
+    } = db
         .create_user_from_invite(
             &user2_invite,
             NewUserParams {
@@ -970,7 +974,8 @@ async fn test_invite_codes() {
         .unwrap();
     let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap();
     assert_eq!(invite_count, 1);
-    assert_eq!(inviter, Some(user1));
+    assert_eq!(inviting_user_id, Some(user1));
+    assert_eq!(signup_device_id.unwrap(), "user-2-device-id");
     assert_eq!(
         db.get_contacts(user1).await.unwrap(),
         [
@@ -1004,10 +1009,14 @@ async fn test_invite_codes() {
 
     // User 3 redeems the invite code and becomes a contact of user 1.
     let user3_invite = db
-        .create_invite_from_code(&invite_code, "u3@example.com")
+        .create_invite_from_code(&invite_code, "u3@example.com", None)
         .await
         .unwrap();
-    let (user3, inviter, _) = db
+    let NewUserResult {
+        user_id: user3,
+        inviting_user_id,
+        signup_device_id,
+    } = db
         .create_user_from_invite(
             &user3_invite,
             NewUserParams {
@@ -1020,7 +1029,8 @@ async fn test_invite_codes() {
         .unwrap();
     let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap();
     assert_eq!(invite_count, 0);
-    assert_eq!(inviter, Some(user1));
+    assert_eq!(inviting_user_id, Some(user1));
+    assert!(signup_device_id.is_none());
     assert_eq!(
         db.get_contacts(user1).await.unwrap(),
         [
@@ -1057,7 +1067,7 @@ async fn test_invite_codes() {
     );
 
     // Trying to reedem the code for the third time results in an error.
-    db.create_invite_from_code(&invite_code, "u4@example.com")
+    db.create_invite_from_code(&invite_code, "u4@example.com", Some("user-4-device-id"))
         .await
         .unwrap_err();
 
@@ -1069,10 +1079,10 @@ async fn test_invite_codes() {
 
     // User 4 can now redeem the invite code and becomes a contact of user 1.
     let user4_invite = db
-        .create_invite_from_code(&invite_code, "u4@example.com")
+        .create_invite_from_code(&invite_code, "u4@example.com", Some("user-4-device-id"))
         .await
         .unwrap();
-    let (user4, _, _) = db
+    let user4 = db
         .create_user_from_invite(
             &user4_invite,
             NewUserParams {
@@ -1082,7 +1092,8 @@ async fn test_invite_codes() {
             },
         )
         .await
-        .unwrap();
+        .unwrap()
+        .user_id;
 
     let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap();
     assert_eq!(invite_count, 1);
@@ -1126,7 +1137,7 @@ async fn test_invite_codes() {
     );
 
     // An existing user cannot redeem invite codes.
-    db.create_invite_from_code(&invite_code, "u2@example.com")
+    db.create_invite_from_code(&invite_code, "u2@example.com", Some("user-2-device-id"))
         .await
         .unwrap_err();
     let (_, invite_count) = db.get_invite_code_for_user(user1).await.unwrap().unwrap();
@@ -1147,7 +1158,7 @@ async fn test_signups() {
             platform_windows: i % 4 == 0,
             editor_features: vec!["speed".into()],
             programming_languages: vec!["rust".into(), "c".into()],
-            device_id: format!("device_id_{i}"),
+            device_id: Some(format!("device_id_{i}")),
         })
         .await
         .unwrap();
@@ -1217,7 +1228,11 @@ async fn test_signups() {
 
     // user completes the signup process by providing their
     // github account.
-    let (user_id, inviter_id, signup_device_id) = db
+    let NewUserResult {
+        user_id,
+        inviting_user_id,
+        signup_device_id,
+    } = db
         .create_user_from_invite(
             &Invite {
                 email_address: signups_batch1[0].email_address.clone(),
@@ -1232,11 +1247,11 @@ async fn test_signups() {
         .await
         .unwrap();
     let user = db.get_user_by_id(user_id).await.unwrap().unwrap();
-    assert!(inviter_id.is_none());
+    assert!(inviting_user_id.is_none());
     assert_eq!(user.github_login, "person-0");
     assert_eq!(user.email_address.as_deref(), Some("person-0@example.com"));
     assert_eq!(user.invite_count, 5);
-    assert_eq!(signup_device_id, "device_id_0");
+    assert_eq!(signup_device_id.unwrap(), "device_id_0");
 
     // cannot redeem the same signup again.
     db.create_user_from_invite(