Allow filling co-authors in the git panel's commit input (#23329)

https://github.com/user-attachments/assets/78db908e-cfe5-4803-b0dc-4f33bc457840


* starts to extract usernames out of `users/` GitHub API responses, and
pass those along with e-mails in the collab sessions as part of the
`User` data

* adjusts various prefill and seed test methods so that the new data can
be retrieved from GitHub properly

* if there's an active call, where guests have write permissions and
e-mails, allow to trigger `FillCoAuthors` action in the context of the
git panel, that will fill in `co-authored-by:` lines, using e-mail and
names (or GitHub handle names if name is absent)

* the action tries to not duplicate such entries, if any are present
already, and adds those below the rest of the commit input's text

Concerns:

* users with write permissions and no e-mails will be silently omitted
— adding odd entries that try to indicate this or raising pop-ups is
very intrusive (maybe, we can add `#`-prefixed comments?), logging seems
pointless

* it's not clear whether the data prefill will run properly on the
existing users — seems tolerable now, as it seems that we get e-mails
properly already, so we'll see GitHub handles instead of names in the
worst case. This can be prefilled better later.

* e-mails and names for a particular project may be not what the user
wants.
E.g. my `.gitconfig` has
```
[user]
    email = mail4score@gmail.com

# .....snip

[includeif "gitdir:**/work/zed/**/.git"]
    path = ~/.gitconfig.work
```

and that one has

```
[user]
    email = kirill@zed.dev
```

while my GitHub profile is configured so, that `mail4score@gmail.com` is
the public, commit e-mail.

So, when I'm a participant in a Zed session, wrong e-mail will be
picked.
The problem is, it's impossible for a host to get remote's collaborator
git metadata for a particular project, as that might not even exist on
disk for the client.

Seems that we might want to add some "project git URL <-> user name and
email" mapping in the settings(?).
The design of this is not very clear, so the PR concentrates on the
basics for now.

When https://github.com/zed-industries/zed/pull/23308 lands, most of the
issues can be solved by collaborators manually, before committing.

Release Notes:

- N/A
This commit is contained in:
Kirill Bulatov 2025-01-18 22:57:17 +02:00 committed by GitHub
parent ac214c52c9
commit 0199eca289
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
32 changed files with 215 additions and 14 deletions

View file

