From 78f18d53e83585c9b0b90827a1bcee6edd64227b Mon Sep 17 00:00:00 2001 From: John Karahalis Date: Tue, 26 Jan 2016 16:56:17 -0500 Subject: [PATCH] Fix #79: Never post the same comment twice --- lib/commenter.js | 92 ++++++++++++++++----- migrations/20160121091841-create-comment.js | 39 +++++++++ models/comment.js | 17 ++++ routes/hook/index.js | 5 +- worker.js | 2 +- 5 files changed, 131 insertions(+), 24 deletions(-) create mode 100644 migrations/20160121091841-create-comment.js create mode 100644 models/comment.js diff --git a/lib/commenter.js b/lib/commenter.js index f0b9e02..bbd0f24 100644 --- a/lib/commenter.js +++ b/lib/commenter.js @@ -5,39 +5,87 @@ * create_pull_request_comment. */ +var Q = require('q'); var request = require('request'); var config = require('./config'); +var models = require('../models'); /** * Post a line comment on a particular pull request. commentURL should be an * instance of review_comments_url. * https://developer.github.com/v3/activity/events/types/#pullrequestevent */ -function postPullRequestComment(commentURL, comment, filename, commitSHA, line, done) { - - request({ - url: commentURL, - method: 'POST', - headers: { - 'User-Agent': config.brand, - 'Authorization': 'token ' + config.token - }, - body: JSON.stringify({ - body: comment, - path: filename, - commit_id: commitSHA, - position: line - }) - }, function(error, response) { - // If the comment could not be submitted, notify Redis so that the job - // can be re-attempted later. Otherwise, mark the job as done. - if (error || response.statusCode === 403) { - return done(new Error('Comment could not be submitted')); - } else { - done(); +function postPullRequestComment(commentURL, commitSHA, repo, pr, filename, line, comment, incompatibleFeature, done) { + var alreadyCommented = commentExists(repo, pr, filename, line, incompatibleFeature); + alreadyCommented.then(function(alreadyCommented) { + if (!alreadyCommented) { + request({ + url: commentURL, + method: 'POST', + headers: { + 'User-Agent': config.brand, + 'Authorization': 'token ' + config.token + }, + body: JSON.stringify({ + body: comment, + path: filename, + commit_id: commitSHA, + position: line + }) + }, function(error, response) { + // If the comment could not be submitted, notify Redis so that the job + // can be re-attempted later. Otherwise, mark the job as done. + if (error || response.statusCode === 403) { + return done(new Error('Comment could not be submitted')); + } else { + models.Comment.create({ + repo: repo, + pr: pr, + filename: filename, + line: line, + feature: incompatibleFeature + }).then(function() { + done(); + }); + } + }); + } }); } +/** + * Return true if a comment has already been posted about this incompatibility. + */ +function commentExists(repo, pr, filename, line, incompatibleFeature) { + var deferred = Q.defer(); + + models.Comment.count({ + where: { + repo: repo, + pr: pr, + filename: filename, + line: line, + feature: incompatibleFeature + } + }).then( + // Success callback (a matching comment was found) + function(count) { + if (count > 0) { + deferred.resolve(true); + } else { + deferred.resolve(false); + } + }, + + // Failure callback (no matching comments were found) + function(error) { + deferred.reject(error); + } + ); + + return deferred.promise; +} + exports.postPullRequestComment = postPullRequestComment; diff --git a/migrations/20160121091841-create-comment.js b/migrations/20160121091841-create-comment.js new file mode 100644 index 0000000..6aacd2d --- /dev/null +++ b/migrations/20160121091841-create-comment.js @@ -0,0 +1,39 @@ +'use strict'; +module.exports = { + up: function(queryInterface, Sequelize) { + return queryInterface.createTable('Comments', { + id: { + allowNull: false, + autoIncrement: true, + primaryKey: true, + type: Sequelize.INTEGER + }, + repo: { + type: Sequelize.TEXT + }, + pr: { + type: Sequelize.INTEGER + }, + filename: { + type: Sequelize.TEXT + }, + line: { + type: Sequelize.INTEGER + }, + feature: { + type: Sequelize.TEXT + }, + createdAt: { + allowNull: false, + type: Sequelize.DATE + }, + updatedAt: { + allowNull: false, + type: Sequelize.DATE + } + }); + }, + down: function(queryInterface, Sequelize) { + return queryInterface.dropTable('Comments'); + } +}; diff --git a/models/comment.js b/models/comment.js new file mode 100644 index 0000000..726f0b9 --- /dev/null +++ b/models/comment.js @@ -0,0 +1,17 @@ +'use strict'; +module.exports = function(sequelize, DataTypes) { + var Comment = sequelize.define('Comment', { + repo: DataTypes.TEXT, + pr: DataTypes.INTEGER, + filename: DataTypes.TEXT, + line: DataTypes.INTEGER, + feature: DataTypes.TEXT + }, { + classMethods: { + associate: function(models) { + // associations can be defined here + } + } + }); + return Comment; +}; diff --git a/routes/hook/index.js b/routes/hook/index.js index a704c50..900c6a1 100644 --- a/routes/hook/index.js +++ b/routes/hook/index.js @@ -80,9 +80,12 @@ function processPullRequest(destinationRepo, originRepo, originBranch, prNumber, var commentJob = redisQueue.create('comment', { commentURL: commentURL, sha: currentCommit.sha, + repo: destinationRepo, + pr: prNumber, filename: file.filename, line: line, - comment: comment + comment: comment, + incompatibleFeature: incompatibility.featureData.title }); // If the comment is rejected, re-attempt several times with diff --git a/worker.js b/worker.js index 4b69fb1..6db75ae 100644 --- a/worker.js +++ b/worker.js @@ -23,7 +23,7 @@ redisQueue.process('comment', function(job, done) { } setTimeout(function() { - commenter.postPullRequestComment(job.data.commentURL, job.data.comment, job.data.filename, job.data.sha, job.data.line, done); + commenter.postPullRequestComment(job.data.commentURL, job.data.sha, job.data.repo, job.data.pr, job.data.filename, job.data.line, job.data.comment, job.data.incompatibleFeature, done); lastCommentTimestamp = Date.now(); // Use a new, fresh timestamp }, waitRemaining); });