From e747558e218b9644fd3f0f7c1517bfc63103e5f6 Mon Sep 17 00:00:00 2001 From: apiraino Date: Sat, 26 Sep 2026 18:16:44 +0200 Subject: [PATCH] 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. --- src/zulip.rs | 35 ++++++++++++++++++++++++++++------- src/zulip/client.rs | 5 +++++ src/zulip/commands.rs | 39 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/zulip.rs b/src/zulip.rs index c22880636..4c31c8965 100644 --- a/src/zulip.rs +++ b/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 = 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, prio: IssuePrio, -) -> anyhow::Result> { +) -> 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 = 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::() + .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. diff --git a/src/zulip/client.rs b/src/zulip/client.rs index 058ecfc29..ca6f9d400 100644 --- a/src/zulip/client.rs +++ b/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}")) diff --git a/src/zulip/commands.rs b/src/zulip/commands.rs index 896d52f50..13d339da1 100644 --- a/src/zulip/commands.rs +++ b/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, }, /// 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::(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 + } + ); + } }