Merge pull request #1941 from zed-industries/Allow-overwriting-signup-data

Joseph T. Lyons created

Allow overwriting signup data if a user signs up more than once with the same email address

Change summary

Cargo.lock                     | 34 ++++++++++++++
crates/collab/Cargo.toml       |  1 
crates/collab/src/db.rs        | 25 ++++++++++
crates/collab/src/db/signup.rs |  3 
crates/collab/src/db/tests.rs  | 84 ++++++++++++++++++++++++++++++++++++
5 files changed, 145 insertions(+), 2 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -1160,6 +1160,7 @@ dependencies = [
  "lsp",
  "nanoid",
  "parking_lot 0.11.2",
+ "pretty_assertions",
  "project",
  "prometheus",
  "rand 0.8.5",
@@ -1730,6 +1731,12 @@ dependencies = [
  "workspace",
 ]
 
+[[package]]
+name = "diff"
+version = "0.1.13"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8"
+
 [[package]]
 name = "digest"
 version = "0.9.0"
@@ -4005,6 +4012,15 @@ dependencies = [
  "workspace",
 ]
 
+[[package]]
+name = "output_vt100"
+version = "0.1.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "628223faebab4e3e40667ee0b2336d34a5b960ff60ea743ddfdbcf7770bcfb66"
+dependencies = [
+ "winapi 0.3.9",
+]
+
 [[package]]
 name = "overload"
 version = "0.1.1"
@@ -4346,6 +4362,18 @@ version = "0.2.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de"
 
+[[package]]
+name = "pretty_assertions"
+version = "1.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a25e9bcb20aa780fd0bb16b72403a9064d6b3f22f026946029acb941a50af755"
+dependencies = [
+ "ctor",
+ "diff",
+ "output_vt100",
+ "yansi",
+]
+
 [[package]]
 name = "proc-macro-crate"
 version = "0.1.5"
@@ -8065,6 +8093,12 @@ dependencies = [
  "linked-hash-map",
 ]
 
+[[package]]
+name = "yansi"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec"
+
 [[package]]
 name = "zed"
 version = "0.67.0"

crates/collab/Cargo.toml 🔗

@@ -64,6 +64,7 @@ fs = { path = "../fs", features = ["test-support"] }
 git = { path = "../git", features = ["test-support"] }
 live_kit_client = { path = "../live_kit_client", features = ["test-support"] }
 lsp = { path = "../lsp", features = ["test-support"] }
+pretty_assertions = "1.3.0"
 project = { path = "../project", features = ["test-support"] }
 rpc = { path = "../rpc", features = ["test-support"] }
 settings = { path = "../settings", features = ["test-support"] }

crates/collab/src/db.rs 🔗

@@ -669,7 +669,15 @@ impl Database {
             })
             .on_conflict(
                 OnConflict::column(signup::Column::EmailAddress)
-                    .update_column(signup::Column::EmailAddress)
+                    .update_columns([
+                        signup::Column::PlatformMac,
+                        signup::Column::PlatformWindows,
+                        signup::Column::PlatformLinux,
+                        signup::Column::EditorFeatures,
+                        signup::Column::ProgrammingLanguages,
+                        signup::Column::DeviceId,
+                        signup::Column::AddedToMailingList,
+                    ])
                     .to_owned(),
             )
             .exec(&*tx)
@@ -679,6 +687,21 @@ impl Database {
         .await
     }
 
