Split the `prioritize` feature in smaller parts

src/config.rs

@@ -23,6 +23,8 @@ pub(crate) struct Config {
     pub(crate) prioritize: Option<PrioritizeConfig>,
     pub(crate) major_change: Option<MajorChangeConfig>,
     pub(crate) glacier: Option<GlacierConfig>,
+    pub(crate) autolabel: Option<AutolabelConfig>,
+    pub(crate) notify_zulip: Option<NotifyZulipConfig>,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -79,11 +81,45 @@ pub(crate) struct RelabelConfig {
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 pub(crate) struct PrioritizeConfig {
     pub(crate) label: String,
-    #[serde(default)]
-    pub(crate) prioritize_on: Vec<String>,
+}
+
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct AutolabelConfig {
+    #[serde(flatten)]
+    pub(crate) labels: HashMap<String, AutolabelLabelConfig>,
+}
+
+impl AutolabelConfig {
+    pub(crate) fn get_by_trigger(&self, trigger: &str) -> Vec<(&str, &AutolabelLabelConfig)> {
+        let mut results = Vec::new();
+        for (label, cfg) in self.labels.iter() {
+            if cfg.trigger_labels.iter().any(|l| l == trigger) {
+                results.push((label.as_str(), cfg));
+            }
+        }
+        results
+    }
+}
+
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct AutolabelLabelConfig {
+    pub(crate) trigger_labels: Vec<String>,
     #[serde(default)]
     pub(crate) exclude_labels: Vec<String>,
+}
+
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct NotifyZulipConfig {
+    #[serde(flatten)]
+    pub(crate) labels: HashMap<String, NotifyZulipLabelConfig>,
+}
+
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct NotifyZulipLabelConfig {
     pub(crate) zulip_stream: u64,
+    pub(crate) topic: String,
+    pub(crate) message_on_add: Option<String>,
+    pub(crate) message_on_remove: Option<String>,
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
@@ -231,6 +267,8 @@ mod tests {
                 prioritize: None,
                 major_change: None,
                 glacier: None,
+                autolabel: None,
+                notify_zulip: None,
             }
         );
     }

src/handlers.rs

@@ -82,6 +82,8 @@ handlers! {
     major_change = major_change::MajorChangeHandler,
     //tracking_issue = tracking_issue::TrackingIssueHandler,
     glacier = glacier::GlacierHandler,
+    autolabel = autolabel::AutolabelHandler,
+    notify_zulip = notify_zulip::NotifyZulipHandler,
 }
 
 pub struct Context {

src/handlers/autolabel.rs

@@ -0,0 +1,94 @@
+use crate::{
+    config::AutolabelConfig,
+    github::{self, Event, Label},
+    handlers::{Context, Handler},
+};
+use futures::future::{BoxFuture, FutureExt};
+pub(super) struct AutolabelInput {
+    labels: Vec<Label>
+}
+
+pub(super) struct AutolabelHandler;
+
+impl Handler for AutolabelHandler {
+    type Input = AutolabelInput;
+    type Config = AutolabelConfig;
+
+    fn parse_input(
+        &self,
+        _ctx: &Context,
+        event: &Event,
+        config: Option<&Self::Config>,
+    ) -> Result<Option<Self::Input>, String> {
+        if let Event::Issue(e) = event {
+            if e.action == github::IssuesAction::Labeled {
+                if let Some(config) = config {
+                    let mut autolabels = Vec::new();
+                    let applied_label = &e.label.as_ref().expect("label").name;
+
+                    'outer: for (label, config) in config.get_by_trigger(applied_label) {
+                        let exclude_patterns: Vec<glob::Pattern> = config
+                            .exclude_labels
+                            .iter()
+                            .filter_map(|label| {
+                                match glob::Pattern::new(label) {
+                                    Ok(exclude_glob) => {
+                                        Some(exclude_glob)
+                                    }
+                                    Err(error) => {
+                                        log::error!("Invalid glob pattern: {}", error);
+                                        None
+                                    }
+                                }
+                            })
+                            .collect();
+
+                        for label in event.issue().unwrap().labels() {
+                            for pat in &exclude_patterns {
+                                if pat.matches(&label.name) {
+                                    // If we hit an excluded label, ignore this autolabel and check the next
+                                    continue 'outer;
+                                }
+                            }
+                        }
+
+                        // If we reach here, no excluded labels were found, so we should apply the autolabel.
+                        autolabels.push(Label { name: label.to_owned() });
+                    }
+                    if !autolabels.is_empty() {
+                        return Ok(Some(AutolabelInput { labels: autolabels }));
+                    }
+                }
+            }
+        }
+        Ok(None)
+    }
+
+    fn handle_input<'a>(
+        &self,
+        ctx: &'a Context,
+        config: &'a Self::Config,
+        event: &'a Event,
+        input: Self::Input,
+    ) -> BoxFuture<'a, anyhow::Result<()>> {
+        handle_input(ctx, config, event, input).boxed()
+    }
+}
+
+async fn handle_input(
+    ctx: &Context,
+    _config: &AutolabelConfig,
+    event: &Event,
+    input: AutolabelInput,
+) -> anyhow::Result<()> {
+    let issue = event.issue().unwrap();
+    let mut labels = issue.labels().to_owned();
+    for label in input.labels {
+        // Don't add the label if it's already there
+        if !labels.contains(&label) {
+            labels.push(label);
+        }
+    }
+    issue.set_labels(&ctx.github, labels).await?;
+    Ok(())
+}

