Merge pull request #232 from pr-triage/revert-230-revert-223-feat.194…

….partial-approval

Revert "Revert "feat: support multiple PR reviews""

index.js

@@ -1,11 +1,12 @@
 const debug = require("debug")("probot:pr-triage");
-const Raven = require("raven");
+const Sentry = require("@sentry/node");
 const PRTriage = require("./lib/pr-triage");
 
-Raven.config(
-  process.env.NODE_ENV === "production" &&
-    "https://[email protected]/1222067"
-).install();
+if (process.env.NODE_ENV === "production") {
+  Sentry.init({
+    dsn: "https://[email protected]/1222067"
+  });
+}
 
 function probotPlugin(robot) {
   const events = [
@@ -26,16 +27,14 @@ async function triage(context) {
   const prTriage = forRepository(context);
   const pullRequest = getPullRequest(context);
 
-  Raven.context(() => {
-    Raven.setContext({
-      extra: {
-        owner: context.repo()["owner"],
-        repo: context.repo()["repo"],
-        number: pullRequest.number
-      }
+  try {
+    Sentry.configureScope(scope => {
+      scope.setExtra("pull_request_url", pullRequest.url);
     });
     prTriage.triage(pullRequest);
-  });
+  } catch (e) {
+    Sentry.captureMessage(e);
+  }
 }
 
 function forRepository(context) {

lib/defaults.js

@@ -19,5 +19,9 @@ module.exports = {
   labelDraft: {
     name: "PR: draft",
     color: "6a737d"
+  },
+  labelPartiallyApproved: {
+    name: "PR: partially-approved",
+    color: "7E9C82"
   }
 };

lib/pr-triage.js

@@ -15,6 +15,7 @@ class PRTriage {
       UNREVIED: "labelUnreviewed",
       APPROVED: "labelApproved",
       CHANGES_REQUESTED: "labelChangesRequested",
+      PARTIALLY_APPROVED: "labelPartiallyApproved",
       MERGED: "labelMerged",
       DRAFT: "labelDraft"
     });
@@ -31,29 +32,18 @@ class PRTriage {
 
   async triage(pullRequest) {
     Object.assign(this.pullRequest, pullRequest);
-    const { owner, repo } = this.config;
-    const number = this.pullRequest.number;
 
     await this._ensurePRTriageLabelExists();
     const state = await this._getState();
-
     switch (state) {
       case PRTriage.STATE.WIP:
       case PRTriage.STATE.UNREVIED:
       case PRTriage.STATE.CHANGES_REQUESTED:
+      case PRTriage.STATE.PARTIALLY_APPROVED:
       case PRTriage.STATE.APPROVED:
       case PRTriage.STATE.DRAFT:
       case PRTriage.STATE.MERGED:
         this._updateLabel(state);
-        this.logger(
-          "%s/%s#%s is labeld as %s",
-          owner,
-          repo,
-          number,
-          Object.keys(PRTriage.STATE).find(k => {
-            return PRTriage.STATE[k] === state;
-          })
-        );
         break;
       default:
         throw new Error("Undefined state");
@@ -70,6 +60,9 @@ class PRTriage {
     }
 
     const reviews = await this._getUniqueReviews();
+    const requiredNumberOfReviews = await this._getRequiredNumberOfReviews();
+    const numRequestedReviewsRemaining = await this._getRequestedNumberOfReviews();
+
     if (reviews.length === 0) {
       return PRTriage.STATE.UNREVIED;
     } else {
@@ -82,21 +75,24 @@ class PRTriage {
 
       if (changeRequestedReviews.length > 0) {
         return PRTriage.STATE.CHANGES_REQUESTED;
+      } else if (
+        approvedReviews.length < requiredNumberOfReviews ||
+        numRequestedReviewsRemaining > 0
+      ) {
+        // Mark if partially approved if:
+        // 1) Branch protections require more approvals
+        //  - or -
+        // 2) not everyone requested has approved (requested remaining > 0)
+        return PRTriage.STATE.PARTIALLY_APPROVED;
       } else if (reviews.length === approvedReviews.length) {
         return PRTriage.STATE.APPROVED;
       }
     }
   }
 
   async _getUniqueReviews() {
-    const { owner, repo } = this.config;
-    const pull_number = this.pullRequest.number;
+    const reviews = await this._getReviews();
     const sha = this.pullRequest.head.sha;
-
-    const reviews =
-      (await this.github.pulls.listReviews({ owner, repo, pull_number }))
-        .data || [];
-
     const uniqueReviews = reviews
       .filter(review => review.commit_id === sha)
       .filter(
@@ -131,16 +127,90 @@ class PRTriage {
     return Object.values(uniqueReviews);
   }
 
+  /**
+   * Get the number of required number of reviews according to branch protections
+   * @return {Promise<number>} The number of required approving reviews, or 1 if Administration Permission is not granted or Branch Protection is not set up
+   * @private
+   */
+  async _getRequiredNumberOfReviews() {
+    const { owner, repo } = this.config;
+    const branch = this.pullRequest.base.ref;
+    return (
+      this.github.repos
+        // See: https://developer.github.com/v3/previews/#require-multiple-approving-reviews
+        .getBranchProtection({
+          owner,
+          repo,
+          branch,
+          mediaType: {
+            previews: ["luke-cage"]
+          }
+        })
+        .then(res => {
+          return res.data.required_pull_request_reviews
+            .required_approving_review_count;
+        })
+        .catch(err => {
+          // Return the minium number of reviews if it's 403 or 403 because Administration Permission is not granted (403) or Branch Protection is not set up(404)
+          if (err.status === 404 || err.status === 403) {
+            return 1;
+          }
+          throw err;
+        })
+    );
+  }
+
+  /**
+   * Get the number of users and teams that have been requested to review the PR
+   * @return {Promise<number>}
+   * @private
+   */
+  async _getRequestedNumberOfReviews() {
+    const { owner, repo } = this.config;
+    const pull_number = this.pullRequest.number;
+    return this.github.pulls
+      .listReviewRequests({ owner, repo, pull_number })
+      .then(res => res.data.teams.length + res.data.users.length);
+  }
+
+  async _getReviews() {
+    const { owner, repo } = this.config;
+    // Ignore inconsitent variable name conversation
+    // because of https://octokit.github.io/rest.js/v17#pulls-list-reviews
+    const pull_number = this.pullRequest.number;
+    return this.github.pulls
+      .listReviews({ owner, repo, pull_number })
+      .then(res => res.data || []);
+  }
+
   async _ensurePRTriageLabelExists() {
-    for (const labelObj in this._getFilteredConfigObjByRegex(/label_*/)) {
-      await this._createLabel(labelObj);
+    const labelKeys = Object.keys(this._getFilteredConfigObjByRegex(/label_*/));
+    for (let i = 0; i < labelKeys.length; i++) {
+      await this._createLabel(labelKeys[i]);
+    }
+  }
+
+  async _updateLabel(labelKey) {
+    const currentLabelKey = await this._getCurrentLabelKey();
+    if (currentLabelKey) {
+      if (labelKey === PRTriage.STATE.WIP) {
+        this._removeLabel(currentLabelKey);
+      } else if (currentLabelKey !== labelKey) {
+        this._removeLabel(currentLabelKey);
+        this._addLabel(labelKey);
+      }
+    } else {
+      if (labelKey !== PRTriage.STATE.WIP) {
+        this._addLabel(labelKey);
+      }
     }
   }
 
   async _createLabel(key) {
     const { owner, repo } = this.config;
     const labelObj = this._getConfigObj(key);
 
+    // Create a label to repository if the label is not created.
     return this.github.issues
       .getLabel({ owner, repo, name: labelObj.name })
       .catch(() => {
@@ -155,15 +225,15 @@ class PRTriage {
 
   async _addLabel(key) {
     const { owner, repo } = this.config;
-    const number = this.pullRequest.number;
+    const issue_number = this.pullRequest.number;
     const labelObj = this._getConfigObj(key);
 
-    // Check if a label does not exist. If it does, it addes the label.
+    // Add a label to issue if it does not exist.
     return this._getLabel(key).catch(() => {
       return this.github.issues.addLabels({
         owner,
         repo,
-        number,
+        issue_number,
         labels: [labelObj.name]
       });
     });
@@ -174,14 +244,14 @@ class PRTriage {
     const issue_number = this.pullRequest.number;
     const labelObj = this._getConfigObj(key);
 
-    // Check if a label exists. If it does, it removes the label.
+    // Remove the label from a issue if it exists.
     return this._getLabel(key).then(
       labelObj => {
         return this.github.issues
           .removeLabel({ owner, repo, issue_number, name: labelObj.name })
           .catch(err => {
             // Ignore if it's a 404 because then the label was already removed
-            if (err.code !== 404) {
+            if (err.status !== 404) {
               throw err;
             }
           });
@@ -190,22 +260,6 @@ class PRTriage {
     ); // Do nothing for error handling.
   }
 
-  async _updateLabel(labelKey) {
-    const currentLabelKey = await this._getCurrentLabelKey();
-    if (currentLabelKey) {
-      if (labelKey === PRTriage.STATE.WIP) {
-        this._removeLabel(currentLabelKey);
-      } else if (currentLabelKey !== labelKey) {
-        this._removeLabel(currentLabelKey);
-        this._addLabel(labelKey);
-      }
-    } else {
-      if (labelKey !== PRTriage.STATE.WIP) {
-        this._addLabel(labelKey);
-      }
-    }
-  }
-
   _getLabel(key) {
     return new Promise((resolve, reject) => {
       for (const label of this.pullRequest.labels) {

package.json

@@ -4,7 +4,7 @@
   "description": "GitHub App built with Probot that support pull request workflow",
   "main": "index.js",
   "scripts": {
-    "start": "DEBUG=probot* probot run ./index.js",
+    "start": "probot run ./index.js",
     "start:dev": "nodemon --exec \"npm start\"",
     "commit": "git-cz",
     "lint": "prettier --write {index,lib/**/*,test/pr-triage.test}.js",
@@ -32,6 +32,7 @@
   "author": "Sam Yamashita",
   "license": "Apache-2.0",
   "dependencies": {
+    "@sentry/node": "^5.14.2",
     "debug": "^4.1.1",
     "dotenv": "^8.0.0",
     "probot": "^9.2.10",
@@ -48,7 +49,6 @@
     "nodemon": "^2.0.1",
     "prettier": "1.19.1",
     "probot-config": "^1.0.0",
-    "raven": "^2.6.3",
     "semantic-release": "^17.0.2",
     "smee-client": "^1.1.0",
     "validate-commit-msg": "^2.14.0"