From a35835017b5e2aa66205f991712420da1507a097 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philip=20J=C3=A4genstedt?= <philip@foolip.org>
Date: Mon, 28 Oct 2019 23:58:22 +0100
Subject: [PATCH] Don't react to PR comments

This simplifies the filtering and removes the need to fetch the pull
request in `getPullRequest`.
---
 index.js       | 44 ++++++++------------------------------------
 lib/filter.js  |  4 ++++
 test/filter.js |  7 ++++++-
 3 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/index.js b/index.js
index d8f11a2..45648b7 100644
--- a/index.js
+++ b/index.js
@@ -9,17 +9,10 @@ var express = require("express"),
     labelModel = require('./lib/label-model'),
     metadata = require('./lib/metadata'),
     comment = require('./lib/comment'),
-    github = require('./lib/github'),
     checkRequest = require('./lib/check-request'),
     filter = require('./lib/filter'),
     q = require('q');
 
-function promise(value) {
-    var deferred = q.defer();
-    deferred.resolve(value);
-    return deferred.promise;
-}
-
 function waitFor(ms) {
     var deferred = q.defer();
     setTimeout(function() { deferred.resolve(); }, ms);
@@ -43,13 +36,6 @@ function funkLogErr(num, msg) {
     return function(err) { logArgs("#" + num + ": " + msg + "\n", err); };
 }
 
-function getPullRequest(n, body) {
-    if (body.pull_request) {
-        return promise(body.pull_request);
-    }
-    return github.get("/repos/:owner/:repo/pulls/:number", { number: n });
-}
-
 var currentlyRunning = {};
 
 app.post('/github-hook', function (req, res) {
@@ -59,7 +45,6 @@ app.post('/github-hook', function (req, res) {
         } else if (process.env.NODE_ENV != 'production' || checkRequest(body, req.headers["x-hub-signature"], process.env.GITHUB_SECRET)) {
             res.send(new Date().toISOString());
 
-            // FILTER ALL THE THINGS
             try {
                 body = JSON.parse(body);
             } catch(e) {
@@ -68,38 +53,25 @@ app.post('/github-hook', function (req, res) {
             if (!filter.event(body, logArgs)) {
                 return;
             }
-            if (!body.pull_request && (body.issue && !body.issue.pull_request)) {
-                logArgs("Ignoring event: not a pull request");
-                return;
-            }
-            if (body.pull_request && !filter.pullRequest(body.pull_request, logArgs)) {
+            if (!filter.pullRequest(body.pull_request, logArgs)) {
                 return;
             }
-            // Note: `filter.pullRequest` is checked again after a timeout.
-            // END FILTERNG //
 
             var action = body.action;
-            var isComment = !!body.comment;
-            var issue = body.pull_request || body.issue;
-            var n = issue.number;
-            var u = (issue.user && issue.user.login) || null;
-            var content = issue.body || "";
+            var pr = body.pull_request;
+            var n = pr.number;
+            var u = (pr.user && pr.user.login) || null;
+            var content = pr.body || "";
             if (action == "opened" || action == "synchronize" ||
-                action == "ready_for_review" ||
-                (isComment && action == "created")) {
+                action == "ready_for_review") {
                 if (n in currentlyRunning) {
                     logArgs("#" + n + " is already being processed.");
                     return;
                 }
                 currentlyRunning[n] = true;
-                logArgs("#" + n, isComment ? "comment" : "pull request", action);
+                logArgs("#" + n, action);
 
                 waitFor(5 * 1000).then(function() { // Avoid race condition
-                    return getPullRequest(n, body);
-                }).then(function(pull_request) {
-                    if (!filter.pullRequest(pull_request, logArgs)) {
-                        return;
-                    }
                     return metadata(n, u, content).then(function(metadata) {
                         logArgs(metadata);
                         return labelModel.post(n, metadata.labels).then(
@@ -119,7 +91,7 @@ app.post('/github-hook', function (req, res) {
                     funkLogErr(n, "THIS SHOULDN'T EVER HAPPEN")(err);
                 });
             } else {
-                logArgs("#" + n + ": not handled.", "action:", action, "isComment:", isComment);
+                logArgs("#" + n + ": not handled.", "action:", action);
             }
         } else {
             logArgs("Unverified request", req);
diff --git a/lib/filter.js b/lib/filter.js
index 4272302..1242fbf 100644
--- a/lib/filter.js
+++ b/lib/filter.js
@@ -9,6 +9,10 @@ function filterEvent(body, log) {
         log("Ignoring event: no body");
         return false;
     }
+    if (!body.pull_request) {
+        log("Ignoring event: not a pull request");
+        return false;
+    }
     if (body.sender && body.sender.login == "wpt-pr-bot") {
         log("Ignoring event: sender is wpt-pr-bot");
         return false;
diff --git a/test/filter.js b/test/filter.js
index 43402de..7f47b8b 100644
--- a/test/filter.js
+++ b/test/filter.js
@@ -12,10 +12,14 @@ suite('Event filtering', function() {
         assert.strictEqual(filter.event(null, nullLog), false);
     });
     test('empty body', function() {
-        assert.strictEqual(filter.event({}, nullLog), true);
+        assert.strictEqual(filter.event({}, nullLog), false);
+    });
+    test('pull request', function() {
+        assert.strictEqual(filter.event({pull_request: {}}, nullLog), true);
     });
     test('wpt-pr-bot sender', function() {
         assert.strictEqual(filter.event({
+            pull_request: {},
             sender: {
                 login: 'wpt-pr-bot'
             }
@@ -23,6 +27,7 @@ suite('Event filtering', function() {
     });
     test('other sender', function() {
         assert.strictEqual(filter.event({
+            pull_request: {},
             sender: {
                 login: 'some-other-user'
             }