src/handlers/notify_zulip.rs

@@ -0,0 +1,97 @@
+use crate::{
+    config::NotifyZulipConfig,
+    github::{self, Event},
+    handlers::{Context, Handler},
+};
+use futures::future::{BoxFuture, FutureExt};
+
+pub(super) struct NotifyZulipInput {
+    notification_type: NotificationType,
+}
+
+pub(super) enum NotificationType {
+    Labeled,
+    Unlabeled,
+}
+
+pub(super) struct NotifyZulipHandler;
+
+impl Handler for NotifyZulipHandler {
+    type Input = NotifyZulipInput;
+    type Config = NotifyZulipConfig;
+
+    fn parse_input(
+        &self,
+        _ctx: &Context,
+        event: &Event,
+        config: Option<&Self::Config>,
+    ) -> Result<Option<Self::Input>, String> {
+        if let Event::Issue(e) = event {
+            if let github::IssuesAction::Labeled | github::IssuesAction::Unlabeled = e.action {
+                let applied_label = &e.label.as_ref().expect("label").name;
+                if let Some(config) = config.and_then(|c| c.labels.get(applied_label)) {
+                    if e.action == github::IssuesAction::Labeled && config.message_on_add.is_some() {
+                        return Ok(Some(NotifyZulipInput {
+                            notification_type: NotificationType::Labeled,
+                        }));
+                    } else if config.message_on_remove.is_some() {
+                        return Ok(Some(NotifyZulipInput {
+                            notification_type: NotificationType::Unlabeled,
+                        }));
+                    }
+                }
+            }
+        }
+        Ok(None)
+    }
+
+    fn handle_input<'b>(
+        &self,
+        ctx: &'b Context,
+        config: &'b Self::Config,
+        event: &'b Event,
+        input: Self::Input,
+    ) -> BoxFuture<'b, anyhow::Result<()>> {
+        handle_input(ctx, config, event, input).boxed()
+    }
+}
+
+async fn handle_input<'a>(
+    ctx: &Context,
+    config: &NotifyZulipConfig,
+    event: &Event,
+    input: NotifyZulipInput,
+) -> anyhow::Result<()> {
+    let event = match event {
+        Event::Issue(e) => e,
+        _ => unreachable!()
+    };
+    let config = config.labels.get(&event.label.as_ref().unwrap().name).unwrap();
+
+    let mut topic = config.topic.clone();
+    topic = topic.replace("{number}", &event.issue.number.to_string());
+    topic = topic.replace("{title}", &event.issue.title);
+    topic.truncate(60); // Zulip limitation
+
+    let mut msg = match input.notification_type {
+        NotificationType::Labeled => {
+            config.message_on_add.as_ref().unwrap().clone()
+        }
+        NotificationType::Unlabeled => {
+            config.message_on_add.as_ref().unwrap().clone()
+        }
+    };
+
+    msg = msg.replace("{number}", &event.issue.number.to_string());
+    msg = msg.replace("{title}", &event.issue.title);
+
+    let zulip_req = crate::zulip::MessageApiRequest {
+            type_: "stream",
+            to: &config.zulip_stream.to_string(),
+            topic: Some(&topic),
+            content: &msg,
+        };
+    zulip_req.send(&ctx.github.raw()).await?;
+
+    Ok(())
+}

src/handlers/prioritize.rs

@@ -9,21 +9,15 @@ use parser::command::{Command, Input};
 
 pub(super) struct PrioritizeHandler;
 
