diff --git a/README.md b/README.md index c07a2f1..24b9641 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,40 @@ useAssigneeGroups: false # - wip ``` +#### Add Members in Github Team(s) to Reviewers List + +Randomly add members in Github team(s) to the pull request based on number of reviewers listed using the `org/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) +reviewersInTeams: + - org/teamReviewerA + - org/teamReviewerB + +# Number of reviewers has no impact on Github teams +# Set 0 to add all the reviewers (default: 0) +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 +``` + ### Assign Author as Assignee Add the PR creator as assignee to the pull request. diff --git a/app.yml b/app.yml index c103c8f..0a4cb4a 100644 --- a/app.yml +++ b/app.yml @@ -102,7 +102,7 @@ default_permissions: # Organization members and teams. # https://developer.github.com/v3/apps/permissions/#permission-on-members - # members: read + members: read # View and manage users blocked by the organization. # https://developer.github.com/v3/apps/permissions/#permission-on-organization-user-blocking diff --git a/src/handler.ts b/src/handler.ts index 608ae78..02cd098 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -13,6 +13,61 @@ export interface Config { useAssigneeGroups: boolean reviewGroups: { [key: string]: string[] } assigneeGroups: { [key: string]: string[] } + reviewersInTeams: string[] +} + +export async function getTeamMembers( + context: Context, + { org, teamSlug }: { org: string; teamSlug: string } +): Promise { + let members: { login: string }[] = [] + + try { + members = ( + await context.github.teams.listMembersInOrg({ + org, + // eslint-disable-next-line @typescript-eslint/camelcase + team_slug: teamSlug + }) + ).data + } catch (error) { + context.log(error) + } + + if (members.length === 0) { + let team + + try { + team = ( + await context.github.teams.getByName({ + org, + // eslint-disable-next-line @typescript-eslint/camelcase + team_slug: teamSlug + }) + ).data + } catch (error) { + context.log(error) + throw new Error( + `Cannot fetch team id for team ${teamSlug} under organisation ${org}.` + ) + } + + try { + members = ( + await context.github.teams.listMembersLegacy({ + // eslint-disable-next-line @typescript-eslint/camelcase + team_id: team.id + }) + ).data + } catch (error) { + context.log(error) + throw new Error( + `Cannot fetch list of members in team ${teamSlug} under organisation ${org}.` + ) + } + } + + return members.map((member): string => member.login) } export async function handlePullRequest(context: Context): Promise { @@ -30,7 +85,8 @@ export async function handlePullRequest(context: Context): Promise { useAssigneeGroups, assigneeGroups, addReviewers, - addAssignees + addAssignees, + reviewersInTeams } = config if (skipKeywords && includesSkipKeywords(title, skipKeywords)) { @@ -54,6 +110,24 @@ export async function handlePullRequest(context: Context): Promise { ) } + if (reviewersInTeams && reviewersInTeams.length > 0) { + for (const team of reviewersInTeams) { + const result = /^([\w\-]+)\/([\w\-]+)$/.exec(team) + if (result === null) { + throw new Error( + 'Error in configuration file to expand a team reviewersInTeams must be a list of org/team_slug.' + ) + } + const reviewers = await getTeamMembers(context, { + org: result[1], + teamSlug: result[2] + }) + config.reviewers = Array.from( + new Set((config.reviewers || []).concat(reviewers)) + ) + } + } + const owner = context.payload.pull_request.user.login if (addReviewers) { diff --git a/test/handler.test.ts b/test/handler.test.ts index eb7dd13..9bccc6d 100644 --- a/test/handler.test.ts +++ b/test/handler.test.ts @@ -1,5 +1,5 @@ import { Context } from 'probot' -import { handlePullRequest } from '../src/handler' +import { handlePullRequest, getTeamMembers } from '../src/handler' describe('handlePullRequest', () => { let event: any @@ -94,7 +94,7 @@ describe('handlePullRequest', () => { } as any const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') - const createReviewRequestSpy = jest.spyOn( + const createReviewRequestSpy: any = jest.spyOn( context.github.pulls, 'createReviewRequest' ) @@ -131,7 +131,7 @@ describe('handlePullRequest', () => { } as any const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') - const createReviewRequestSpy = jest.spyOn( + const createReviewRequestSpy: any = jest.spyOn( context.github.pulls, 'createReviewRequest' ) @@ -156,7 +156,7 @@ describe('handlePullRequest', () => { context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => { }) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -219,7 +219,7 @@ describe('handlePullRequest', () => { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') const createReviewRequestSpy = jest.spyOn( context.github.pulls, 'createReviewRequest' @@ -258,7 +258,7 @@ describe('handlePullRequest', () => { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') const createReviewRequestSpy = jest.spyOn( context.github.pulls, 'createReviewRequest' @@ -293,8 +293,8 @@ describe('handlePullRequest', () => { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') - const createReviewRequestSpy = jest.spyOn( + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') + const createReviewRequestSpy: any = jest.spyOn( context.github.pulls, 'createReviewRequest' ) @@ -365,7 +365,7 @@ describe('handlePullRequest', () => { }) } as any - const spy = jest.spyOn(context.github.issues, 'addAssignees') + const spy: any = jest.spyOn(context.github.issues, 'addAssignees') await handlePullRequest(context) @@ -396,7 +396,7 @@ describe('handlePullRequest', () => { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const spy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const spy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') await handlePullRequest(context) @@ -463,7 +463,7 @@ describe('handlePullRequest', () => { context.github.pulls = { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const createReviewRequestSpy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const createReviewRequestSpy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) @@ -496,7 +496,7 @@ describe('handlePullRequest', () => { context.github.pulls = { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const createReviewRequestSpy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const createReviewRequestSpy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) @@ -532,7 +532,7 @@ describe('handlePullRequest', () => { context.github.pulls = { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const createReviewRequestSpy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const createReviewRequestSpy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) @@ -574,7 +574,7 @@ describe('handlePullRequest', () => { context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -614,7 +614,7 @@ describe('handlePullRequest', () => { context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => { }) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -653,7 +653,7 @@ describe('handlePullRequest', () => { context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -687,12 +687,12 @@ describe('handlePullRequest', () => { context.github.pulls = { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const createReviewRequestSpy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const createReviewRequestSpy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -730,12 +730,12 @@ describe('handlePullRequest', () => { context.github.pulls = { createReviewRequest: jest.fn().mockImplementation(async () => {}) } as any - const createReviewRequestSpy = jest.spyOn(context.github.pulls, 'createReviewRequest') + const createReviewRequestSpy: any = jest.spyOn(context.github.pulls, 'createReviewRequest') context.github.issues = { addAssignees: jest.fn().mockImplementation(async () => {}) } as any - const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees') + const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees') // GIVEN context.config = jest.fn().mockImplementation(async () => { @@ -766,4 +766,228 @@ describe('handlePullRequest', () => { expect(createReviewRequestSpy.mock.calls[0][0].reviewers[2]).toMatch(/group2-reviewer/) expect(createReviewRequestSpy.mock.calls[0][0].reviewers[3]).toMatch(/group3-reviewer/) }) + + test('should throw error if reviewersInTeams contain a single string without org or team_slug', async () => { + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + numberOfReviewers: 1, + reviewersInTeams: ['justice-league'] + } + }) + + try { + await handlePullRequest(context) + } catch (error) { + expect(error).toEqual(new Error("Error in configuration file to expand a team reviewersInTeams must be a list of org/team_slug.")) + } + }) + + test('should throw error if reviewerInTeams contains only the team_slug', async () => { + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + numberOfReviewers: 1, + reviewersInTeams: ['/teen_titans'] + } + }) + + try { + await handlePullRequest(context) + } catch (error) { + expect(error).toEqual(new Error("Error in configuration file to expand a team reviewersInTeams must be a list of org/team_slug.")) + } + }) + + test('adds reviewers to pull requests from list in reviewersInTeams', async () => { + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + numberOfReviewers: 1, + reviewersInTeams: ['justice-league/teen_titans'] + } + }) + + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: [{login: 'Robin'}, {login: 'Raven'}]})), + } as any + + context.github.pulls = { + // tslint:disable-next-line:no-empty + createReviewRequest: jest.fn().mockImplementation(async () => {}) + } as any + + const createReviewRequestSpy: any = jest.spyOn( + context.github.pulls, + 'createReviewRequest' + ) + + await handlePullRequest(context) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toHaveLength(1) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers[0]).toMatch(/[Robin, Raven]/) + }) + + test('adds reviewers to pull requests from list in reviewersInTeams and list of reviewers in config', async () => { + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + numberOfReviewers: 2, + reviewersInTeams: ['justice-league/teen_titans'], + reviewers: ['Robin'], + } + }) + + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: [{login: 'Raven'}]})), + } as any + + context.github.pulls = { + // tslint:disable-next-line:no-empty + createReviewRequest: jest.fn().mockImplementation(async () => {}) + } as any + + const createReviewRequestSpy: any = jest.spyOn( + context.github.pulls, + 'createReviewRequest' + ) + + await handlePullRequest(context) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toHaveLength(2) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toEqual(expect.arrayContaining(['Robin', 'Raven'])); + }) + + test('adds unique reviewers to pull requests from list in reviewersInTeams and list of reviewers in config', async () => { + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + numberOfReviewers: 2, + reviewersInTeams: ['justice-league/teen_titans'], + reviewers: ['Robin'], + } + }) + + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: [{login: 'Robin'}, {login: 'Raven'}]})), + } as any + + context.github.pulls = { + // tslint:disable-next-line:no-empty + createReviewRequest: jest.fn().mockImplementation(async () => {}) + } as any + + const createReviewRequestSpy: any = jest.spyOn( + context.github.pulls, + 'createReviewRequest' + ) + + await handlePullRequest(context) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toHaveLength(2) + expect(createReviewRequestSpy.mock.calls[0][0].reviewers).toEqual(expect.arrayContaining(['Robin', 'Raven'])); + }) +}) + +describe('handlePullRequest', () => { + let event: any + let context: Context + beforeEach(async () => { + event = { + id: '456', + name: 'pull_request', + payload: { + action: 'opened', + number: '1', + pull_request: { + number: '1', + title: 'test', + user: { + login: 'pr-creator' + } + }, + repository: { + name: 'auto-assign', + owner: { + login: 'itomtom' + } + } + }, + draft: false + } + + context = new Context(event, {} as any, {} as any) + context.log = jest.fn() as any + }) + + it('should return team members if listMembersInOrg is available', async () => { + context.github.teams = { + // tslint:disable-next-line:no-empty + listMembersInOrg: jest.fn().mockImplementation(async () => ({data: [{login: 'Robin'}, {login: 'Raven'}]})), + } as any + const spy = jest.spyOn(context, 'log') + + const result = await getTeamMembers(context, {org: 'justice-league', teamSlug: 'teen_titans'}) + expect(result).toEqual(['Robin', 'Raven']) + expect(spy).not.toBeCalled() + }) + + it('should throw an error if team id is not returned', async () => { + context.github.teams = { + // tslint:disable-next-line:no-empty + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: [{login: 'Robin'}, {login: 'Raven'}]})), + } as any + const spy = jest.spyOn(context.github.teams, 'listMembersLegacy') + + try { + await getTeamMembers(context, {org: 'justice-league', teamSlug: 'teen_titans'}) + } catch(error) { + expect(error).toEqual(new Error('Cannot fetch team id for team teen_titans under organisation justice-league.')) + } + expect(spy).not.toBeCalled() + }) + + it('should throw an error if team id is returned but the list of members in the team cannot be fetched', async () => { + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + } as any + const spy = jest.spyOn(context, 'log') + + try { + await getTeamMembers(context, {org: 'justice-league', teamSlug: 'teen_titans'}) + } catch(error) { + expect(error).toEqual(new Error('Cannot fetch list of members in team teen_titans under organisation justice-league.')) + } + expect(spy).toBeCalled() + }) + + it('should return list of members in the team', async () => { + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: [{login: 'Robin'}, {login: 'Raven'}]})), + } as any + const spy = jest.spyOn(context, 'log') + + const result = await getTeamMembers(context, {org: 'justice-league', teamSlug: 'teen_titans'}) + expect(result).toEqual(['Robin', 'Raven']) + expect(spy).toBeCalled() + }) + + it('should return an empty list of members if listMembersLegacy is empty', async () => { + context.github.teams = { + // tslint:disable-next-line:no-empty + getByName: jest.fn().mockImplementation(async () => ({data: {id: '1'}})), + listMembersLegacy: jest.fn().mockImplementation(async () => ({data: []})), + } as any + const spy = jest.spyOn(context, 'log') + + const result = await getTeamMembers(context, {org: 'justice-league', teamSlug: 'teen_titans'}) + expect(result).toEqual([]) + expect(spy).toBeCalled() + }) })