From b74c40939cbe5b32d04c278b7074d67055ca8e4d Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Sun, 6 Nov 2022 21:12:16 +0000 Subject: [PATCH] Allow commands in review submissions (#149) --- README.md | 3 ++ app/commands/close-command.js | 11 ++-- app/commands/label-command.js | 11 ++-- app/commands/remove-label-command.js | 13 +++-- app/commands/reopen-command.js | 15 ++++-- app/commands/reviewer-command.js | 13 +++-- app/commands/transfer-command.js | 11 ++-- app/comment-extractor.js | 21 ++++++++ app/comment-extractor.test.js | 79 ++++++++++++++++++++++++++++ app/router.js | 13 ++++- index.js | 17 +++++- package.json | 1 + 12 files changed, 187 insertions(+), 21 deletions(-) create mode 100644 app/comment-extractor.js create mode 100644 app/comment-extractor.test.js diff --git a/README.md b/README.md index 13eb5c2..3121566 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,8 @@ Run `smee --path /api/github/webhooks` and point your webhook to the smee url th The application runs on port 3000 by default, this can be customized with the `PORT` environment variable. +You can also use the npm script `npm run tunnel` which will run smee for you. + ### Chart deployment You can deploy this application to Kubernetes with the helm chart included in this repo: @@ -69,6 +71,7 @@ See more in the [chart README](charts/github-comment-ops/README.md) - `GITHUB_APP_ID` - `GITHUB_APP_PRIVATE_KEY` + - Either the private key as a string or a file path prefixed by `file:`, e.g. `file:my-key.pem` - `WEBHOOK_SECRET` ## Supported commands diff --git a/app/commands/close-command.js b/app/commands/close-command.js index e83ea43..d71c92c 100644 --- a/app/commands/close-command.js +++ b/app/commands/close-command.js @@ -3,6 +3,11 @@ import { closeMatcher } from "../matchers.js"; import { closeEnabled } from "../command-enabled.js"; import { closeIssue } from "../github.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/close-command"); @@ -12,7 +17,7 @@ export class CloseCommand extends Command { } matches() { - return closeMatcher(this.payload.comment.body); + return closeMatcher(extractBody(this.payload)); } enabled(config) { @@ -38,13 +43,13 @@ export class CloseCommand extends Command { } logger.info( - `Closing issue ${this.payload.issue.html_url}, reason: ${reason}` + `Closing issue ${extractHtmlUrl(this.payload)}, reason: ${reason}` ); try { await closeIssue( authToken, sourceRepo, - this.payload.issue.node_id, + extractLabelableId(this.payload), reason ); } catch (error) { diff --git a/app/commands/label-command.js b/app/commands/label-command.js index 8354b9a..0cd8de5 100644 --- a/app/commands/label-command.js +++ b/app/commands/label-command.js @@ -5,6 +5,11 @@ import { Command } from "./command.js"; import { extractCommaSeparated } from "../converters.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/label-command"); @@ -14,7 +19,7 @@ export class LabelCommand extends Command { } matches() { - return labelMatcher(this.payload.comment.body); + return labelMatcher(extractBody(this.payload)); } enabled(config) { @@ -32,14 +37,14 @@ export class LabelCommand extends Command { }); logger.info( - `Labeling issue ${this.payload.issue.html_url} with labels ${labels}` + `Labeling issue ${extractHtmlUrl(this.payload)} with labels ${labels}` ); try { await addLabel( authToken, this.payload.repository.owner.login, sourceRepo, - this.payload.issue.node_id, + extractLabelableId(this.payload), labels ); } catch (error) { diff --git a/app/commands/remove-label-command.js b/app/commands/remove-label-command.js index da1ea82..b9a155a 100644 --- a/app/commands/remove-label-command.js +++ b/app/commands/remove-label-command.js @@ -4,6 +4,11 @@ import { removeLabel } from "../github.js"; import { Command } from "./command.js"; import { extractCommaSeparated } from "../converters.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/remove-label-command"); @@ -13,7 +18,7 @@ export class RemoveLabelCommand extends Command { } matches() { - return removeLabelMatcher(this.payload.comment.body); + return removeLabelMatcher(extractBody(this.payload)); } enabled(config) { @@ -31,14 +36,16 @@ export class RemoveLabelCommand extends Command { }); logger.info( - `Removing label(s) from issue ${this.payload.issue.html_url}, labels ${labels}` + `Removing label(s) from issue ${extractHtmlUrl( + this.payload + )}, labels ${labels}` ); try { await removeLabel( authToken, this.payload.repository.owner.login, sourceRepo, - this.payload.issue.node_id, + extractLabelableId(this.payload), labels ); } catch (error) { diff --git a/app/commands/reopen-command.js b/app/commands/reopen-command.js index 0de44d9..1825f44 100644 --- a/app/commands/reopen-command.js +++ b/app/commands/reopen-command.js @@ -3,6 +3,11 @@ import { reopenEnabled } from "../command-enabled.js"; import { reopenIssue } from "../github.js"; import { Command } from "./command.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/reopen-command"); @@ -12,7 +17,7 @@ export class ReopenCommand extends Command { } matches() { - return reopenMatcher(this.payload.comment.body); + return reopenMatcher(extractBody(this.payload)); } enabled(config) { @@ -32,10 +37,14 @@ export class ReopenCommand extends Command { return; } - logger.info(`Re-opening issue ${this.payload.issue.html_url}`); + logger.info(`Re-opening issue ${extractHtmlUrl(this.payload)}`); try { - await reopenIssue(authToken, sourceRepo, this.payload.issue.node_id); + await reopenIssue( + authToken, + sourceRepo, + extractLabelableId(this.payload) + ); } catch (error) { logger.error( `Failed to reopen issue ${ diff --git a/app/commands/reviewer-command.js b/app/commands/reviewer-command.js index f62a988..6c4bbf7 100644 --- a/app/commands/reviewer-command.js +++ b/app/commands/reviewer-command.js @@ -4,6 +4,11 @@ import { requestReviewers } from "../github.js"; import { Command } from "./command.js"; import { extractUsersAndTeams } from "../converters.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/reviewer-command"); @@ -13,7 +18,7 @@ export class ReviewerCommand extends Command { } matches() { - return reviewerMatcher(this.payload.comment.body); + return reviewerMatcher(extractBody(this.payload)); } enabled(config) { @@ -30,7 +35,9 @@ export class ReviewerCommand extends Command { }); logger.info( - `Requesting review for ${reviewerMatches} at ${this.payload.issue.html_url}` + `Requesting review for ${reviewerMatches} at ${extractHtmlUrl( + this.payload + )}` ); const reviewers = extractUsersAndTeams( this.payload.repository.owner.login, @@ -41,7 +48,7 @@ export class ReviewerCommand extends Command { authToken, this.payload.repository.owner.login, sourceRepo, - this.payload.issue.node_id, + extractLabelableId(this.payload), reviewers.users, reviewers.teams ); diff --git a/app/commands/transfer-command.js b/app/commands/transfer-command.js index 1561514..f084688 100644 --- a/app/commands/transfer-command.js +++ b/app/commands/transfer-command.js @@ -3,6 +3,11 @@ import { transferEnabled } from "../command-enabled.js"; import { transferIssue } from "../github.js"; import { Command } from "./command.js"; import { getLogger } from "../logger.js"; +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "../comment-extractor.js"; const classLogger = getLogger("commands/transfer-command"); @@ -12,7 +17,7 @@ export class TransferCommand extends Command { } matches() { - return transferMatcher(this.payload.comment.body); + return transferMatcher(extractBody(this.payload)); } enabled(config) { @@ -29,7 +34,7 @@ export class TransferCommand extends Command { }); logger.info( - `Transferring issue ${this.payload.issue.html_url} to repo ${targetRepo}` + `Transferring issue ${extractHtmlUrl(this.payload)} to repo ${targetRepo}` ); try { await transferIssue( @@ -37,7 +42,7 @@ export class TransferCommand extends Command { this.payload.repository.owner.login, sourceRepo, targetRepo, - this.payload.issue.node_id + extractLabelableId(this.payload) ); } catch (error) { logger.error( diff --git a/app/comment-extractor.js b/app/comment-extractor.js new file mode 100644 index 0000000..b3b47ca --- /dev/null +++ b/app/comment-extractor.js @@ -0,0 +1,21 @@ +export function extractBody(payload) { + if (payload.review) { + return payload.review.body; + } + return payload.comment.body; +} + +export function extractHtmlUrl(payload) { + if (payload.review) { + return payload.pull_request.html_url; + } + return payload.issue.html_url; +} + +export function extractLabelableId(payload) { + if (payload.review) { + return payload.pull_request.node_id; + } + + return payload.issue.node_id; +} diff --git a/app/comment-extractor.test.js b/app/comment-extractor.test.js new file mode 100644 index 0000000..ff81c71 --- /dev/null +++ b/app/comment-extractor.test.js @@ -0,0 +1,79 @@ +import { + extractBody, + extractHtmlUrl, + extractLabelableId, +} from "./comment-extractor.js"; + +describe("extractors", () => { + describe("extractBody", () => { + test("is a review", () => { + expect( + extractBody({ + review: { + body: "review body", + }, + }) + ).toEqual("review body"); + }); + + test("is an issue", () => { + expect( + extractBody({ + comment: { + body: "comment body", + }, + }) + ).toEqual("comment body"); + }); + }); + + describe("extractHtmlUrl", () => { + test("is a review", () => { + expect( + extractHtmlUrl({ + review: { + body: "review body", + }, + pull_request: { + html_url: "https://github.com/some-org/some-repo/pull/1", + }, + }) + ).toEqual("https://github.com/some-org/some-repo/pull/1"); + }); + + test("is an issue", () => { + expect( + extractHtmlUrl({ + issue: { + html_url: "https://github.com/some-org/some-repo/issue/1", + }, + }) + ).toEqual("https://github.com/some-org/some-repo/issue/1"); + }); + }); + + describe("extractLabelableId", () => { + test("is a review", () => { + expect( + extractLabelableId({ + review: { + body: "review body", + }, + pull_request: { + node_id: "PR_aaaaaa", + }, + }) + ).toEqual("PR_aaaaaa"); + }); + + test("is an issue", () => { + expect( + extractLabelableId({ + issue: { + node_id: "PR_abcdefgh", + }, + }) + ).toEqual("PR_abcdefgh"); + }); + }); +}); diff --git a/app/router.js b/app/router.js index b0eba6a..ff3f135 100644 --- a/app/router.js +++ b/app/router.js @@ -11,6 +11,7 @@ import { getCommands } from "./commands.js"; const OctokitConfig = Octokit.plugin(octoKitConfig); import { getLogger } from "./logger.js"; +import { extractLabelableId } from "./comment-extractor.js"; const classLogger = getLogger("router"); @@ -30,7 +31,7 @@ export async function router(auth, id, payload, verbose) { const octokit = new OctokitConfig({ auth: authToken }); try { - await addReaction(authToken, payload.comment.node_id, "THUMBS_UP"); + await addReaction(authToken, getCommentNodeId(payload), "THUMBS_UP"); } catch (error) { logger.error( `Failed to add reaction ${ @@ -54,9 +55,17 @@ export async function router(auth, id, payload, verbose) { const result = command.enabled(config); await (result.enabled ? command.run(authToken) - : reportError(authToken, payload.issue.node_id, result.error)); + : reportError(authToken, extractLabelableId(payload), result.error)); } } catch (error) { logger.error(error); } } + +function getCommentNodeId(payload) { + if (payload.review) { + return payload.review.node_id; + } + + return payload.comment.node_id; +} diff --git a/index.js b/index.js index f30b5af..a48454d 100755 --- a/index.js +++ b/index.js @@ -7,6 +7,8 @@ import { router } from "./app/router.js"; import { getLogger } from "./app/logger.js"; +import fs from "node:fs/promises"; + const verbose = process.env.VERBOSE === "true"; const secret = process.env.WEBHOOK_SECRET; @@ -18,15 +20,28 @@ const webhooks = new Webhooks({ secret, }); +function getPrivateKey(key) { + if (key.startsWith("file:")) { + const filePath = key.replace("file:", ""); + return fs.readFile(filePath, { encoding: "utf8" }); + } + + return key; +} + const auth = createAppAuth({ appId: process.env.GITHUB_APP_ID, - privateKey: process.env.GITHUB_APP_PRIVATE_KEY, + privateKey: await getPrivateKey(process.env.GITHUB_APP_PRIVATE_KEY), }); webhooks.on("issue_comment.created", async ({ id, payload }) => { await router(auth, id, payload, verbose); }); +webhooks.on("pull_request_review", async ({ id, payload }) => { + await router(auth, id, payload, verbose); +}); + createServer( createNodeMiddleware(webhooks, { // Return 200 for health probes diff --git a/package.json b/package.json index bc576f2..c353f68 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "main": "index.js", "scripts": { "start": "node index.js", + "tunnel": "smee --path /api/github/webhooks", "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js", "lint": "eslint . && prettier --check .", "lint:fix": "prettier --write . && eslint --fix ."