Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear issue status on label removal from user #560

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/api/github/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
43 changes: 43 additions & 0 deletions src/brain/issueLabelHandler/followups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,49 @@ export async function updateCommunityFollowups({
tx.finish();
}

export async function clearWaitingForProductOwnerStatus({
id,
payload,
...rest
}: EmitterWebhookEvent<'issues.unlabeled'>) {
const tx = Sentry.startTransaction({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eating our own 🐶 🥫 , nice!

op: 'brain',
name: 'issueLabelHandler.isNotWaitingForProductOwnerLabel',
});

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;
// Here label will never be undefined, ts is erroring here but is handled in the shouldSkip above
// @ts-ignore
const labelName = label.name;
hubertdeng123 marked this conversation as resolved.
Show resolved Hide resolved

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,
Expand Down
60 changes: 56 additions & 4 deletions src/brain/issueLabelHandler/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.';

Expand Down Expand Up @@ -152,6 +153,21 @@ describe('issueLabelHandler', function () {
);
}

async function removeLabel(label: string, repo?: string, sender?: string) {
await createGitHubEvent(
fastify,
'issues.unlabeled',
makePayload({
repo,
label,
sender,
state: undefined,
author_association: undefined,
})
);
org.api.issues.removeLabel(label);
}

async function addLabel(label: string, repo?: string, state?: string) {
await createGitHubEvent(
fastify,
Expand Down Expand Up @@ -546,17 +562,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();
Expand Down Expand Up @@ -748,5 +768,37 @@ 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 removeLabel(label);
hubertdeng123 marked this conversation as resolved.
Show resolved Hide resolved
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 removeLabel(label, undefined, 'getsentry-bot');
expect(clearProjectIssueFieldSpy).not.toHaveBeenCalled();
}
);
});
});
6 changes: 6 additions & 0 deletions src/brain/issueLabelHandler/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { githubEvents } from '@api/github';

import {
clearWaitingForProductOwnerStatus,
ensureOneWaitingForLabel,
updateCommunityFollowups,
} from './followups';
Expand Down Expand Up @@ -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);
}
8 changes: 4 additions & 4 deletions test/github-orgs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ getsentry:
privateKey: 'STUB'
installationId: -1
project:
nodeId: ''
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes assertions more accurate in tests to populate these with values

nodeId: 'projectNodeId'
fieldIds:
productArea: ''
status: ''
responseDue: ''
productArea: 'productAreaFieldId'
status: 'statusFieldId'
responseDue: 'responseDueFieldId'
repos:
withRouting:
- routing-repo
Expand Down