From 9886259b3aea123000307c5e11e089ed98a8924d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 20 Sep 2022 09:44:56 -0700 Subject: [PATCH] Start storing users' github user id --- .../20220913211150_create_signups.down.sql | 4 +- .../20220913211150_create_signups.up.sql | 5 +- crates/collab/src/api.rs | 14 +- crates/collab/src/db.rs | 25 +- crates/collab/src/db_tests.rs | 226 ++++++++++++++++-- crates/collab/src/integration_tests.rs | 30 ++- 6 files changed, 262 insertions(+), 42 deletions(-) diff --git a/crates/collab/migrations/20220913211150_create_signups.down.sql b/crates/collab/migrations/20220913211150_create_signups.down.sql index 14b4e43cea..ec02ac3322 100644 --- a/crates/collab/migrations/20220913211150_create_signups.down.sql +++ b/crates/collab/migrations/20220913211150_create_signups.down.sql @@ -1,8 +1,10 @@ DROP TABLE signups; ALTER TABLE users + DROP COLUMN github_user_id, DROP COLUMN metrics_id; DROP SEQUENCE metrics_id_seq; -DROP INDEX index_users_on_email_address; \ No newline at end of file +DROP INDEX index_users_on_email_address; +DROP INDEX index_users_on_github_user_id; \ No newline at end of file diff --git a/crates/collab/migrations/20220913211150_create_signups.up.sql b/crates/collab/migrations/20220913211150_create_signups.up.sql index 6c9380275d..3de683c58e 100644 --- a/crates/collab/migrations/20220913211150_create_signups.up.sql +++ b/crates/collab/migrations/20220913211150_create_signups.up.sql @@ -19,12 +19,15 @@ CREATE TABLE IF NOT EXISTS "signups" ( "programming_languages" VARCHAR[] ); -CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); CREATE UNIQUE INDEX "index_signups_on_email_address" ON "signups" ("email_address"); CREATE INDEX "index_signups_on_email_confirmation_sent" ON "signups" ("email_confirmation_sent"); ALTER TABLE "users" + ADD "github_user_id" INTEGER, ADD "metrics_id" INTEGER DEFAULT nextval('metrics_id_seq'); +CREATE INDEX "index_users_on_email_address" ON "users" ("email_address"); +CREATE INDEX "index_users_on_github_user_id" ON "users" ("github_user_id"); + UPDATE users SET metrics_id = nextval('metrics_id_seq'); diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index a390b4392a..de8ec44c78 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -112,9 +112,11 @@ async fn get_users( #[derive(Deserialize, Debug)] struct CreateUserParams { + github_user_id: i32, github_login: String, email_address: String, email_confirmation_code: Option, + #[serde(default)] invite_count: i32, } @@ -123,6 +125,11 @@ async fn create_user( Extension(app): Extension>, Extension(rpc_server): Extension>, ) -> Result> { + let user = NewUserParams { + github_login: params.github_login, + github_user_id: params.github_user_id, + invite_count: params.invite_count, + }; let (user_id, inviter_id) = // Creating a user via the normal signup process if let Some(email_confirmation_code) = params.email_confirmation_code { @@ -132,10 +139,7 @@ async fn create_user( email_address: params.email_address, email_confirmation_code, }, - NewUserParams { - github_login: params.github_login, - invite_count: params.invite_count, - }, + user, ) .await? } @@ -143,7 +147,7 @@ async fn create_user( else { ( app.db - .create_user(¶ms.github_login, ¶ms.email_address, false) + .create_user(¶ms.email_address, false, user) .await?, None, ) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 85f13a6a11..f31defa577 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -14,9 +14,9 @@ use time::{OffsetDateTime, PrimitiveDateTime}; pub trait Db: Send + Sync { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result; async fn get_all_users(&self, page: u32, limit: u32) -> Result>; async fn fuzzy_search_users(&self, query: &str, limit: u32) -> Result>; @@ -196,19 +196,20 @@ impl Db for PostgresDb { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result { let query = " - INSERT INTO users (github_login, email_address, admin) - VALUES ($1, $2, $3) + INSERT INTO users (email_address, github_login, github_user_id, admin) + VALUES ($1, $2, $3, $4) ON CONFLICT (github_login) DO UPDATE SET github_login = excluded.github_login RETURNING id "; Ok(sqlx::query_scalar(query) - .bind(github_login) .bind(email_address) + .bind(params.github_login) + .bind(params.github_user_id) .bind(admin) .fetch_one(&self.pool) .await @@ -431,14 +432,15 @@ impl Db for PostgresDb { let user_id: UserId = sqlx::query_scalar( " INSERT INTO users - (email_address, github_login, admin, invite_count, invite_code, metrics_id) + (email_address, github_login, github_user_id, admin, invite_count, invite_code, metrics_id) VALUES - ($1, $2, 'f', $3, $4, $5) + ($1, $2, $3, 'f', $4, $5, $6) RETURNING id ", ) .bind(&invite.email_address) .bind(&user.github_login) + .bind(&user.github_user_id) .bind(&user.invite_count) .bind(random_invite_code()) .bind(metrics_id) @@ -1508,6 +1510,7 @@ id_type!(UserId); pub struct User { pub id: UserId, pub github_login: String, + pub github_user_id: i32, pub email_address: Option, pub admin: bool, pub invite_code: Option, @@ -1637,6 +1640,7 @@ pub struct Invite { #[derive(Debug, Serialize, Deserialize)] pub struct NewUserParams { pub github_login: String, + pub github_user_id: i32, pub invite_count: i32, } @@ -1719,16 +1723,16 @@ mod test { impl Db for FakeDb { async fn create_user( &self, - github_login: &str, email_address: &str, admin: bool, + params: NewUserParams, ) -> Result { self.background.simulate_random_delay().await; let mut users = self.users.lock(); if let Some(user) = users .values() - .find(|user| user.github_login == github_login) + .find(|user| user.github_login == params.github_login) { Ok(user.id) } else { @@ -1737,7 +1741,8 @@ mod test { user_id, User { id: user_id, - github_login: github_login.to_string(), + github_login: params.github_login, + github_user_id: params.github_user_id, email_address: Some(email_address.to_string()), admin, invite_code: None, diff --git a/crates/collab/src/db_tests.rs b/crates/collab/src/db_tests.rs index 16eba2fb22..87033fab38 100644 --- a/crates/collab/src/db_tests.rs +++ b/crates/collab/src/db_tests.rs @@ -12,10 +12,54 @@ async fn test_get_users_by_ids() { ] { let db = test_db.db(); - let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); - let user4 = db.create_user("u4", "u4@example.com", false).await.unwrap(); + let user1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 2, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 3, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user4 = db + .create_user( + "u4@example.com", + false, + NewUserParams { + github_login: "u4".into(), + github_user_id: 4, + invite_count: 0, + }, + ) + .await + .unwrap(); assert_eq!( db.get_users_by_ids(vec![user1, user2, user3, user4]) @@ -25,6 +69,7 @@ async fn test_get_users_by_ids() { User { id: user1, github_login: "u1".to_string(), + github_user_id: 1, email_address: Some("u1@example.com".to_string()), admin: false, ..Default::default() @@ -32,6 +77,7 @@ async fn test_get_users_by_ids() { User { id: user2, github_login: "u2".to_string(), + github_user_id: 2, email_address: Some("u2@example.com".to_string()), admin: false, ..Default::default() @@ -39,6 +85,7 @@ async fn test_get_users_by_ids() { User { id: user3, github_login: "u3".to_string(), + github_user_id: 3, email_address: Some("u3@example.com".to_string()), admin: false, ..Default::default() @@ -46,6 +93,7 @@ async fn test_get_users_by_ids() { User { id: user4, github_login: "u4".to_string(), + github_user_id: 4, email_address: Some("u4@example.com".to_string()), admin: false, ..Default::default() @@ -60,7 +108,18 @@ async fn test_worktree_extensions() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); let project = db.register_project(user).await.unwrap(); db.update_worktree_extensions(project, 100, Default::default()) @@ -120,9 +179,42 @@ async fn test_user_activity() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let user_1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); let project_1 = db.register_project(user_1).await.unwrap(); db.update_worktree_extensions( project_1, @@ -340,7 +432,18 @@ async fn test_recent_channel_messages() { TestDb::fake(build_background_executor()), ] { let db = test_db.db(); - let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let user = db + .create_user( + "u@example.com", + false, + NewUserParams { + github_login: "u".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); let org = db.create_org("org", "org").await.unwrap(); let channel = db.create_org_channel(org, "channel").await.unwrap(); for i in 0..10 { @@ -373,7 +476,18 @@ async fn test_channel_message_nonces() { TestDb::fake(build_background_executor()), ] { let db = test_db.db(); - let user = db.create_user("u", "u@example.com", false).await.unwrap(); + let user = db + .create_user( + "user@example.com", + false, + NewUserParams { + github_login: "user".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); let org = db.create_org("org", "org").await.unwrap(); let channel = db.create_org_channel(org, "channel").await.unwrap(); @@ -404,7 +518,18 @@ async fn test_channel_message_nonces() { async fn test_create_access_tokens() { let test_db = TestDb::postgres().await; let db = test_db.db(); - let user = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); db.create_access_token_hash(user, "h1", 3).await.unwrap(); db.create_access_token_hash(user, "h2", 3).await.unwrap(); @@ -443,7 +568,7 @@ fn test_fuzzy_like_string() { async fn test_fuzzy_search_users() { let test_db = TestDb::postgres().await; let db = test_db.db(); - for github_login in [ + for (i, github_login) in [ "California", "colorado", "oregon", @@ -451,10 +576,21 @@ async fn test_fuzzy_search_users() { "florida", "delaware", "rhode-island", - ] { - db.create_user(github_login, &format!("{github_login}@example.com"), false) - .await - .unwrap(); + ] + .into_iter() + .enumerate() + { + db.create_user( + &format!("{github_login}@example.com"), + false, + NewUserParams { + github_login: github_login.into(), + github_user_id: i as i32, + invite_count: 0, + }, + ) + .await + .unwrap(); } assert_eq!( @@ -484,9 +620,42 @@ async fn test_add_contacts() { ] { let db = test_db.db(); - let user_1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); - let user_2 = db.create_user("u2", "u2@example.com", false).await.unwrap(); - let user_3 = db.create_user("u3", "u3@example.com", false).await.unwrap(); + let user_1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_2 = db + .create_user( + "u2@example.com", + false, + NewUserParams { + github_login: "u2".into(), + github_user_id: 1, + invite_count: 0, + }, + ) + .await + .unwrap(); + let user_3 = db + .create_user( + "u3@example.com", + false, + NewUserParams { + github_login: "u3".into(), + github_user_id: 2, + invite_count: 0, + }, + ) + .await + .unwrap(); // User starts with no contacts assert_eq!( @@ -700,7 +869,18 @@ async fn test_add_contacts() { async fn test_invite_codes() { let postgres = TestDb::postgres().await; let db = postgres.db(); - let user1 = db.create_user("u1", "u1@example.com", false).await.unwrap(); + let user1 = db + .create_user( + "u1@example.com", + false, + NewUserParams { + github_login: "u1".into(), + github_user_id: 0, + invite_count: 0, + }, + ) + .await + .unwrap(); // Initially, user 1 has no invite code assert_eq!(db.get_invite_code_for_user(user1).await.unwrap(), None); @@ -724,6 +904,7 @@ async fn test_invite_codes() { &user2_invite, NewUserParams { github_login: "user2".into(), + github_user_id: 2, invite_count: 7, }, ) @@ -773,6 +954,7 @@ async fn test_invite_codes() { &user3_invite, NewUserParams { github_login: "user-3".into(), + github_user_id: 3, invite_count: 3, }, ) @@ -837,6 +1019,7 @@ async fn test_invite_codes() { &user4_invite, NewUserParams { github_login: "user-4".into(), + github_user_id: 4, invite_count: 5, }, ) @@ -983,6 +1166,7 @@ async fn test_signups() { }, NewUserParams { github_login: "person-0".into(), + github_user_id: 0, invite_count: 5, }, ) @@ -1002,6 +1186,7 @@ async fn test_signups() { }, NewUserParams { github_login: "some-other-github_account".into(), + github_user_id: 1, invite_count: 5, }, ) @@ -1016,6 +1201,7 @@ async fn test_signups() { }, NewUserParams { github_login: "person-1".into(), + github_user_id: 2, invite_count: 5, }, ) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 1a4e4381c1..94811b0951 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -1,5 +1,5 @@ use crate::{ - db::{ProjectId, TestDb, UserId}, + db::{NewUserParams, ProjectId, TestDb, UserId}, rpc::{Executor, Server, Store}, AppState, }; @@ -4641,7 +4641,15 @@ async fn test_random_collaboration( let mut server = TestServer::start(cx.foreground(), cx.background()).await; let db = server.app_state.db.clone(); let host_user_id = db - .create_user("host", "host@example.com", false) + .create_user( + "host@example.com", + false, + NewUserParams { + github_login: "host".into(), + github_user_id: 0, + invite_count: 0, + }, + ) .await .unwrap(); let mut available_guests = vec![ @@ -4651,9 +4659,17 @@ async fn test_random_collaboration( "guest-4".to_string(), ]; - for username in &available_guests { + for (ix, username) in available_guests.iter().enumerate() { let guest_user_id = db - .create_user(username, &format!("{username}@example.com"), false) + .create_user( + &format!("{username}@example.com"), + false, + NewUserParams { + github_login: username.into(), + github_user_id: ix as i32, + invite_count: 0, + }, + ) .await .unwrap(); assert_eq!(*username, format!("guest-{}", guest_user_id)); @@ -5163,7 +5179,11 @@ impl TestServer { } else { self.app_state .db - .create_user(name, &format!("{name}@example.com"), false) + .create_user(&format!("{name}@example.com"), false, NewUserParams { + github_login: name.into(), + github_user_id: 0, + invite_count: 0, + }) .await .unwrap() };