chore(handlers/assign): replace const + replace patterns with typed functions

shard77 · shard77 · commit dd2a7d83cc45 · 2025-06-07T12:37:06.000+02:00
diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs
@@ -43,61 +43,14 @@ use tokio::sync::RwLock;
 use tokio_postgres::Client as DbClient;
 use tracing as log;
 
+mod messages;
+
 #[cfg(test)]
 mod tests {
     mod tests_candidates;
     mod tests_from_diff;
 }
 
-const NEW_USER_WELCOME_MESSAGE: &str = "Thanks for the pull request, and welcome! \
-The Rust team is excited to review your changes, and you should hear from {who} \
-some time within the next two weeks.";
-
-const CONTRIBUTION_MESSAGE: &str = "Please see [the contribution \
-instructions]({contributing_url}) for more information. Namely, in order to ensure the \
-minimum review times lag, PR authors and assigned reviewers should ensure that the review \
-label (`S-waiting-on-review` and `S-waiting-on-author`) stays updated, invoking these commands \
-when appropriate:
-
-- `@{bot} author`: the review is finished, PR author should check the comments and take action accordingly
-- `@{bot} review`: the author is ready for a review, this PR will be queued again in the reviewer's queue";
-
-const WELCOME_WITH_REVIEWER: &str = "@{assignee} (or someone else)";
-
-const WELCOME_WITHOUT_REVIEWER: &str = "@Mark-Simulacrum (NB. this repo may be misconfigured)";
-
-const RETURNING_USER_WELCOME_MESSAGE: &str = "r? @{assignee}
-
-{bot} has assigned @{assignee}.
-They will have a look at your PR within the next two weeks and either review your PR or \
-reassign to another reviewer.
-
-Use `r?` to explicitly pick a reviewer";
-
-const RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER: &str =
-    "@{author}: no appropriate reviewer found, use `r?` to override";
-
-fn reviewer_off_rotation_message(username: &str) -> String {
-    format!(
-        r"`{username}` is not available for reviewing at the moment.
-
-Please choose another assignee."
-    )
-}
-
-const REVIEWER_IS_PR_AUTHOR: &str = "Pull request author cannot be assigned as reviewer.
-
-Please choose another assignee.";
-
-const REVIEWER_ALREADY_ASSIGNED: &str =
-    "Requested reviewer is already assigned to this pull request.
-
-Please choose another assignee.";
-
-const REVIEWER_ASSIGNED_BEFORE: &str = "Requested reviewer @{username} was already assigned before.
-
-Please choose another assignee by using `r? @reviewer`.";
-
 // Special account that we use to prevent assignment.
 const GHOST_ACCOUNT: &str = "ghost";
 
@@ -211,11 +164,7 @@ pub(super) async fn handle_input(
                         AuthorAssociation::FirstTimer | AuthorAssociation::FirstTimeContributor
                     ) {
                         welcome.push_str("\n\n");
-                        welcome.push_str(
-                            &CONTRIBUTION_MESSAGE
-                                .replace("{contributing_url}", contrib)
-                                .replace("{bot}", &ctx.username),
-                        );
+                        welcome.push_str(&messages::contribution_message(contrib, &ctx.username));
                     }
                 }
                 Some(welcome)
