Fix backport label handler

src/handlers/backport.rs

@@ -1,9 +1,14 @@
+//! Handle backport labels for PRs fixing P-high/critical regressions
+//!
+//! Configuration is done with the `[backport]` table.
+//!
 use std::collections::HashMap;
 use std::sync::LazyLock;
 
 use crate::config::BackportConfig;
 use crate::github::{IssuesAction, IssuesEvent, Label};
 use crate::handlers::Context;
+use crate::utils::contains_any;
 use anyhow::Context as AnyhowContext;
 use futures::future::join_all;
 use regex::Regex;
@@ -15,12 +20,9 @@ static CLOSES_ISSUE_REGEXP: LazyLock<Regex> = LazyLock::new(|| {
     Regex::new("(?i)(?P<action>close[sd]*|fix([e]*[sd]*)?|resolve[sd]*)(?P<spaces>:? +)(?P<org_repo>[a-zA-Z0-9_-]*/[a-zA-Z0-9_-]*)?#(?P<issue_num>[0-9]+)").unwrap()
 });
 
-const BACKPORT_LABELS: [&str; 4] = [
-    "beta-nominated",
-    "beta-accepted",
-    "stable-nominated",
-    "stable-accepted",
-];
+const BACKPORT_ACCEPTED_LABELS: [&str; 2] = ["beta-accepted", "stable-accepted"];
+
+const BACKPORT_NOMINATED_LABELS: [&str; 2] = ["beta-nominated", "stable-nominated"];
 
 const REGRESSION_LABELS: [&str; 3] = [
     "regression-from-stable-to-nightly",
@@ -41,7 +43,7 @@ pub(crate) struct BackportInput {
 }
 
 pub(super) async fn parse_input(
-    _ctx: &Context,
+    ctx: &Context,
     event: &IssuesEvent,
     config: Option<&BackportConfig>,
 ) -> Result<Option<BackportInput>, String> {
@@ -53,13 +55,17 @@ pub(super) async fn parse_input(
     // - is opened (and not a draft)
     // - is converted from draft to ready for review
     // - when the first comment is edited
+    // - when a label is added (later we check which one)
     let skip_check = !matches!(
         event.action,
-        IssuesAction::Opened | IssuesAction::Edited | IssuesAction::ReadyForReview
+        IssuesAction::Opened
+            | IssuesAction::Edited
+            | IssuesAction::ReadyForReview
+            | IssuesAction::Labeled { label: _ }
     );
     if skip_check || !event.issue.is_pr() || event.issue.draft {
         log::debug!(
-            "Skipping backport event because: IssuesAction = {:?}, issue.is_pr() {}, draft = {}",
+            "Skipping backport event because: IssuesAction = {:?}, issue.is_pr() = {}, draft = {}",
             event.action,
             event.issue.is_pr(),
             event.issue.draft
@@ -69,9 +75,28 @@ pub(super) async fn parse_input(
     let pr = &event.issue;
 
     let pr_labels: Vec<&str> = pr.labels.iter().map(|l| l.name.as_str()).collect();
-    if contains_any(&pr_labels, &BACKPORT_LABELS) {
-        log::debug!("PR #{} already has a backport label", pr.number);
-        return Ok(None);
+
+    // If a "-nominated" label is added to an already "-accepted" PR, remove the label added by github
+    if let IssuesAction::Labeled { label } = &event.action
+        && BACKPORT_NOMINATED_LABELS.contains(&label.name.as_str())
+    {
+        if contains_any(&pr_labels, &BACKPORT_ACCEPTED_LABELS) {
+            log::debug!(
+                "Refusing to backport nominate, PR #{} is already backport accepted (found labels: {:?})",
+                pr.number,
+                pr_labels
+            );
+            let label_to_remove = vec![Label {
+                name: label.name.clone(),
+            }];
+            let _ = &event
+                .issue
+                .remove_labels(&ctx.github, label_to_remove)
+                .await
+                .context("failed to remove labels from the issue")
+                .unwrap();
+            return Ok(None);
+        }
     }
 
     // Retrieve backport config for this PR, based on its team label(s)
@@ -187,7 +212,8 @@ pub(super) async fn handle_input(
             continue;
         }
 
-        // Add backport nomination label(s) to PR
+        // When backport nominating/accepting a PR, if a `[notify-zulip]` table is configured in triagebot.toml
+        // that will trigger the notify_zulip handler
         let mut new_labels = pr.labels().to_owned();
         new_labels.extend(
             add_labels
@@ -210,16 +236,12 @@ pub(super) async fn handle_input(
     Ok(())
 }
 
-fn contains_any(haystack: &[&str], needles: &[&str]) -> bool {
-    needles.iter().any(|needle| haystack.contains(needle))
-}
-
 #[cfg(test)]
 mod tests {
     use crate::handlers::backport::CLOSES_ISSUE_REGEXP;
 
-    #[tokio::test]
-    async fn backport_match_comment() {
+    #[test]
+    fn backport_match_comment() {
         let test_strings = vec![
             ("close #10", vec![10]),
             ("closes #10", vec![10]),

src/utils.rs

@@ -56,3 +56,7 @@ pub(crate) async fn is_issue_under_rfcbot_fcp(
 
     false
 }
+
+pub fn contains_any(haystack: &[&str], needles: &[&str]) -> bool {
+    needles.iter().any(|needle| haystack.contains(needle))
+}