diff --git a/dist/index.js b/dist/index.js index a0e90b7..ae5047a 100644 --- a/dist/index.js +++ b/dist/index.js @@ -16001,6 +16001,27 @@ async function fetch_changed_files() { return changed_files; } +async function fetch_current_reviewers() { + const context = get_context(); + const octokit = get_octokit(); + + const current_reviewers = []; + + // API docs + // Generated octokit methods: https://github.com/octokit/plugin-rest-endpoint-methods.js/blob/main/src/generated/endpoints.ts + // Available Rest APIs: https://docs.github.com/en/rest/pulls/review-requests?apiVersion=2022-11-28#get-all-requested-reviewers-for-a-pull-request + const { data: response_body } = await octokit.pulls.listRequestedReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + }); + + current_reviewers.push(...response_body.users.map((user) => user.login)); + current_reviewers.push(...response_body.teams.map((team) => 'team:'.concat(team.slug))); + + return current_reviewers; +} + async function assign_reviewers(reviewers) { const context = get_context(); const octokit = get_octokit(); @@ -16061,6 +16082,7 @@ module.exports = { get_pull_request, fetch_config, fetch_changed_files, + fetch_current_reviewers, assign_reviewers, clear_cache, }; @@ -16118,6 +16140,10 @@ async function run() { core.info('Fetching changed files in the pull request'); const changed_files = await github.fetch_changed_files(); + core.info('Fetching currently requested reviewers'); + const current_reviewers = await github.fetch_current_reviewers(); + core.info(`Already in review: ${current_reviewers.join(', ')}`); + core.info('Identifying reviewers based on the changed files'); const reviewers_based_on_files = identify_reviewers_by_changed_files({ config, changed_files, excludes: [ author ] }); @@ -16142,11 +16168,18 @@ async function run() { reviewers.push(...default_reviewers); } + core.info(`Possible Reviewers ${reviewers.join(', ')}, prepare filtering out already requested reviewers`); + reviewers = reviewers.filter((reviewer) => !current_reviewers.includes(reviewer)); + core.info('Randomly picking reviewers if the number of reviewers is set'); reviewers = randomly_pick_reviewers({ reviewers, config }); - core.info(`Requesting review to ${reviewers.join(', ')}`); - await github.assign_reviewers(reviewers); + if (reviewers.length > 0) { + core.info(`Requesting review to ${reviewers.join(', ')}`); + await github.assign_reviewers(reviewers); + } else { + core.info('No new reviewers to assign to PR'); + } } module.exports = { diff --git a/src/github.js b/src/github.js index 11ff8ad..b02ae4c 100644 --- a/src/github.js +++ b/src/github.js @@ -96,6 +96,27 @@ async function fetch_changed_files() { return changed_files; } +async function fetch_current_reviewers() { + const context = get_context(); + const octokit = get_octokit(); + + const current_reviewers = []; + + // API docs + // Generated octokit methods: https://github.com/octokit/plugin-rest-endpoint-methods.js/blob/main/src/generated/endpoints.ts + // Available Rest APIs: https://docs.github.com/en/rest/pulls/review-requests?apiVersion=2022-11-28#get-all-requested-reviewers-for-a-pull-request + const { data: response_body } = await octokit.pulls.listRequestedReviewers({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + }); + + current_reviewers.push(...response_body.users.map((user) => user.login)); + current_reviewers.push(...response_body.teams.map((team) => 'team:'.concat(team.slug))); + + return current_reviewers; +} + async function assign_reviewers(reviewers) { const context = get_context(); const octokit = get_octokit(); @@ -156,6 +177,7 @@ module.exports = { get_pull_request, fetch_config, fetch_changed_files, + fetch_current_reviewers, assign_reviewers, clear_cache, }; diff --git a/src/index.js b/src/index.js index 5d23032..1207a47 100644 --- a/src/index.js +++ b/src/index.js @@ -44,6 +44,10 @@ async function run() { core.info('Fetching changed files in the pull request'); const changed_files = await github.fetch_changed_files(); + core.info('Fetching currently requested reviewers'); + const current_reviewers = await github.fetch_current_reviewers(); + core.info(`Already in review: ${current_reviewers.join(', ')}`); + core.info('Identifying reviewers based on the changed files'); const reviewers_based_on_files = identify_reviewers_by_changed_files({ config, changed_files, excludes: [ author ] }); @@ -68,11 +72,18 @@ async function run() { reviewers.push(...default_reviewers); } + core.info(`Possible Reviewers ${reviewers.join(', ')}, prepare filtering out already requested reviewers`); + reviewers = reviewers.filter((reviewer) => !current_reviewers.includes(reviewer)); + core.info('Randomly picking reviewers if the number of reviewers is set'); reviewers = randomly_pick_reviewers({ reviewers, config }); - core.info(`Requesting review to ${reviewers.join(', ')}`); - await github.assign_reviewers(reviewers); + if (reviewers.length > 0) { + core.info(`Requesting review to ${reviewers.join(', ')}`); + await github.assign_reviewers(reviewers); + } else { + core.info('No new reviewers to assign to PR'); + } } module.exports = { diff --git a/test/github.test.js b/test/github.test.js index 7a21ca5..a65c194 100644 --- a/test/github.test.js +++ b/test/github.test.js @@ -12,6 +12,7 @@ const { get_pull_request, fetch_config, fetch_changed_files, + fetch_current_reviewers, assign_reviewers, clear_cache, } = require('../src/github'); @@ -123,6 +124,66 @@ describe('github', function() { }); }); + describe('fetch_current_reviewers()', function() { + const stub = sinon.stub(); + const octokit = { + pulls: { + listRequestedReviewers: stub, + }, + }; + + beforeEach(function() { + github.getOctokit.returns(octokit); + }); + + it('fetches current reviewers - user only', async function() { + stub.returns({ + data: { + users: [ + { login: 'super/mario/64' }, + ], + teams: [], + }, + }); + const expected = [ 'super/mario/64' ]; + const actual = await fetch_current_reviewers(); + expect(actual).to.deep.equal(expected); + }); + + it('fetches current reviewers - team only', async function() { + stub.returns({ + data: { + users: [ ], + teams: [ + { slug: 'super_marios' }, + ], + }, + }); + const expected = [ 'team:super_marios' ]; + const actual = await fetch_current_reviewers(); + expect(actual).to.deep.equal(expected); + }); + + it('fetches current reviewers - combined users and teams', async function() { + stub.returns({ + data: { + users: [ + { login: 'bowser' }, + { login: 'peach' }, + { login: 'luigi' }, + ], + teams: [ + { slug: 'super_marios' }, + { slug: 'toads' }, + ], + }, + }); + const expected = [ 'bowser', 'peach', 'luigi', 'team:super_marios', 'team:toads' ]; + const actual = await fetch_current_reviewers(); + expect(actual).to.deep.equal(expected); + }); + }); + describe('assign_reviewers()', function() { const spy = sinon.spy(); const octokit = { diff --git a/test/index.test.js b/test/index.test.js index 4bbfe21..a9d8db4 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -14,6 +14,7 @@ describe('index', function() { sinon.stub(github, 'get_pull_request'); sinon.stub(github, 'fetch_config'); sinon.stub(github, 'fetch_changed_files'); + sinon.stub(github, 'fetch_current_reviewers'); sinon.stub(github, 'assign_reviewers'); }); @@ -21,6 +22,7 @@ describe('index', function() { github.get_pull_request.restore(); github.fetch_config.restore(); github.fetch_changed_files.restore(); + github.fetch_current_reviewers.restore(); github.assign_reviewers.restore(); }); @@ -49,12 +51,116 @@ describe('index', function() { const changed_fiels = [ 'path/to/file.js' ]; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true; expect(github.assign_reviewers.lastCall.args[0]).to.have.members([ 'mario', 'princess-peach' ]); }); + it('skips single alias if already a reviewer', async function() { + const config = { + reviewers: { + defaults: [ 'dr-mario' ], + groups: { + 'mario-brothers': [ 'mario', 'luigi' ], + }, + }, + files: { + '**/*.js': [ 'mario-brothers', 'princess-peach' ], + '**/*.rb': [ 'wario', 'waluigi' ], + }, + }; + github.fetch_config.returns(config); + + const pull_request = { + title: 'Nice Pull Request', + is_draft: false, + author: 'luigi', + }; + github.get_pull_request.returns(pull_request); + + const changed_files = [ 'path/to/file.js' ]; + github.fetch_changed_files.returns(changed_files); + + const current_reviewers = [ 'princess-peach' ]; + github.fetch_current_reviewers.returns(current_reviewers); + + await run(); + + expect(github.assign_reviewers.calledOnce).to.be.true; + expect(github.assign_reviewers.lastCall.args[0]).to.have.members([ 'mario' ]); + }); + + it('skips team alias if already a reviewer', async function() { + const config = { + reviewers: { + defaults: [ 'dr-mario' ], + groups: { + 'mario-brothers': [ 'mario', 'luigi' ], + }, + }, + files: { + '**/*.js': [ 'mario-brothers', 'team:peach-alliance' ], + '**/*.rb': [ 'wario', 'waluigi', 'team:bowser-and-co' ], + }, + }; + github.fetch_config.returns(config); + + const pull_request = { + title: 'Nice Pull Request', + is_draft: false, + author: 'luigi', + }; + github.get_pull_request.returns(pull_request); + + const changed_files = [ 'path/to/file.js', 'path/to/file.rb' ]; + github.fetch_changed_files.returns(changed_files); + + const current_reviewers = [ 'team:bowser-and-co' ]; + github.fetch_current_reviewers.returns(current_reviewers); + + await run(); + + expect(github.assign_reviewers.calledOnce).to.be.true; + expect(github.assign_reviewers.lastCall.args[0]).to.have.members([ 'mario', 'team:peach-alliance', 'wario', 'waluigi' ]); + }); + + it('skips caling assign if no reviewers', async function() { + const config = { + reviewers: { + defaults: [ 'dr-mario' ], + groups: { + 'mario-brothers': [ 'mario', 'luigi' ], + }, + }, + files: { + '**/*.js': [ 'mario-brothers', 'princess-peach' ], + '**/*.rb': [ 'wario', 'waluigi' ], + }, + }; + github.fetch_config.returns(config); + + const pull_request = { + title: 'Nice Pull Request', + is_draft: false, + author: 'luigi', + }; + github.get_pull_request.returns(pull_request); + + const changed_files = [ 'path/to/file.js' ]; + github.fetch_changed_files.returns(changed_files); + + const current_reviewers = [ 'princess-peach', 'mario' ]; + github.fetch_current_reviewers.returns(current_reviewers); + + await run(); + + expect(github.assign_reviewers.calledOnce).to.be.false; + }); + it('requests review based on groups that author belongs to', async function() { const config = { reviewers: { @@ -80,6 +186,9 @@ describe('index', function() { const changed_fiels = []; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true; @@ -107,6 +216,9 @@ describe('index', function() { const changed_fiels = [ 'path/to/file.js' ]; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.false; @@ -133,6 +245,9 @@ describe('index', function() { const changed_fiels = [ 'path/to/file.js' ]; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.false; @@ -162,6 +277,9 @@ describe('index', function() { const changed_fiels = [ 'path/to/file.py' ]; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.false; @@ -192,6 +310,9 @@ describe('index', function() { const changed_fiels = [ 'path/to/file.py' ]; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true; @@ -223,6 +344,9 @@ describe('index', function() { const changed_fiels = []; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true; @@ -254,6 +378,9 @@ describe('index', function() { const changed_fiels = []; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true; @@ -283,6 +410,9 @@ describe('index', function() { const changed_fiels = []; github.fetch_changed_files.returns(changed_fiels); + const current_reviewers = []; + github.fetch_current_reviewers.returns(current_reviewers); + await run(); expect(github.assign_reviewers.calledOnce).to.be.true;