Disable routing/triaging for external collaborators (#546)

* disable routing/triaging for external collaborators
getsentry · Jul 27, 2023 · bd3931c · bd3931c
1 parent 1dc70ea
commit bd3931c
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 15 deletions.
diff --git a/src/brain/issueLabelHandler/followups.ts b/src/brain/issueLabelHandler/followups.ts
@@ -31,7 +31,7 @@ function isWaitingForSupport(payload) {
   );
 }
 
-function isContractor(payload) {
+function isCommentFromCollaborator(payload) {
   // Contractors are outside collaborators on GitHub
   return payload.comment.author_association === 'COLLABORATOR';
 }
@@ -57,8 +57,8 @@ export async function updateCommunityFollowups({
   const reasonsToDoNothing = [
     isNotInARepoWeCareAboutForFollowups,
     isNotFromAnExternalOrGTMUser,
+    isCommentFromCollaborator,
     isWaitingForSupport,
-    isContractor,
     isPullRequest,
     isFromABot,
   ];

diff --git a/src/brain/issueLabelHandler/index.test.ts b/src/brain/issueLabelHandler/index.test.ts
@@ -100,12 +100,7 @@ describe('issueLabelHandler', function () {
     });
   }
 
-  function makePayload(
-    repo?: string,
-    label?: string,
-    sender?: string,
-    state?: string
-  ) {
+  function makePayload({ repo, label, sender, state, author_association }) {
     repo = repo || 'test-ttt-simple';
     state = state || 'open';
 
@@ -116,7 +111,7 @@ describe('issueLabelHandler', function () {
         name: repo,
         owner: { login: GETSENTRY_ORG.slug },
       },
-      issue: { state, labels }, // mix in labels stored in mock
+      issue: { state, labels, author_association }, // mix in labels stored in mock
     };
 
     if (label) {
@@ -125,27 +120,49 @@ describe('issueLabelHandler', function () {
     return payload;
   }
 
-  async function createIssue(repo?: string, username?: string) {
+  async function createIssue(
+    repo?: string,
+    username?: string,
+    author_association?: string
+  ) {
     await createGitHubEvent(
       fastify,
       'issues.opened',
-      makePayload(repo, undefined, username)
+      makePayload({
+        repo,
+        label: undefined,
+        sender: username,
+        state: undefined,
+        author_association,
+      })
     );
   }
 
   async function createPR(repo?: string, username?: string) {
     await createGitHubEvent(
       fastify,
       'pull_request.opened',
-      makePayload(repo, undefined, username)
+      makePayload({
+        repo,
+        label: undefined,
+        sender: username,
+        state: undefined,
+        author_association: undefined,
+      })
     );
   }
 
   async function addLabel(label: string, repo?: string, state?: string) {
     await createGitHubEvent(
       fastify,
       'issues.labeled',
-      makePayload(repo, label, undefined, state)
+      makePayload({
+        repo,
+        label,
+        sender: undefined,
+        state,
+        author_association: undefined,
+      })
     );
     org.api.issues.addLabels({ labels: [label] });
   }
@@ -162,7 +179,13 @@ describe('issueLabelHandler', function () {
       throw `Unknown user: '${username}'`;
     }
     const payload = {
-      ...makePayload(repo, undefined, username),
+      ...makePayload({
+        repo,
+        label: undefined,
+        sender: username,
+        state: undefined,
+        author_association: undefined,
+      }),
       comment: { author_association: membership },
     };
     if (isPR) {
@@ -249,6 +272,13 @@ describe('issueLabelHandler', function () {
       expect(addIssueToGlobalIssuesProjectSpy).toHaveBeenCalled();
     });
 
+    it('skips adding `Waiting for: Product Owner` for external collaborators', async function () {
+      await createIssue(undefined, 'External User', 'COLLABORATOR');
+      expectNotWaitingforProductOwner();
+      expectNoAdding();
+      expect(addIssueToGlobalIssuesProjectSpy).not.toHaveBeenCalled();
+    });
+
     it('skips adding `Waiting for: Product Owner` in untracked repos', async function () {
       await createIssue('other-repo');
       expectNotWaitingforProductOwner();
@@ -372,6 +402,14 @@ describe('issueLabelHandler', function () {
       expect(addIssueToGlobalIssuesProjectSpy).not.toHaveBeenCalled();
     });
 
+    it('skips adding `Waiting for: Support` for external collaborators', async function () {
+      await createIssue('sentry-docs', 'External User', 'COLLABORATOR');
+      expectNotWaitingForSupport();
+      expect(org.api.issues._labels).not.toContain(WAITING_FOR_SUPPORT_LABEL);
+      expect(org.api.issues._comments).toEqual([]);
+      expect(addIssueToGlobalIssuesProjectSpy).not.toHaveBeenCalled();
+    });
+
     it('skips adding `Waiting for: Support` in untracked repos', async function () {
       await createIssue('Pizza Sandwich');
       expectNotWaitingForSupport();

diff --git a/src/brain/issueLabelHandler/route.ts b/src/brain/issueLabelHandler/route.ts
@@ -10,6 +10,7 @@ import {
   WAITING_FOR_PRODUCT_OWNER_LABEL,
   WAITING_FOR_SUPPORT_LABEL,
 } from '@/config';
+import { isFromOutsideCollaborator } from '@/utils/isFromOutsideCollaborator';
 import { isNotFromAnExternalOrGTMUser } from '@utils/isNotFromAnExternalOrGTMUser';
 import { shouldSkip } from '@utils/shouldSkip';
 import { slugizeProductArea } from '@utils/slugizeProductArea';
@@ -61,6 +62,7 @@ export async function markWaitingForSupport({
     isNotInARepoWeCareAboutForRouting,
     isAlreadyWaitingForSupport,
     isNotFromAnExternalOrGTMUser,
+    isFromOutsideCollaborator,
   ];
   if (await shouldSkip(payload, org, reasonsToSkip)) {
     return;

diff --git a/src/brain/issueLabelHandler/triage.ts b/src/brain/issueLabelHandler/triage.ts
@@ -1,7 +1,11 @@
 import { EmitterWebhookEvent } from '@octokit/webhooks';
 import * as Sentry from '@sentry/node';
 
-import { GH_ORGS, WAITING_FOR_PRODUCT_OWNER_LABEL } from '@/config';
+import {
+  GH_ORGS,
+  WAITING_FOR_PRODUCT_OWNER_LABEL,
+} from '@/config';
+import { isFromOutsideCollaborator } from '@/utils/isFromOutsideCollaborator';
 import { isFromABot } from '@utils/isFromABot';
 import { isNotFromAnExternalOrGTMUser } from '@utils/isNotFromAnExternalOrGTMUser';
 import { shouldSkip } from '@utils/shouldSkip';
@@ -42,6 +46,7 @@ export async function markWaitingForProductOwner({
     isNotInARepoWeCareAboutForTriage,
     isAlreadyUntriaged,
     isNotFromAnExternalOrGTMUser,
+    isFromOutsideCollaborator,
   ];
   if (await shouldSkip(payload, org, reasonsToSkipTriage)) {
     return;

diff --git a/src/utils/isFromOutsideCollaborator.ts b/src/utils/isFromOutsideCollaborator.ts
@@ -0,0 +1,9 @@
+import { getOssUserType } from '@utils/getOssUserType';
+
+export async function isFromOutsideCollaborator(payload) {
+  const type = await getOssUserType(payload);
+  // Need to check user type to make sure issues created by GTM are routed/triaged
+  return (
+    type === 'external' && payload.issue.author_association === 'COLLABORATOR'
+  );
+}