From 688e511fae1b0f6331b10068847d7505280db198 Mon Sep 17 00:00:00 2001 From: Niel Markwick Date: Tue, 9 Jul 2024 22:22:45 +0200 Subject: [PATCH] Allow pr-bot to re-assign reviewers when stopped (#31436) Fixes #31437 Enables the command `assign set of reviewers` to restart notifications if stopped --- scripts/ci/pr-bot/processPrUpdate.ts | 23 ++++++-- scripts/ci/pr-bot/shared/commentStrings.ts | 2 +- scripts/ci/pr-bot/shared/userCommand.ts | 64 ++++++++++++++-------- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/scripts/ci/pr-bot/processPrUpdate.ts b/scripts/ci/pr-bot/processPrUpdate.ts index f9aa15713216..d38fa452a4b6 100644 --- 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 index 138a494feb27..d1b366bcf773 100644 --- 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 index e32746eb7fce..6980468c3b19 100644 --- 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,