Skip to content

Commit

Permalink
Add initial waiting for label logic (#446)
Browse files Browse the repository at this point in the history
* add initial waiting for label logic

* use label constants in test

* clean up tests
  • Loading branch information
hubertdeng123 authored May 8, 2023
1 parent e808090 commit 57ee6e7
Show file tree
Hide file tree
Showing 8 changed files with 510 additions and 50 deletions.
106 changes: 106 additions & 0 deletions src/brain/issueLabelHandler/followups.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import { EmitterWebhookEvent } from '@octokit/webhooks';
import * as Sentry from '@sentry/node';

import { isFromABot } from '@utils/isFromABot';

const REPOS_TO_TRACK_FOR_TRIAGE = new Set(['sentry', 'sentry-docs']);
import { ClientType } from '@/api/github/clientType';
import {
isNotFromAnExternalOrGTMUser,
shouldSkip,
} from '@/brain/issueLabelHandler/helpers';
import {
WAITING_FOR_COMMUNITY_LABEL,
WAITING_FOR_LABEL_PREFIX,
WAITING_FOR_PRODUCT_OWNER_LABEL,
} from '@/config';
import { getClient } from '@api/github/getClient';

function isNotInARepoWeCareAboutForTriage(payload) {
return !REPOS_TO_TRACK_FOR_TRIAGE.has(payload.repository.name);
}

function isNotWaitingForCommunity(payload) {
const { issue } = payload;
return !issue?.labels.some(
({ name }) => name === WAITING_FOR_COMMUNITY_LABEL
);
}

// Markers of State

export async function updateCommunityFollowups({
id,
payload,
...rest
}: EmitterWebhookEvent<'issue_comment.created'>) {
const tx = Sentry.startTransaction({
op: 'brain',
name: 'issueLabelHandler.updateCommunityFollowups',
});

const reasonsToDoNothing = [
isNotInARepoWeCareAboutForTriage,
isNotFromAnExternalOrGTMUser,
isNotWaitingForCommunity,
isFromABot,
];
if (await shouldSkip(payload, reasonsToDoNothing)) {
return;
}

const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);

await octokit.issues.removeLabel({
owner,
repo: payload.repository.name,
issue_number: payload.issue.number,
name: WAITING_FOR_COMMUNITY_LABEL,
});

await octokit.issues.addLabels({
owner,
repo: payload.repository.name,
issue_number: payload.issue.number,
labels: [WAITING_FOR_PRODUCT_OWNER_LABEL],
});

tx.finish();
}

export async function ensureOneWaitingForLabel({
id,
payload,
...rest
}: EmitterWebhookEvent<'issues.labeled'>) {
const tx = Sentry.startTransaction({
op: 'brain',
name: 'issueLabelHandler.ensureOneWaitingForLabel',
});

const reasonsToDoNothing = [isFromABot];
if (await shouldSkip(payload, reasonsToDoNothing)) {
return;
}

const { issue, label } = payload;
const owner = payload.repository.owner.login;
const octokit = await getClient(ClientType.App, owner);

if (label?.name.startsWith(WAITING_FOR_LABEL_PREFIX)) {
const labelToRemove =
issue.labels?.find(
({ name }) =>
name.startsWith(WAITING_FOR_LABEL_PREFIX) && name != label?.name
)?.name || '';
await octokit.issues.removeLabel({
owner: owner,
repo: payload.repository.name,
issue_number: payload.issue.number,
name: labelToRemove,
});
}

tx.finish();
}
22 changes: 22 additions & 0 deletions src/brain/issueLabelHandler/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { getOssUserType } from '@utils/getOssUserType';

// Validation Helpers

export async function shouldSkip(payload, reasonsToSkip) {
// Could do Promise-based async here, but that was getting complicated[1] and
// there's not really a performance concern (famous last words).
//
// [1] https://github.com/getsentry/eng-pipes/pull/212#discussion_r657365585

for (const skipIf of reasonsToSkip) {
if (await skipIf(payload)) {
return true;
}
}
return false;
}

export async function isNotFromAnExternalOrGTMUser(payload) {
const type = await getOssUserType(payload);
return !(type === 'external' || type === 'gtm');
}
111 changes: 108 additions & 3 deletions src/brain/issueLabelHandler/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { createGitHubEvent } from '@test/utils/github';

