Merge pull request #2190 from Urgau/remove_labels

Improve removal of multiple GitHub labels at the same time

Author: Urgau

src/github.rs

@@ -785,33 +785,47 @@ impl Issue {
         Ok(())
     }
 
-    pub async fn remove_label(&self, client: &GithubClient, label: &str) -> anyhow::Result<()> {
-        log::info!("remove_label from {}: {:?}", self.global_id(), label);
-        // DELETE /repos/:owner/:repo/issues/:number/labels/{name}
-        let url = format!(
-            "{repo_url}/issues/{number}/labels/{name}",
-            repo_url = self.repository().url(client),
-            number = self.number,
-            name = label,
+    pub async fn remove_labels(
+        &self,
+        client: &GithubClient,
+        labels: Vec<Label>,
+    ) -> anyhow::Result<()> {
+        log::info!("remove_labels from {}: {:?}", self.global_id(), labels);
+
+        // Don't try to remove labels not already present on this issue.
+        let labels = labels
+            .into_iter()
+            .filter(|l| self.labels().contains(l))
+            .collect::<Vec<_>>();
+
+        log::info!(
+            "remove_labels: {} filtered to {:?}",
+            self.global_id(),
+            labels
         );
 
-        if !self
-            .labels()
-            .iter()
-            .any(|l| l.name.to_lowercase() == label.to_lowercase())
-        {
-            log::info!(
-                "remove_label from {}: {:?} already not present, skipping",
-                self.global_id(),
-                label
-            );
+        if labels.is_empty() {
             return Ok(());
         }
 
-        client
-            .send_req(client.delete(&url))
-            .await
-            .context("failed to delete label")?;
+        // There is no API to remove all labels at once, so we issue as many
+        // API requests are required in parallel.
+        let requests = labels.into_iter().map(|label| async move {
+            // DELETE /repos/:owner/:repo/issues/:number/labels/{name}
+            let url = format!(
+                "{repo_url}/issues/{number}/labels/{name}",
+                repo_url = self.repository().url(client),
+                number = self.number,
+                name = label.name,
+            );
+
+            client
+                .send_req(client.delete(&url))
+                .await
+                .with_context(|| format!("failed to remove {label:?}"))
+        });
+
+        futures::future::try_join_all(requests).await?;
 
         Ok(())
     }

src/handlers/autolabel.rs

@@ -222,18 +222,11 @@ pub(super) async fn handle_input(
         }
     }
 
-    for label in input.remove {
-        event
-            .issue
-            .remove_label(&ctx.github, &label.name)
-            .await
-            .with_context(|| {
-                format!(
-                    "failed to remove {:?} from {:?}",
-                    label,
-                    event.issue.global_id()
-                )
-            })?;
-    }
+    event
+        .issue
+        .remove_labels(&ctx.github, input.remove)
+        .await
+        .context("failed to remove labels from the issue")?;
+
     Ok(())
 }

src/handlers/check_commits.rs

@@ -243,13 +243,17 @@ async fn handle_new_state(
 
         // Remove the labels no longer required
         if !labels_to_remove.is_empty() {
-            for label in labels_to_remove {
-                event
-                    .issue
-                    .remove_label(&ctx.github, &label)
-                    .await
-                    .context("failed to remove a label in check_commits")?;
-            }
+            event
+                .issue
+                .remove_labels(
+                    &ctx.github,
+                    labels_to_remove
+                        .into_iter()
+                        .map(|name| Label { name })
+                        .collect(),
+                )
+                .await
+                .context("failed to remove a label in check_commits")?;
         }
 
         // Add the labels that are now required

src/handlers/concern.rs

@@ -152,12 +152,19 @@ pub(super) async fn handle_command(
             ).await.context("unable to post the comment failure it-self")?;
         }
     } else {
-        for l in &config.labels {
-            issue
-                .remove_label(&ctx.github, &l)
-                .await
-                .context("unable to remove the concern labels")?;
-        }
+        issue
+            .remove_labels(
+                &ctx.github,
+                config
+                    .labels
+                    .iter()
+                    .map(|l| Label {
+                        name: l.to_string(),
+                    })
+                    .collect(),
+            )
+            .await
+            .context("unable to remove the concern labels")?;
     }
 
     // Apply the new markdown concerns list to the issue

src/handlers/major_change.rs

@@ -708,7 +708,12 @@ As the automated representative, I would like to thank the author for their work
         .await
         .context("unable to add the accept label")?;
     issue
-        .remove_label(&ctx.github, &config.second_label)
+        .remove_labels(
+            &ctx.github,
+            vec![Label {
+                name: config.second_label.clone(),
+            }],
+        )
         .await
         .context("unable to remove the second label")?;
     issue

src/handlers/merge_conflicts.rs

@@ -288,15 +288,19 @@ async fn maybe_add_comment(
 
     let current_labels: HashSet<_> = issue.labels.iter().map(|l| l.name.clone()).collect();
     if current_labels.is_disjoint(&config.unless) {
-        for label in &config.remove {
-            issue.remove_label(gh, label).await?;
-        }
         let to_add = config
             .add
             .iter()
             .map(|l| Label { name: l.clone() })
             .collect();
+        let to_remove = config
+            .remove
+            .iter()
+            .map(|l| Label { name: l.clone() })
+            .collect();
+
         issue.add_labels(gh, to_add).await?;
+        issue.remove_labels(gh, to_remove).await?;
     }
 
     Ok(())

src/handlers/relabel.rs

@@ -73,16 +73,14 @@ pub(super) async fn handle_command(
     }
 
     // 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,
-                issue.global_id(),
-                e
-            );
-            return Err(e);
-        }
+    if let Err(e) = issue.remove_labels(&ctx.github, to_remove.clone()).await {
+        tracing::error!(
+            "failed to remove {:?} from issue {}: {:?}",
+            to_remove,
+            issue.global_id(),
+            e
+        );
+        return Err(e);
     }
 
     Ok(())

src/handlers/review_requested.rs

@@ -52,9 +52,18 @@ pub(crate) async fn handle_input(
         )
         .await?;
 
-    for label in &config.remove_labels {
-        event.issue.remove_label(&ctx.github, label).await?;
-    }
+    event
+        .issue
+        .remove_labels(
+            &ctx.github,
+            config
+                .remove_labels
+                .iter()
+                .cloned()
+                .map(|name| Label { name })
+                .collect(),
+        )
+        .await?;
 
     Ok(())
 }

src/handlers/review_submitted.rs

@@ -23,9 +23,20 @@ pub(crate) async fn handle(
 
         if event.issue.assignees.contains(&event.comment.user) {
             // Remove review labels
-            for label in &config.review_labels {
-                event.issue.remove_label(&ctx.github, &label).await?;
-            }
+            event
+                .issue
+                .remove_labels(
+                    &ctx.github,
+                    config
+                        .review_labels
+                        .iter()
+                        .map(|label| Label {
+                            name: label.clone(),
+                        })
+                        .collect(),
+                )
+                .await?;
+
             // Add waiting on author
             event
                 .issue

src/handlers/shortcut.rs

@@ -50,11 +50,17 @@ pub(super) async fn handle_command(
     };
 
     if !issue_labels.iter().any(|l| l.name == add) {
-        for remove in status_labels {
-            if remove != add {
-                issue.remove_label(&ctx.github, remove).await?;
-            }
-        }
+        issue
+            .remove_labels(
+                &ctx.github,
+                status_labels
+                    .iter()
+                    .filter(|l| **l != add)
+                    .map(|l| Label { name: (*l).into() })
+                    .collect(),
+            )
+            .await?;
+
         issue
             .add_labels(
                 &ctx.github,