Clear issue status on label removal from user (#560)

* clear issue status on label removal from user
getsentry · Jul 28, 2023 · 2cb4d38 · 2cb4d38
1 parent 77f544c
commit 2cb4d38
Show file tree

Hide file tree

Showing 5 changed files with 152 additions and 10 deletions.
diff --git a/src/api/github/org.ts b/src/api/github/org.ts
@@ -157,6 +157,32 @@ export class GitHubOrg {
     await this.sendGraphQuery(modifyProjectIssueFieldMutation, data);
   }
 
+  async clearProjectIssueField(
+    itemId: string,
+    projectFieldOption: string,
+    fieldId: string
+  ) {
+    const modifyProjectIssueFieldMutation = `mutation {
+      clearProjectV2ItemFieldValue(
+        input: {
+          projectId: "${this.project.nodeId}"
+          itemId: "${itemId}"
+          fieldId: "${fieldId}"
+        }
+      ) {
+        projectV2Item {
+          id
+        }
+      }
+    }`;
+    const data = {
+      itemId,
+      projectFieldOption,
+      fieldId,
+    };
+    await this.sendGraphQuery(modifyProjectIssueFieldMutation, data);
+  }
+
   async modifyDueByDate(
     itemId: string,
     projectFieldOption: string,

diff --git a/src/brain/issueLabelHandler/followups.ts b/src/brain/issueLabelHandler/followups.ts
@@ -105,6 +105,53 @@ export async function updateCommunityFollowups({
   tx.finish();
 }
 
+export async function clearWaitingForProductOwnerStatus({
+  id,
+  payload,
+  ...rest
+}: EmitterWebhookEvent<'issues.unlabeled'>) {
+  const tx = Sentry.startTransaction({
+    op: 'brain',
+    name: 'issueLabelHandler.clearWaitingForProductOwnerStatus',
+  });
+
+  const org = GH_ORGS.getForPayload(payload);
+
+  const reasonsToDoNothing = [
+    isNotInARepoWeCareAboutForFollowups,
+    isNotWaitingForLabel,
+    isFromABot,
+  ];
+  if (await shouldSkip(payload, org, reasonsToDoNothing)) {
+    return;
+  }
+
+  const { label } = payload;
+  const repo = payload.repository.name;
+  const issueNumber = payload.issue.number;
+  if (label?.name == null) {
+    Sentry.captureException(
+      `Webhook label name is undefined for ${repo}/${issueNumber}`
+    );
+    return;
+  }
+  const labelName = label.name;
+
+  const itemId: string = await org.addIssueToGlobalIssuesProject(
+    payload.issue.node_id,
+    repo,
+    issueNumber
+  );
+
+  await org.clearProjectIssueField(
+    itemId,
+    labelName,
+    org.project.fieldIds.status
+  );
+
+  tx.finish();
+}
+
 export async function ensureOneWaitingForLabel({
   id,
   payload,
@@ -128,8 +175,12 @@ export async function ensureOneWaitingForLabel({
   const { issue, label } = payload;
   const repo = payload.repository.name;
   const issueNumber = payload.issue.number;
-  // Here label will never be undefined, ts is erroring here but is handled in the shouldSkip above
-  // @ts-ignore
+  if (label?.name == null) {
+    Sentry.captureException(
+      `Webhook label name is undefined for ${repo}/${issueNumber}`
+    );
+    return;
+  }
   const labelName = label.name;
 
   const labelToRemove = issue.labels?.find(

diff --git a/src/brain/issueLabelHandler/index.test.ts b/src/brain/issueLabelHandler/index.test.ts
@@ -15,6 +15,7 @@ import { defaultErrorHandler, githubEvents } from '@api/github';
 import { MockOctokitError } from '@api/github/__mocks__/mockError';
 import * as businessHourFunctions from '@utils/businessHours';
 import { db } from '@utils/db';
+import * as isFromABot from '@utils/isFromABot';
 
 import { issueLabelHandler } from '.';
 
@@ -546,17 +547,21 @@ describe('issueLabelHandler', function () {
   describe('followups test cases', function () {
     let modifyProjectIssueFieldSpy,
       modifyDueByDateSpy,
-      addIssueToGlobalIssuesProjectSpy;
+      addIssueToGlobalIssuesProjectSpy,
+      clearProjectIssueFieldSpy;
     beforeAll(function () {
       modifyProjectIssueFieldSpy = jest
         .spyOn(org, 'modifyProjectIssueField')
         .mockImplementation(jest.fn());
-      modifyDueByDateSpy = jest
-        .spyOn(org, 'modifyDueByDate')
-        .mockImplementation(jest.fn());
       addIssueToGlobalIssuesProjectSpy = jest
         .spyOn(org, 'addIssueToGlobalIssuesProject')
         .mockReturnValue('itemId');
+      modifyDueByDateSpy = jest
+        .spyOn(org, 'modifyDueByDate')
+        .mockImplementation(jest.fn());
+      clearProjectIssueFieldSpy = jest
+        .spyOn(org, 'clearProjectIssueField')
+        .mockImplementation(jest.fn());
     });
     afterEach(function () {
       jest.clearAllMocks();
@@ -748,5 +753,59 @@ describe('issueLabelHandler', function () {
         org.project.fieldIds.responseDue
       );
     });
+
+    it.each([
+      WAITING_FOR_PRODUCT_OWNER_LABEL,
+      WAITING_FOR_SUPPORT_LABEL,
+      WAITING_FOR_COMMUNITY_LABEL,
+    ])(
+      "should clear project issue status field if user removes '%s'",
+      async function (label) {
+        await setupIssue();
+        await addLabel(label, 'routing-repo');
+        await createGitHubEvent(
+          fastify,
+          'issues.unlabeled',
+          makePayload({
+            repo: 'routing-repo',
+            label,
+            sender: undefined,
+            state: undefined,
+            author_association: undefined,
+          })
+        );
+        org.api.issues.removeLabel(label);
+        expect(clearProjectIssueFieldSpy).toHaveBeenLastCalledWith(
+          'itemId',
+          label,
+          org.project.fieldIds.status
+        );
+      }
+    );
+
+    it.each([
+      WAITING_FOR_PRODUCT_OWNER_LABEL,
+      WAITING_FOR_SUPPORT_LABEL,
+      WAITING_FOR_COMMUNITY_LABEL,
+    ])(
+      "should not clear project issue status field if bot removes '%s'",
+      async function (label) {
+        await setupIssue();
+        await addLabel(label, 'routing-repo');
+        await createGitHubEvent(
+          fastify,
+          'issues.unlabeled',
+          makePayload({
+            repo: 'routing-repo',
+            label,
+            sender: 'getsentry-bot',
+            state: undefined,
+            author_association: undefined,
+          })
+        );
+        org.api.issues.removeLabel(label);
+        expect(clearProjectIssueFieldSpy).not.toHaveBeenCalled();
+      }
+    );
   });
 });
diff --git a/src/brain/issueLabelHandler/index.ts b/src/brain/issueLabelHandler/index.ts
@@ -1,6 +1,7 @@
 import { githubEvents } from '@api/github';
 
 import {
+  clearWaitingForProductOwnerStatus,
   ensureOneWaitingForLabel,
   updateCommunityFollowups,
 } from './followups';
@@ -28,4 +29,9 @@ export async function issueLabelHandler() {
   githubEvents.on('issue_comment.created', updateCommunityFollowups);
   githubEvents.removeListener('issues.labeled', ensureOneWaitingForLabel);
   githubEvents.on('issues.labeled', ensureOneWaitingForLabel);
+  githubEvents.removeListener(
+    'issues.unlabeled',
+    clearWaitingForProductOwnerStatus
+  );
+  githubEvents.on('issues.unlabeled', clearWaitingForProductOwnerStatus);
 }
diff --git a/test/github-orgs.yml b/test/github-orgs.yml
@@ -4,11 +4,11 @@ getsentry:
     privateKey: 'STUB'
     installationId: -1
   project:
-    nodeId: ''
+    nodeId: 'projectNodeId'
     fieldIds:
-      productArea: ''
-      status: ''
-      responseDue: ''
+      productArea: 'productAreaFieldId'
+      status: 'statusFieldId'
+      responseDue: 'responseDueFieldId'
   repos:
     withRouting:
       - routing-repo