Merge pull request #1733 from WaffleLapkin/review-requested

Allow switching prs to waiting on review when author has requested a review from an assignee

Author: Mark-Simulacrum

src/config.rs

@@ -30,6 +30,7 @@ pub(crate) struct Config {
     pub(crate) notify_zulip: Option<NotifyZulipConfig>,
     pub(crate) github_releases: Option<GitHubReleasesConfig>,
     pub(crate) review_submitted: Option<ReviewSubmittedConfig>,
+    pub(crate) review_requested: Option<ReviewRequestedConfig>,
     pub(crate) shortcut: Option<ShortcutConfig>,
     pub(crate) note: Option<NoteConfig>,
     pub(crate) mentions: Option<MentionsConfig>,
@@ -253,6 +254,12 @@ pub(crate) struct ReviewSubmittedConfig {
     pub(crate) reviewed_label: String,
 }
 
+#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
+pub(crate) struct ReviewRequestedConfig {
+    pub(crate) remove_labels: Vec<String>,
+    pub(crate) add_labels: Vec<String>,
+}
+
 pub(crate) async fn get(
     gh: &GithubClient,
     repo: &Repository,
@@ -421,6 +428,7 @@ mod tests {
                 notify_zulip: None,
                 github_releases: None,
                 review_submitted: None,
+                review_requested: None,
                 mentions: None,
                 no_merges: None,
             }

src/github.rs

@@ -923,7 +923,7 @@ pub struct IssueCommentEvent {
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
-#[serde(rename_all = "snake_case")]
+#[serde(rename_all = "snake_case", tag = "action")]
 pub enum IssuesAction {
     Opened,
     Edited,
@@ -935,13 +935,22 @@ pub enum IssuesAction {
     Reopened,
     Assigned,
     Unassigned,
-    Labeled,
-    Unlabeled,
+    Labeled {
+        /// The label added from the issue
+        label: Label,
+    },
+    Unlabeled {
+        /// The label removed from the issue
+        label: Label,
+    },
     Locked,
     Unlocked,
     Milestoned,
     Demilestoned,
-    ReviewRequested,
+    ReviewRequested {
+        /// The person requested to review the pull request
+        requested_reviewer: User,
+    },
     ReviewRequestRemoved,
     ReadyForReview,
     Synchronize,
@@ -952,13 +961,14 @@ pub enum IssuesAction {
 
 #[derive(Debug, serde::Deserialize)]
 pub struct IssuesEvent {
+    #[serde(flatten)]
     pub action: IssuesAction,
     #[serde(alias = "pull_request")]
     pub issue: Issue,
     pub changes: Option<Changes>,
     pub repository: Repository,
-    /// Some if action is IssuesAction::Labeled, for example
-    pub label: Option<Label>,
+    /// The GitHub user that triggered the event.
+    pub sender: User,
 }
 
 #[derive(Debug, serde::Deserialize)]

src/handlers.rs

@@ -41,6 +41,7 @@ mod notify_zulip;
 mod ping;
 mod prioritize;
 mod relabel;
+mod review_requested;
 mod review_submitted;
 mod rfc_helper;
 pub mod rustc_commits;
@@ -164,6 +165,7 @@ issue_handlers! {
     mentions,
     no_merges,
     notify_zulip,
+    review_requested,
 }
 
 macro_rules! command_handlers {

src/handlers/autolabel.rs

@@ -95,9 +95,9 @@ pub(super) async fn parse_input(
         }
     }
 
-    if event.action == IssuesAction::Labeled {
+    if let IssuesAction::Labeled { label } = &event.action {
         let mut autolabels = Vec::new();
-        let applied_label = &event.label.as_ref().expect("label").name;
+        let applied_label = &label.name;
 
         'outer: for (label, config) in config.get_by_trigger(applied_label) {
             let exclude_patterns: Vec<glob::Pattern> = config

src/handlers/major_change.rs

@@ -55,11 +55,7 @@ pub(super) async fn parse_input(
     }
 
     // If we were labeled with accepted, then issue that event
-    if event.action == IssuesAction::Labeled
-        && event
-            .label
-            .as_ref()
-            .map_or(false, |l| l.name == config.accept_label)
+    if matches!(&event.action, IssuesAction::Labeled { label } if label.name == config.accept_label)
     {
         return Ok(Some(Invocation::AcceptedProposal));
     }
@@ -70,17 +66,8 @@ pub(super) async fn parse_input(
     // We want to treat reopened issues as new proposals but if the
     // issue is freshly opened, we only want to trigger once;
     // currently we do so on the label event.
-    if (event.action == IssuesAction::Reopened
-        && event
-            .issue
-            .labels()
-            .iter()
-            .any(|l| l.name == enabling_label))
-        || (event.action == IssuesAction::Labeled
-            && event
-                .label
-                .as_ref()
-                .map_or(false, |l| l.name == enabling_label))
+    if matches!(event.action, IssuesAction::Reopened if event.issue.labels().iter().any(|l| l.name == enabling_label))
+        || matches!(&event.action, IssuesAction::Labeled { label } if label.name == enabling_label)
     {
         return Ok(Some(Invocation::NewProposal));
     }

src/handlers/notify_zulip.rs

@@ -31,9 +31,9 @@ pub(super) async fn parse_input(
         None => return Ok(None),
     };
 
-    match event.action {
-        IssuesAction::Labeled | IssuesAction::Unlabeled => {
-            let applied_label = event.label.as_ref().expect("label").clone();
+    match &event.action {
+        IssuesAction::Labeled { label } | IssuesAction::Unlabeled { label } => {
+            let applied_label = label.clone();
             Ok(config
                 .labels
                 .get(&applied_label.name)
@@ -60,14 +60,16 @@ fn parse_label_change_input(
     }
 
     match event.action {
-        IssuesAction::Labeled if config.message_on_add.is_some() => Some(NotifyZulipInput {
+        IssuesAction::Labeled { .. } if config.message_on_add.is_some() => Some(NotifyZulipInput {
             notification_type: NotificationType::Labeled,
             label,
         }),
-        IssuesAction::Unlabeled if config.message_on_remove.is_some() => Some(NotifyZulipInput {
-            notification_type: NotificationType::Unlabeled,
-            label,
-        }),
+        IssuesAction::Unlabeled { .. } if config.message_on_remove.is_some() => {
+            Some(NotifyZulipInput {
+                notification_type: NotificationType::Unlabeled,
+                label,
+            })
+        }
         _ => None,
     }
 }

src/handlers/review_requested.rs

@@ -0,0 +1,53 @@
+use crate::config::ReviewRequestedConfig;
+use crate::github::{IssuesAction, IssuesEvent, Label};
+use crate::handlers::Context;
+
+pub(crate) struct ReviewRequestedInput {}
+
+pub(crate) async fn parse_input(
+    _ctx: &Context,
+    event: &IssuesEvent,
+    _config: Option<&ReviewRequestedConfig>,
+) -> Result<Option<ReviewRequestedInput>, String> {
+    // PR author requests a review from one of the assignees
+
+    let IssuesAction::ReviewRequested { requested_reviewer } = &event.action else {
+        return Ok(None);
+    };
+
+    if event.sender != event.issue.user {
+        return Ok(None);
+    }
+
+    if !event.issue.assignees.contains(requested_reviewer) {
+        return Ok(None);
+    }
+
+    Ok(Some(ReviewRequestedInput {}))
+}
+
+pub(crate) async fn handle_input(
+    ctx: &Context,
+    config: &ReviewRequestedConfig,
+    event: &IssuesEvent,
+    ReviewRequestedInput {}: ReviewRequestedInput,
+) -> anyhow::Result<()> {
+    event
+        .issue
+        .add_labels(
+            &ctx.github,
+            config
+                .add_labels
+                .iter()
+                .cloned()
+                .map(|name| Label { name })
+                .collect(),
+        )
+        .await?;
+
+    for label in &config.remove_labels {
+        event.issue.remove_label(&ctx.github, label).await?;
+    }
+
+    Ok(())
+}