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
This commit is contained in:
Marshall Bowers 2024-08-30 17:09:59 -04:00 committed by GitHub
parent 9a8c301a7d
commit bb9f2f8713
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 26 additions and 26 deletions

View File

@ -12,6 +12,7 @@ use async_tungstenite::tungstenite::{
error::Error as WebsocketError, error::Error as WebsocketError,
http::{HeaderValue, Request, StatusCode}, http::{HeaderValue, Request, StatusCode},
}; };
use chrono::{DateTime, Utc};
use clock::SystemClock; use clock::SystemClock;
use collections::HashMap; use collections::HashMap;
use futures::{ use futures::{
@ -1400,6 +1401,7 @@ impl Client {
struct GithubUser { struct GithubUser {
id: i32, id: i32,
login: String, login: String,
created_at: DateTime<Utc>,
} }
let request = { let request = {
@ -1445,13 +1447,15 @@ impl Client {
user 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. // of the impersonated user.
let mut url = self.rpc_url(http.clone(), None).await?; let mut url = self.rpc_url(http.clone(), None).await?;
url.set_path("/user"); url.set_path("/user");
url.set_query(Some(&format!( url.set_query(Some(&format!(
"github_login={}&github_user_id={}", "github_login={login}&github_user_id={id}&github_user_created_at={created_at}",
github_user.login, github_user.id login = github_user.login,
id = github_user.id,
created_at = github_user.created_at.to_rfc3339()
))); )));
let request: http_client::Request<AsyncBody> = Request::get(url.as_str()) let request: http_client::Request<AsyncBody> = Request::get(url.as_str())
.header("Authorization", format!("token {api_token}")) .header("Authorization", format!("token {api_token}"))

View File

@ -111,7 +111,7 @@ struct AuthenticatedUserParams {
github_user_id: i32, github_user_id: i32,
github_login: String, github_login: String,
github_email: Option<String>, github_email: Option<String>,
github_user_created_at: Option<chrono::DateTime<chrono::Utc>>, github_user_created_at: chrono::DateTime<chrono::Utc>,
} }
#[derive(Debug, Serialize)] #[derive(Debug, Serialize)]

View File

@ -65,7 +65,7 @@ impl Database {
github_login: &str, github_login: &str,
github_user_id: i32, github_user_id: i32,
github_email: Option<&str>, github_email: Option<&str>,
github_user_created_at: Option<DateTimeUtc>, github_user_created_at: DateTimeUtc,
initial_channel_id: Option<ChannelId>, initial_channel_id: Option<ChannelId>,
) -> Result<()> { ) -> Result<()> {
self.transaction(|tx| async move { self.transaction(|tx| async move {
@ -74,7 +74,7 @@ impl Database {
github_login, github_login,
github_user_id, github_user_id,
github_email, github_email,
github_user_created_at.map(|time| time.naive_utc()), github_user_created_at.naive_utc(),
initial_channel_id, initial_channel_id,
&tx, &tx,
) )

View File

@ -101,7 +101,7 @@ impl Database {
github_login: &str, github_login: &str,
github_user_id: i32, github_user_id: i32,
github_email: Option<&str>, github_email: Option<&str>,
github_user_created_at: Option<DateTimeUtc>, github_user_created_at: DateTimeUtc,
initial_channel_id: Option<ChannelId>, initial_channel_id: Option<ChannelId>,
) -> Result<User> { ) -> Result<User> {
self.transaction(|tx| async move { self.transaction(|tx| async move {
@ -109,7 +109,7 @@ impl Database {
github_login, github_login,
github_user_id, github_user_id,
github_email, github_email,
github_user_created_at.map(|created_at| created_at.naive_utc()), github_user_created_at.naive_utc(),
initial_channel_id, initial_channel_id,
&tx, &tx,
) )
@ -123,7 +123,7 @@ impl Database {
github_login: &str, github_login: &str,
github_user_id: i32, github_user_id: i32,
github_email: Option<&str>, github_email: Option<&str>,
github_user_created_at: Option<NaiveDateTime>, github_user_created_at: NaiveDateTime,
initial_channel_id: Option<ChannelId>, initial_channel_id: Option<ChannelId>,
tx: &DatabaseTransaction, tx: &DatabaseTransaction,
) -> Result<User> { ) -> Result<User> {
@ -134,10 +134,8 @@ impl Database {
{ {
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() { user_by_github_user_id.github_user_created_at =
user_by_github_user_id.github_user_created_at = ActiveValue::set(Some(github_user_created_at));
ActiveValue::set(github_user_created_at);
}
Ok(user_by_github_user_id.update(tx).await?) Ok(user_by_github_user_id.update(tx).await?)
} else if let Some(user_by_github_login) = user::Entity::find() } else if let Some(user_by_github_login) = user::Entity::find()
.filter(user::Column::GithubLogin.eq(github_login)) .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(); 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); 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 =
user_by_github_login.github_user_created_at = ActiveValue::set(Some(github_user_created_at));
ActiveValue::set(github_user_created_at);
}
Ok(user_by_github_login.update(tx).await?) Ok(user_by_github_login.update(tx).await?)
} else { } else {
let user = user::Entity::insert(user::ActiveModel { let user = user::Entity::insert(user::ActiveModel {
email_address: ActiveValue::set(github_email.map(|email| email.into())), email_address: ActiveValue::set(github_email.map(|email| email.into())),
github_login: ActiveValue::set(github_login.into()), github_login: ActiveValue::set(github_login.into()),
github_user_id: ActiveValue::set(github_user_id), 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), admin: ActiveValue::set(false),
invite_count: ActiveValue::set(0), invite_count: ActiveValue::set(0),
invite_code: ActiveValue::set(None), invite_code: ActiveValue::set(None),

View File

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

View File

@ -101,7 +101,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc<Database>) {
.user_id; .user_id;
let user = db 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 .await
.unwrap(); .unwrap();
assert_eq!(user.id, user_id2); assert_eq!(user.id, user_id2);
@ -113,7 +113,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc<Database>) {
"login3", "login3",
103, 103,
Some("user3@example.com"), Some("user3@example.com"),
Some(Utc::now()), Utc::now(),
None, None,
) )
.await .await

View File

@ -131,7 +131,7 @@ pub async fn seed(config: &Config, db: &Database, force: bool) -> anyhow::Result
&github_user.login, &github_user.login,
github_user.id, github_user.id,
github_user.email.as_deref(), github_user.email.as_deref(),
Some(github_user.created_at), github_user.created_at,
None, None,
) )
.await .await

View File

@ -168,7 +168,7 @@ async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut Tes
server server
.app_state .app_state
.db .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 .await
.unwrap(); .unwrap();
@ -266,7 +266,7 @@ async fn test_channel_requires_zed_cla(cx_a: &mut TestAppContext, cx_b: &mut Tes
server server
.app_state .app_state
.db .db
.add_contributor("user_b", 100, None, Some(Utc::now()), None) .add_contributor("user_b", 100, None, Utc::now(), None)
.await .await
.unwrap(); .unwrap();

View File

@ -86,7 +86,7 @@ impl UserBackfiller {
&user.github_login, &user.github_login,
github_user.id, github_user.id,
user.email_address.as_deref(), user.email_address.as_deref(),
Some(github_user.created_at), github_user.created_at,
initial_channel_id, initial_channel_id,
) )
.await?; .await?;