fix: labels when trop PR closed unmerged

spec/index.spec.ts

@@ -181,10 +181,10 @@ describe('trop', () => {
     });
 
     it('adds a label when a backport PR has been merged', async () => {
-      Object.defineProperty(utils, 'labelMergedPR', { value: jest.fn() });
+      Object.defineProperty(utils, 'labelClosedPR', { value: jest.fn() });
       await robot.receive(backportPRClosedBotEvent);
 
-      expect(utils.labelMergedPR).toHaveBeenCalled();
+      expect(utils.labelClosedPR).toHaveBeenCalled();
     });
 
     it('labels the original PR when a manual backport PR has been merged', async () => {
@@ -194,7 +194,7 @@ describe('trop', () => {
     });
 
     it('adds a label when a backport PR has been merged', async () => {
-      Object.defineProperty(utils, 'labelMergedPR', { value: jest.fn() });
+      Object.defineProperty(utils, 'labelClosedPR', { value: jest.fn() });
       await robot.receive(backportPRClosedEvent);
 
       expect(updateManualBackport).toHaveBeenCalled();

src/index.ts

@@ -1,6 +1,6 @@
 import { Application, Context } from 'probot';
 
-import { backportImpl, labelMergedPR } from './utils';
+import { backportImpl, labelClosedPR } from './utils';
 import { labelToTargetBranch, labelExistsOnPR } from './utils/label-utils';
 import { TropConfig } from './interfaces';
 import { CHECK_PREFIX, SKIP_CHECK_LABEL } from './constants';
@@ -19,13 +19,17 @@ import { getSupportedBranches, getBackportPattern } from './utils/branch-util';
 import { updateBackportValidityCheck } from './utils/checks-util';
 
 const probotHandler = async (robot: Application) => {
-  const labelMergedPRs = async (context: Context, pr: PullsGetResponse) => {
+  const labelClosedPRs = async (
+    context: Context,
+    pr: PullsGetResponse,
+    change: PRChange,
+  ) => {
     for (const label of pr.labels) {
       const targetBranch = label.name.match(
         /^(\d)+-(?:(?:[0-9]+-x$)|(?:x+-y$))$/,
       );
       if (targetBranch && targetBranch[0]) {
-        await labelMergedPR(context, pr, label.name);
+        await labelClosedPR(context, pr, label.name, change);
       }
     }
   };
@@ -37,6 +41,27 @@ const probotHandler = async (robot: Application) => {
     }
   };
 
+  const handleTropBackportClosed = async (
+    context: Context,
+    pr: PullsGetResponse,
+    change: PRChange,
+  ) => {
+    const closeType = change === PRChange.MERGE ? 'merged' : 'closed';
+    robot.log(
+      `Updating labels on original PR for ${closeType} PR: #${pr.number}`,
+    );
+    await labelClosedPRs(context, pr, change);
+
+    robot.log(`Deleting base branch: ${pr.head.ref}`);
+    try {
+      await context.github.git.deleteRef(
+        context.repo({ ref: `heads/${pr.head.ref}` }),
+      );
+    } catch (e) {
+      robot.log('Failed to delete backport branch: ', e);
+    }
+  };
+
   const runCheck = async (context: Context, pr: PullsGetResponse) => {
     const allChecks = await context.github.checks.listForRef(
       context.repo({
@@ -327,44 +352,43 @@ const probotHandler = async (robot: Application) => {
     if (pr.merged) {
       if (oldPRNumbers.length > 0) {
         robot.log(`Automatic backport merged for: #${pr.number}`);
-
         robot.log(`Labeling original PR for merged PR: #${pr.number}`);
         for (const oldPRNumber of oldPRNumbers) {
           await updateManualBackport(context, PRChange.MERGE, oldPRNumber);
         }
-        await labelMergedPRs(context, pr);
+        await labelClosedPRs(context, pr, PRChange.MERGE);
       }
 
       // Check that the closed PR is trop's own and act accordingly.
       if (pr.user.login === getEnvVar('BOT_USER_NAME')) {
-        robot.log(`Labeling original PR for merged PR: #${pr.number}`);
-        await labelMergedPRs(context, pr);
-
-        robot.log(`Deleting base branch: ${pr.head.ref}`);
-        try {
-          await context.github.git.deleteRef(
-            context.repo({ ref: `heads/${pr.head.ref}` }),
-          );
-        } catch (e) {
-          robot.log('Failed to delete backport branch: ', e);
-        }
+        await handleTropBackportClosed(context, pr, PRChange.MERGE);
       } else {
         robot.log(
           `Backporting #${pr.number} to all branches specified by labels`,
         );
         backportAllLabels(context, pr);
       }
     } else {
-      if (oldPRNumbers.length > 0) {
-        robot.log(
-          `Automatic backport #${pr.number} closed with unmerged commits`,
-        );
+      robot.log(
+        `Automatic backport #${pr.number} closed with unmerged commits`,
+      );
 
+      if (oldPRNumbers.length > 0) {
         robot.log(`Updating label on original PR for closed PR: #${pr.number}`);
         for (const oldPRNumber of oldPRNumbers) {
           await updateManualBackport(context, PRChange.CLOSE, oldPRNumber);
         }
-        await labelMergedPRs(context, pr);
+        await labelClosedPRs(context, pr, PRChange.CLOSE);
+      }
+
+      // Check that the closed PR is trop's own and act accordingly.
+      if (pr.user.login === getEnvVar('BOT_USER_NAME')) {
+        await handleTropBackportClosed(context, pr, PRChange.CLOSE);
+      } else {
+        robot.log(
+          `Backporting #${pr.number} to all branches specified by labels`,
+        );
+        backportAllLabels(context, pr);
       }
     }
   });

src/utils.ts

@@ -17,7 +17,7 @@ import {
   BACKPORT_REQUESTED_LABEL,
   DEFAULT_BACKPORT_REVIEW_TEAM,
 } from './constants';
-import { PRStatus, BackportPurpose, LogLevel } from './enums';
+import { PRStatus, BackportPurpose, LogLevel, PRChange } from './enums';
 
 import * as labelUtils from './utils/label-utils';
 import { initRepo } from './operations/init-repo';
@@ -31,13 +31,14 @@ import { log } from './utils/log-util';
 const makeQueue: IQueue = require('queue');
 const { parse: parseDiff } = require('what-the-diff');
 
-export const labelMergedPR = async (
+export const labelClosedPR = async (
   context: Context,
   pr: PullsGetResponse,
   targetBranch: String,
+  change: PRChange,
 ) => {
   log(
-    'labelMergedPR',
+    'labelClosedPR',
     LogLevel.INFO,
     `Labeling original PRs for PR at #${pr.number}`,
   );
@@ -53,11 +54,14 @@ export const labelMergedPR = async (
   }
 
   for (const prNumber of backportNumbers) {
-    const labelToAdd = PRStatus.MERGED + targetBranch;
     const labelToRemove = PRStatus.IN_FLIGHT + targetBranch;
 
+    if (change === PRChange.MERGE) {
+      const labelToAdd = PRStatus.MERGED + targetBranch;
+      await labelUtils.addLabel(context, prNumber, [labelToAdd]);
+    }
+
     await labelUtils.removeLabel(context, prNumber, labelToRemove);
-    await labelUtils.addLabel(context, prNumber, [labelToAdd]);
   }
 };