Add ability to add collaborator

.eslintrc.yaml

@@ -2,6 +2,7 @@
    extends:
       - eslint:recommended
       - google
+   parser: babel-eslint
    parserOptions:
       ecmaVersion: 6
       sourceType: module
@@ -25,7 +26,6 @@
             MethodDefinition: true
       spaced-comment: error
       valid-jsdoc: [error, {requireParamDescription: true}]
-
    env:
       es6: true
       node: true

.gitignore

@@ -9,3 +9,4 @@ node_modules/
 .DS_Store
 npm-debug.log
 sauce.json
+package-lock.json

.npmignore

@@ -5,3 +5,4 @@ lib/
 .nyc_output/
 .DS_Store
 sauce.json
+package-lock.json

.travis.yml

@@ -1,9 +1,8 @@
 sudo: false
 language: node_js
 node_js:
+  - '8'
   - '6'
-  - '5'
-  - '4'
 
 cache:
   directories:

lib/Repository.js

@@ -436,13 +436,27 @@ class Repository extends Requestable {
     * List the users who are collaborators on the repository. The currently authenticated user must have
     * push access to use this method
     * @see https://developer.github.com/v3/repos/collaborators/#list-collaborators
-    * @param {Requestable.callback} cb - will receive the list of collaborators
+    * @param {Requestable.callback} cb - will receive the fetched data
     * @return {Promise} - the promise for the http request
     */
    getCollaborators(cb) {
       return this._request('GET', `/repos/${this.__fullname}/collaborators`, null, cb);
    }
 
+   /**
+    * Adds user as a collaborator on the repository. The currently authenticated user must have admin access for the
+    * repo to use this method
+    * @see https://developer.github.com/enterprise/2.10/v3/repos/collaborators/#add-user-as-a-collaborator
+    * @param {string} username - the user to add as a collaborator
+    * @param {Object} [options] - collaborator permissions, only applicable on repos in an org
+    * @param {Object} [options.permission=push] - can be one of: `pull`, `push`, or `admin`
+    * @param {Requestable.callback} cb - will receive the information about the newly added contributor
+    * @return {Promise} - the promise for the http request
+    */
+   addCollaborator(username, options, cb) {
+      return this._request('PUT', `/repos/${this.__fullname}/collaborators/${username}`, options, cb);
+   }
+
    /**
     * Check if a user is a collaborator on the repository
     * @see https://developer.github.com/v3/repos/collaborators/#check-if-a-user-is-a-collaborator
@@ -451,7 +465,7 @@ class Repository extends Requestable {
     * @return {Promise} - the promise for the http request {Boolean} [description]
     */
    isCollaborator(username, cb) {
-      return this._request('GET', `/repos/${this.__fullname}/collaborators/${username}`, null, cb);
+      return this._request204or404(`/repos/${this.__fullname}/collaborators/${username}`, null, cb);
    }
 
    /**

lib/Requestable.js

@@ -235,15 +235,18 @@ class Requestable {
     * @param {string} path - the path to request
     * @param {Object} options - the query parameters to include
     * @param {Requestable.callback} [cb] - the function to receive the data. The returned data will always be an array.
-    * @param {Object[]} results - the partial results. This argument is intended for interal use only.
     * @return {Promise} - a promise which will resolve when all pages have been fetched
     * @deprecated This will be folded into {@link Requestable#_request} in the 2.0 release.
     */
-   _requestAllPages(path, options, cb, results) {
-      results = results || [];
+   async _requestAllPages(path, options, cb) {
+      let currentPath = path;
+      let results = [];
+      let response;
+
+      try {
+         do {
+            response = await this._request('GET', currentPath, options);
 
-      return this._request('GET', path, options)
-         .then((response) => {
             let thisGroup;
             if (response.data instanceof Array) {
                thisGroup = response.data;
@@ -255,19 +258,18 @@ class Requestable {
             }
             results.push(...thisGroup);
 
-            const nextUrl = getNextPage(response.headers.link);
-            if (nextUrl && typeof options.page !== 'number') {
-               log(`getting next page: ${nextUrl}`);
-               return this._requestAllPages(nextUrl, options, cb, results);
-            }
+            currentPath = getNextPage(response.headers.link);
+         } while(currentPath);
 
-            if (cb) {
-               cb(null, results, response);
-            }
+         if (cb) {
+            cb(null, results, response);
+         }
 
-            response.data = results;
-            return response;
-         }).catch(callbackErrorOrThrow(cb, path));
+         response.data = results;
+         return response;
+      } catch (err) {
+         callbackErrorOrThrow(cb, path)(err);
+      }
    }
 }
 
@@ -283,6 +285,7 @@ function methodHasNoBody(method) {
 
 function getNextPage(linksHeader = '') {
    const links = linksHeader.split(/\s*,\s*/); // splits and strips the urls
+   // TODO: Change this to early abort once it finds the link in question
    return links.reduce(function(nextUrl, link) {
       if (link.search(/rel="next"/) !== -1) {
          return (link.match(/<(.*)>/) || [])[1];

lib/Team.js

@@ -90,8 +90,8 @@ class Team extends Requestable {
 
    /**
     * Add a member to the Team
-    * @see https://developer.github.com/v3/orgs/teams/#add-team-membership
-    * @param {string} username - can be one of: `all`, `maintainer`, or `member`
+    * @see https://developer.github.com/v3/orgs/teams/#add-or-update-team-membership
+    * @param {string} username - user to add or update membership for
     * @param {object} options - Parameters for adding a team member
     * @param {string} [options.role=member] - The role that this user should have in the team. Can be one
     * of: `member`, or `maintainer`

package.json

@@ -25,7 +25,14 @@
   },
   "babel": {
     "presets": [
-      "es2015"
+      [
+        "env",
+        {
+          "targets": {
+            "node": 6
+          }
+        }
+      ]
     ],
     "plugins": [
       [
@@ -59,9 +66,11 @@
   },
   "devDependencies": {
     "babel-core": "^6.7.7",
+    "babel-eslint": "^7.2.3",
     "babel-plugin-add-module-exports": "^0.2.1",
     "babel-plugin-istanbul": "3.0.0",
     "babel-plugin-transform-es2015-modules-umd": "^6.5.0",
+    "babel-preset-env": "^1.6.0",
     "babel-preset-es2015": "^6.5.0",
     "babel-register": "^6.7.2",
     "babelify": "^7.3.0",

test/fixtures/alt-user.json

@@ -0,0 +1,3 @@
+{
+   "USERNAME": "mtscout6-test"
+}

test/organization.spec.js

@@ -21,7 +21,9 @@ describe('Organization', function() {
    });
 
    after(function() {
-      return github.getProject(createdProject.id).deleteProject();
+      if (createdProject) {
+         return github.getProject(createdProject.id).deleteProject();
+      }
    });
 
    describe('reading', function() {
@@ -95,13 +97,10 @@ describe('Organization', function() {
          }));
       });
 
-      it('should list the teams in the organization', function() {
-         return organization.getTeams()
-           .then(({data}) => {
-              const hasTeam = data.some((member) => member.slug === testRepoName);
-
-              expect(hasTeam).to.be.true();
-           });
+      it('should list the teams in the organization', async function() {
+         const {data} = await organization.getTeams();
+         const hasTeam = data.some((member) => member.slug === testRepoName);
+         expect(hasTeam).to.be.true();
       });
 
       it('should create a project', function(done) {

test/rate-limit.spec.js

@@ -2,7 +2,6 @@ import expect from 'must';
 
 import Github from '../lib/GitHub';
 import testUser from './fixtures/user.json';
-import {assertSuccessful} from './helpers/callbacks';
 
 describe('RateLimit', function() {
    let github;
@@ -18,18 +17,15 @@ describe('RateLimit', function() {
       rateLimit = github.getRateLimit();
    });
 
-   it('should get rate limit', function(done) {
-      rateLimit.getRateLimit(assertSuccessful(done, function(err, rateInfo) {
-         const rate = rateInfo.rate;
+   it('should get rate limit', async function() {
+      const {data: rateInfo} = await rateLimit.getRateLimit();
+      const rate = rateInfo.rate;
 
-         expect(rate).to.be.an.object();
-         expect(rate).to.have.own('limit');
-         expect(rate).to.have.own('remaining');
-         expect(rate.limit).to.be.a.number();
-         expect(rate.remaining).to.be.a.number();
-         expect(rate.remaining).to.be.at.most(rateInfo.rate.limit);
-
-         done();
-      }));
+      expect(rate).to.be.an.object();
+      expect(rate).to.have.own('limit');
+      expect(rate).to.have.own('remaining');
+      expect(rate.limit).to.be.a.number();
+      expect(rate.remaining).to.be.a.number();
+      expect(rate.remaining).to.be.at.most(rateInfo.rate.limit);
    });
 });

test/repository.spec.js

@@ -3,6 +3,7 @@ import expect from 'must';
 import Github from '../lib/GitHub';
 import wait from './helpers/wait';
 import testUser from './fixtures/user.json';
+import altUser from './fixtures/alt-user.json';
 import loadImage from './fixtures/imageBlob';
 import {assertSuccessful, assertFailure} from './helpers/callbacks';
 import getTestRepoName from './helpers/getTestRepoName';
@@ -343,8 +344,18 @@ describe('Repository', function() {
          }));
       });
 
-      it('should test whether user is collaborator', function(done) {
-         remoteRepo.isCollaborator(testUser.USERNAME, assertSuccessful(done));
+      it('should add repo collaborator', async function() {
+         expect(await remoteRepo.isCollaborator(altUser.USERNAME)).to.be.false;
+         await remoteRepo.addCollaborator(altUser.USERNAME);
+         expect(await remoteRepo.isCollaborator(altUser.USERNAME)).to.be.true;
+      });
+
+      it('should test whether user is collaborator', async function() {
+         expect(await remoteRepo.isCollaborator(testUser.USERNAME)).to.be.true;
+      });
+
+      it('should test whether user is not collaborator', async function() {
+         expect(await remoteRepo.isCollaborator(altUser.USERNAME)).to.be.false;
       });
 
       it('should write to repo', function(done) {

test/team.spec.js

@@ -1,13 +1,10 @@
+import assert from 'assert';
 import expect from 'must';
 
 import Github from '../lib/GitHub';
 import testUser from './fixtures/user.json';
-import {assertFailure} from './helpers/callbacks';
 import getTestRepoName from './helpers/getTestRepoName';
-
-const altUser = {
-   USERNAME: 'mtscout6-test',
-};
+import altUser from './fixtures/alt-user.json';
 
 function createTestTeam() {
    const name = getTestRepoName();
@@ -145,9 +142,15 @@ describe('Team', function() { // Isolate tests that need a new team per test
    });
 
    // Test for Team deletion
-   afterEach(function(done) {
-      team.deleteTeam()
-         .then(() => team.getTeam(assertFailure(done)));
+   afterEach(async function() {
+      await team.deleteTeam();
+
+      try {
+         await team.getTeam();
+         assert.fail(undefined, undefined, 'Failed to delete the team');
+      } catch (error) {
+         // Swallow intentionally
+      }
    });
 
    it('should update team', function() {
@@ -158,13 +161,11 @@ describe('Team', function() { // Isolate tests that need a new team per test
          });
    });
 
-   it('should add membership for a given user', function() {
-      return team.addMembership(testUser.USERNAME)
-         .then(({data}) => {
-            const {state, role} = data;
-            expect(state === 'active' || state === 'pending').to.be.true();
-            expect(role).to.equal('member');
-         });
+   it('should add membership for a given user', async function() {
+      const {data: {state, role}} = await team.addMembership(testUser.USERNAME);
+      expect(state === 'active' || state === 'pending').to.be.true();
+      // TODO: This does not appear to match the documentation...
+      expect(role).to.equal('maintainer');
    });
 
    it('should add membership as a maintainer for a given user', function() {