diff --git a/src/config.rs b/src/config.rs index 510a012e6..c44deefd3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -96,11 +96,13 @@ pub(crate) struct AssignReviewPrefsConfig {} #[derive(PartialEq, Eq, Debug, serde::Deserialize)] #[serde(rename_all = "kebab-case")] #[serde(deny_unknown_fields)] -pub(crate) struct AssignCustomWelcomeMessages { - /// Welcome message with reviewer automaticaly chosen (`{assignee}`) - pub(crate) welcome_message: String, - /// Welcome message without a reviewer automaticaly chosen - pub(crate) welcome_message_no_reviewer: String, +pub(crate) struct AssignCustomMessages { + /// Message with reviewer automaticaly chosen (`{assignee}`) + #[serde(alias = "welcome-message")] + pub(crate) auto_assign_someone: Option, + /// Message without a reviewer automaticaly chosen + #[serde(alias = "welcome-message-no-reviewer")] + pub(crate) auto_assign_no_one: String, } #[derive(PartialEq, Eq, Debug, serde::Deserialize)] @@ -127,7 +129,8 @@ pub(crate) struct AssignConfig { pub(crate) review_prefs: Option, /// Custom welcome messages #[serde(default)] - pub(crate) custom_welcome_messages: Option, + #[serde(alias = "custom_welcome_messages")] + pub(crate) custom_messages: Option, } impl AssignConfig { @@ -694,7 +697,7 @@ mod tests { owners: HashMap::new(), users_on_vacation: HashSet::from(["jyn514".into()]), review_prefs: None, - custom_welcome_messages: None, + custom_messages: None, }), note: Some(NoteConfig { _empty: () }), ping: Some(PingConfig { teams: ping_teams }), @@ -739,9 +742,9 @@ mod tests { [assign] warn_non_default_branch.enable = true - [assign.custom_welcome_messages] - welcome-message = "Welcome message, assigning {assignee}!" - welcome-message-no-reviewer = "Welcome message for when no reviewer could be found!" + [assign.custom_messages] + auto-assign-someone = "Welcome message, assigning {assignee}!" + auto-assign-no-one = "Welcome message for when no reviewer could be found!" [[assign.warn_non_default_branch.exceptions]] title = "[beta" @@ -776,10 +779,12 @@ mod tests { }, ], }, - custom_welcome_messages: Some(AssignCustomWelcomeMessages { - welcome_message: "Welcome message, assigning {assignee}!".to_string(), - welcome_message_no_reviewer: - "Welcome message for when no reviewer could be found!".to_string() + custom_messages: Some(AssignCustomMessages { + auto_assign_someone: Some( + "Welcome message, assigning {assignee}!".to_string() + ), + auto_assign_no_one: "Welcome message for when no reviewer could be found!" + .to_string() }), contributing_url: None, adhoc_groups: HashMap::new(), @@ -829,4 +834,20 @@ mod tests { Some(AssignReviewPrefsConfig {}) )); } + + #[test] + fn assign_custom_welcome_message_old() { + let config = r#" + [assign.custom_welcome_messages] + welcome-message-no-reviewer = "welcome message!" + "#; + let config = toml::from_str::(&config).unwrap(); + assert_eq!( + config.assign.and_then(|c| c.custom_messages), + Some(AssignCustomMessages { + auto_assign_someone: None, + auto_assign_no_one: "welcome message!".to_string(), + }) + ); + } } diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index 72909ab83..42e9f3e9a 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -145,20 +145,19 @@ pub(super) async fn handle_input( // want any assignments or noise. return Ok(()); } - let welcome = if let Some(custom_welcome_messages) = &config.custom_welcome_messages { + let welcome = if let Some(custom_messages) = &config.custom_messages { if !from_comment { let mut welcome = match &assignee { - Some(assignee) => custom_welcome_messages - .welcome_message - .trim() - .replace("{assignee}", &assignee.name), - None => custom_welcome_messages - .welcome_message_no_reviewer - .trim() - .to_string(), + Some(assignee) => custom_messages + .auto_assign_someone + .as_ref() + .map(|wm| wm.trim().replace("{assignee}", &assignee.name)), + None => Some(custom_messages.auto_assign_no_one.trim().to_string()), }; - if let Some(contrib) = &config.contributing_url { + if let Some(ref mut welcome) = welcome + && let Some(contrib) = &config.contributing_url + { if matches!( event.issue.author_association, AuthorAssociation::FirstTimer | AuthorAssociation::FirstTimeContributor @@ -167,7 +166,7 @@ pub(super) async fn handle_input( welcome.push_str(&messages::contribution_message(contrib, &ctx.username)); } } - Some(welcome) + welcome } else { // No welcome is posted if they used `r?` in the opening body. None diff --git a/src/handlers/check_commits/validate_config.rs b/src/handlers/check_commits/validate_config.rs index 46964b60f..561297a43 100644 --- a/src/handlers/check_commits/validate_config.rs +++ b/src/handlers/check_commits/validate_config.rs @@ -33,29 +33,44 @@ pub(super) async fn validate_config( let triagebot_content = triagebot_content.unwrap_or_default(); let triagebot_content = String::from_utf8_lossy(&*triagebot_content); - let Err(e) = toml::from_str::(&triagebot_content) else { - return Ok(None); - }; + match toml::from_str::(&triagebot_content) { + Err(e) => { + let position = match e.span() { + // toml sometimes gives bad spans, see https://github.com/toml-rs/toml/issues/589 + Some(span) if span != (0..0) => { + let (line, col) = translate_position(&triagebot_content, span.start); + let url = format!( + "https://github.com/{}/blob/{}/{CONFIG_FILE_NAME}#L{line}", + repo.full_name, pr_source.sha + ); + format!(" at position [{line}:{col}]({url})",) + } + Some(_) | None => String::new(), + }; - let position = match e.span() { - // toml sometimes gives bad spans, see https://github.com/toml-rs/toml/issues/589 - Some(span) if span != (0..0) => { - let (line, col) = translate_position(&triagebot_content, span.start); - let url = format!( - "https://github.com/{}/blob/{}/{CONFIG_FILE_NAME}#L{line}", - repo.full_name, pr_source.sha - ); - format!(" at position [{line}:{col}]({url})",) + Ok(Some(format!( + "Invalid `triagebot.toml`{position}:\n\ + `````\n\ + {e}\n\ + `````", + ))) } - Some(_) | None => String::new(), - }; + Ok(config) => { + // Error if `[assign.owners]` is not empty (ie auto-assign) and the custom welcome message for assignee isn't set. + if let Some(assign) = config.assign + && !assign.owners.is_empty() + && let Some(custom_messages) = &assign.custom_messages + && custom_messages.auto_assign_someone.is_none() + { + return Ok(Some( + "Invalid `triagebot.toml`:\n\ + `[assign.owners]` is populated but `[assign.custom_messages.auto-assign-someone]` is not set!".to_string() + )); + } - Ok(Some(format!( - "Invalid `triagebot.toml`{position}:\n\ - `````\n\ - {e}\n\ - `````", - ))) + Ok(None) + } + } } /// Helper to translate a toml span to a `(line_no, col_no)` (1-based).