Change passport-github to passport-github2 and @octokit/rest to v18

.gitignore

@@ -4,3 +4,4 @@ node_modules
 /views/standard/css
 pulldasher.pid
 dist
+/.env.db

config.example.js

@@ -51,7 +51,7 @@ module.exports = {
       hook_secret:  "some random string to use (?secret=oxwm5gks) to 'secure' github hook handlers",
       // Limit access to specific users or teams.
       requireOrg:   "Limit access to users belonging to this github organization",
-      requireTeam:  "[Optional] Limit access to users belonging to this team name within the above github organization"
+      requireTeam:  "[Optional] Limit access to users belonging to this team name within the above github organization, uses the slug version of the team name, i.e. 'some-team' from @SomeOrg/some-team"
    },
    session: {
       secret: "secret for signing session cookies"

lib/authentication.js

@@ -3,7 +3,7 @@ var config = require('./config-loader'),
     github = require('./git-manager').github,
     passport = require('passport'),
     _ = require('underscore'),
-    GitHubStrategy = require('passport-github').Strategy;
+    GitHubStrategy = require('passport-github2').Strategy;
 
 const FAKE_USER = process.env.MOCK_AUTH_AS_USER;
 
@@ -98,27 +98,12 @@ module.exports = {
 let checkOrgMembershipRequirements;
 
 if (config.github.requireTeam) {
-   // All this logic is needed to get the team id from a team name
-   const getTeamId = github.orgs.getTeams({org: config.github.requireOrg})
-   .then(function(teams) {
-      const team = _.findWhere(teams, {name: config.github.requireTeam});
-      if (!team) {
-         const m = "Team " + (config.github.requireTeam) + " not found in Organization: " + config.github.requireOrg;
-         debug(m);
-         console.error(m); // eslint-disable-line no-console
-         process.exit(1);
-      }
-      return team.id;
-   });
-
    checkOrgMembershipRequirements = function(options) {
-      return getTeamId.then(function(teamId) {
-         options.id = teamId;
-         return github.orgs.getTeamMembership(options);
-      });
+      options.team_slug = config.github.requireTeam;
+      return github.teams.getMembershipForUserInOrg(options);
    };
 } else {
-   checkOrgMembershipRequirements = github.orgs.checkMembership;
+   checkOrgMembershipRequirements = github.orgs.checkMembershipForUser;
 }
 
 function confirmOrgMembership(user) {

lib/git-manager.js

@@ -1,10 +1,9 @@
-var GithubApi = require('@octokit/rest'),
+var { Octokit } = require('@octokit/rest'),
+    { throttling } = require("@octokit/plugin-throttling"),
+    MyOctokit = Octokit.plugin(throttling),
     config = require('./config-loader'),
     Promise = require('bluebird'),
     _ = require('underscore'),
-    github = new GithubApi({
-      version: '3.0.0'
-    }),
     debug = require('./debug')('pulldasher:github'),
     utils = require('./utils'),
     Pull = require('../models/pull'),
@@ -14,25 +13,45 @@ var GithubApi = require('@octokit/rest'),
     Label = require('../models/label'),
     Status = require('../models/status'),
     Signature = require('../models/signature'),
-    getLogin  = require('./get-user-login'),
-    rateLimit = require('./rate-limit.js');
+    getLogin  = require('./get-user-login');
+
+const github = new MyOctokit({
+   auth: config.github.token,
+   throttle: {
+      onRateLimit: (retryAfter, options) => {
+        github.log.warn(
+          `Request quota exhausted for request ${options.method} ${options.url}`
+        );
+
+        // Retry five times after hitting a rate limit error, then give up
+        if (options.request.retryCount <= 5) {
+          github.log.debug(`Retrying after ${retryAfter} seconds!`);
+          return true;
+        }
+      },
+      onAbuseLimit: (retryAfter, options) => {
+        // does not retry, only logs a warning
+        github.log.warn(
+          `Abuse detected for request ${options.method} ${options.url}`
+        );
+      },
+    }
+}); 
+
+const githubRest = github.rest;
 
-github.authenticate({
-   type: 'oauth',
-   token: config.github.token
-});
 
 module.exports = {
-   github: github,
+   github: githubRest,
 
    /**
     * Returns a promise which resolves to a GitHub API response to
     * a query for a particular Pull Request.
     */
    getPull: function(repo, number) {
       debug("Getting pull %s", number);
-      return rateLimit(github.pullRequests.get)(params({ number }, repo))
-                                .then(res => res.data);
+      return githubRest.pulls.get(params({ pull_number: number }, repo))
+       .then(res => res.data);
    },
 
    /**
@@ -42,7 +61,7 @@ module.exports = {
     */
    getOpenPulls: function(repo) {
       debug("Getting open pulls in repo %s", repo);
-      return rateLimit(github.pullRequests.getAll)(params({}, repo)).then(paginate);
+      return github.paginate(githubRest.pulls.list, params({state: 'open'}, repo));
    },
 
    /**
@@ -52,8 +71,7 @@ module.exports = {
     */
    getAllPulls: function(repo) {
       debug("Getting all pulls in repo %s", repo);
-      return rateLimit(github.pullRequests.getAll)(params({ state: 'all' }, repo))
-                                .then(paginate);
+      return github.paginate(githubRest.pulls.list, params({ state: 'all'}, repo));
    },
 
    /**
@@ -62,11 +80,11 @@ module.exports = {
     * Returns a promise which resolves to a github issue
     */
    getIssue: function(repo, number) {
-      const searchParams = params({ number }, repo);
+      const searchParams = params({ issue_number: number }, repo);
       debug("Getting issue %s in repo %s", number, repo);
-      return rateLimit(github.issues.get)(searchParams)
-                          .then(res => res.data)
-                          .then(addRepo(searchParams));
+      return githubRest.issues.get(searchParams)
+       .then(res => res.data)
+       .then(addRepo(searchParams));
    },
 
    /**
@@ -75,12 +93,11 @@ module.exports = {
     * Returns a promise which resolves to an array of all open issues
     */
    getOpenIssues: function(repo) {
-      const searchParams = params({}, repo);
-      debug("Getting open issues");
-      return rateLimit(github.issues.getForRepo)(searchParams)
-      .then(paginate)
-      .then(filterOutPulls)
-      .then(addRepo(searchParams));
+      const searchParams = params({state: 'open'}, repo);
+      debug("Getting open issues in repo %s", repo);
+      return github.paginate(githubRest.issues.listForRepo, searchParams)
+       .then(filterOutPulls)
+       .then(addRepo(searchParams));
    },
 
    /**
@@ -91,10 +108,9 @@ module.exports = {
    getAllIssues: function(repo) {
       const searchParams = params({state: 'all'}, repo);
       debug("Getting all issues");
-      return rateLimit(github.issues.getForRepo)(searchParams)
-      .then(paginate)
-      .then(filterOutPulls)
-      .then(addRepo(searchParams));
+      return github.paginate(githubRest.issues.listForRepo, searchParams)
+       .then(filterOutPulls)
+       .then(addRepo(searchParams));
    },
 
    /**
@@ -225,18 +241,6 @@ module.exports = {
    },
 };
 
-async function paginate(res) {
-   let response = res;
-   let all = res.data;
-
-   while (github.hasNextPage(response)) {
-      response = await rateLimit(github.getNextPage)(response);
-      all = all.concat(response.data);
-   }
-
-   return all;
-}
-
 /**
  * Get array of Label objects from complete list of a Issue's events.
  *
@@ -322,7 +326,6 @@ function params(apiParams, fullRepoName) {
    return _.extend({
       owner,
       repo,
-      per_page:   100
    }, apiParams);
 }
 
@@ -360,36 +363,31 @@ function parseRepo(repo) {
  */
 function getIssueEvents(repo, number) {
    debug("Getting events for issue #%s", number);
-   return rateLimit(github.issues.getEvents)(params({ number }, repo))
-                       .then(paginate);
+   return github.paginate(githubRest.issues.listEvents, params({ issue_number: number }, repo))
 }
 
 function getIssueComments(repo, number) {
    debug("Getting comments for issue #%s", number);
-   return rateLimit(github.issues.getComments)(params({ number }, repo))
-                       .then(paginate);
+   return github.paginate(githubRest.issues.listComments, params({ issue_number: number }, repo));
 }
 
 function getReviews(repo, number) {
    debug("Getting reviews for pull #%s", number);
-   return rateLimit(github.pullRequests.listReviews)(params({ number }, repo))
-                       .then(paginate);
+   return github.paginate(githubRest.pulls.listReviews, params({ pull_number: number }, repo));
 }
 
 function getPullReviewComments(repo, number) {
    debug("Getting pull review comments for pull #%s", number);
-   return rateLimit(github.pullRequests.getComments)(params({ number }, repo))
-                .then(paginate);
+   return github.paginate(githubRest.pulls.listReviewComments, params({ pull_number: number }, repo))
 }
 
 function getCommit(repo, sha) {
-   return rateLimit(github.repos.getCommit)(params({ sha }, repo))
-                      .then(res => res.data);
+   return githubRest.repos.getCommit(params({ ref: sha }, repo)).then(res => res.data);
 }
 
 function getCommitStatuses(repo, ref) {
    debug("Getting commit status for %s", ref);
-   return rateLimit(github.repos.getCombinedStatusForRef)(params({ ref }, repo))
+   return githubRest.repos.getCombinedStatusForRef(params({ ref }, repo))
       .then(res => res.data.statuses)
       .then(statuses => statuses || [])
 }

lib/utils.js

@@ -5,7 +5,9 @@ var config = require('./config-loader'),
 
 // Set the global Promise object up with the done method so that any
 // promise by other libraries will have a .done()
-Promise.prototype.done = Bluebird.prototype.done;
+Promise.prototype.done = function (callback) {
+   return Bluebird.cast(this).done(callback);
+};
 Promise = Bluebird;
 
 module.exports = {

migrations/0014-pulldasher-reviews-body-nullable.sql

@@ -0,0 +1 @@
+ALTER TABLE reviews MODIFY body text