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

… aliases (#5)

When the action attempts to assign a list of aliases as reviewers, the batch call will fail if any of the aliases are not a collaborator of the repository and therefore don't have access:

```
HttpError: Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the <owner>/<repo> repository
```

The error does not provide information on which aliases are not collaborators, and the current GH [Rest](https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28#list-repository-collaborators)/[GraphQL](https://docs.github.com/en/graphql/reference/objects#repository) APIs do not have a "filtered list of collaborators" method. To retrieve collaborator status, we would either need to make an individual network call per alias, or retrieve all collaborators which could be pages of information returned.

The proposed fix is to simplify the approach and update the current rest call to not be a batch call. If a particular alias no longer contains access, the requestReviewers call will print out an error. However, the error will not stop execution (all collaborators get added) and the action will not register as failed.

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',
+ ],
+ });
  });
  });
 });