Merge pull request #2112 from Urgau/assign-custom-message-without-auto-assign

Be less strict for `[assign]` custom welcome message without auto-assign

Author: Urgau

src/config.rs

@@ -98,7 +98,7 @@ pub(crate) struct AssignReviewPrefsConfig {}
 #[serde(deny_unknown_fields)]
 pub(crate) struct AssignCustomWelcomeMessages {
     /// Welcome message with reviewer automaticaly chosen (`{assignee}`)
-    pub(crate) welcome_message: String,
+    pub(crate) welcome_message: Option<String>,
     /// Welcome message without a reviewer automaticaly chosen
     pub(crate) welcome_message_no_reviewer: String,
 }
@@ -777,7 +777,7 @@ mod tests {
                         ],
                     },
                     custom_welcome_messages: Some(AssignCustomWelcomeMessages {
-                        welcome_message: "Welcome message, assigning {assignee}!".to_string(),
+                        welcome_message: Some("Welcome message, assigning {assignee}!".to_string()),
                         welcome_message_no_reviewer:
                             "Welcome message for when no reviewer could be found!".to_string()
                     }),
@@ -829,4 +829,20 @@ mod tests {
             Some(AssignReviewPrefsConfig {})
         ));
     }
+
+    #[test]
+    fn assign_custom_welcome_messaga() {
+        let config = r#"
+            [assign.custom_welcome_messages]
+            welcome-message-no-reviewer = "welcome message!"
+        "#;
+        let config = toml::from_str::<Config>(&config).unwrap();
+        assert_eq!(
+            config.assign.and_then(|c| c.custom_welcome_messages),
+            Some(AssignCustomWelcomeMessages {
+                welcome_message: None,
+                welcome_message_no_reviewer: "welcome message!".to_string(),
+            })
+        );
+    }
 }

src/handlers/assign.rs

@@ -150,15 +150,19 @@ pub(super) async fn handle_input(
                 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(),
+                        .as_ref()
+                        .map(|wm| wm.trim().replace("{assignee}", &assignee.name)),
+                    None => Some(
+                        custom_welcome_messages
+                            .welcome_message_no_reviewer
+                            .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 +171,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

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::<crate::handlers::Config>(&triagebot_content) else {
-        return Ok(None);
-    };
+    match toml::from_str::<crate::handlers::Config>(&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_welcome_messages) = &assign.custom_welcome_messages
+                && custom_welcome_messages.welcome_message.is_none()
+            {
+                return Ok(Some(
+                    "Invalid `triagebot.toml`:\n\
+                    `[assign.owners]` is populated but `[assign.custom_welcome_messages.welcome-message]` 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).