fix: check for conflicting backport labels on PR

spec/operations.spec.ts

@@ -22,8 +22,9 @@ const backportPRMergedEvent = require('./fixtures/backport_pull_request.merged.j
 const backportPROpenedEvent = require('./fixtures/backport_pull_request.opened.json');
 
 jest.mock('../src/utils', () => ({
-  tagBackportReviewers: jest.fn().mockReturnValue(Promise.resolve()),
+  tagBackportReviewers: jest.fn(),
   isSemverMinorPR: jest.fn().mockReturnValue(false),
+  needsSemverMinorBackportLabel: jest.fn().mockReturnValue(false),
 }));
 
 jest.mock('../src/utils/label-utils', () => ({

spec/utils.spec.ts

@@ -1,6 +1,11 @@
-import * as logUtils from '../src/utils/log-util';
 import { LogLevel } from '../src/enums';
-import { tagBackportReviewers } from '../src/utils';
+import {
+  needsSemverMinorBackportLabel,
+  tagBackportReviewers,
+} from '../src/utils';
+import * as utils from '../src/utils';
+import * as labelUtils from '../src/utils/label-utils';
+import * as logUtils from '../src/utils/log-util';
 
 const backportPROpenedEvent = require('./fixtures/backport_pull_request.opened.json');
 
@@ -74,4 +79,40 @@ describe('utils', () => {
       );
     });
   });
+
+  describe('needsSemverMinorBackportLabel()', () => {
+    const context = {
+      octokit: {},
+      repo: {},
+      ...backportPROpenedEvent,
+    };
+    const pr = context.payload.pull_request;
+    let labelsToAdd: string[];
+
+    beforeEach(() => {
+      labelsToAdd = [];
+    });
+
+    it('should should return true if PR is semver minor and not already approved', async () => {
+      jest.spyOn(labelUtils, 'labelExistsOnPR').mockResolvedValue(false);
+      jest.spyOn(utils, 'isSemverMinorPR').mockResolvedValue(true);
+
+      expect(await needsSemverMinorBackportLabel(context, pr)).toBe(true);
+    });
+
+    it('should return false if PR is not semver minor', async () => {
+      jest.spyOn(utils, 'isSemverMinorPR').mockResolvedValue(false);
+
+      expect(await needsSemverMinorBackportLabel(context, pr)).toBe(false);
+    });
+
+    it('should return false if PR is already approved', async () => {
+      jest.spyOn(utils, 'isSemverMinorPR').mockResolvedValue(true);
+
+      // Mocking labelExistsOnPR to return true (indicating backport is already approved)
+      jest.spyOn(labelUtils, 'labelExistsOnPR').mockResolvedValue(true);
+
+      expect(await needsSemverMinorBackportLabel(context, pr)).toBe(false);
+    });
+  });
 });

src/constants.ts

@@ -19,11 +19,11 @@ export const SEMVER_LABELS = {
   MAJOR: 'semver/major',
 };
 
-export const SKIP_CHECK_LABEL =
-  process.env.SKIP_CHECK_LABEL || 'backport-check-skip';
-
-export const BACKPORT_REQUESTED_LABEL =
-  process.env.BACKPORT_REQUESTED_LABEL || 'backport/requested 🗳';
+export const BACKPORT_REVIEW_LABELS = {
+  SKIP: process.env.SKIP_CHECK_LABEL || 'backport-check-skip',
+  REQUESTED: process.env.BACKPORT_REQUESTED_LABEL || 'backport/requested 🗳',
+  APPROVED: 'backport/approved ✅',
+};
 
 export const DEFAULT_BACKPORT_REVIEW_TEAM =
   process.env.DEFAULT_BACKPORT_REVIEW_TEAM;

src/index.ts

@@ -11,7 +11,11 @@ import {
   labelExistsOnPR,
   removeLabel,
 } from './utils/label-utils';
-import { CHECK_PREFIX, NO_BACKPORT_LABEL, SKIP_CHECK_LABEL } from './constants';
+import {
+  BACKPORT_REVIEW_LABELS,
+  CHECK_PREFIX,
+  NO_BACKPORT_LABEL,
+} from './constants';
 import { getEnvVar } from './utils/env-util';
 import { PRChange, PRStatus, BackportPurpose, CheckRunStatus } from './enums';
 import { Label } from '@octokit/webhooks-types';
