diff --git a/README.md b/README.md index 760013e..0df41f5 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,8 @@ reviewers: designer_b: - lead_desinger # username - desinger_a # username + team:engineering-managers: + - engineers files: # Keys are glob expressions. diff --git a/dist/index.js b/dist/index.js index b80ea99..79b6dd1 100644 --- a/dist/index.js +++ b/dist/index.js @@ -16017,6 +16017,20 @@ async function assign_reviewers(reviewers) { }); } +// https://docs.github.com/en/rest/teams/members?apiVersion=2022-11-28#list-team-members +async function get_team_members(team) { + const context = get_context(); + const octokit = get_octokit(); + const org = context.payload.repository.owner.login; + + const { data } = await octokit.teams.listMembersInOrg({ + org, + team_slug: team, + }); + + return data?.map((member) => member.login); +} + /* Private */ let context_cache; @@ -16063,6 +16077,7 @@ module.exports = { fetch_changed_files, assign_reviewers, clear_cache, + get_team_members, }; @@ -16122,7 +16137,7 @@ async function run() { const reviewers_based_on_files = identify_reviewers_by_changed_files({ config, changed_files, excludes: [ author ] }); core.info('Identifying reviewers based on the author'); - const reviewers_based_on_author = identify_reviewers_by_author({ config, author }); + const reviewers_based_on_author = await identify_reviewers_by_author({ config, author }); core.info('Adding other group members to reviewers if group assignment feature is on'); const reviewers_from_same_teams = fetch_other_group_members({ config, author }); @@ -16170,6 +16185,7 @@ if (process.env.NODE_ENV !== 'automated-testing') { const core = __nccwpck_require__(2186); const minimatch = __nccwpck_require__(3973); const sample_size = __nccwpck_require__(2199); +const github = __nccwpck_require__(8396); // Don't destructure this object to stub with sinon in tests function fetch_other_group_members({ author, config }) { const DEFAULT_OPTIONS = { @@ -16232,21 +16248,46 @@ function identify_reviewers_by_changed_files({ config, changed_files, excludes = return [ ...new Set(individuals) ].filter((reviewer) => !excludes.includes(reviewer)); } -function identify_reviewers_by_author({ config, 'author': specified_author }) { +async function identify_reviewers_by_author({ config, 'author': specified_author }) { if (!(config.reviewers && config.reviewers.per_author)) { core.info('"per_author" is not set; returning no reviewers for the author.'); return []; } + // async behavior must happen first + const team_member_promises = await Object.keys(config.reviewers.per_author).map(async (author) => { + if (author.startsWith('team:')) { + const team = author.replace('team:', ''); + + return { + author, + members: await github.get_team_members(team) || [], + }; + } + + return { + author, + members: [], + }; + }); + const team_members = await Promise.all(team_member_promises); + // More than one author can be matched because groups are set as authors const matching_authors = Object.keys(config.reviewers.per_author).filter((author) => { if (author === specified_author) { return true; } + if (author.startsWith('team:')) { + const { members } = team_members.find((team) => team.author === author); + if (members.includes(specified_author)) { + return true; + } + } + const individuals_in_author_setting = replace_groups_with_individuals({ reviewers: [ author ], config }); - if (individuals_in_author_setting.includes(specified_author)) { + if (individuals_in_author_setting?.includes(specified_author)) { return true; } diff --git a/src/github.js b/src/github.js index 11ff8ad..377c4a9 100644 --- a/src/github.js +++ b/src/github.js @@ -112,6 +112,20 @@ async function assign_reviewers(reviewers) { }); } +// https://docs.github.com/en/rest/teams/members?apiVersion=2022-11-28#list-team-members +async function get_team_members(team) { + const context = get_context(); + const octokit = get_octokit(); + const org = context.payload.repository.owner.login; + + const { data } = await octokit.teams.listMembersInOrg({ + org, + team_slug: team, + }); + + return data?.map((member) => member.login); +} + /* Private */ let context_cache; @@ -158,4 +172,5 @@ module.exports = { fetch_changed_files, assign_reviewers, clear_cache, + get_team_members, }; diff --git a/src/index.js b/src/index.js index 5d23032..d72319a 100644 --- a/src/index.js +++ b/src/index.js @@ -48,7 +48,7 @@ async function run() { const reviewers_based_on_files = identify_reviewers_by_changed_files({ config, changed_files, excludes: [ author ] }); core.info('Identifying reviewers based on the author'); - const reviewers_based_on_author = identify_reviewers_by_author({ config, author }); + const reviewers_based_on_author = await identify_reviewers_by_author({ config, author }); core.info('Adding other group members to reviewers if group assignment feature is on'); const reviewers_from_same_teams = fetch_other_group_members({ config, author }); diff --git a/src/reviewer.js b/src/reviewer.js index 086ef66..569ba2e 100644 --- a/src/reviewer.js +++ b/src/reviewer.js @@ -3,6 +3,7 @@ const core = require('@actions/core'); const minimatch = require('minimatch'); const sample_size = require('lodash/sampleSize'); +const github = require('./github'); // Don't destructure this object to stub with sinon in tests function fetch_other_group_members({ author, config }) { const DEFAULT_OPTIONS = { @@ -65,21 +66,46 @@ function identify_reviewers_by_changed_files({ config, changed_files, excludes = return [ ...new Set(individuals) ].filter((reviewer) => !excludes.includes(reviewer)); } -function identify_reviewers_by_author({ config, 'author': specified_author }) { +async function identify_reviewers_by_author({ config, 'author': specified_author }) { if (!(config.reviewers && config.reviewers.per_author)) { core.info('"per_author" is not set; returning no reviewers for the author.'); return []; } + // async behavior must happen first + const team_member_promises = await Object.keys(config.reviewers.per_author).map(async (author) => { + if (author.startsWith('team:')) { + const team = author.replace('team:', ''); + + return { + author, + members: await github.get_team_members(team) || [], + }; + } + + return { + author, + members: [], + }; + }); + const team_members = await Promise.all(team_member_promises); + // More than one author can be matched because groups are set as authors const matching_authors = Object.keys(config.reviewers.per_author).filter((author) => { if (author === specified_author) { return true; } + if (author.startsWith('team:')) { + const { members } = team_members.find((team) => team.author === author); + if (members.includes(specified_author)) { + return true; + } + } + const individuals_in_author_setting = replace_groups_with_individuals({ reviewers: [ author ], config }); - if (individuals_in_author_setting.includes(specified_author)) { + if (individuals_in_author_setting?.includes(specified_author)) { return true; } diff --git a/test/github.test.js b/test/github.test.js index 7a21ca5..a770d91 100644 --- a/test/github.test.js +++ b/test/github.test.js @@ -14,6 +14,7 @@ const { fetch_changed_files, assign_reviewers, clear_cache, + get_team_members, } = require('../src/github'); describe('github', function() { @@ -155,4 +156,38 @@ describe('github', function() { }); }); }); + + describe('get_team_members()', function() { + const stub = sinon.stub(); + const octokit = { + teams: { + listMembersInOrg: stub, + }, + }; + + beforeEach(function() { + github.getOctokit.resetBehavior(); + github.getOctokit.returns(octokit); + }); + + it('gets team members', async function() { + stub.returns({ + data: [ + { login: 'bowser' }, + { login: 'king-boo' }, + { login: 'goomboss' }, + ], + }); + + const team = 'koopa-troop'; + const actual = await get_team_members(team); + + expect(stub.calledOnce).to.be.true; + expect(stub.lastCall.args[0]).to.deep.equal({ + org: 'necojackarc', + team_slug: 'koopa-troop', + }); + expect(actual).to.deep.equal([ 'bowser', 'king-boo', 'goomboss' ]); + }); + }); }); diff --git a/test/reviewer.test.js b/test/reviewer.test.js index 83facc1..1b5cf21 100644 --- a/test/reviewer.test.js +++ b/test/reviewer.test.js @@ -9,6 +9,8 @@ const { randomly_pick_reviewers, } = require('../src/reviewer'); const { expect } = require('chai'); +const github = require('../src/github'); +const sinon = require('sinon'); describe('reviewer', function() { describe('fetch_other_group_members()', function() { @@ -136,36 +138,45 @@ describe('reviewer', function() { designers: [ 'mario', 'princess-peach', 'princess-daisy' ], }, per_author: { - engineers: [ 'engineers', 'dr-mario' ], - designers: [ 'designers' ], - yoshi: [ 'mario', 'luige' ], + 'engineers': [ 'engineers', 'dr-mario' ], + 'designers': [ 'designers' ], + 'yoshi': [ 'mario', 'luige' ], + 'team:koopa-troop': [ 'mario' ], }, }, }; - it('returns nothing when config does not have a "per-author" key', function() { + const stub = sinon.stub(github, 'get_team_members'); + stub.withArgs('koopa-troop').returns([ 'bowser', 'king-boo', 'goomboss' ]); + + it('returns nothing when config does not have a "per-author" key', async function() { const author = 'THIS DOES NOT MATTER'; - expect(identify_reviewers_by_author({ config: { reviewers: {} }, author })).to.deep.equal([]); + expect(await identify_reviewers_by_author({ config: { reviewers: {} }, author })).to.deep.equal([]); }); - it('returns nothing when the author does not exist in the "per-author" settings', function() { + it('returns nothing when the author does not exist in the "per-author" settings', async function() { const author = 'toad'; - expect(identify_reviewers_by_author({ config, author })).to.deep.equal([]); + expect(await identify_reviewers_by_author({ config, author })).to.deep.equal([]); }); - it('returns the reviewers for the author', function() { + it('returns the reviewers for the author', async function() { const author = 'yoshi'; - expect(identify_reviewers_by_author({ config, author })).to.have.members([ 'mario', 'luige' ]); + expect(await identify_reviewers_by_author({ config, author })).to.have.members([ 'mario', 'luige' ]); }); - it('works when a author setting is specified with a group', function() { + it('works when a author setting is specified with a group', async function() { const author = 'luigi'; - expect(identify_reviewers_by_author({ config, author })).to.have.members([ 'mario', 'wario', 'waluigi', 'dr-mario' ]); + expect(await identify_reviewers_by_author({ config, author })).to.have.members([ 'mario', 'wario', 'waluigi', 'dr-mario' ]); }); - it('works when the author belongs to more than one group', function() { + it('works when the author belongs to more than one group', async function() { const author = 'mario'; - expect(identify_reviewers_by_author({ config, author })).to.have.members([ 'dr-mario', 'luigi', 'wario', 'waluigi', 'princess-peach', 'princess-daisy' ]); + expect(await identify_reviewers_by_author({ config, author })).to.have.members([ 'dr-mario', 'luigi', 'wario', 'waluigi', 'princess-peach', 'princess-daisy' ]); + }); + + it('works when gh team slug used for auther', async function() { + const author = 'bowser'; + expect(await identify_reviewers_by_author({ config, author })).to.have.members([ 'mario' ]); }); });