@@ -227,35 +176,29 @@ pub(super) async fn handle_input(
             event.issue.author_association,
             AuthorAssociation::FirstTimer | AuthorAssociation::FirstTimeContributor
         ) {
-            let who_text = match &assignee {
-                Some(assignee) => WELCOME_WITH_REVIEWER.replace("{assignee}", &assignee.name),
-                None => WELCOME_WITHOUT_REVIEWER.to_string(),
+            let assignee_text = match &assignee {
+                Some(assignee) => messages::welcome_with_reviewer(&assignee.name),
+                None => messages::WELCOME_WITHOUT_REVIEWER.to_string(),
             };
-            let mut welcome = NEW_USER_WELCOME_MESSAGE.replace("{who}", &who_text);
+            let mut welcome = messages::new_user_welcome_message(&assignee_text);
             if let Some(contrib) = &config.contributing_url {
                 welcome.push_str("\n\n");
-                welcome.push_str(
-                    &CONTRIBUTION_MESSAGE
-                        .replace("{contributing_url}", contrib)
-                        .replace("{bot}", &ctx.username),
-                );
+                welcome.push_str(&messages::contribution_message(contrib, &ctx.username));
             }
             Some(welcome)
         } else if !from_comment {
             match &assignee {
-                Some(assignee) => Some(
-                    RETURNING_USER_WELCOME_MESSAGE
-                        .replace("{assignee}", &assignee.name)
-                        .replace("{bot}", &ctx.username),
-                ),
+                Some(assignee) => Some(messages::returning_user_welcome_message(
+                    &assignee.name,
+                    &ctx.username,
+                )),
                 None => {
                     // If the assign fallback group is empty, then we don't expect any automatic
                     // assignment, and this message would just be spam.
                     if config.fallback_review_group().is_some() {
-                        Some(
-                            RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER
-                                .replace("{author}", &event.issue.user.login),
-                        )
+                        Some(messages::returning_user_welcome_message_no_reviewer(
+                            &event.issue.user.login,
+                        ))
                     } else {
                         None
                     }
@@ -807,28 +750,16 @@ impl fmt::Display for FindReviewerError {
                 )
             }
             FindReviewerError::ReviewerOffRotation { username } => {
-                write!(f, "{}", reviewer_off_rotation_message(username))
+                write!(f, "{}", messages::reviewer_off_rotation_message(username))
             }
-            FindReviewerError::ReviewerIsPrAuthor { username } => {
-                write!(
-                    f,
-                    "{}",
-                    REVIEWER_IS_PR_AUTHOR.replace("{username}", username)
-                )
+            FindReviewerError::ReviewerIsPrAuthor { .. } => {
+                write!(f, "{}", messages::REVIEWER_IS_PR_AUTHOR)
             }
-            FindReviewerError::ReviewerAlreadyAssigned { username } => {
-                write!(
-                    f,
-                    "{}",
-                    REVIEWER_ALREADY_ASSIGNED.replace("{username}", username)
-                )
+            FindReviewerError::ReviewerAlreadyAssigned { .. } => {
+                write!(f, "{}", messages::REVIEWER_ALREADY_ASSIGNED)
             }
             FindReviewerError::ReviewerPreviouslyAssigned { username } => {
-                write!(
-                    f,
-                    "{}",
-                    REVIEWER_ASSIGNED_BEFORE.replace("{username}", username)
-                )
+                write!(f, "{}", messages::reviewer_assigned_before(username))
             }
             FindReviewerError::DatabaseError(error) => {
                 write!(f, "Database error: {error}")
diff --git a/src/handlers/assign/messages.rs b/src/handlers/assign/messages.rs
@@ -0,0 +1,73 @@
+//! Assignment messages functions and constants.
+//!
+//! This module contains the different constants and functions related
+//! to assignment messages.
+
+pub fn new_user_welcome_message(reviewer: &str) -> String {
+    format!(
+        "Thanks for the pull request, and welcome! \
+The Rust team is excited to review your changes, and you should hear from {reviewer} \
+some time within the next two weeks."
+    )
+}
+
+pub fn contribution_message(contributing_url: &str, bot: &str) -> String {
+    format!(
+        "Please see [the contribution \
+instructions]({contributing_url}) for more information. Namely, in order to ensure the \
+minimum review times lag, PR authors and assigned reviewers should ensure that the review \
+label (`S-waiting-on-review` and `S-waiting-on-author`) stays updated, invoking these commands \
+when appropriate:
+
+- `@{bot} author`: the review is finished, PR author should check the comments and take action accordingly
+- `@{bot} review`: the author is ready for a review, this PR will be queued again in the reviewer's queue"
+    )
+}
+
+pub fn welcome_with_reviewer(assignee: &str) -> String {
+    format!("@{assignee} (or someone else)")
+}
+
+pub fn returning_user_welcome_message(assignee: &str, bot: &str) -> String {
+    format!(
+        "r? @{assignee}
+
+{bot} has assigned @{assignee}.
+They will have a look at your PR within the next two weeks and either review your PR or \
+reassign to another reviewer.
+
+Use `r?` to explicitly pick a reviewer"
+    )
+}
+
+pub fn returning_user_welcome_message_no_reviewer(pr_author: &str) -> String {
+    format!("@{pr_author}: no appropriate reviewer found, use `r?` to override")
+}
+
+pub fn reviewer_off_rotation_message(username: &str) -> String {
+    format!(
+        r"`{username}` is not available for reviewing at the moment.
+
+Please choose another assignee."
+    )
+}
+
+pub fn reviewer_assigned_before(username: &str) -> String {
+    format!(
+        "Requested reviewer @{username} was already assigned before.
+
+Please choose another assignee by using `r? @reviewer`."
+    )
+}
+
+pub const WELCOME_WITHOUT_REVIEWER: &str = "@Mark-Simulacrum (NB. this repo may be misconfigured)";
+
+pub const REVIEWER_IS_PR_AUTHOR: &str = "Pull request author cannot be assigned as reviewer.
+
+
+Please choose another assignee.";
+
+pub const REVIEWER_ALREADY_ASSIGNED: &str =
+    "Requested reviewer is already assigned to this pull request.
+
+Please choose another assignee.";
diff --git a/src/handlers/project_goals.rs b/src/handlers/project_goals.rs
@@ -17,19 +17,29 @@ const RUST_PROJECT_GOALS_REPO: &'static str = "rust-lang/rust-project-goals";
 const GOALS_STREAM: u64 = 435869; // #project-goals
 const C_TRACKING_ISSUE: &str = "C-tracking-issue";
 
-const MESSAGE: &str = r#"
-Dear $OWNERS, it's been $DAYS days since the last update to your goal *$GOAL*.
+fn message(
+    zulip_owners: &str,
+    days: &str,
+    issue_number: u64,
+    issue_title: &str,
+    next_update: &str,
+) -> String {
+    format!(
+        r#"
+Dear {zulip_owners}, it's been {days} days since the last update to your goal *{issue_title}*.
 
-We will begin drafting the next blog post collecting goal updates $NEXT_UPDATE.
+We will begin drafting the next blog post collecting goal updates {next_update}.
 
-Please comment on the github tracking issue goals#$GOALNUM before then. Thanks! <3
+Please comment on the github tracking issue goals#{issue_number} before then. Thanks! <3
 
 Here is a suggested template for updates (feel free to drop the items that don't apply):
 
 * **Key developments:** *What has happened since the last time. It's perfectly ok to list "nothing" if that's the truth, we know people get busy.*
 * **Blockers:** *List any Rust teams you are waiting on and what you are waiting for.*
 * **Help wanted:** *Are there places where you are looking for contribution or feedback from the broader community?*
-"#;
+"#
+    )
+}
 
 pub struct ProjectGoalsUpdateJob;
 
@@ -146,19 +156,17 @@ pub async fn ping_project_goals_owners(
             continue;
         };
 
-        let message = MESSAGE
-            .replace("$OWNERS", &zulip_owners)
-            .replace(
-                "$DAYS",
-                &if comments <= 1 {
-                    "∞".to_string()
-                } else {
-                    days_since_last_comment.to_string()
-                },
-            )
-            .replace("$GOALNUM", &issue.number.to_string())
-            .replace("$GOAL", &issue.title)
-            .replace("$NEXT_UPDATE", next_update);
+        let message = message(
+            &zulip_owners,
+            &if comments <= 1 {
+                "∞".to_string()
+            } else {
+                days_since_last_comment.to_string()
+            },
+            issue.number,
+            &issue.title,
+            next_update,
+        );
 
         let zulip_req = crate::zulip::MessageApiRequest {
             recipient: Recipient::Stream {
