Prevent an alias who is not a collaborator from failing to assign any aliases

src/github.js

@@ -170,13 +170,32 @@ async function assign_reviewers(reviewers) {
  const [ teams_with_prefix, individuals ] = partition(reviewers, (reviewer) => reviewer.startsWith('team:'));
  const teams = teams_with_prefix.map((team_with_prefix) => team_with_prefix.replace('team:', ''));
 
- return octokit.pulls.requestReviewers({
- owner: context.repo.owner,
- repo: context.repo.repo,
- pull_number: context.payload.pull_request.number,
- reviewers: individuals,
- team_reviewers: teams,
+ const request_review_responses = [];
+
+ // Github's requestReviewers API will fail to add all reviewers if any of the aliases are not collaborators.
+ // Github also does not support a batch call to determine which aliases in the list are not collaborators.
+
+ // We therefore make each call individually so that we add all reviewers that are collaborators,
+ // and log failure for aliases that no longer have access.
+ teams.forEach((team) => {
+ request_review_responses.push(octokit.pulls.requestReviewers({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ pull_number: context.payload.pull_request.number,
+ team_reviewers: [ team ],
+ }).catch((error) => core.error(`Team: ${team} failed to be added with error: ${error}`)));
  });
+
+ individuals.forEach((login) => {
+ request_review_responses.push(octokit.pulls.requestReviewers({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ pull_number: context.payload.pull_request.number,
+ reviewers: [ login ],
+ }).catch((error) => core.error(`Individual: ${login} failed to be added with error: ${error}`)));
+ });
+
+ return Promise.allSettled(request_review_responses);
 }
 
 /* Private */

test/github.test.js

@@ -279,10 +279,10 @@ describe('github', function() {
  });
 
  describe('assign_reviewers()', function() {
- const spy = sinon.spy();
+ const stub = sinon.stub();
  const octokit = {
  pulls: {
- requestReviewers: spy,
+ requestReviewers: stub,
  },
  };
 
@@ -291,26 +291,146 @@ describe('github', function() {
  restoreModule = rewired_github.__set__('octokit_cache', octokit);
  });
  afterEach(function() {
+ stub.reset();
  restoreModule();
  });
 
- it('assigns reviewers', async function() {
+ it('assigns reviewers - mixed success', async function() {
+ stub.onFirstCall().resolves({});
+ stub.onSecondCall().resolves({});
+ stub.onThirdCall().resolves({});
+
  const reviewers = [ 'mario', 'princess-peach', 'team:koopa-troop' ];
  await rewired_github.assign_reviewers(reviewers);
 
- expect(spy.calledOnce).to.be.true;
- expect(spy.lastCall.args[0]).to.deep.equal({
+ expect(stub.calledThrice).to.be.true;
+ expect(stub.firstCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ team_reviewers: [
+ 'koopa-troop',
+ ],
+ });
+
+ expect(stub.secondCall.args[0]).to.deep.equal({
  owner: 'necojackarc',
  pull_number: 18,
  repo: 'auto-request-review',
  reviewers: [
  'mario',
+ ],
+ });
+
+ expect(stub.thirdCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ reviewers: [
+ 'princess-peach',
+ ],
+ });
+ });
+
+ it('assigns reviewers - continues after failure individual', async function() {
+ stub.onFirstCall().resolves({});
+ let isResolved = false;
+ let isRejected = false;
+ const error = new Error('Alias is not a collaborator');
+ const failedNetworkCall = Promise.reject(error).then(
+ function(value) {
+ isResolved = true; return value;
+ },
+ function(error) {
+ isRejected = true; throw error;
+ }
+ );
+ stub.onSecondCall().returns(failedNetworkCall);
+ stub.onThirdCall().resolves({});
+
+ const reviewers = [ 'team:super_marios', 'princess-peach', 'luigi' ];
+ await rewired_github.assign_reviewers(reviewers);
+
+ expect(stub.calledThrice).to.be.true;
+ expect(stub.firstCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ team_reviewers: [
+ 'super_marios',
+ ],
+ });
+
+ expect(stub.secondCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ reviewers: [
  'princess-peach',
  ],
+ });
+ expect(isRejected).to.be.true;
+ expect(isResolved).to.be.false;
+
+ expect(stub.thirdCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ reviewers: [
+ 'luigi',
+ ],
+ });
+ });
+
+ it('assigns reviewers - continues after failure team', async function() {
+ let isResolved = false;
+ let isRejected = false;
+ const error = new Error('Alias is not a collaborator');
+ const failedNetworkCall = Promise.reject(error).then(
+ function(value) {
+ isResolved = true; return value;
+ },
+ function(error) {
+ isRejected = true; throw error;
+ }
+ );
+ stub.onFirstCall().returns(failedNetworkCall);
+ stub.onSecondCall().resolves({});
+ stub.onThirdCall().resolves({});
+
+ const reviewers = [ 'team:toads', 'team:koopa-troop', 'mario' ];
+ await rewired_github.assign_reviewers(reviewers);
+
+ expect(stub.calledThrice).to.be.true;
+
+ expect(stub.firstCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ team_reviewers: [
+ 'toads',
+ ],
+ });
+ expect(isRejected).to.be.true;
+ expect(isResolved).to.be.false;
+
+ expect(stub.secondCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
  team_reviewers: [
  'koopa-troop',
  ],
  });
+
+ expect(stub.thirdCall.args[0]).to.deep.equal({
+ owner: 'necojackarc',
+ pull_number: 18,
+ repo: 'auto-request-review',
+ reviewers: [
+ 'mario',
+ ],
+ });
  });
  });
 });