Honor GitHub PR approvals as CR signoffs

README.md

@@ -49,12 +49,17 @@ the pull is considered ready. Conversely, if a pull only touches test code, you
 might put only `qa_req 0` on it to say that it doesn't need to be QAed, since
 it should break tests if there's anything wrong with it.
 
-When you CR a pull, add a comment to it with `CR :emoji:` in it.
+When you CR a pull, you can either approve it on GitHub (using GitHub's native
+**Approve** review action) or add a comment containing `CR :emoji:`.
 (`emoji` is simply a word or words between colons; we often use GitHub emoji,
-which follow this format.) This comment is considered a _signoff_. Pulldasher
-will update the pull's display to indicate that one of the required CRs is
-completed. Similarly, when you QA a pull, add a comment containing
-`QA :emoji:`, and the number of QA signoffs will increase.
+which follow this format.) A GitHub approval and a `CR :emoji:` comment are both
+considered _signoffs_. Pulldasher will update the pull's display to indicate
+that one of the required CRs is completed. Similarly, when you QA a pull, add a
+comment containing `QA :emoji:`, and the number of QA signoffs will increase.
+
+To honor GitHub approvals, enable the **Pull request reviews** webhook event on
+tracked repositories. Set `useGithubApprovalForCr: false` in `config.js` to
+disable this and require `CR :emoji:` comments (legacy behavior).
 
 ## Feature Details
 

config.example.js

@@ -40,8 +40,8 @@ module.exports = {
   //     containing the secret from `hook_secret` below.
   //   - Content type should be `application/x-www-form-urlencoded`
   //   - Choose to be sent individual events, and then check the Issue comments,
-  //     Issues, Pull requests, Pull request review comments, Pushes, and
-  //     Statuses boxes
+  //     Issues, Pull requests, Pull request reviews, Pull request review
+  //     comments, Pushes, and Statuses boxes
   github: {
     // Get this from the GitHub application setup page.
     clientId: "your github application client id",
@@ -63,6 +63,10 @@ module.exports = {
     secret: "secret for signing session cookies",
   },
 
+  // When true (default), GitHub PR reviews with state APPROVED count as CR
+  // signoffs without requiring a CR :emoji: tag in the review body.
+  useGithubApprovalForCr: true,
+
   // List of repositories for pulldasher to watch.
   repos: [
     // Just listing the repo as a string indicates pulldasher should consider

models/signature.js

@@ -54,9 +54,32 @@ Signature.parseReview = function parseReview(review, repoFullName, pullNumber) {
     user: review.user,
     created_at: review.submitted_at,
     id: review.id,
+    state: review.state,
   };
 
-  return parseSignatures(reviewData);
+  const signatures = parseSignatures(reviewData);
+
+  // A GitHub "Approve" review counts as a CR signoff even without a
+  // `CR :emoji:` tag in the body. Skip if the body already produced one.
+  if (
+    useGithubApprovalForCr() &&
+    review.state === "APPROVED" &&
+    !signatures.some((sig) => sig.data.type === "CR")
+  ) {
+    signatures.push(
+      new Signature({
+        repo: repoFullName,
+        number: pullNumber,
+        user: review.user,
+        type: "CR",
+        created_at: review.submitted_at,
+        active: true,
+        comment_id: review.id,
+      })
+    );
+  }
+
+  return signatures;
 };
 
 function parseSignatures(data) {
@@ -114,7 +137,11 @@ Signature.compare = function (a, b) {
 };
 
 function hasTag(body, tag) {
-  return tag.regex.test(body);
+  return tag.regex.test(body || "");
+}
+
+function useGithubApprovalForCr() {
+  return config.useGithubApprovalForCr !== false;
 }
 
 export default Signature;

package.json

@@ -66,6 +66,7 @@
     "frontend:watch": "webpack --watch --config ./frontend/webpack.dev.config.js",
     "postinstall": "npm run build",
     "build": "webpack --config ./frontend/webpack.prod.config.js",
+    "test": "node --test test/*.test.js",
     "precommit": "lint-staged"
   },
   "lint-staged": {

test/signature.test.js

@@ -0,0 +1,62 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+process.env.CONFIG_PATH = "../test/test-config.js";
+
+const Signature = (await import("../models/signature.js")).default;
+
+const reviewUser = { id: 1, login: "reviewer" };
+const repo = "owner/repo";
+const number = 42;
+
+test("parseReview creates CR signature from APPROVED review with empty body", () => {
+  const signatures = Signature.parseReview(
+    {
+      id: 100,
+      body: "",
+      state: "APPROVED",
+      user: reviewUser,
+      submitted_at: "2024-01-01T00:00:00Z",
+    },
+    repo,
+    number
+  );
+
+  assert.equal(signatures.length, 1);
+  assert.equal(signatures[0].data.type, "CR");
+  assert.equal(signatures[0].data.comment_id, 100);
+  assert.equal(signatures[0].data.user.login, "reviewer");
+});
+
+test("parseReview keeps single CR signature when body already contains CR tag", () => {
+  const signatures = Signature.parseReview(
+    {
+      id: 101,
+      body: "CR :thumbsup:",
+      state: "APPROVED",
+      user: reviewUser,
+      submitted_at: "2024-01-01T00:00:00Z",
+    },
+    repo,
+    number
+  );
+
+  assert.equal(signatures.length, 1);
+  assert.equal(signatures[0].data.type, "CR");
+});
+
+test("parseReview ignores CHANGES_REQUESTED reviews", () => {
+  const signatures = Signature.parseReview(
+    {
+      id: 102,
+      body: "Please fix this",
+      state: "CHANGES_REQUESTED",
+      user: reviewUser,
+      submitted_at: "2024-01-01T00:00:00Z",
+    },
+    repo,
+    number
+  );
+
+  assert.equal(signatures.length, 0);
+});

test/test-config.js

@@ -0,0 +1,19 @@
+var emoji =
+  "(\u00a9|\u00ae|[\u2000-\u3300]|\ud83c[\ud000-\udfff]|\ud83d[\ud000-\udfff]|\ud83e[\ud000-\udfff])";
+var emojiText = ":[^\n:]+:";
+var signature = "(" + emojiText + "|" + emoji + ")";
+
+export default {
+  useGithubApprovalForCr: true,
+  repos: ["owner/repo"],
+  tags: [
+    {
+      name: "CR",
+      regex: new RegExp("\\bCR " + signature, "i"),
+    },
+    {
+      name: "QA",
+      regex: new RegExp("\\bQA " + signature, "i"),
+    },
+  ],
+};