import * as helpers from '@/brain/issueLabelHandler/helpers';
import { getLabelsTable, slackHandler } from '@/brain/issueNotifier';
import { buildServer } from '@/buildServer';
import { UNROUTED_LABEL, UNTRIAGED_LABEL } from '@/config';
import {
UNROUTED_LABEL,
UNTRIAGED_LABEL,
WAITING_FOR_COMMUNITY_LABEL,
WAITING_FOR_PRODUCT_OWNER_LABEL,
WAITING_FOR_SUPPORT_LABEL,
} from '@/config';
import { Fastify } from '@/types';
import { defaultErrorHandler, githubEvents } from '@api/github';
import { MockOctokitError } from '@api/github/__mocks__/mockError';
Expand Down Expand Up @@ -135,6 +142,15 @@ describe('issueLabelHandler', function () {
octokit.issues.addLabels({ labels: [label] });
}

async function addComment(repo?: string, username?: string) {
await createGitHubEvent(
fastify,
// @ts-expect-error
'issue_comment.created',
makePayload(repo, undefined, username)
);
}

// Expectations

function expectUnrouted() {
Expand Down Expand Up @@ -260,17 +276,19 @@ describe('issueLabelHandler', function () {
});

describe('[routing](https://open.sentry.io/triage/#2-route) test cases', function () {
it('adds `Status: Unrouted` to new issues', async function () {
it('adds `Status: Unrouted` and `Waiting for: Support` to new issues', async function () {
await createIssue('sentry-docs');
expectUnrouted();
expect(octokit.issues._labels).toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._comments).toEqual([
'Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2022-12-20T00:00:00.000Z>Monday, December 19th at 4:00 pm</time> (sfo)**. ⏲️',
]);
});

it('adds `Status: Unrouted` for GTM users', async function () {
it('adds `Status: Unrouted` and `Waiting for: Support` for GTM users', async function () {
await createIssue('sentry-docs', 'Troi');
expectUnrouted();
expect(octokit.issues._labels).toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._comments).toEqual([
'Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2022-12-20T00:00:00.000Z>Monday, December 19th at 4:00 pm</time> (sfo)**. ⏲️',
]);
Expand All @@ -279,12 +297,14 @@ describe('issueLabelHandler', function () {
it('skips adding `Status: Unrouted` for internal users', async function () {
await createIssue('sentry-docs', 'Picard');
expectRouted();
expect(octokit.issues._labels).not.toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._comments).toEqual([]);
});

it('skips adding `Status: Unrouted` in untracked repos', async function () {
await createIssue('Pizza Sandwich');
expectRouted();
expect(octokit.issues._labels).not.toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._comments).toEqual([]);
});

Expand All @@ -293,6 +313,8 @@ describe('issueLabelHandler', function () {
await addLabel('Product Area: Test', 'sentry-docs');
expectUntriaged();
expectRouted();
expect(octokit.issues._labels).not.toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._labels).toContain(WAITING_FOR_PRODUCT_OWNER_LABEL);
expect(octokit.issues._comments).toEqual([
'Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2022-12-20T00:00:00.000Z>Monday, December 19th at 4:00 pm</time> (sfo)**. ⏲️',
'Routing to @getsentry/product-owners-test for [triage](https://develop.sentry.dev/processing-tickets/#3-triage), due by **<time datetime=2022-12-21T00:00:00.000Z>Tuesday, December 20th at 4:00 pm</time> (sfo)**. ⏲️',
Expand All @@ -303,6 +325,7 @@ describe('issueLabelHandler', function () {
await createIssue('sentry-docs');
await addLabel('Status: Needs More Information', 'sentry-docs');
expectUnrouted();
expect(octokit.issues._labels).toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._comments).toEqual([
'Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2022-12-20T00:00:00.000Z>Monday, December 19th at 4:00 pm</time> (sfo)**. ⏲️',
]);
Expand All @@ -313,6 +336,8 @@ describe('issueLabelHandler', function () {
await addLabel('Product Area: Does Not Exist', 'sentry-docs');
expectUntriaged();
expectRouted();
expect(octokit.issues._labels).not.toContain(WAITING_FOR_SUPPORT_LABEL);
expect(octokit.issues._labels).toContain(WAITING_FOR_PRODUCT_OWNER_LABEL);
expect(octokit.issues._comments).toEqual([
'Assigning to @getsentry/support for [routing](https://open.sentry.io/triage/#2-route), due by **<time datetime=2022-12-20T00:00:00.000Z>Monday, December 19th at 4:00 pm</time> (sfo)**. ⏲️',
'Failed to route for Product Area: Does Not Exist. Defaulting to @getsentry/open-source for [triage](https://develop.sentry.dev/processing-tickets/#3-triage), due by **<time datetime=2022-12-21T00:00:00.000Z>Tuesday, December 20th at 4:00 pm</time> (sfo)**. ⏲️',
Expand Down Expand Up @@ -405,4 +430,84 @@ describe('issueLabelHandler', function () {
]);
});
});

describe('followups test cases', function () {
const setupIssue = async () => {
await createIssue('sentry-docs');
await addLabel('Product Area: Test', 'sentry-docs');
};

it('should remove `Waiting for: Product Owner` label when another `Waiting for: *` label is added', async function () {
await setupIssue();
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
WAITING_FOR_PRODUCT_OWNER_LABEL,
'Product Area: Test',
])
);
await addLabel('Waiting for: Community', 'sentry-docs');
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
'Product Area: Test',
WAITING_FOR_COMMUNITY_LABEL,
])
);
await addLabel('Waiting for: Support', 'sentry-docs');
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
'Product Area: Test',
WAITING_FOR_SUPPORT_LABEL,
])
);
});

it('should not add `Waiting for: Product Owner` label when product owner/GTM member comments and issue is waiting for community', async function () {
await setupIssue();
await addLabel(WAITING_FOR_COMMUNITY_LABEL, 'sentry-docs');
jest.spyOn(helpers, 'isNotFromAnExternalOrGTMUser').mockReturnValue(true);
await addComment('sentry-docs', 'Picard');
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
'Product Area: Test',
WAITING_FOR_COMMUNITY_LABEL,
])
);
});

it('should add `Waiting for: Product Owner` label when community member comments and issue is waiting for community', async function () {
await setupIssue();
await addLabel(WAITING_FOR_COMMUNITY_LABEL, 'sentry-docs');
jest
.spyOn(helpers, 'isNotFromAnExternalOrGTMUser')
.mockReturnValue(false);
await addComment('sentry-docs', 'Picard');
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
'Product Area: Test',
WAITING_FOR_PRODUCT_OWNER_LABEL,
])
);
});

