refactor: move release notes check after failureMap

Author: alicelovescake

spec/utils.spec.ts

@@ -109,7 +109,7 @@ describe('utils', () => {
       ).toBe(true);
     });
 
-    it('should update backport PR if release notes do not match original PR for single PR', async () => {
+    it('should return not valid if release notes do not match original PR for single PR', async () => {
       const context = { ...backportPRMissingReleaseNotes };
       expect(
         await isValidManualBackportReleaseNotes(context, [
@@ -118,7 +118,7 @@ describe('utils', () => {
       ).toBe(false);
     });
 
-    it('should update backport PR if release notes do not match original PR for multiple PR', async () => {
+    it('should return not valid if release notes do not match original PR for multiple PR', async () => {
       const context = { ...backportPRMissingReleaseNotes };
       expect(
         await isValidManualBackportReleaseNotes(context, [

src/index.ts

@@ -272,8 +272,6 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
         ];
         const FASTTRACK_LABELS: string[] = ['fast-track 🚅'];
 
-        const failureMap = new Map();
-
         // There are several types of PRs which might not target main yet which are
         // inherently valid; e.g roller-bot PRs. Check for and allow those here.
         if (oldPRNumbers.length === 0) {
@@ -309,6 +307,7 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
           robot.log(
             `#${pr.number} has backport numbers - checking their validity now`,
           );
+          const failureMap = new Map();
           const supported = await getSupportedBranches(context);
           const oldPRs = [];
 
@@ -319,7 +318,9 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
                 pull_number: oldPRNumber,
               }),
             );
+
             oldPRs.push(oldPR);
+
             // The current PR is only valid if the PR it is backporting
             // was merged to main or to a supported release branch.
             if (
@@ -337,9 +338,33 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
             }
           }
 
+          for (const oldPRNumber of oldPRNumbers) {
+            if (failureMap.has(oldPRNumber)) {
+              robot.log(
+                `#${pr.number} is targeting a branch that is not ${
+                  pr.base.repo.default_branch
+                } - ${failureMap.get(oldPRNumber)}`,
+              );
+              await updateBackportValidityCheck(context, checkRun, {
+                title: 'Invalid Backport',
+                summary: `This PR is targeting a branch that is not ${
+                  pr.base.repo.default_branch
+                } but ${failureMap.get(oldPRNumber)}`,
+                conclusion: CheckRunStatus.FAILURE,
+              });
+            } else {
+              robot.log(`#${pr.number} is a valid backport of #${oldPRNumber}`);
+              await updateBackportValidityCheck(context, checkRun, {
+                title: 'Valid Backport',
+                summary: `This PR is declared as backporting "#${oldPRNumber}" which is a valid PR that has been merged into ${pr.base.repo.default_branch}`,
+                conclusion: CheckRunStatus.SUCCESS,
+              });
+            }
+          }
+
           if (['opened', 'edited'].includes(action)) {
             robot.log(`Checking validity of release notes`);
-            // Check to make sure backport PR has the same release notes as at least on of the old prs
+            // Check to make sure backport PR has the same release notes as at least one of the old prs
             const isValidReleaseNotes = await isValidManualBackportReleaseNotes(
               context,
               oldPRs as WebHookPR[],
@@ -356,30 +381,6 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
             }
           }
         }
-
-        for (const oldPRNumber of oldPRNumbers) {
-          if (failureMap.has(oldPRNumber)) {
-            robot.log(
-              `#${pr.number} is targeting a branch that is not ${
-                pr.base.repo.default_branch
-              } - ${failureMap.get(oldPRNumber)}`,
-            );
-            await updateBackportValidityCheck(context, checkRun, {
-              title: 'Invalid Backport',
-              summary: `This PR is targeting a branch that is not ${
-                pr.base.repo.default_branch
-              } but ${failureMap.get(oldPRNumber)}`,
-              conclusion: CheckRunStatus.FAILURE,
-            });
-          } else {
-            robot.log(`#${pr.number} is a valid backport of #${oldPRNumber}`);
-            await updateBackportValidityCheck(context, checkRun, {
-              title: 'Valid Backport',
-              summary: `This PR is declared as backporting "#${oldPRNumber}" which is a valid PR that has been merged into ${pr.base.repo.default_branch}`,
-              conclusion: CheckRunStatus.SUCCESS,
-            });
-          }
-        }
       } else {
         // If we're somehow targeting main and have a check run,
         // we mark this check as cancelled.