diff --git a/controllers/githubHooks.js b/controllers/githubHooks.js index 0b63a916..97e77eaf 100644 --- a/controllers/githubHooks.js +++ b/controllers/githubHooks.js @@ -3,7 +3,6 @@ import Promise from "bluebird"; import debug from "../lib/debug.js"; import Pull from "../models/pull.js"; import Signature from "../models/signature.js"; -import Issue from "../models/issue.js"; import Comment from "../models/comment.js"; import Review from "../models/review.js"; import Status from "../models/status.js"; @@ -196,51 +195,16 @@ function handleIssueEvent(body) { var doneHandling = handleLabelEvents(body); - // If we get issue events for pull requests, we have to go refresh from - // the api cause the body here is just a subset of pull request fields - // See https://docs.github.com/en/rest/reference/issues#list-issues-assigned-to-the-authenticated-user - // - // > GitHub's REST API v3 considers every pull request an issue, but - // not every issue is a pull request. For this reason, "Issues" - // endpoints may return both issues and pull requests in the response. - // You can identify pull requests by the pull_request key. - if (body.issue.pull_request) { - return doneHandling.then(function () { - // Not returning here cause we don't want to delay replying to the - // hook with a 200 since we know what needs to be done. - refreshPullOrIssue(body); - }); - } - - switch (body.action) { - case "opened": - // Always do this for opened issues because a full refresh - // is the easiest way to get *who* assigned the initial labels. - return doneHandling.then(function () { - // Not returning here cause we don't want to delay replying to the - // hook with a 200 since we know what needs to be done. - refreshPullOrIssue(body); - }); - - case "reopened": - case "closed": - case "edited": - case "assigned": - case "unassigned": - // Default case is update the issue - } - - return doneHandling - .then(function () { - // Copy the full name of the repo into the issue object so we can - // normalize the structure. - body.issue.repo = body.repository.full_name; - return Issue.getFromGH(body.issue); - }) - .then(dbManager.updateIssue) - .then(function () { - return reprocessLabels(body.repository.full_name, body.issue.number); - }); + // Always refresh from the API rather than upserting the webhook body + // directly. The body is just a subset of the fields (and for pull requests, + // not even an issue), and it carries no events, which we need to attribute + // labels and to date the current assignment (date_assigned). + // refreshPullOrIssue dispatches pull-vs-issue from the body itself. + return doneHandling.then(function () { + // Not returning here cause we don't want to delay replying to the + // hook with a 200 since we know what needs to be done. + refreshPullOrIssue(body); + }); } /** @@ -274,18 +238,6 @@ function handleLabelEvents(body) { return Promise.resolve(); } -/** - * After a label has been added or removed we have to re-process all the labels - * in case one of them matches one of our configured label updaters. - */ -function reprocessLabels(repo, issueNumber) { - if (!config.labels || !config.labels.length) { - return; - } - debug("Reprocessing labels for Issue #%s in repo %s", issueNumber, repo); - return dbManager.getIssue(repo, issueNumber).then(dbManager.updateIssue); -} - function refreshPullOrIssue(responseBody) { var repo = responseBody.repository.full_name; diff --git a/lib/git-manager.js b/lib/git-manager.js index 9c31b82c..073489f3 100644 --- a/lib/git-manager.js +++ b/lib/git-manager.js @@ -305,6 +305,9 @@ export default { // multiple repos. The ghIssue object doesn't contain the repo name. var labels = getLabelsFromEvents(events, ghIssue); + // The issue object has no assignment timestamp; derive it from events. + ghIssue.date_assigned = getAssignedDateFromEvents(events, ghIssue); + return Issue.getFromGH(ghIssue, labels); }); }, @@ -402,6 +405,32 @@ function getLabelsFromEvents(events, ghIssue) { }); } +/** + * Find when the issue's current assignee was assigned, from its events. + * + * The issue object carries the assignee but no assignment time, so we read it + * from the most recent `assigned` event for that assignee. Returns the event's + * raw `created_at` timestamp (normalized by `getFromGH`, like `closed_at`), or + * null when the issue is unassigned or no matching event exists. + */ +function getAssignedDateFromEvents(events, ghIssue) { + var assignee = ghIssue.assignee && ghIssue.assignee.login; + if (!assignee) { + return null; + } + + var assignments = _.filter(events, function (event) { + return ( + event.event === "assigned" && + event.assignee && + event.assignee.login === assignee + ); + }); + + var latest = _.last(_.sortBy(assignments, "created_at")); + return latest ? latest.created_at : null; +} + /** * Return the default api params merged with the overrides */ diff --git a/migrations/0019-issues--add-author-state-reason-date-assigned.sql b/migrations/0019-issues--add-author-state-reason-date-assigned.sql new file mode 100644 index 00000000..e86f1030 --- /dev/null +++ b/migrations/0019-issues--add-author-state-reason-date-assigned.sql @@ -0,0 +1,4 @@ +ALTER TABLE `issues` +ADD COLUMN `author` varchar(255) NOT NULL DEFAULT 'ghost' AFTER `assignee`, +ADD COLUMN `state_reason` varchar(30) DEFAULT NULL AFTER `status`, +ADD COLUMN `date_assigned` int DEFAULT NULL AFTER `date_created`; diff --git a/migrations/schema.sql b/migrations/schema.sql index 0972e13b..b4aa0753 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -76,9 +76,12 @@ CREATE TABLE IF NOT EXISTS `issues` ( `milestone_title` varchar(255) COLLATE utf8mb4_general_ci DEFAULT NULL, `milestone_due_on` int DEFAULT NULL, `assignee` varchar(255) COLLATE utf8mb4_general_ci DEFAULT NULL, + `author` varchar(255) COLLATE utf8mb4_general_ci NOT NULL DEFAULT 'ghost', `status` varchar(30) COLLATE utf8mb4_general_ci DEFAULT NULL, + `state_reason` varchar(30) COLLATE utf8mb4_general_ci DEFAULT NULL, `date_closed` int DEFAULT NULL, `date_created` int DEFAULT NULL, + `date_assigned` int DEFAULT NULL, PRIMARY KEY (`repo`,`number`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/models/db_issue.js b/models/db_issue.js index 2453c77a..b2e4a3f4 100644 --- a/models/db_issue.js +++ b/models/db_issue.js @@ -10,8 +10,11 @@ class DBIssue { repo: issue.repo, number: issue.number, title: issue.title, + author: issue.author, assignee: issue.assignee, status: issue.status, + state_reason: issue.state_reason, + date_assigned: utils.toUnixTime(issue.date_assigned), date_created: utils.toUnixTime(issue.date_created), date_closed: utils.toUnixTime(issue.date_closed), difficulty: issue.difficulty, diff --git a/models/issue.js b/models/issue.js index 22b25df9..9e4b109d 100644 --- a/models/issue.js +++ b/models/issue.js @@ -72,7 +72,10 @@ class Issue { repo: data.repo, number: data.number, title: data.title, + author: getLogin(data.user), status: data.state, + state_reason: data.state_reason, + date_assigned: utils.fromDateString(data.date_assigned), date_created: new Date(data.created_at), date_closed: utils.fromDateString(data.closed_at), milestone: data.milestone @@ -96,7 +99,10 @@ class Issue { repo: data.repo, number: data.number, title: data.title, + author: data.author, status: data.status, + state_reason: data.state_reason, + date_assigned: utils.fromUnixTime(data.date_assigned), date_created: utils.fromUnixTime(data.date_created), date_closed: utils.fromUnixTime(data.date_closed), milestone: data.milestone_title