@ -3,6 +3,7 @@ CREATE TABLE "users" (
"github_login" VARCHAR,
"admin" BOOLEAN,
"email_address" VARCHAR(255) DEFAULT NULL,
"name" TEXT,
"invite_code" VARCHAR(64),
"invite_count" INTEGER NOT NULL DEFAULT 0,
"inviter_id" INTEGER REFERENCES users (id),

View file

@ -0,0 +1 @@
ALTER TABLE users ADD COLUMN name TEXT;

View file

@ -144,6 +144,7 @@ struct AuthenticatedUserParams {
github_user_id: i32,
github_login: String,
github_email: Option<String>,
github_name: Option<String>,
github_user_created_at: chrono::DateTime<chrono::Utc>,
}
@ -165,6 +166,7 @@ async fn get_authenticated_user(
&params.github_login,
params.github_user_id,
params.github_email.as_deref(),
params.github_name.as_deref(),
params.github_user_created_at,
initial_channel_id,
)

View file

@ -115,6 +115,7 @@ async fn add_contributor(
&params.github_login,
params.github_user_id,
params.github_email.as_deref(),
params.github_name.as_deref(),
params.github_user_created_at,
initial_channel_id,
)

View file

@ -248,6 +248,7 @@ mod test {
let user = db
.create_user(
"example@example.com",
None,
false,
NewUserParams {
github_login: "example".into(),

View file

@ -726,6 +726,8 @@ impl Database {
user.github_login
),
github_login: user.github_login,
name: user.name,
email: user.email_address,
})
}
proto::ChannelMember {

View file

@ -65,6 +65,7 @@ impl Database {
github_login: &str,
github_user_id: i32,
github_email: Option<&str>,
github_name: Option<&str>,
github_user_created_at: DateTimeUtc,
initial_channel_id: Option<ChannelId>,
) -> Result<()> {
@ -74,6 +75,7 @@ impl Database {
github_login,
github_user_id,
github_email,
github_name,
github_user_created_at.naive_utc(),
initial_channel_id,
&tx,

View file

@ -7,6 +7,7 @@ impl Database {
pub async fn create_user(
&self,
email_address: &str,
name: Option<&str>,
admin: bool,
params: NewUserParams,
) -> Result<NewUserResult> {
@ -14,6 +15,7 @@ impl Database {
let tx = tx;
let user = user::Entity::insert(user::ActiveModel {
email_address: ActiveValue::set(Some(email_address.into())),
name: ActiveValue::set(name.map(|s| s.into())),
github_login: ActiveValue::set(params.github_login.clone()),
github_user_id: ActiveValue::set(params.github_user_id),
admin: ActiveValue::set(admin),
@ -101,6 +103,7 @@ impl Database {
github_login: &str,
github_user_id: i32,
github_email: Option<&str>,
github_name: Option<&str>,
github_user_created_at: DateTimeUtc,
initial_channel_id: Option<ChannelId>,
) -> Result<User> {
@ -109,6 +112,7 @@ impl Database {
github_login,
github_user_id,
github_email,
github_name,
github_user_created_at.naive_utc(),
initial_channel_id,
&tx,
@ -118,11 +122,13 @@ impl Database {
.await
}
#[allow(clippy::too_many_arguments)]
pub async fn get_or_create_user_by_github_account_tx(
&self,
github_login: &str,
github_user_id: i32,
github_email: Option<&str>,
github_name: Option<&str>,
github_user_created_at: NaiveDateTime,
initial_channel_id: Option<ChannelId>,
tx: &DatabaseTransaction,
@ -150,6 +156,7 @@ impl Database {
} else {
let user = user::Entity::insert(user::ActiveModel {
email_address: ActiveValue::set(github_email.map(|email| email.into())),
name: ActiveValue::set(github_name.map(|name| name.into())),
github_login: ActiveValue::set(github_login.into()),
github_user_id: ActiveValue::set(github_user_id),
github_user_created_at: ActiveValue::set(Some(github_user_created_at)),

View file

@ -13,6 +13,7 @@ pub struct Model {
pub github_user_id: i32,
pub github_user_created_at: Option<NaiveDateTime>,
pub email_address: Option<String>,
pub name: Option<String>,
pub admin: bool,
pub invite_code: Option<String>,
pub invite_count: i32,

View file

@ -177,6 +177,7 @@ static GITHUB_USER_ID: AtomicI32 = AtomicI32::new(5);
async fn new_test_user(db: &Arc<Database>, email: &str) -> UserId {
db.create_user(
email,
None,
false,
NewUserParams {
github_login: email[0..email.find('@').unwrap()].to_string(),

View file

@ -13,6 +13,7 @@ async fn test_channel_buffers(db: &Arc<Database>) {
let a_id = db
.create_user(
"user_a@example.com",
None,
false,
NewUserParams {
github_login: "user_a".into(),
@ -25,6 +26,7 @@ async fn test_channel_buffers(db: &Arc<Database>) {
let b_id = db
.create_user(
"user_b@example.com",
None,
false,
NewUserParams {
github_login: "user_b".into(),
@ -39,6 +41,7 @@ async fn test_channel_buffers(db: &Arc<Database>) {
let c_id = db
.create_user(
"user_c@example.com",
None,
false,
NewUserParams {
github_login: "user_c".into(),
@ -176,6 +179,7 @@ async fn test_channel_buffers_last_operations(db: &Database) {
let user_id = db
.create_user(
"user_a@example.com",
None,
false,
NewUserParams {
github_login: "user_a".into(),
@ -188,6 +192,7 @@ async fn test_channel_buffers_last_operations(db: &Database) {
let observer_id = db
.create_user(
"user_b@example.com",
None,
false,
NewUserParams {
github_login: "user_b".into(),

View file

@ -269,6 +269,7 @@ async fn test_channel_renames(db: &Arc<Database>) {
let user_1 = db
.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".into(),
@ -282,6 +283,7 @@ async fn test_channel_renames(db: &Arc<Database>) {
let user_2 = db
.create_user(
"user2@example.com",
None,
false,
NewUserParams {
github_login: "user2".into(),
@ -318,6 +320,7 @@ async fn test_db_channel_moving(db: &Arc<Database>) {
let a_id = db
.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".into(),
@ -372,6 +375,7 @@ async fn test_db_channel_moving_bugs(db: &Arc<Database>) {
let user_id = db
.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".into(),

View file

@ -13,6 +13,7 @@ test_both_dbs!(
async fn test_contributors(db: &Arc<Database>) {
db.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".to_string(),
@ -25,7 +26,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", 1, None, user1_created_at, None)
db.add_contributor("user1", 1, None, None, user1_created_at, None)
.await
.unwrap();
assert_eq!(
@ -34,7 +35,7 @@ async fn test_contributors(db: &Arc<Database>) {
);
let user2_created_at = Utc::now();
db.add_contributor("user2", 2, None, user2_created_at, None)
db.add_contributor("user2", 2, None, None, user2_created_at, None)
.await
.unwrap();
assert_eq!(

View file

@ -17,6 +17,7 @@ async fn test_get_users(db: &Arc<Database>) {
let user = db
.create_user(
&format!("user{i}@example.com"),
None,
false,
NewUserParams {
github_login: format!("user{i}"),
@ -79,6 +80,7 @@ test_both_dbs!(
async fn test_get_or_create_user_by_github_account(db: &Arc<Database>) {
db.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "login1".into(),
@ -90,6 +92,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc<Database>) {
let user_id2 = db
.create_user(
"user2@example.com",
None,
false,
NewUserParams {
github_login: "login2".into(),
@ -101,7 +104,7 @@ 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("the-new-login2", 102, None, Utc::now(), None)
.get_or_create_user_by_github_account("the-new-login2", 102, None, None, Utc::now(), None)
.await
.unwrap();
assert_eq!(user.id, user_id2);
@ -113,6 +116,7 @@ async fn test_get_or_create_user_by_github_account(db: &Arc<Database>) {
"login3",
103,
Some("user3@example.com"),
None,
Utc::now(),
None,
)
@ -133,6 +137,7 @@ async fn test_create_access_tokens(db: &Arc<Database>) {
let user_1 = db
.create_user(
"u1@example.com",
None,
false,
NewUserParams {
github_login: "u1".into(),
@ -145,6 +150,7 @@ async fn test_create_access_tokens(db: &Arc<Database>) {
let user_2 = db
.create_user(
"u2@example.com",
None,
false,
NewUserParams {
github_login: "u2".into(),
@ -296,6 +302,7 @@ async fn test_add_contacts(db: &Arc<Database>) {
user_ids.push(
db.create_user(
&format!("user{i}@example.com"),
None,
false,
NewUserParams {
github_login: format!("user{i}"),
@ -457,6 +464,7 @@ async fn test_metrics_id(db: &Arc<Database>) {
} = db
.create_user(
"person1@example.com",
None,
false,
NewUserParams {
github_login: "person1".into(),
@ -472,6 +480,7 @@ async fn test_metrics_id(db: &Arc<Database>) {
} = db
.create_user(
"person2@example.com",
None,
false,
NewUserParams {
github_login: "person2".into(),
@ -500,6 +509,7 @@ async fn test_project_count(db: &Arc<Database>) {
let user1 = db
.create_user(
"admin@example.com",
None,
true,
NewUserParams {
github_login: "admin".into(),
@ -511,6 +521,7 @@ async fn test_project_count(db: &Arc<Database>) {
let user2 = db
.create_user(
"user@example.com",
None,
false,
NewUserParams {
github_login: "user".into(),
@ -588,6 +599,7 @@ async fn test_fuzzy_search_users(cx: &mut gpui::TestAppContext) {
{
db.create_user(
&format!("{github_login}@example.com"),
None,
false,
NewUserParams {
github_login: github_login.into(),

View file

@ -15,6 +15,7 @@ async fn test_get_user_flags(db: &Arc<Database>) {
let user_1 = db
.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".to_string(),
@ -28,6 +29,7 @@ async fn test_get_user_flags(db: &Arc<Database>) {
let user_2 = db
.create_user(
"user2@example.com",
None,
false,
NewUserParams {
github_login: "user2".to_string(),

View file

@ -16,6 +16,7 @@ async fn test_accepted_tos(db: &Arc<Database>) {
let user_id = db
.create_user(
"user1@example.com",
None,
false,
NewUserParams {
github_login: "user1".to_string(),

View file

@ -189,6 +189,7 @@ mod tests {
let user_1 = db
.create_user(
"user-1@zed.dev",
None,
false,
NewUserParams {
github_login: "user-1".into(),
@ -201,6 +202,7 @@ mod tests {
let user_2 = db
.create_user(
"user-2@zed.dev",
None,
false,
NewUserParams {
github_login: "user-2".into(),

View file

@ -2418,6 +2418,8 @@ async fn get_users(
id: user.id.to_proto(),
avatar_url: format!("https://github.com/{}.png?size=128", user.github_login),
github_login: user.github_login,
email: user.email_address,
name: user.name,
})
.collect();
response.send(proto::UsersResponse { users })?;
@ -2449,6 +2451,8 @@ async fn fuzzy_search_users(
id: user.id.to_proto(),
avatar_url: format!("https://github.com/{}.png?size=128", user.github_login),
github_login: user.github_login,
name: user.name,
email: user.email_address,
})
.collect();
response.send(proto::UsersResponse { users })?;

View file

@ -16,6 +16,7 @@ struct GithubUser {
id: i32,
login: String,
email: Option<String>,
name: Option<String>,
created_at: DateTime<Utc>,
}
@ -75,6 +76,7 @@ pub async fn seed(config: &Config, db: &Database, force: bool) -> anyhow::Result
let user = db
.create_user(
&user.email.unwrap_or(format!("{admin_login}@example.com")),
user.name.as_deref(),
true,
NewUserParams {
github_login: user.login,
@ -129,6 +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(),
github_user.name.as_deref(),
github_user.created_at,
None,
)
@ -152,14 +155,20 @@ fn load_admins(path: impl AsRef<Path>) -> anyhow::Result<SeedConfig> {
}
async fn fetch_github<T: DeserializeOwned>(client: &reqwest::Client, url: &str) -> T {
let response = client
.get(url)
let mut request_builder = client.get(url);
if let Ok(github_token) = std::env::var("GITHUB_TOKEN") {
request_builder =
request_builder.header("Authorization", format!("Bearer {}", github_token));
}
let response = request_builder
.header("user-agent", "zed")
.send()
.await
.unwrap_or_else(|error| panic!("failed to fetch '{url}': {error}"));
response
.json()
.await
.unwrap_or_else(|error| panic!("failed to deserialize github user from '{url}': {error}"))
let response_text = response.text().await.unwrap_or_else(|error| {
panic!("failed to fetch '{url}': {error}");
});
serde_json::from_str(&response_text).unwrap_or_else(|error| {
panic!("failed to deserialize github user from '{url}'. Error: '{error}', text: '{response_text}'");
})
}

View file

@ -174,7 +174,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, Utc::now(), None)
.get_or_create_user_by_github_account("user_b", 100, None, None, Utc::now(), None)
.await
.unwrap();
@ -278,7 +278,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, Utc::now(), None)
.add_contributor("user_b", 100, None, None, Utc::now(), None)
.await
.unwrap();

View file

@ -1840,6 +1840,8 @@ async fn test_active_call_events(
id: client_a.user_id().unwrap(),
github_login: "user_a".to_string(),
avatar_uri: "avatar_a".into(),
name: None,
email: None,
}),
project_id: project_a_id,
worktree_root_names: vec!["a".to_string()],
@ -1858,6 +1860,8 @@ async fn test_active_call_events(
id: client_b.user_id().unwrap(),
github_login: "user_b".to_string(),
avatar_uri: "avatar_b".into(),
name: None,
email: None,
}),
project_id: project_b_id,
worktree_root_names: vec!["b".to_string()]

View file

@ -220,6 +220,7 @@ impl<T: RandomizedTest> TestPlan<T> {
.db
.create_user(
&format!("{username}@example.com"),
None,
false,
NewUserParams {
github_login: username.clone(),

View file

@ -186,6 +186,7 @@ impl TestServer {
.db
.create_user(
&format!("{name}@example.com"),
None,
false,
NewUserParams {
github_login: name.into(),

View file

@ -86,6 +86,7 @@ impl UserBackfiller {
&user.github_login,
github_user.id,
user.email_address.as_deref(),
user.name.as_deref(),
github_user.created_at,
initial_channel_id,
)
@ -159,4 +160,5 @@ impl UserBackfiller {
struct GithubUser {
id: i32,
created_at: DateTime<Utc>,
name: Option<String>,
}