Improvements to the assign-priority command

This patch adds a few small changes to the issue prioritization triagebot
Zulip command:
- It is now accepted the shortcut "prio"
- The issue number is optional. If omitted it is searched in Zulip topic title
- When we edit the Zulip topic, we ensure we do not set the same topic
  (the Zulip API returns an error)

Examples:
```
triagebot assign-priority high 123456
triagebot prio medium 123456
triagebot prio high
```

This will change the syntax of the command so I will update the documentation.

Author: apiraino

src/zulip.rs

@@ -30,15 +30,18 @@ use axum::extract::rejection::JsonRejection;
 use axum::response::IntoResponse;
 use chrono::{DateTime, Duration, Utc};
 use itertools::Itertools;
+use regex::Regex;
 use rust_team_data::v1::{TeamKind, TeamMember};
 use secrecy::{ExposeSecret, SecretString};
 use std::cmp::{Ordering, Reverse};
 use std::collections::{HashMap, HashSet};
 use std::fmt::Write as _;
-use std::sync::Arc;
+use std::sync::{Arc, LazyLock};
 use subtle::ConstantTimeEq;
 use tracing::log;
 
+static RE_ISSUE_NUM: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"#[0-9]*").unwrap());
+
 fn get_text_backport_approved(
     channel: &BackportChannelArgs,
     verb: &BackportVerbArgs,
@@ -71,6 +74,7 @@ pub struct Request {
     token: SecretString,
 }
 
+// Zulip webhook payload: https://rust-lang.zulipchat.com/api/outgoing-webhook-payload
 #[derive(Clone, Debug, serde::Deserialize)]
 struct Message {
     id: u64,
@@ -396,7 +400,7 @@ async fn handle_command<'a>(
                             }
                         }
                         Err(err) => {
-                            log::error!("Could not assign priority to #{}: {:?}", issue_num, err);
+                            log::error!("Could not assign priority to #{:?}: {:?}", issue_num, err);
                             ctx.zulip.add_reaction(message_data.id, "scream").await?;
                         }
                     };
@@ -445,7 +449,7 @@ async fn accept_decline_backport(
     let mut pr_num = pr_num;
     let mut channel = channel.clone();
 
-    // Parse the stream subjetc if the channel or the pr num are not provided
+    // Parse the stream subject if the channel or the pr num are not provided
     if pr_num.is_none() || channel.is_none() {
         let (maybe_pr_num, maybe_channel) = subject
             .rsplit_once(':')
@@ -559,9 +563,9 @@ async fn accept_decline_backport(
 async fn assign_issue_prio(
     ctx: &Context,
     message_data: &Message,
-    issue_num: PullRequestNumber,
+    issue_num: Option<PullRequestNumber>,
     prio: IssuePrio,
-) -> anyhow::Result<Option<String>> {
+) -> anyhow::Result<()> {
     let message = message_data.clone();
     let stream_id = message.stream_id.unwrap();
     let subject = message.subject.unwrap();
@@ -579,6 +583,23 @@ async fn assign_issue_prio(
         parent: None,
     };
 
+    // Parse the Zulip topic subject if an issue num was not provided
+    // Hopefully it is something like "#123456 Some text" or "✔ #123456 Some text"
+    let mut issue_num: Option<u64> = issue_num;
+    if issue_num.is_none() {
+        issue_num = Some(
+            RE_ISSUE_NUM
+                .find(&subject)
+                .expect("Cannot parse issue num")
+                .as_str()
+                .strip_prefix('#')
+                .expect("Cannot parse issue num")
+                .parse::<u64>()
+                .expect("Cannot parse issue num"),
+        );
+    }
+    let issue_num = issue_num.context("No issue number to apply to")?;
+
     // Ensure this is an issue and not a pull request
     let issue = repository
         .get_issue(&ctx.github, issue_num)
@@ -615,7 +636,7 @@ async fn assign_issue_prio(
 
     // if just removing priority, nothing else to do
     if prio == IssuePrio::None {
-        return Ok(None);
+        return Ok(());
     }
 
     // post a comment on GitHub
@@ -640,7 +661,7 @@ async fn assign_issue_prio(
         .await
         .context(format!("failed to add labels to issue #{}", issue_num))?;
 
-    Ok(None)
+    Ok(())
 }
 
 /// Unlock a specific issue in our managed repos.

src/zulip/client.rs

@@ -221,6 +221,11 @@ impl ZulipClient {
         }
 
         let topic = format_resolved_topic(message_topic);
+        // Zulip returns a BAD_REQUEST error if attempting to set the same title for a topic
+        // so in this case just return
+        if topic == message_topic {
+            return Ok(());
+        }
 
         let resp = self
             .make_request(Method::PATCH, &format!("messages/{message_id}"))

src/zulip/commands.rs

@@ -173,11 +173,12 @@ pub enum StreamCommand {
     #[clap(subcommand)]
     Lookup(LookupCmd),
     /// Label assignment: add one of `P-{low,medium,high,critical}` and remove `I-prioritize`
+    #[clap(alias = "prio")]
     AssignPriority {
-        /// Issue target of the prioritization
-        issue_num: PullRequestNumber,
         /// Issue priority. Allowed: "low", "medium", "high", "critical", "none" (to just remove the prioritization)
         prio: IssuePrio,
+        /// Issue target of the prioritization
+        issue_num: Option<PullRequestNumber>,
     },
     /// Unlock a specific GitHub issue or pull-request.
     Unlock {
@@ -198,7 +199,7 @@ pub enum StreamCommand {
         /// Version of the crate to yank
         version: semver::Version,
     },
-    /// Unynk a specific crate version from crates.io.
+    /// Unyank a specific crate version from crates.io.
     /// Can only be performed by members of teams that own the crate.
     Unyank {
         /// Crate to unyank.
@@ -519,4 +520,36 @@ mod tests {
     fn parse_stream(input: &[&str]) -> StreamCommand {
         parse_cli::<StreamCommand, _>(input.into_iter().copied()).unwrap()
     }
+
+    #[test]
+    fn parse_assign_prio_command() {
+        assert_eq!(
+            parse_stream(&["assign-priority", "medium", "123456"]),
+            StreamCommand::AssignPriority {
+                prio: IssuePrio::Medium,
+                issue_num: Some(123456)
+            }
+        );
+        assert_eq!(
+            parse_stream(&["assign-priority", "none"]),
+            StreamCommand::AssignPriority {
+                prio: IssuePrio::None,
+                issue_num: None
+            }
+        );
+        assert_eq!(
+            parse_stream(&["prio", "medium", "123456"]),
+            StreamCommand::AssignPriority {
+                prio: IssuePrio::Medium,
+                issue_num: Some(123456)
+            }
+        );
+        assert_eq!(
+            parse_stream(&["prio", "medium"]),
+            StreamCommand::AssignPriority {
+                prio: IssuePrio::Medium,
+                issue_num: None
+            }
+        );
+    }
 }