feat: add support for separate team reviewers in configuration

README.md

@@ -29,8 +29,15 @@ reviewers:
   - reviewerB
   - reviewerC
 
+# A list of team reviewers to be added to pull requests (GitHub team slug)
+team_reviewers:
+  - org/teamReviewerA
+  - org/teamReviewerB
+  - teamReviewerC
+
 # A number of reviewers added to the pull request
 # Set 0 to add all the reviewers (default: 0)
+# Note: This only affects individual reviewers, not team reviewers
 numberOfReviewers: 0
 
 # A list of assignees, overrides reviewers if set

auto_assign_example.yml

@@ -0,0 +1,42 @@
+# Configuration example for auto-assign with separate team_reviewers
+
+# Set to true to add reviewers to pull requests
+addReviewers: true
+
+# Set to true to add assignees to pull requests
+addAssignees: false
+
+# A list of individual reviewers to be added to pull requests (GitHub user name)
+reviewers:
+  - reviewerA
+  - reviewerB
+  - reviewerC
+
+# A list of team reviewers to be added to pull requests (GitHub team slug)
+# Teams can be specified with or without the org prefix
+team_reviewers:
+  - frontend-team
+  - backend-team
+  - org/security-team
+
+# A number of individual reviewers added to the pull request
+# Set 0 to add all the reviewers (default: 0)
+# Note: This only affects individual reviewers, not team reviewers
+numberOfReviewers: 2
+
+# A list of assignees, overrides reviewers if set
+# assignees:
+#   - assigneeA
+
+# A number of assignees to add to the pull request
+# Set to 0 to add all of the assignees.
+# Uses numberOfReviewers if unset.
+# numberOfAssignees: 2
+
+# A list of keywords to be skipped the process that add reviewers if pull requests include it
+# skipKeywords:
+#   - wip
+
+# A list of users to be skipped by both the add reviewers and add assignees processes
+# skipUsers:
+#   - dependabot[bot]

src/handler.ts

@@ -9,6 +9,7 @@ export interface Config {
   addReviewers: boolean
   addAssignees: boolean
   reviewers: string[]
+  teamReviewers: string[]
   assignees: string[]
   numberOfAssignees: number
   numberOfReviewers: number

src/util.ts

@@ -62,7 +62,13 @@ export function chooseReviewers(
   reviewers: string[]
   team_reviewers: string[]
 } {
-  const { useReviewGroups, reviewGroups, numberOfReviewers, reviewers } = config
+  const {
+    useReviewGroups,
+    reviewGroups,
+    numberOfReviewers,
+    reviewers,
+    teamReviewers: team_reviewers,
+  } = config
   const useGroups: boolean =
     useReviewGroups && Object.keys(reviewGroups).length > 0
 
@@ -79,11 +85,21 @@ export function chooseReviewers(
     }
   }
 
-  const chosenReviewers = chooseUsers(reviewers, numberOfReviewers, owner)
+  // Filter out the owner from individual reviewers
+  const filteredReviewers = reviewers
+    ? reviewers.filter((reviewer) => reviewer !== owner)
+    : []
+
+  // Choose individual reviewers
+  const chosenIndividualReviewers =
+    numberOfReviewers > 0
+      ? _.sampleSize(filteredReviewers, numberOfReviewers)
+      : filteredReviewers
 
+  // Return both individual reviewers and team reviewers
   return {
-    reviewers: chosenReviewers.users,
-    team_reviewers: chosenReviewers.teams,
+    reviewers: chosenIndividualReviewers,
+    team_reviewers: team_reviewers || [],
   }
 }
 

test/handler.test.ts

@@ -142,7 +142,8 @@ describe('handlePullRequest', () => {
         addAssignees: false,
         addReviewers: true,
         numberOfReviewers: 0,
-        reviewers: ['/team_reviewer1'],
+        reviewers: [],
+        teamReviewers: ['team_reviewer1'],
         skipKeywords: ['wip'],
       }
     })
