Allow pr-bot to re-assign reviewers when stopped (apache#31436)

Fixes apache#31437 Enables the command `assign set of reviewers` to restart notifications if stopped
acrites · Jul 17, 2024 · 688e511 · 688e511
1 parent 7395713
commit 688e511
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 30 deletions.
diff --git a/scripts/ci/pr-bot/processPrUpdate.ts b/scripts/ci/pr-bot/processPrUpdate.ts
@@ -83,6 +83,18 @@ async function processPrComment(
     stateClient,
     reviewerConfig
   );
+
+  // Check to see if notifications have been stopped before processing further.
+  // Notifications can be stopped by an "R: reviewer" comment,
+  // and then restarted by adding "assign set of reviewers" comment.
+  if (
+    (await stateClient.getPrState(getPullNumberFromPayload(payload)))
+      .stopReviewerNotifications
+  ) {
+    console.log("Notifications have been paused for this pull - skipping");
+    return;
+  }
+
   // If there's been a comment by a non-author, we can remove the slow review label
   if (commentAuthor !== pullAuthor && commentAuthor !== BOT_NAME) {
     await removeSlowReviewLabel(payload);
@@ -140,11 +152,6 @@ async function processPrUpdate() {
   const pullNumber = getPullNumberFromPayload(payload);
 
   const stateClient = new PersistentState();
-  const prState = await stateClient.getPrState(pullNumber);
-  if (prState.stopReviewerNotifications) {
-    console.log("Notifications have been paused for this pull - skipping");
-    return;
-  }
 
   switch (github.context.eventName) {
     case "issue_comment":
@@ -156,6 +163,12 @@ async function processPrUpdate() {
       await processPrComment(payload, stateClient, reviewerConfig);
       break;
     case "pull_request_target":
+      if (
+        (await stateClient.getPrState(pullNumber)).stopReviewerNotifications
+      ) {
+        console.log("Notifications have been paused for this pull - skipping");
+        return;
+      }
       if (payload.action === "synchronize") {
         console.log("Processing synchronize action");
         await setNextActionReviewers(payload, stateClient);

diff --git a/scripts/ci/pr-bot/shared/commentStrings.ts b/scripts/ci/pr-bot/shared/commentStrings.ts
@@ -58,7 +58,7 @@ export function someChecksFailing(reviewersToNotify: string[]): string {
 }
 
 export function stopNotifications(reason: string): string {
-  return `Stopping reviewer notifications for this pull request: ${reason}`;
+  return `Stopping reviewer notifications for this pull request: ${reason}. If you'd like to restart, comment \`assign set of reviewers\``;
 }
 
 export function remindReviewerAfterTestsPass(requester: string): string {

diff --git a/scripts/ci/pr-bot/shared/userCommand.ts b/scripts/ci/pr-bot/shared/userCommand.ts
@@ -39,32 +39,42 @@ export async function processCommand(
 
   const pullNumber = payload.issue?.number || payload.pull_request?.number;
   commentText = commentText.toLowerCase();
-  if (commentText.indexOf("r: @") > -1) {
-    await manuallyAssignedToReviewer(pullNumber, stateClient);
-  } else if (commentText.indexOf("assign to next reviewer") > -1) {
-    await assignToNextReviewer(
-      payload,
-      commentAuthor,
-      pullNumber,
-      stateClient,
-      reviewerConfig
-    );
-  } else if (commentText.indexOf("stop reviewer notifications") > -1) {
-    await stopReviewerNotifications(
-      pullNumber,
-      stateClient,
-      "requested by reviewer"
-    );
-  } else if (commentText.indexOf("remind me after tests pass") > -1) {
-    await remindAfterTestsPass(pullNumber, commentAuthor, stateClient);
-  } else if (commentText.indexOf("waiting on author") > -1) {
-    await waitOnAuthor(payload, pullNumber, stateClient);
-  } else if (commentText.indexOf("assign set of reviewers") > -1) {
-    await assignReviewerSet(payload, pullNumber, stateClient, reviewerConfig);
+
+  let prState = await stateClient.getPrState(pullNumber);
+  if(prState.stopReviewerNotifications) {
+    // Notifications stopped, only "allow assign set of reviewers"
+    if (commentText.indexOf("assign set of reviewers") > -1) {
+      await assignReviewerSet(payload, pullNumber, stateClient, reviewerConfig);
+    } else {
+      return false;
+    }
   } else {
-    return false;
+    if (commentText.indexOf("r: @") > -1) {
+      await manuallyAssignedToReviewer(pullNumber, stateClient);
+    } else if (commentText.indexOf("assign to next reviewer") > -1) {
+      await assignToNextReviewer(
+        payload,
+        commentAuthor,
+        pullNumber,
+        stateClient,
+        reviewerConfig
+      );
+    } else if (commentText.indexOf("stop reviewer notifications") > -1) {
+      await stopReviewerNotifications(
+        pullNumber,
+        stateClient,
+        "requested by reviewer"
+      );
+    } else if (commentText.indexOf("remind me after tests pass") > -1) {
+      await remindAfterTestsPass(pullNumber, commentAuthor, stateClient);
+    } else if (commentText.indexOf("waiting on author") > -1) {
+      await waitOnAuthor(payload, pullNumber, stateClient);
+    } else if (commentText.indexOf("assign set of reviewers") > -1) {
+      await assignReviewerSet(payload, pullNumber, stateClient, reviewerConfig);
+    } else {
+      return false;
+    }
   }
-
   return true;
 }
 
@@ -175,6 +185,12 @@ async function assignReviewerSet(
   reviewerConfig: typeof ReviewerConfig
 ) {
   let prState = await stateClient.getPrState(pullNumber);
+  if(prState.stopReviewerNotifications) {
+    // Restore notifications, and clear any existing reviewer set to
+    // allow new reviewers to be assigned.
+    prState.stopReviewerNotifications = false;
+    prState.reviewersAssignedForLabels = {};
+  }
   if (Object.values(prState.reviewersAssignedForLabels).length > 0) {
     await github.addPrComment(
       pullNumber,