it('should not modify labels when community member comments and issue is waiting for product owner', async function () {
await setupIssue();
await addLabel(WAITING_FOR_PRODUCT_OWNER_LABEL, 'sentry-docs');
jest
.spyOn(helpers, 'isNotFromAnExternalOrGTMUser')
.mockReturnValue(false);
await addComment('sentry-docs', 'Picard');
expect(octokit.issues._labels).toEqual(
new Set([
UNTRIAGED_LABEL,
'Product Area: Test',
WAITING_FOR_PRODUCT_OWNER_LABEL,
])
);
});
});
});
11 changes: 11 additions & 0 deletions src/brain/issueLabelHandler/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { githubEvents } from '@api/github';

import {
ensureOneWaitingForLabel,
updateCommunityFollowups,
} from './followups';
import { markRouted, markUnrouted } from './route';
import { markTriaged, markUntriaged } from './triage';

Expand All @@ -14,4 +18,11 @@ export async function issueLabelHandler() {
githubEvents.on('issues.opened', markUnrouted);
githubEvents.removeListener('issues.labeled', markRouted);
githubEvents.on('issues.labeled', markRouted);
githubEvents.removeListener(
'issue_comment.created',
updateCommunityFollowups
);
githubEvents.on('issue_comment.created', updateCommunityFollowups);
githubEvents.removeListener('issues.labeled', ensureOneWaitingForLabel);
githubEvents.on('issues.labeled', ensureOneWaitingForLabel);
}
Loading

0 comments on commit 57ee6e7

Please sign in to comment.