+    pub async fn get_signup(&self, email_address: &str) -> Result<signup::Model> {
+        self.transaction(|tx| async move {
+            let signup = signup::Entity::find()
+                .filter(signup::Column::EmailAddress.eq(email_address))
+                .one(&*tx)
+                .await?
+                .ok_or_else(|| {
+                    anyhow!("signup with email address {} doesn't exist", email_address)
+                })?;
+
+            Ok(signup)
+        })
+        .await
+    }
+
     pub async fn get_waitlist_summary(&self) -> Result<WaitlistSummary> {
         self.transaction(|tx| async move {
             let query = "

crates/collab/src/db/signup.rs 🔗

@@ -34,7 +34,7 @@ pub struct Invite {
     pub email_confirmation_code: String,
 }
 
-#[derive(Clone, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct NewSignup {
     pub email_address: String,
     pub platform_mac: bool,
@@ -44,6 +44,7 @@ pub struct NewSignup {
     pub programming_languages: Vec<String>,
     pub device_id: Option<String>,
     pub added_to_mailing_list: bool,
+    pub created_at: Option<DateTime>,
 }
 
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, FromQueryResult)]

crates/collab/src/db/tests.rs 🔗

@@ -2,6 +2,9 @@ use super::*;
 use gpui::executor::{Background, Deterministic};
 use std::sync::Arc;
 
+#[cfg(test)]
+use pretty_assertions::{assert_eq, assert_ne};
+
 macro_rules! test_both_dbs {
     ($postgres_test_name:ident, $sqlite_test_name:ident, $db:ident, $body:block) => {
         #[gpui::test]
@@ -804,6 +807,86 @@ async fn test_invite_codes() {
     assert!(db.has_contact(user5, user1).await.unwrap());
 }
 
+#[gpui::test]
+async fn test_multiple_signup_overwrite() {
+    let test_db = TestDb::postgres(build_background_executor());
+    let db = test_db.db();
+
+    let email_address = "user_1@example.com".to_string();
+
+    let initial_signup_created_at_milliseconds = 0;
+
+    let initial_signup = NewSignup {
+        email_address: email_address.clone(),
+        platform_mac: false,
+        platform_linux: true,
+        platform_windows: false,
+        editor_features: vec!["speed".into()],
+        programming_languages: vec!["rust".into(), "c".into()],
+        device_id: Some(format!("device_id")),
+        added_to_mailing_list: false,
+        created_at: Some(
+            DateTime::from_timestamp_millis(initial_signup_created_at_milliseconds).unwrap(),
+        ),
+    };
+
+    db.create_signup(&initial_signup).await.unwrap();
+
+    let initial_signup_from_db = db.get_signup(&email_address).await.unwrap();
+
+    assert_eq!(
+        initial_signup_from_db.clone(),
+        signup::Model {
+            email_address: initial_signup.email_address,
+            platform_mac: initial_signup.platform_mac,
+            platform_linux: initial_signup.platform_linux,
+            platform_windows: initial_signup.platform_windows,
+            editor_features: Some(initial_signup.editor_features),
+            programming_languages: Some(initial_signup.programming_languages),
+            added_to_mailing_list: initial_signup.added_to_mailing_list,
+            ..initial_signup_from_db
+        }
+    );
+
+    let subsequent_signup = NewSignup {
+        email_address: email_address.clone(),
+        platform_mac: true,
+        platform_linux: false,
+        platform_windows: true,
+        editor_features: vec!["git integration".into(), "clean design".into()],
+        programming_languages: vec!["d".into(), "elm".into()],
+        device_id: Some(format!("different_device_id")),
+        added_to_mailing_list: true,
+        // subsequent signup happens next day
+        created_at: Some(
+            DateTime::from_timestamp_millis(
+                initial_signup_created_at_milliseconds + (1000 * 60 * 60 * 24),
+            )
+            .unwrap(),
+        ),
+    };
+
+    db.create_signup(&subsequent_signup).await.unwrap();
+
+    let subsequent_signup_from_db = db.get_signup(&email_address).await.unwrap();
+
+    assert_eq!(
+        subsequent_signup_from_db.clone(),
+        signup::Model {
+            platform_mac: subsequent_signup.platform_mac,
+            platform_linux: subsequent_signup.platform_linux,
+            platform_windows: subsequent_signup.platform_windows,
+            editor_features: Some(subsequent_signup.editor_features),
+            programming_languages: Some(subsequent_signup.programming_languages),
+            device_id: subsequent_signup.device_id,
+            added_to_mailing_list: subsequent_signup.added_to_mailing_list,
+            // shouldn't overwrite their creation Datetime - user shouldn't lose their spot in line
+            created_at: initial_signup_from_db.created_at,
+            ..subsequent_signup_from_db
+        }
+    );
+}
+
 #[gpui::test]
 async fn test_signups() {
     let test_db = TestDb::postgres(build_background_executor());
@@ -823,6 +906,7 @@ async fn test_signups() {
             programming_languages: vec!["rust".into(), "c".into()],
             device_id: Some(format!("device_id_{i}")),
             added_to_mailing_list: i != 0, // One user failed to subscribe
+            created_at: Some(DateTime::from_timestamp_millis(i as i64).unwrap()), // Signups are consecutive
         })
         .collect::<Vec<NewSignup>>();