@@ -1008,4 +1009,73 @@ describe('handlePullRequest', () => {
     expect(requestReviewersSpy).not.toBeCalled()
     expect(addAssigneesSpy).not.toBeCalled()
   })
+
+  test('adds team reviewers from separate team_reviewers config', async () => {
+    // GIVEN
+    context.config = jest.fn().mockImplementation(async () => {
+      return {
+        addReviewers: true,
+        addAssignees: false,
+        reviewers: ['reviewer1', 'reviewer2'],
+        teamReviewers: ['team1', 'org/team2'],
+        numberOfReviewers: 0,
+      }
+    })
+
+    context.octokit.pulls = {
+      requestReviewers: jest.fn().mockImplementation(async () => ({})),
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    } as any
+
+    const requestReviewersSpy = jest.spyOn(
+      context.octokit.pulls,
+      'requestReviewers'
+    )
+
+    // WHEN
+    await handlePullRequest(context)
+
+    // THEN
+    expect(requestReviewersSpy).toBeCalledTimes(1)
+    expect(requestReviewersSpy.mock.calls[0][0]?.reviewers).toEqual([
+      'reviewer1',
+      'reviewer2',
+    ])
+    expect(requestReviewersSpy.mock.calls[0][0]?.team_reviewers).toEqual([
+      'team1',
+      'org/team2',
+    ])
+  })
+
+  test('handles empty team_reviewers config gracefully', async () => {
+    // GIVEN
+    context.config = jest.fn().mockImplementation(async () => {
+      return {
+        addReviewers: true,
+        addAssignees: false,
+        reviewers: ['reviewer1'],
+        numberOfReviewers: 0,
+      }
+    })
+
+    context.octokit.pulls = {
+      requestReviewers: jest.fn().mockImplementation(async () => ({})),
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    } as any
+
+    const requestReviewersSpy = jest.spyOn(
+      context.octokit.pulls,
+      'requestReviewers'
+    )
+
+    // WHEN
+    await handlePullRequest(context)
+
+    // THEN
+    expect(requestReviewersSpy).toBeCalledTimes(1)
+    expect(requestReviewersSpy.mock.calls[0][0]?.reviewers).toEqual([
+      'reviewer1',
+    ])
+    expect(requestReviewersSpy.mock.calls[0][0]?.team_reviewers).toEqual([])
+  })
 })

test/util.test.ts

