Skip to content

Commit

Permalink
Skip Re-Assigning Aliases to PRs (#3)
Browse files Browse the repository at this point in the history
In the existing action, the workflow will assign a reviewer regardless of if that reviewer has already been assigned to the pull request: necojackarc#110

This additionally has the issue of sending extra notifications about being added to a review that you have previously been added to, making it more difficult to manage PR review requests.

To solve this, we add an additional REST call to GH to retrieve the current users that have already been assigned to the PR. We then remove those previously assigned aliases from the list of aliases the workflow will assign.

There is also a slight optimization to not make the PUT REST call to add reviewers if there exists no list of reviewers to add (instead of making the call with no reviewers).
  • Loading branch information
jamoor-moj authored Feb 21, 2024
1 parent 465de57 commit d8fc68e
Show file tree
Hide file tree
Showing 5 changed files with 261 additions and 4 deletions.
37 changes: 35 additions & 2 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions src/github.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -156,6 +177,7 @@ module.exports = {
get_pull_request,
fetch_config,
fetch_changed_files,
fetch_current_reviewers,
assign_reviewers,
clear_cache,
};
15 changes: 13 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] });

Expand All @@ -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 = {
Expand Down
61 changes: 61 additions & 0 deletions test/github.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
get_pull_request,
fetch_config,
fetch_changed_files,
fetch_current_reviewers,
assign_reviewers,
clear_cache,
} = require('../src/github');
Expand Down Expand Up @@ -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 = {
Expand Down
Loading

0 comments on commit d8fc68e

Please sign in to comment.