collab: Make users.github_user_id required and unique (#16704)

This PR makes the `github_user_id` column on the `users` table required
and replaces the index with a unique index.

I have gone through and ensured that all users have a unique
`github_user_id` in the staging and production databases.

Release Notes:

- N/A

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,

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

crates/collab/src/api.rs

@@ -108,7 +108,7 @@ pub async fn validate_api_token<B>(req: Request<B>, next: Next<B>) -> impl IntoR
 
 #[derive(Debug, Deserialize)]
 struct AuthenticatedUserParams {
-    github_user_id: Option<i32>,
+    github_user_id: i32,
     github_login: String,
     github_email: Option<String>,
     github_user_created_at: Option<chrono::DateTime<chrono::Utc>>,

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<i32>,
+        github_user_id: i32,
         github_email: Option<&str>,
         github_user_created_at: Option<DateTimeUtc>,
         initial_channel_id: Option<ChannelId>,

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<i32>,
+        github_user_id: i32,
         github_email: Option<&str>,
         github_user_created_at: Option<DateTimeUtc>,
         initial_channel_id: Option<ChannelId>,
@@ -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<i32>,
+        github_user_id: i32,
         github_email: Option<&str>,
         github_user_created_at: Option<NaiveDateTime>,
         initial_channel_id: Option<ChannelId>,
         tx: &DatabaseTransaction,
     ) -> Result<User> {
-        if let Some(github_user_id) = github_user_id {
+        if let Some(user_by_github_user_id) = user::Entity::find()
-            if let Some(user_by_github_user_id) = user::Entity::find()
+            .filter(user::Column::GithubUserId.eq(github_user_id))
-                .filter(user::Column::GithubUserId.eq(github_user_id))
+            .one(tx)
-                .one(tx)
+            .await?
-                .await?
+        {
-            {
+            let mut user_by_github_user_id = user_by_github_user_id.into_active_model();
-                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());
-                user_by_github_user_id.github_login = ActiveValue::set(github_login.into());
+            if github_user_created_at.is_some() {
-                if github_user_created_at.is_some() {
+                user_by_github_user_id.github_user_created_at =
-                    user_by_github_user_id.github_user_created_at =
+                    ActiveValue::set(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)
             }
+            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()
+            let user = user::Entity::insert(user::ActiveModel {
-                .filter(user::Column::GithubLogin.eq(github_login))
+                email_address: ActiveValue::set(github_email.map(|email| email.into())),
-                .one(tx)
+                github_login: ActiveValue::set(github_login.into()),
-                .await?
+                github_user_id: ActiveValue::set(github_user_id),
-                .ok_or_else(|| anyhow!("no such user {}", github_login))?;
+                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)
         }
     }

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<i32>,
+    pub github_user_id: i32,
     pub github_user_created_at: Option<NaiveDateTime>,
     pub email_address: Option<String>,
     pub admin: bool,

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

@@ -42,7 +42,7 @@ async fn test_channel_buffers(db: &Arc<Database>) {
             false,
             NewUserParams {
                 github_login: "user_c".into(),
-                github_user_id: 102,
+                github_user_id: 103,
             },
         )
         .await

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

@@ -25,7 +25,7 @@ async fn test_contributors(db: &Arc<Database>) {
     assert_eq!(db.get_contributors().await.unwrap(), Vec::<String>::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<Database>) {
     );
 
     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!(

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

@@ -45,25 +45,25 @@ async fn test_get_users(db: &Arc<Database>) {
             (
                 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<Database>) {
         .user_id;
 
     let user = db
-        .get_or_create_user_by_github_account(
+        .get_or_create_user_by_github_account("the-new-login2", 102, None, Some(Utc::now()), None)
-            "the-new-login2",
-            Some(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<Database>) {
         .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()));
 }
 

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,

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

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,