From 2cb4d38f18e0d58c98b4a7cca6b509c234d56ea8 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Fri, 28 Jul 2023 16:14:59 -0700 Subject: [PATCH] Clear issue status on label removal from user (#560) * clear issue status on label removal from user --- src/api/github/org.ts | 26 +++++++++ src/brain/issueLabelHandler/followups.ts | 55 ++++++++++++++++++- src/brain/issueLabelHandler/index.test.ts | 67 +++++++++++++++++++++-- src/brain/issueLabelHandler/index.ts | 6 ++ test/github-orgs.yml | 8 +-- 5 files changed, 152 insertions(+), 10 deletions(-) diff --git a/src/api/github/org.ts b/src/api/github/org.ts index d3c2d5bb..81aa87fd 100644 --- 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 index d14502d1..115f5edc 100644 --- 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 index 1f4d23c7..c88e7938 100644 --- 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 index 469587c7..a1266328 100644 --- 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 index 3cf989e3..e7511522 100644 --- 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