-pub(crate) enum Prioritize {
-    Label,
-    Start,
-    End,
-}
-
 impl Handler for PrioritizeHandler {
-    type Input = Prioritize;
+    type Input = PrioritizeCommand;
     type Config = PrioritizeConfig;
 
     fn parse_input(
         &self,
         ctx: &Context,
         event: &Event,
-        config: Option<&Self::Config>,
+        _config: Option<&Self::Config>,
     ) -> Result<Option<Self::Input>, String> {
         let body = if let Some(b) = event.comment_body() {
             b
@@ -32,66 +26,9 @@ impl Handler for PrioritizeHandler {
             return Ok(None);
         };
 
-        if let Event::Issue(e) = event {
-            if e.action == github::IssuesAction::Labeled {
-                if let Some(config) = config {
-                    let applied_label = &e.label.as_ref().expect("label").name;
-
-                    if *applied_label == config.label {
-                        // We need to take the exact same action in this case.
-                        return Ok(Some(Prioritize::Start));
-                    } else {
-                        // Checks if an `applied_label` needs prioritization according to the
-                        // labels the issue already have and the config's `exclude_labels` pattern
-                        // which are the ones we don't want to prioritize.
-                        if config.prioritize_on.iter().any(|l| l == applied_label) {
-                            let mut prioritize = false;
-
-                            'outer: for label in event.issue().unwrap().labels() {
-                                for exclude_label in &config.exclude_labels {
-                                    match glob::Pattern::new(exclude_label) {
-                                        Ok(exclude_glob) => {
-                                            prioritize = !exclude_glob.matches(&label.name);
-                                        }
-                                        Err(error) => {
-                                            log::error!("Invalid glob pattern: {}", error);
-                                        }
-                                    }
-
-                                    if !prioritize {
-                                        break 'outer;
-                                    }
-                                }
-                            }
-
-                            if prioritize {
-                                return Ok(Some(Prioritize::Label));
-                            }
-                        }
-                    }
-                }
-            }
-
-            if e.action == github::IssuesAction::Unlabeled {
-                if let Some(config) = config {
-                    if e.label.as_ref().expect("label").name == config.label {
-                        // We need to take the exact same action in this case.
-                        return Ok(Some(Prioritize::End));
-                    }
-                }
-            }
-
-            if e.action != github::IssuesAction::Opened {
-                log::debug!("skipping event, issue was {:?}", e.action);
-                // skip events other than opening the issue to avoid retriggering commands in the
-                // issue body
-                return Ok(None);
-            }
-        }
-
         let mut input = Input::new(&body, &ctx.username);
         match input.parse_command() {
-            Command::Prioritize(Ok(PrioritizeCommand)) => Ok(Some(Prioritize::Label)),
+            Command::Prioritize(Ok(PrioritizeCommand)) => Ok(Some(PrioritizeCommand)),
             _ => Ok(None),
         }
     }
@@ -111,70 +48,16 @@ async fn handle_input(
     ctx: &Context,
     config: &PrioritizeConfig,
     event: &Event,
-    cmd: Prioritize,
+    _: PrioritizeCommand,
 ) -> anyhow::Result<()> {
     let issue = event.issue().unwrap();
-
     let mut labels = issue.labels().to_owned();
-    let content = match cmd {
-        Prioritize::Label => {
-            // Don't add the label if it's already there
-            if !labels.iter().any(|l| l.name == config.label) {
-                labels.push(github::Label {
-                    name: config.label.clone(),
-                });
-            } else {
-                // TODO maybe send a GitHub message if the label is already there,
-                // i.e. the issue has already been requested for prioritization?
-                return Ok(());
-            }
-            None
-        }
-        Prioritize::Start => Some(format!(
-            "@*WG-prioritization* issue [#{}]({}) has been requested for prioritization.",
-            issue.number,
-            event.html_url().unwrap()
-        )),
-        Prioritize::End => {
-            // Shouldn't be necessary in practice as we only end on label
-            // removal, but if we add support in the future let's be sure to do
-            // the right thing.
-            if let Some(idx) = labels.iter().position(|l| l.name == config.label) {
-                labels.remove(idx);
-            }
-            Some(format!(
-                "Issue [#{}]({})'s prioritization request has been removed.",
-                issue.number,
-                event.html_url().unwrap()
-            ))
-        }
-    };
 
-    let github_req = issue.set_labels(&ctx.github, labels);
-
-    if let Some(content) = content {
-        let mut zulip_topic = format!(
-            "{} {} {}",
-            config.label,
-            issue.zulip_topic_reference(),
-            issue.title
-        );
-        zulip_topic.truncate(60); // Zulip limitation
-
-        let zulip_stream = config.zulip_stream.to_string();
-        let zulip_req = crate::zulip::MessageApiRequest {
-            type_: "stream",
-            to: &zulip_stream,
-            topic: Some(&zulip_topic),
-            content: &content,
-        };
-        let zulip_req = zulip_req.send(&ctx.github.raw());
-        let (gh_res, zulip_res) = futures::join!(github_req, zulip_req);
-        gh_res?;
-        zulip_res?;
-        Ok(())
-    } else {
-        github_req.await?;
-        Ok(())
+    // Don't add the label if it's already there
+    if !labels.iter().any(|l| l.name == config.label) {
+        labels.push(github::Label { name: config.label.to_owned() });
     }
+
+    issue.set_labels(&ctx.github, labels).await?;
+    Ok(())
 }