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),
+  handleSemverMinorBackportLabel: jest.fn(),
 }));
 
 jest.mock('../src/utils/label-utils', () => ({

spec/utils.spec.ts

@@ -1,6 +1,12 @@
-import * as logUtils from '../src/utils/log-util';
+import { BACKPORT_REVIEW_LABELS } from '../src/constants';
 import { LogLevel } from '../src/enums';
-import { tagBackportReviewers } from '../src/utils';
+import {
+  handleSemverMinorBackportLabel,
+  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 +80,60 @@ describe('utils', () => {
       );
     });
   });
+
+  describe('handleSemverMinorBackportLabel()', () => {
+    const context = {
+      octokit: {},
+      repo: {},
+      ...backportPROpenedEvent,
+    };
+    const pr = context.payload.pull_request;
+    let labelsToAdd: string[];
+
+    beforeEach(() => {
+      labelsToAdd = [];
+    });
+
+    it('should add BACKPORT_REQUESTED_LABEL if PR is semver minor and not already approved', async () => {
+      jest.spyOn(labelUtils, 'labelExistsOnPR').mockResolvedValue(false);
+      jest.spyOn(utils, 'isSemverMinorPR').mockResolvedValue(true);
+
+      await handleSemverMinorBackportLabel(
+        context,
+        pr,
+        labelsToAdd,
+        'testFunction',
+      );
+      expect(labelsToAdd).toContain(BACKPORT_REVIEW_LABELS.REQUESTED);
+    });
+
+    it('should not add BACKPORT_REVIEW_LABELS.REQUESTED if PR is not semver minor', async () => {
+      jest.spyOn(utils, 'isSemverMinorPR').mockResolvedValue(false);
+
+      await handleSemverMinorBackportLabel(
+        context,
+        pr,
+        labelsToAdd,
+        'testFunction',
+      );
+
+      expect(labelsToAdd).not.toContain(BACKPORT_REVIEW_LABELS.REQUESTED);
+    });
+
+    it('should not add BACKPORT_REQUESTED_LABEL 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);
+
+      await handleSemverMinorBackportLabel(
+        context,
+        pr,
+        labelsToAdd,
+        'testFunction',
+      );
+
+      expect(labelsToAdd).not.toContain(BACKPORT_REVIEW_LABELS.REQUESTED);
+    });
+  });
 });

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 { handleSemverMinorBackportLabel, 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,14 +94,12 @@ export const updateManualBackport = async (
       }
     }
 
-    if (await isSemverMinorPR(context, pr)) {
-      log(
-        'updateManualBackport',
-        LogLevel.INFO,
-        `Determined that ${pr.number} is semver-minor`,
-      );
-      newPRLabelsToAdd.push(BACKPORT_REQUESTED_LABEL);
-    }
+    await handleSemverMinorBackportLabel(
+      context,
+      pr,
+      newPRLabelsToAdd,
+      'updateManualBackport',
+    );
 
     // We should only comment if there is not a previous existing comment
     const commentBody = `@${pr.user.login} has manually backported this PR to "${pr.base.ref}", \

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,14 +640,12 @@ export const backportImpl = async (
 
         const labelsToAdd = [BACKPORT_LABEL, `${targetBranch}`];
 
-        if (await isSemverMinorPR(context, pr)) {
-          log(
-            'backportImpl',
-            LogLevel.INFO,
-            `Determined that ${pr.number} is semver-minor`,
-          );
-          labelsToAdd.push(BACKPORT_REQUESTED_LABEL);
-        }
+        await handleSemverMinorBackportLabel(
+          context,
+          pr,
+          labelsToAdd,
+          'backportImpl',
+        );
 
         const semverLabel = labelUtils.getSemverLabel(pr);
         if (semverLabel) {
@@ -787,3 +785,37 @@ export const backportImpl = async (
     },
   );
 };
+
+export const handleSemverMinorBackportLabel = async (
+  context: SimpleWebHookRepoContext,
+  pr: WebHookPR,
+  labelsToAdd: string[],
+  functionName: string,
+) => {
+  if (!(await isSemverMinorPR(context, pr))) {
+    return;
+  }
+
+  log(
+    functionName,
+    LogLevel.INFO,
+    `Determined that ${pr.number} is semver-minor`,
+  );
+  // Check to see if backport has already been approved.
+  const hasApprovedLabel = await labelUtils.labelExistsOnPR(
+    context,
+    pr.number,
+    BACKPORT_REVIEW_LABELS.APPROVED,
+  );
+
+  if (!hasApprovedLabel) {
+    labelsToAdd.push(BACKPORT_REVIEW_LABELS.REQUESTED);
+    return;
+  }
+
+  log(
+    functionName,
+    LogLevel.WARN,
+    `The backport for ${pr.number} has already been approved. No requested label added.`,
+  );
+};