Merge pull request #2189 from Urgau/relabel-deltas-left-right

Apply labels from left to right in relabel

Author: Urgau

parser/src/command/relabel.rs

@@ -19,7 +19,7 @@ pub enum LabelDelta {
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
-pub struct Label(String);
+pub struct Label(pub String);
 
 #[derive(PartialEq, Eq, Debug)]
 pub enum ParseError {

src/github.rs

@@ -324,7 +324,7 @@ impl User {
     }
 }
 
-#[derive(PartialEq, Eq, Debug, Clone, serde::Deserialize)]
+#[derive(PartialEq, Eq, Debug, Clone, Ord, PartialOrd, serde::Deserialize)]
 pub struct Label {
     pub name: String,
 }

src/handlers/relabel.rs

@@ -8,6 +8,9 @@
 //! If the command was successful, there will be no feedback beyond the label change to reduce
 //! notification noise.
 
+use std::collections::BTreeSet;
+
+use crate::github::Label;
 use crate::team_data::TeamClient;
 use crate::{
     config::RelabelConfig,
@@ -24,8 +27,11 @@ pub(super) async fn handle_command(
     event: &Event,
     input: RelabelCommand,
 ) -> anyhow::Result<()> {
-    let mut results = vec![];
-    let mut to_add = vec![];
+    let Some(issue) = event.issue() else {
+        anyhow::bail!("event is not an issue");
+    };
+
+    // Check label authorization for the current user
     for delta in &input.0 {
         let name = delta.label().as_str();
         let err = match check_filter(name, config, is_member(&event.user(), &ctx.team).await) {
@@ -42,54 +48,37 @@ pub(super) async fn handle_command(
             Err(err) => Some(err),
         };
         if let Some(msg) = err {
-            let cmnt = ErrorComment::new(&event.issue().unwrap(), msg);
+            let cmnt = ErrorComment::new(issue, msg);
             cmnt.post(&ctx.github).await?;
             return Ok(());
         }
-        match delta {
-            LabelDelta::Add(label) => {
-                to_add.push(github::Label {
-                    name: label.to_string(),
-                });
-            }
-            LabelDelta::Remove(label) => {
-                results.push((
-                    label,
-                    event.issue().unwrap().remove_label(&ctx.github, &label),
-                ));
-            }
-        }
     }
 
-    if let Err(e) = event
-        .issue()
-        .unwrap()
-        .add_labels(&ctx.github, to_add.clone())
-        .await
-    {
+    // Compute the labels to add and remove
+    let (to_add, to_remove) = compute_label_deltas(&input.0);
+
+    // Add labels
+    if let Err(e) = issue.add_labels(&ctx.github, to_add.clone()).await {
         tracing::error!(
             "failed to add {:?} from issue {}: {:?}",
             to_add,
-            event.issue().unwrap().global_id(),
+            issue.global_id(),
             e
         );
         if let Some(err @ UnknownLabels { .. }) = e.downcast_ref() {
-            event
-                .issue()
-                .unwrap()
-                .post_comment(&ctx.github, &err.to_string())
-                .await?;
+            issue.post_comment(&ctx.github, &err.to_string()).await?;
         }
 
         return Err(e);
     }
 
-    for (label, res) in results {
-        if let Err(e) = res.await {
+    // Remove labels
+    for label in to_remove {
+        if let Err(e) = issue.remove_label(&ctx.github, &label.name).await {
             tracing::error!(
                 "failed to remove {:?} from issue {}: {:?}",
                 label,
-                event.issue().unwrap().global_id(),
+                issue.global_id(),
                 e
             );
             return Err(e);
@@ -182,10 +171,41 @@ fn match_pattern(pattern: &str, label: &str) -> anyhow::Result<MatchPatternResul
     })
 }
 
+fn compute_label_deltas(deltas: &[LabelDelta]) -> (Vec<Label>, Vec<Label>) {
+    let mut add = BTreeSet::new();
+    let mut remove = BTreeSet::new();
+
+    for delta in deltas {
+        match delta {
+            LabelDelta::Add(label) => {
+                let label = Label {
+                    name: label.to_string(),
+                };
+                if !remove.remove(&label) {
+                    add.insert(label);
+                }
+            }
+            LabelDelta::Remove(label) => {
+                let label = Label {
+                    name: label.to_string(),
+                };
+                if !add.remove(&label) {
+                    remove.insert(label);
+                }
+            }
+        }
+    }
+
+    (add.into_iter().collect(), remove.into_iter().collect())
+}
+
 #[cfg(test)]
 mod tests {
+    use parser::command::relabel::{Label, LabelDelta};
+
     use super::{
-        CheckFilterResult, MatchPatternResult, TeamMembership, check_filter, match_pattern,
+        CheckFilterResult, MatchPatternResult, TeamMembership, check_filter, compute_label_deltas,
+        match_pattern,
     };
     use crate::config::RelabelConfig;
 
@@ -252,4 +272,55 @@ mod tests {
         }
         Ok(())
     }
+
+    #[test]
+    fn test_compute_label_deltas() {
+        use crate::github::Label as GitHubLabel;
+
+        let mut deltas = vec![
+            LabelDelta::Add(Label("I-nominated".to_string())),
+            LabelDelta::Add(Label("I-nominated".to_string())),
+            LabelDelta::Add(Label("I-lang-nominated".to_string())),
+            LabelDelta::Add(Label("I-libs-nominated".to_string())),
+            LabelDelta::Remove(Label("I-lang-nominated".to_string())),
+        ];
+
+        assert_eq!(
+            compute_label_deltas(&deltas),
+            (
+                vec![
+                    GitHubLabel {
+                        name: "I-libs-nominated".to_string()
+                    },
+                    GitHubLabel {
+                        name: "I-nominated".to_string()
+                    },
+                ],
+                vec![],
+            ),
+        );
+
+        deltas.push(LabelDelta::Remove(Label("needs-triage".to_string())));
+        deltas.push(LabelDelta::Add(Label("I-lang-nominated".to_string())));
+
+        assert_eq!(
+            compute_label_deltas(&deltas),
+            (
+                vec![
+                    GitHubLabel {
+                        name: "I-lang-nominated".to_string()
+                    },
+                    GitHubLabel {
+                        name: "I-libs-nominated".to_string()
+                    },
+                    GitHubLabel {
+                        name: "I-nominated".to_string()
+                    },
+                ],
+                vec![GitHubLabel {
+                    name: "needs-triage".to_string()
+                }],
+            ),
+        );
+    }
 }