From f143e829319a1aa11fd4a1e085003b184f7aa9d9 Mon Sep 17 00:00:00 2001
From: Alice Zhao <alice@makenotion.com>
Date: Thu, 25 Apr 2024 15:45:48 -0700
Subject: [PATCH] refactor: move release notes check after failureMap

---
 spec/utils.spec.ts |  4 ++--
 src/index.ts       | 55 +++++++++++++++++++++++-----------------------
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/spec/utils.spec.ts b/spec/utils.spec.ts
index c2be816..e9f765b 100644
--- a/spec/utils.spec.ts
+++ b/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, [
diff --git a/src/index.ts b/src/index.ts
index 9f08739..e25d6ab 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -250,8 +250,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) {
@@ -287,6 +285,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 = [];
 
@@ -297,7 +296,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 (
@@ -315,9 +316,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[],
@@ -334,30 +359,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.