Add support for Github teams as reviewers (#120)

* Add support for Github teams as reviewers by prefixing them with slash

* Add support for org in team slug

* Fix typo

* Update README

README.md

@@ -51,6 +51,43 @@ numberOfReviewers: 0
 #   - wip
 ```
 
+#### Add Github Team to Single Reviewers List
+
+Add Github team to the pull request based on single reviewers list using the `org/team_slug` or `/team_slug` syntax.
+
+```yaml
+# 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 team reviewers to be added to pull requests (GitHub team slug)
+reviewers:
+  - org/teamReviewerA
+  - org/teamReviewerB
+  - /teamReviewerC
+
+# Number of reviewers has no impact on Github teams
+# Set 0 to add all the reviewers (default: 0)
+numberOfReviewers: 0
+
+# 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
+```
+
+>Note: Number of reviewers has currently no impact on Github teams and all teams will be added as reviewers.
+
 ### Multiple Reviewers List
 Add reviewers/assignees to the pull request based on multiple reviewers list.
 
@@ -109,7 +146,7 @@ useAssigneeGroups: false
 Add the PR creator as assignee to the pull request.
 
 ```yaml
-# Set to author to set pr creater as assignee
+# Set to author to set pr creator as assignee
 addAssignees: author
 ```
 

src/handler.ts

@@ -58,10 +58,13 @@ export async function handlePullRequest(context: Context): Promise<void> {
 
   if (addReviewers) {
     try {
-      const reviewers = chooseReviewers(owner, config)
+      // eslint-disable-next-line @typescript-eslint/camelcase
+      const { reviewers, team_reviewers } = chooseReviewers(owner, config)
 
-      if (reviewers.length > 0) {
-        const params = context.issue({ reviewers })
+      // eslint-disable-next-line @typescript-eslint/camelcase
+      if (reviewers.length > 0 || team_reviewers.length > 0) {
+        // eslint-disable-next-line @typescript-eslint/camelcase
+        const params = context.issue({ reviewers, team_reviewers })
         const result = await context.github.pulls.createReviewRequest(params)
         context.log(result)
       }

src/util.ts

@@ -1,23 +1,46 @@
 import _ from 'lodash'
 import { Config } from './handler'
 
+interface ChooseUsersResponse {
+  teams: string[]
+  users: string[]
+}
+
 export function chooseUsers(
   candidates: string[],
   desiredNumber: number,
   filterUser: string = ''
-): string[] {
-  const filteredCandidates = candidates.filter(
-    (reviewer: string): boolean => {
-      return reviewer !== filterUser
+): ChooseUsersResponse {
+  const { teams, users } = candidates.reduce(
+    (acc: ChooseUsersResponse, reviewer: string): ChooseUsersResponse => {
+      const separator = '/'
+      const isTeam = reviewer.includes(separator)
+      if (isTeam) {
+        const team = reviewer.split(separator)[1]
+        acc.teams = [...acc.teams, team]
+      } else if (reviewer !== filterUser) {
+        acc.users = [...acc.users, reviewer]
+      }
+      return acc
+    },
+    {
+      teams: [],
+      users: []
     }
   )
 
   // all-assign
   if (desiredNumber === 0) {
-    return filteredCandidates
+    return {
+      teams,
+      users
+    }
   }
 
-  return _.sampleSize(filteredCandidates, desiredNumber)
+  return {
+    teams,
+    users: _.sampleSize(users, desiredNumber)
+  }
 }
 
 export function chooseUsersFromGroups(
@@ -27,28 +50,43 @@ export function chooseUsersFromGroups(
 ): string[] {
   let users: string[] = []
   for (const group in groups) {
-    users = users.concat(chooseUsers(groups[group], desiredNumber, owner))
+    users = users.concat(chooseUsers(groups[group], desiredNumber, owner).users)
   }
   return users
 }
 
-export function chooseReviewers(owner: string, config: Config): string[] {
+export function chooseReviewers(
+  owner: string,
+  config: Config
+): {
+  reviewers: string[]
+  team_reviewers: string[]
+} {
   const { useReviewGroups, reviewGroups, numberOfReviewers, reviewers } = config
-  let chosenReviewers: string[] = []
   const useGroups: boolean =
     useReviewGroups && Object.keys(reviewGroups).length > 0
 
   if (useGroups) {
-    chosenReviewers = chooseUsersFromGroups(
+    const chosenReviewers = chooseUsersFromGroups(
       owner,
       reviewGroups,
       numberOfReviewers
     )
-  } else {
-    chosenReviewers = chooseUsers(reviewers, numberOfReviewers, owner)
+
+    return {
+      reviewers: chosenReviewers,
+      // eslint-disable-next-line @typescript-eslint/camelcase
+      team_reviewers: []
+    }
   }
 
-  return chosenReviewers
+  const chosenReviewers = chooseUsers(reviewers, numberOfReviewers, owner)
+
+  return {
+    reviewers: chosenReviewers.users,
+    // eslint-disable-next-line @typescript-eslint/camelcase
+    team_reviewers: chosenReviewers.teams
+  }
 }
 
 export function chooseAssignees(owner: string, config: Config): string[] {
@@ -85,7 +123,7 @@ export function chooseAssignees(owner: string, config: Config): string[] {
       candidates,
       numberOfAssignees || numberOfReviewers,
       owner
-    )
+    ).users
   }
 
   return chosenAssignees

test/handler.test.ts

@@ -103,11 +103,49 @@ describe('handlePullRequest', () => {
 
     expect(addAssigneesSpy).not.toBeCalled()
     expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toHaveLength(3)
+    expect(createReviewRequestSpy.mock.calls[0][0].team_reviewers).toHaveLength(0)
     expect(createReviewRequestSpy.mock.calls[0][0].reviewers[0]).toMatch(
       /reviewer/
     )
   })
 
+  test('adds team_reviewers to pull requests if the configuration is enabled, but no assignees', async () => {
+    context.config = jest.fn().mockImplementation(async () => {
+      return {
+        addAssignees: false,
+        addReviewers: true,
+        numberOfReviewers: 0,
+        reviewers: ['/team_reviewer1'],
+        skipKeywords: ['wip']
+      }
+    })
+
+    context.github.issues = {
+      // tslint:disable-next-line:no-empty
+      addAssignees: jest.fn().mockImplementation(async () => {})
+    } as any
+
+    context.github.pulls = {
+      // tslint:disable-next-line:no-empty
+      createReviewRequest: jest.fn().mockImplementation(async () => {})
+    } as any
+
+    const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees')
+    const createReviewRequestSpy = jest.spyOn(
+      context.github.pulls,
+      'createReviewRequest'
+    )
+
+    await handlePullRequest(context)
+
+    expect(addAssigneesSpy).not.toBeCalled()
+    expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toHaveLength(0)
+    expect(createReviewRequestSpy.mock.calls[0][0].team_reviewers).toHaveLength(1)
+    expect(createReviewRequestSpy.mock.calls[0][0].team_reviewers[0]).toMatch(
+      /team_reviewer/
+    )
+  })
+
   test('adds pr-creator as assignee if addAssignees is set to author', async () => {
     // MOCKS
     context.github.pulls = {

test/util.test.ts

@@ -3,12 +3,12 @@ import { chooseUsers, chooseUsersFromGroups, includesSkipKeywords } from '../src
 describe('chooseUsers', () => {
   test('returns the reviewer list without the PR creator', () => {
     const prCreator = 'pr-creator'
-    const reviewers = ['reviewer1','reviewer2', 'reviewer3', 'pr-creator']
+    const reviewers = ['reviewer1', 'reviewer2', 'reviewer3', 'pr-creator']
     const numberOfReviewers = 0
 
     const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
 
-    expect(list).toEqual(['reviewer1','reviewer2', 'reviewer3'])
+    expect(list.users).toEqual(['reviewer1', 'reviewer2', 'reviewer3'])
   })
 
   test('returns the only other reviewer', () => {
@@ -18,17 +18,39 @@ describe('chooseUsers', () => {
 
     const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
 
-    expect(list).toEqual(['reviewer1'])
+    expect(list.users).toEqual(['reviewer1'])
+  })
+
+  test('returns the team reviewers list when reviewers start with slash', () => {
+    const prCreator = 'pr-creator'
+    const reviewers = ['/team_reviewer1', 'pr-creator']
+    const numberOfReviewers = 1
+
+    const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
+
+    expect(list.teams).toEqual(['team_reviewer1'])
+    expect(list.users).toEqual([])
   })
 
-  test('returns the reviewer list if the number of reviewers is setted', () => {
+  test('returns the team reviewers list when reviewers contain slash', () => {
     const prCreator = 'pr-creator'
-    const reviewers = ['reviewer1','reviewer2', 'reviewer3', 'pr-creator']
+    const reviewers = ['org/team_reviewer1', 'pr-creator']
+    const numberOfReviewers = 1
+
+    const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
+
+    expect(list.teams).toEqual(['team_reviewer1'])
+    expect(list.users).toEqual([])
+  })
+
+  test('returns the reviewer list if the number of reviewers is set', () => {
+    const prCreator = 'pr-creator'
+    const reviewers = ['reviewer1', 'reviewer2', 'reviewer3', 'pr-creator']
     const numberOfReviewers = 2
 
     const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
 
-    expect(list.length).toEqual(2)
+    expect(list.users.length).toEqual(2)
   })
 
   test('returns empty array if the reviewer is the PR creator', () => {
@@ -38,7 +60,7 @@ describe('chooseUsers', () => {
 
     const list = chooseUsers(reviewers, numberOfReviewers, prCreator)
 
-    expect(list.length).toEqual(0)
+    expect(list.users.length).toEqual(0)
   })
 
   test('returns full reviewer array if not passing the user to filter out', () => {
@@ -47,7 +69,7 @@ describe('chooseUsers', () => {
 
     const list = chooseUsers(reviewers, numberOfReviewers)
 
-    expect(list).toEqual(expect.arrayContaining(['pr-creator']))
+    expect(list.users).toEqual(expect.arrayContaining(['pr-creator']))
   })
 })