@@ -251,9 +255,11 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
 
         // If a branch is targeting something that isn't main it might not be a backport;
         // allow for a label to skip backport validity check for these branches.
-        if (await labelExistsOnPR(context, pr.number, SKIP_CHECK_LABEL)) {
+        if (
+          await labelExistsOnPR(context, pr.number, BACKPORT_REVIEW_LABELS.SKIP)
+        ) {
           robot.log(
-            `#${pr.number} is labeled with ${SKIP_CHECK_LABEL} - skipping backport validation check`,
+            `#${pr.number} is labeled with ${BACKPORT_REVIEW_LABELS.SKIP} - skipping backport validation check`,
           );
           await updateBackportValidityCheck(context, checkRun, {
             title: 'Backport Check Skipped',

src/operations/update-manual-backport.ts

@@ -1,11 +1,7 @@
-import {
-  BACKPORT_LABEL,
-  BACKPORT_REQUESTED_LABEL,
-  SKIP_CHECK_LABEL,
-} from '../constants';
+import { BACKPORT_LABEL, BACKPORT_REVIEW_LABELS } from '../constants';
 import { PRChange, PRStatus, LogLevel } from '../enums';
 import { WebHookPRContext } from '../types';
-import { isSemverMinorPR, tagBackportReviewers } from '../utils';
+import { needsSemverMinorBackportLabel, tagBackportReviewers } from '../utils';
 import * as labelUtils from '../utils/label-utils';
 import { log } from '../utils/log-util';
 
@@ -59,7 +55,7 @@ export const updateManualBackport = async (
     const skipCheckLabelExists = await labelUtils.labelExistsOnPR(
       context,
       pr.number,
-      SKIP_CHECK_LABEL,
+      BACKPORT_REVIEW_LABELS.SKIP,
     );
     if (!skipCheckLabelExists) {
       newPRLabelsToAdd.push(BACKPORT_LABEL);
@@ -98,13 +94,14 @@ export const updateManualBackport = async (
       }
     }
 
-    if (await isSemverMinorPR(context, pr)) {
+    if (await needsSemverMinorBackportLabel(context, pr)) {
       log(
         'updateManualBackport',
         LogLevel.INFO,
-        `Determined that ${pr.number} is semver-minor`,
+        `Determined that ${pr.number} is semver-minor and needs backport review`,
       );
-      newPRLabelsToAdd.push(BACKPORT_REQUESTED_LABEL);
+
+      newPRLabelsToAdd.push(BACKPORT_REVIEW_LABELS.REQUESTED);
     }
 
     // We should only comment if there is not a previous existing comment

src/utils.ts

@@ -5,9 +5,9 @@ import simpleGit from 'simple-git';
 
 import queue from './Queue';
 import {
-  BACKPORT_REQUESTED_LABEL,
-  DEFAULT_BACKPORT_REVIEW_TEAM,
   BACKPORT_LABEL,
+  BACKPORT_REVIEW_LABELS,
+  DEFAULT_BACKPORT_REVIEW_TEAM,
 } from './constants';
 import { PRStatus, BackportPurpose, LogLevel, PRChange } from './enums';
 
@@ -640,13 +640,14 @@ export const backportImpl = async (
 
         const labelsToAdd = [BACKPORT_LABEL, `${targetBranch}`];
 
-        if (await isSemverMinorPR(context, pr)) {
+        if (await needsSemverMinorBackportLabel(context, pr)) {
           log(
             'backportImpl',
             LogLevel.INFO,
-            `Determined that ${pr.number} is semver-minor`,
+            `Determined that ${pr.number} is semver-minor and needs backport review`,
           );
-          labelsToAdd.push(BACKPORT_REQUESTED_LABEL);
+
+          labelsToAdd.push(BACKPORT_REVIEW_LABELS.REQUESTED);
         }
 
         const semverLabel = labelUtils.getSemverLabel(pr);
@@ -787,3 +788,21 @@ export const backportImpl = async (
     },
   );
 };
+
+export const needsSemverMinorBackportLabel = async (
+  context: SimpleWebHookRepoContext,
+  pr: WebHookPR,
+) => {
+  if (!(await isSemverMinorPR(context, pr))) {
+    return false;
+  }
+
+  // Check to see if backport has already been approved.
+  const hasApprovedLabel = await labelUtils.labelExistsOnPR(
+    context,
+    pr.number,
+    BACKPORT_REVIEW_LABELS.APPROVED,
+  );
+
+  return !hasApprovedLabel;
+};