diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 225971ec32..4db9774c23 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -9,14 +9,14 @@ CREATE TABLE "users" ( "connected_once" BOOLEAN NOT NULL DEFAULT false, "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, "metrics_id" TEXT, - "github_user_id" INTEGER, + "github_user_id" INTEGER NOT NULL, "accepted_tos_at" TIMESTAMP WITHOUT TIME ZONE, "github_user_created_at" TIMESTAMP WITHOUT TIME ZONE ); CREATE UNIQUE INDEX "index_users_github_login" ON "users" ("github_login"); CREATE UNIQUE INDEX "index_invite_code_users" ON "users" ("invite_code"); CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); -CREATE INDEX "index_users_on_github_user_id" ON "users" ("github_user_id"); +CREATE UNIQUE INDEX "index_users_on_github_user_id" ON "users" ("github_user_id"); CREATE TABLE "access_tokens" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, diff --git a/crates/collab/migrations/20240822215737_add_unique_constraint_on_github_user_id_on_users.sql b/crates/collab/migrations/20240822215737_add_unique_constraint_on_github_user_id_on_users.sql new file mode 100644 index 0000000000..3b418f7e26 --- /dev/null +++ b/crates/collab/migrations/20240822215737_add_unique_constraint_on_github_user_id_on_users.sql @@ -0,0 +1,4 @@ +alter table users alter column github_user_id set not null; + +drop index index_users_on_github_user_id; +create unique index uix_users_on_github_user_id on users (github_user_id); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 1aadcc5cd9..8451611427 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -108,7 +108,7 @@ pub async fn validate_api_token(req: Request, next: Next) -> impl IntoR #[derive(Debug, Deserialize)] struct AuthenticatedUserParams { - github_user_id: Option, + github_user_id: i32, github_login: String, github_email: Option, github_user_created_at: Option>, diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index 3078de42ed..65c8bcd12f 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -63,7 +63,7 @@ impl Database { pub async fn add_contributor( &self, github_login: &str, - github_user_id: Option, + github_user_id: i32, github_email: Option<&str>, github_user_created_at: Option, initial_channel_id: Option, diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index 2aa5857560..54c0dc17d1 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -15,7 +15,7 @@ impl Database { let user = user::Entity::insert(user::ActiveModel { email_address: ActiveValue::set(Some(email_address.into())), github_login: ActiveValue::set(params.github_login.clone()), - github_user_id: ActiveValue::set(Some(params.github_user_id)), + github_user_id: ActiveValue::set(params.github_user_id), admin: ActiveValue::set(admin), metrics_id: ActiveValue::set(Uuid::new_v4()), ..Default::default() @@ -99,7 +99,7 @@ impl Database { pub async fn get_or_create_user_by_github_account( &self, github_login: &str, - github_user_id: Option, + github_user_id: i32, github_email: Option<&str>, github_user_created_at: Option, initial_channel_id: Option, @@ -121,70 +121,61 @@ impl Database { pub async fn get_or_create_user_by_github_account_tx( &self, github_login: &str, - github_user_id: Option, + github_user_id: i32, github_email: Option<&str>, github_user_created_at: Option, initial_channel_id: Option, tx: &DatabaseTransaction, ) -> Result { - if let Some(github_user_id) = github_user_id { - if let Some(user_by_github_user_id) = user::Entity::find() - .filter(user::Column::GithubUserId.eq(github_user_id)) - .one(tx) - .await? - { - let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); - user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); - if github_user_created_at.is_some() { - user_by_github_user_id.github_user_created_at = - ActiveValue::set(github_user_created_at); - } - Ok(user_by_github_user_id.update(tx).await?) - } else if let Some(user_by_github_login) = user::Entity::find() - .filter(user::Column::GithubLogin.eq(github_login)) - .one(tx) - .await? - { - let mut user_by_github_login = user_by_github_login.into_active_model(); - user_by_github_login.github_user_id = ActiveValue::set(Some(github_user_id)); - if github_user_created_at.is_some() { - user_by_github_login.github_user_created_at = - ActiveValue::set(github_user_created_at); - } - Ok(user_by_github_login.update(tx).await?) - } else { - let user = user::Entity::insert(user::ActiveModel { - email_address: ActiveValue::set(github_email.map(|email| email.into())), - github_login: ActiveValue::set(github_login.into()), - github_user_id: ActiveValue::set(Some(github_user_id)), - github_user_created_at: ActiveValue::set(github_user_created_at), - admin: ActiveValue::set(false), - invite_count: ActiveValue::set(0), - invite_code: ActiveValue::set(None), - metrics_id: ActiveValue::set(Uuid::new_v4()), - ..Default::default() - }) - .exec_with_returning(tx) - .await?; - if let Some(channel_id) = initial_channel_id { - channel_member::Entity::insert(channel_member::ActiveModel { - id: ActiveValue::NotSet, - channel_id: ActiveValue::Set(channel_id), - user_id: ActiveValue::Set(user.id), - accepted: ActiveValue::Set(true), - role: ActiveValue::Set(ChannelRole::Guest), - }) - .exec(tx) - .await?; - } - Ok(user) + if let Some(user_by_github_user_id) = user::Entity::find() + .filter(user::Column::GithubUserId.eq(github_user_id)) + .one(tx) + .await? + { + let mut user_by_github_user_id = user_by_github_user_id.into_active_model(); + user_by_github_user_id.github_login = ActiveValue::set(github_login.into()); + if github_user_created_at.is_some() { + user_by_github_user_id.github_user_created_at = + ActiveValue::set(github_user_created_at); } + Ok(user_by_github_user_id.update(tx).await?) + } else if let Some(user_by_github_login) = user::Entity::find() + .filter(user::Column::GithubLogin.eq(github_login)) + .one(tx) + .await? + { + let mut user_by_github_login = user_by_github_login.into_active_model(); + user_by_github_login.github_user_id = ActiveValue::set(github_user_id); + if github_user_created_at.is_some() { + user_by_github_login.github_user_created_at = + ActiveValue::set(github_user_created_at); + } + Ok(user_by_github_login.update(tx).await?) } else { - let user = user::Entity::find() - .filter(user::Column::GithubLogin.eq(github_login)) - .one(tx) - .await? - .ok_or_else(|| anyhow!("no such user {}", github_login))?; + let user = user::Entity::insert(user::ActiveModel { + email_address: ActiveValue::set(github_email.map(|email| email.into())), + github_login: ActiveValue::set(github_login.into()), + github_user_id: ActiveValue::set(github_user_id), + github_user_created_at: ActiveValue::set(github_user_created_at), + admin: ActiveValue::set(false), + invite_count: ActiveValue::set(0), + invite_code: ActiveValue::set(None), + metrics_id: ActiveValue::set(Uuid::new_v4()), + ..Default::default() + }) + .exec_with_returning(tx) + .await?; + if let Some(channel_id) = initial_channel_id { + channel_member::Entity::insert(channel_member::ActiveModel { + id: ActiveValue::NotSet, + channel_id: ActiveValue::Set(channel_id), + user_id: ActiveValue::Set(user.id), + accepted: ActiveValue::Set(true), + role: ActiveValue::Set(ChannelRole::Guest), + }) + .exec(tx) + .await?; + } Ok(user) } } diff --git a/crates/collab/src/db/tables/user.rs b/crates/collab/src/db/tables/user.rs index e65b241338..ff4331331b 100644 --- a/crates/collab/src/db/tables/user.rs +++ b/crates/collab/src/db/tables/user.rs @@ -10,7 +10,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: UserId, pub github_login: String, - pub github_user_id: Option, + pub github_user_id: i32, pub github_user_created_at: Option, pub email_address: Option, pub admin: bool, diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index 708461205e..55a8f216c4 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -42,7 +42,7 @@ async fn test_channel_buffers(db: &Arc) { false, NewUserParams { github_login: "user_c".into(), - github_user_id: 102, + github_user_id: 103, }, ) .await diff --git a/crates/collab/src/db/tests/contributor_tests.rs b/crates/collab/src/db/tests/contributor_tests.rs index 0d5c0da6e5..7add7196aa 100644 --- a/crates/collab/src/db/tests/contributor_tests.rs +++ b/crates/collab/src/db/tests/contributor_tests.rs @@ -25,7 +25,7 @@ async fn test_contributors(db: &Arc) { assert_eq!(db.get_contributors().await.unwrap(), Vec::::new()); let user1_created_at = Utc::now(); - db.add_contributor("user1", Some(1), None, Some(user1_created_at), None) + db.add_contributor("user1", 1, None, Some(user1_created_at), None) .await .unwrap(); assert_eq!( @@ -34,7 +34,7 @@ async fn test_contributors(db: &Arc) { ); let user2_created_at = Utc::now(); - db.add_contributor("user2", Some(2), None, Some(user2_created_at), None) + db.add_contributor("user2", 2, None, Some(user2_created_at), None) .await .unwrap(); assert_eq!( diff --git a/crates/collab/src/db/tests/db_tests.rs b/crates/collab/src/db/tests/db_tests.rs index eabc1a9091..df27a3e58a 100644 --- a/crates/collab/src/db/tests/db_tests.rs +++ b/crates/collab/src/db/tests/db_tests.rs @@ -45,25 +45,25 @@ async fn test_get_users(db: &Arc) { ( user_ids[0], "user1".to_string(), - Some(1), + 1, Some("user1@example.com".to_string()), ), ( user_ids[1], "user2".to_string(), - Some(2), + 2, Some("user2@example.com".to_string()), ), ( user_ids[2], "user3".to_string(), - Some(3), + 3, Some("user3@example.com".to_string()), ), ( user_ids[3], "user4".to_string(), - Some(4), + 4, Some("user4@example.com".to_string()), ) ] @@ -101,23 +101,17 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { .user_id; let user = db - .get_or_create_user_by_github_account( - "the-new-login2", - Some(102), - None, - Some(Utc::now()), - None, - ) + .get_or_create_user_by_github_account("the-new-login2", 102, None, Some(Utc::now()), None) .await .unwrap(); assert_eq!(user.id, user_id2); assert_eq!(&user.github_login, "the-new-login2"); - assert_eq!(user.github_user_id, Some(102)); + assert_eq!(user.github_user_id, 102); let user = db .get_or_create_user_by_github_account( "login3", - Some(103), + 103, Some("user3@example.com"), Some(Utc::now()), None, @@ -125,7 +119,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { .await .unwrap(); assert_eq!(&user.github_login, "login3"); - assert_eq!(user.github_user_id, Some(103)); + assert_eq!(user.github_user_id, 103); assert_eq!(user.email_address, Some("user3@example.com".into())); } diff --git a/crates/collab/src/seed.rs b/crates/collab/src/seed.rs index 6b56f46d50..bb5b78b059 100644 --- a/crates/collab/src/seed.rs +++ b/crates/collab/src/seed.rs @@ -127,7 +127,7 @@ pub async fn seed(config: &Config, db: &Database, force: bool) -> anyhow::Result let user = db .get_or_create_user_by_github_account( &github_user.login, - Some(github_user.id), + github_user.id, github_user.email.as_deref(), None, None, diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs index 3415c974fc..b071b583ff 100644 --- a/crates/collab/src/tests/channel_guest_tests.rs +++ b/crates/collab/src/tests/channel_guest_tests.rs @@ -168,7 +168,7 @@ async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut Tes server .app_state .db - .get_or_create_user_by_github_account("user_b", Some(100), None, Some(Utc::now()), None) + .get_or_create_user_by_github_account("user_b", 100, None, Some(Utc::now()), None) .await .unwrap(); @@ -266,7 +266,7 @@ async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut Tes server .app_state .db - .add_contributor("user_b", Some(100), None, Some(Utc::now()), None) + .add_contributor("user_b", 100, None, Some(Utc::now()), None) .await .unwrap(); diff --git a/crates/collab/src/user_backfiller.rs b/crates/collab/src/user_backfiller.rs index 35e02bcd0a..83382e4ec6 100644 --- a/crates/collab/src/user_backfiller.rs +++ b/crates/collab/src/user_backfiller.rs @@ -84,7 +84,7 @@ impl UserBackfiller { self.db .get_or_create_user_by_github_account( &user.github_login, - Some(github_user.id), + github_user.id, user.email_address.as_deref(), Some(github_user.created_at), initial_channel_id,