@@ -1,4 +1,9 @@
-import { chooseUsers, chooseUsersFromGroups, includesSkipKeywords } from '../src/util'
+import {
+  chooseUsers,
+  chooseUsersFromGroups,
+  chooseReviewers,
+  includesSkipKeywords,
+} from '../src/util'
 
 describe('chooseUsers', () => {
   test('returns the reviewer list without the PR creator', () => {
@@ -98,13 +103,8 @@ describe('chooseUsersFromGroups', () => {
     // GIVEN
     const owner = 'owner'
     const reviewers = {
-      groupA: [
-        'owner',
-        'reviewer1'
-      ],
-      groupB: [
-        'reviewer2'
-      ]
+      groupA: ['owner', 'reviewer1'],
+      groupB: ['reviewer2'],
     }
     const numberOfReviewers = 1
 
@@ -119,12 +119,8 @@ describe('chooseUsersFromGroups', () => {
     // GIVEN
     const owner = 'owner'
     const reviewers = {
-      groupA: [
-        'owner'
-      ],
-      groupB: [
-        'reviewer2'
-      ]
+      groupA: ['owner'],
+      groupB: ['reviewer2'],
     }
     const numberOfReviewers = 1
 
@@ -140,20 +136,10 @@ describe('chooseUsersFromGroups', () => {
     // GIVEN
     const owner = 'owner'
     const reviewers = {
-      groupA: [
-        'owner',
-        'groupA-1',
-        'groupA-2'
-      ],
-      groupB: [
-        'groupB-1',
-        'groupB-2'
-      ],
+      groupA: ['owner', 'groupA-1', 'groupA-2'],
+      groupB: ['groupB-1', 'groupB-2'],
       groupC: [],
-      groupD: [
-        'groupD-1',
-        'groupD-2'
-      ]
+      groupD: ['groupD-1', 'groupD-2'],
     }
     const numberOfReviewers = 1
 
@@ -171,11 +157,8 @@ describe('chooseUsersFromGroups', () => {
     // GIVEN
     const owner = 'owner'
     const reviewers = {
-      groupA: [
-        'owner',
-        'reviewer1'
-      ],
-      groupB: []
+      groupA: ['owner', 'reviewer1'],
+      groupB: [],
     }
     const numberOfReviewers = 1
 
@@ -192,10 +175,7 @@ describe('chooseUsersFromGroups', () => {
     const owner = 'owner'
     const reviewers = {
       groupA: [],
-      groupB: [
-        'owner',
-        'reviewer1'
-      ]
+      groupB: ['owner', 'reviewer1'],
     }
     const numberOfReviewers = 2
 
@@ -212,7 +192,7 @@ describe('chooseUsersFromGroups', () => {
     const owner = 'owner'
     const reviewers = {
       groupA: [],
-      groupB: []
+      groupB: [],
     }
     const numberOfReviewers = 2
 
@@ -224,3 +204,86 @@ describe('chooseUsersFromGroups', () => {
     expect(list).toEqual([])
   })
 })
+
+describe('chooseReviewers', () => {
+  test('returns separate reviewers and team_reviewers arrays', () => {
+    // GIVEN
+    const owner = 'pr-creator'
+    const config = {
+      addReviewers: true,
+      addAssignees: false,
+      reviewers: ['reviewer1', 'reviewer2', 'pr-creator'],
+      teamReviewers: ['team1', 'org/team2'],
+      assignees: [],
+      numberOfAssignees: 0,
+      numberOfReviewers: 0,
+      skipKeywords: [],
+      useReviewGroups: false,
+      useAssigneeGroups: false,
+      reviewGroups: {},
+      assigneeGroups: {},
+      skipUsers: [],
+    }
+
+    // WHEN
+    const result = chooseReviewers(owner, config)
+
+    // THEN
+    expect(result.reviewers).toEqual(['reviewer1', 'reviewer2'])
+    expect(result.team_reviewers).toEqual(['team1', 'org/team2'])
+  })
+
+  test('handles empty team_reviewers config', () => {
+    // GIVEN
+    const owner = 'pr-creator'
+    const config = {
+      addReviewers: true,
+      addAssignees: false,
+      reviewers: ['reviewer1'],
+      teamReviewers: [],
+      assignees: [],
+      numberOfAssignees: 0,
+      numberOfReviewers: 0,
+      skipKeywords: [],
+      useReviewGroups: false,
+      useAssigneeGroups: false,
+      reviewGroups: {},
+      assigneeGroups: {},
+      skipUsers: [],
+    }
+
+    // WHEN
+    const result = chooseReviewers(owner, config)
+
+    // THEN
+    expect(result.reviewers).toEqual(['reviewer1'])
+    expect(result.team_reviewers).toEqual([])
+  })
+
+  test('respects numberOfReviewers for individual reviewers', () => {
+    // GIVEN
+    const owner = 'pr-creator'
+    const config = {
+      addReviewers: true,
+      addAssignees: false,
+      reviewers: ['reviewer1', 'reviewer2', 'reviewer3'],
+      teamReviewers: ['team1', 'team2'],
+      assignees: [],
+      numberOfAssignees: 0,
+      numberOfReviewers: 1,
+      skipKeywords: [],
+      useReviewGroups: false,
+      useAssigneeGroups: false,
+      reviewGroups: {},
+      assigneeGroups: {},
+      skipUsers: [],
+    }
+
+    // WHEN
+    const result = chooseReviewers(owner, config)
+
+    // THEN
+    expect(result.reviewers).toHaveLength(1)
+    expect(result.team_reviewers).toEqual(['team1', 'team2']) // Team reviewers are not affected by numberOfReviewers
+  })
+})