From bb9f2f8713aa312e24643283ef6477a10f6c8396 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Fri, 30 Aug 2024 17:09:59 -0400 Subject: [PATCH] collab: Require `github_user_created_at` at ingress points (#17180) This PR makes the `github_user_created_at` field required at ingress points into collab. In practice we already have this value passed up, this change just makes that explicit. This is a precursor to making it required in the database. Release Notes: - N/A --- crates/client/src/client.rs | 10 +++++++--- crates/collab/src/api.rs | 2 +- crates/collab/src/db/queries/contributors.rs | 4 ++-- crates/collab/src/db/queries/users.rs | 20 ++++++++----------- .../collab/src/db/tests/contributor_tests.rs | 4 ++-- crates/collab/src/db/tests/db_tests.rs | 4 ++-- crates/collab/src/seed.rs | 2 +- .../collab/src/tests/channel_guest_tests.rs | 4 ++-- crates/collab/src/user_backfiller.rs | 2 +- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 7588f15f51..f3fc307840 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -12,6 +12,7 @@ use async_tungstenite::tungstenite::{ error::Error as WebsocketError, http::{HeaderValue, Request, StatusCode}, }; +use chrono::{DateTime, Utc}; use clock::SystemClock; use collections::HashMap; use futures::{ @@ -1400,6 +1401,7 @@ impl Client { struct GithubUser { id: i32, login: String, + created_at: DateTime, } let request = { @@ -1445,13 +1447,15 @@ impl Client { user }; - // Use the collab server's admin API to retrieve the id + // Use the collab server's admin API to retrieve the ID // of the impersonated user. let mut url = self.rpc_url(http.clone(), None).await?; url.set_path("/user"); url.set_query(Some(&format!( - "github_login={}&github_user_id={}", - github_user.login, github_user.id + "github_login={login}&github_user_id={id}&github_user_created_at={created_at}", + login = github_user.login, + id = github_user.id, + created_at = github_user.created_at.to_rfc3339() ))); let request: http_client::Request = Request::get(url.as_str()) .header("Authorization", format!("token {api_token}")) diff --git a/crates/collab/src/api.rs b/crates/collab/src/api.rs index 8451611427..0fb9ca8c7f 100644 --- a/crates/collab/src/api.rs +++ b/crates/collab/src/api.rs @@ -111,7 +111,7 @@ struct AuthenticatedUserParams { github_user_id: i32, github_login: String, github_email: Option, - github_user_created_at: Option>, + github_user_created_at: chrono::DateTime, } #[derive(Debug, Serialize)] diff --git a/crates/collab/src/db/queries/contributors.rs b/crates/collab/src/db/queries/contributors.rs index 65c8bcd12f..fda39c34b4 100644 --- a/crates/collab/src/db/queries/contributors.rs +++ b/crates/collab/src/db/queries/contributors.rs @@ -65,7 +65,7 @@ impl Database { github_login: &str, github_user_id: i32, github_email: Option<&str>, - github_user_created_at: Option, + github_user_created_at: DateTimeUtc, initial_channel_id: Option, ) -> Result<()> { self.transaction(|tx| async move { @@ -74,7 +74,7 @@ impl Database { github_login, github_user_id, github_email, - github_user_created_at.map(|time| time.naive_utc()), + github_user_created_at.naive_utc(), initial_channel_id, &tx, ) diff --git a/crates/collab/src/db/queries/users.rs b/crates/collab/src/db/queries/users.rs index b7646e3d4e..b755476e33 100644 --- a/crates/collab/src/db/queries/users.rs +++ b/crates/collab/src/db/queries/users.rs @@ -101,7 +101,7 @@ impl Database { github_login: &str, github_user_id: i32, github_email: Option<&str>, - github_user_created_at: Option, + github_user_created_at: DateTimeUtc, initial_channel_id: Option, ) -> Result { self.transaction(|tx| async move { @@ -109,7 +109,7 @@ impl Database { github_login, github_user_id, github_email, - github_user_created_at.map(|created_at| created_at.naive_utc()), + github_user_created_at.naive_utc(), initial_channel_id, &tx, ) @@ -123,7 +123,7 @@ impl Database { github_login: &str, github_user_id: i32, github_email: Option<&str>, - github_user_created_at: Option, + github_user_created_at: NaiveDateTime, initial_channel_id: Option, tx: &DatabaseTransaction, ) -> Result { @@ -134,10 +134,8 @@ impl Database { { 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); - } + user_by_github_user_id.github_user_created_at = + ActiveValue::set(Some(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)) @@ -146,17 +144,15 @@ impl Database { { 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); - } + user_by_github_login.github_user_created_at = + ActiveValue::set(Some(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(github_user_id), - github_user_created_at: ActiveValue::set(github_user_created_at), + github_user_created_at: ActiveValue::set(Some(github_user_created_at)), admin: ActiveValue::set(false), invite_count: ActiveValue::set(0), invite_code: ActiveValue::set(None), diff --git a/crates/collab/src/db/tests/contributor_tests.rs b/crates/collab/src/db/tests/contributor_tests.rs index 7add7196aa..aaeb2301e4 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", 1, None, Some(user1_created_at), None) + db.add_contributor("user1", 1, None, 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", 2, None, Some(user2_created_at), None) + db.add_contributor("user2", 2, None, 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 df27a3e58a..9a4ca3c11a 100644 --- a/crates/collab/src/db/tests/db_tests.rs +++ b/crates/collab/src/db/tests/db_tests.rs @@ -101,7 +101,7 @@ 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", 102, None, Some(Utc::now()), None) + .get_or_create_user_by_github_account("the-new-login2", 102, None, Utc::now(), None) .await .unwrap(); assert_eq!(user.id, user_id2); @@ -113,7 +113,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc) { "login3", 103, Some("user3@example.com"), - Some(Utc::now()), + Utc::now(), None, ) .await diff --git a/crates/collab/src/seed.rs b/crates/collab/src/seed.rs index 357801eee5..15aa9d1591 100644 --- a/crates/collab/src/seed.rs +++ b/crates/collab/src/seed.rs @@ -131,7 +131,7 @@ pub async fn seed(config: &Config, db: &Database, force: bool) -> anyhow::Result &github_user.login, github_user.id, github_user.email.as_deref(), - Some(github_user.created_at), + github_user.created_at, None, ) .await diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs index b071b583ff..8df318dc29 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", 100, None, Some(Utc::now()), None) + .get_or_create_user_by_github_account("user_b", 100, None, 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", 100, None, Some(Utc::now()), None) + .add_contributor("user_b", 100, None, Utc::now(), None) .await .unwrap(); diff --git a/crates/collab/src/user_backfiller.rs b/crates/collab/src/user_backfiller.rs index 5d9c4e2c16..842128361c 100644 --- a/crates/collab/src/user_backfiller.rs +++ b/crates/collab/src/user_backfiller.rs @@ -86,7 +86,7 @@ impl UserBackfiller { &user.github_login, github_user.id, user.email_address.as_deref(), - Some(github_user.created_at), + github_user.created_at, initial_channel_id, ) .await?;