From 90c30df1e7eede68f2fda4cdf0540475699f1b80 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 26 Jul 2022 09:00:02 +0100 Subject: [PATCH 01/17] Config based approach to commands --- app/command-enabled.js | 29 ++++++ app/default-config.js | 27 ++++++ app/github.js | 2 +- app/router.js | 79 +++++++++++----- package-lock.json | 203 ++++++++++++++++++++++++++++++++++++++++- package.json | 4 +- 6 files changed, 318 insertions(+), 26 deletions(-) create mode 100644 app/command-enabled.js create mode 100644 app/default-config.js diff --git a/app/command-enabled.js b/app/command-enabled.js new file mode 100644 index 0000000..8b113e3 --- /dev/null +++ b/app/command-enabled.js @@ -0,0 +1,29 @@ +export function transferEnabled(octokit, config) { + const transferConfig = config.commands.transfer; + + // TODO check permissions + return transferConfig.enabled; +} + +export function labelEnabled(octokit, config, labels) { + const labelConfig = config.commands.label; + + if (!labelConfig.enabled) { + return { + enabled: false, + error: "/label is not enabled for this repository", + }; + } + + if (!labels.includes(labelConfig.allowed_labels[0])) { + return { + enabled: false, + error: `${labels} doesn't match the allowed labels ${labelConfig.allowed_labels}`, + }; + } + + // TODO check permissions + return { + enabled: true, + }; +} diff --git a/app/default-config.js b/app/default-config.js new file mode 100644 index 0000000..e374cde --- /dev/null +++ b/app/default-config.js @@ -0,0 +1,27 @@ +export const defaultConfig = { + commands: { + label: { + permission: "author-or-member", + allowed_labels: ["*"], + enabled: true, + }, + "remove-label": { + permission: "none", + // TODO wildcard + allowed_labels: ["enhancement"], + enabled: true, + }, + reopen: { + permission: "member", + enabled: true, + }, + reviewer: { + permission: "none", + enabled: true, + }, + transfer: { + permission: "write-single-or-author", + enabled: true, + }, + }, +}; diff --git a/app/github.js b/app/github.js index e134df4..7322b71 100644 --- a/app/github.js +++ b/app/github.js @@ -211,7 +211,7 @@ async function lookupTeam(token, organization, teamName, originalTeamName) { }; } -async function reportError(token, subjectId, comment) { +export async function reportError(token, subjectId, comment) { await graphql( ` mutation ($comment: String!, $subjectId: ID!) { diff --git a/app/router.js b/app/router.js index 6287b22..166f9fb 100644 --- a/app/router.js +++ b/app/router.js @@ -12,28 +12,56 @@ import { removeLabel, reopenIssue, requestReviewers, + reportError, transferIssue, } from "./github.js"; import { getAuthToken } from "./auth.js"; import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; +import { labelEnabled, transferEnabled } from "./command-enabled.js"; +import { defaultConfig } from "./default-config.js"; + +const { Octokit } = require("@octokit/core"); +const { config, composeConfigGet } = require("@probot/octokit-plugin-config"); export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; const transferMatches = transferMatcher(payload.comment.body); const actorRequest = `as requested by ${payload.sender.login}`; + + const authToken = await getAuthToken(auth, payload.installation.id); + const octokit = new Octokit({ auth: authToken }); + + // TODO validate against schema + const { config } = await octokit.config.get({ + owner: payload.repository.owner.login, + repo: sourceRepo, + path: ".github/comment-ops.yml", + defaults: defaultConfig, + }); + if (transferMatches) { - const targetRepo = transferMatches[1]; - console.log( - `${id} Transferring issue ${payload.issue.html_url} to repo ${targetRepo} ${actorRequest}` - ); - await transferIssue( - await getAuthToken(auth, payload.installation.id), - payload.repository.owner.login, - sourceRepo, - targetRepo, - payload.issue.node_id - ); - return; + const enabled = await transferEnabled(octokit, config); + + if (enabled) { + const targetRepo = transferMatches[1]; + console.log( + `${id} Transferring issue ${payload.issue.html_url} to repo ${targetRepo} ${actorRequest}` + ); + await transferIssue( + await getAuthToken(auth, payload.installation.id), + payload.repository.owner.login, + sourceRepo, + targetRepo, + payload.issue.node_id + ); + return; + } else { + await reportError( + authToken, + payload.issue.node_id, + "/transfer is not enabled for this repository" + ); + } } const closeMatches = closeMatcher(payload.comment.body); @@ -70,18 +98,23 @@ export async function router(auth, id, payload, verbose) { const labelMatches = labelMatcher(payload.comment.body); if (labelMatches) { const labels = extractCommaSeparated(labelMatches[1]); + const result = await labelEnabled(octokit, config, labels); - console.log( - `${id} Labeling issue ${payload.issue.html_url} with labels ${labels} ${actorRequest}` - ); - await addLabel( - await getAuthToken(auth, payload.installation.id), - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels - ); - return; + if (result.enabled) { + console.log( + `${id} Labeling issue ${payload.issue.html_url} with labels ${labels} ${actorRequest}` + ); + await addLabel( + await getAuthToken(auth, payload.installation.id), + payload.repository.owner.login, + sourceRepo, + payload.issue.node_id, + labels + ); + return; + } else { + await reportError(authToken, payload.issue.node_id, result.error); + } } const removeLabelMatches = removeLabelMatcher(payload.comment.body); diff --git a/package-lock.json b/package-lock.json index e82b266..eac6202 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,8 +10,10 @@ "license": "MIT", "dependencies": { "@octokit/auth-app": "^4.0.4", + "@octokit/core": "^4.0.4", "@octokit/graphql": "^5.0.0", - "@octokit/webhooks": "^10.0.9" + "@octokit/webhooks": "^10.0.9", + "@probot/octokit-plugin-config": "^1.1.5" }, "devDependencies": { "@types/jest": "^28.1.6", @@ -1260,6 +1262,76 @@ "node": ">= 14" } }, + "node_modules/@octokit/auth-token": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-3.0.0.tgz", + "integrity": "sha512-MDNFUBcJIptB9At7HiV7VCvU3NcL4GnfCQaP8C5lrxWrRPMJBnemYtehaKSOlaM7AYxeRyj9etenu8LVpSpVaQ==", + "dependencies": { + "@octokit/types": "^6.0.3" + }, + "engines": { + "node": ">= 14" + } + }, + "node_modules/@octokit/core": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-4.0.4.tgz", + "integrity": "sha512-sUpR/hc4Gc7K34o60bWC7WUH6Q7T6ftZ2dUmepSyJr9PRF76/qqkWjE2SOEzCqLA5W83SaISymwKtxks+96hPQ==", + "dependencies": { + "@octokit/auth-token": "^3.0.0", + "@octokit/graphql": "^5.0.0", + "@octokit/request": "^6.0.0", + "@octokit/request-error": "^3.0.0", + "@octokit/types": "^6.0.3", + "before-after-hook": "^2.2.0", + "universal-user-agent": "^6.0.0" + }, + "engines": { + "node": ">= 14" + } + }, + "node_modules/@octokit/core/node_modules/@octokit/endpoint": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-7.0.0.tgz", + "integrity": "sha512-Kz/mIkOTjs9rV50hf/JK9pIDl4aGwAtT8pry6Rpy+hVXkAPhXanNQRxMoq6AeRgDCZR6t/A1zKniY2V1YhrzlQ==", + "dependencies": { + "@octokit/types": "^6.0.3", + "is-plain-object": "^5.0.0", + "universal-user-agent": "^6.0.0" + }, + "engines": { + "node": ">= 14" + } + }, + "node_modules/@octokit/core/node_modules/@octokit/request": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-6.2.0.tgz", + "integrity": "sha512-7IAmHnaezZrgUqtRShMlByJK33MT9ZDnMRgZjnRrRV9a/jzzFwKGz0vxhFU6i7VMLraYcQ1qmcAOin37Kryq+Q==", + "dependencies": { + "@octokit/endpoint": "^7.0.0", + "@octokit/request-error": "^3.0.0", + "@octokit/types": "^6.16.1", + "is-plain-object": "^5.0.0", + "node-fetch": "^2.6.7", + "universal-user-agent": "^6.0.0" + }, + "engines": { + "node": ">= 14" + } + }, + "node_modules/@octokit/core/node_modules/@octokit/request-error": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-3.0.0.tgz", + "integrity": "sha512-WBtpzm9lR8z4IHIMtOqr6XwfkGvMOOILNLxsWvDwtzm/n7f5AWuqJTXQXdDtOvPfTDrH4TPhEvW2qMlR4JFA2w==", + "dependencies": { + "@octokit/types": "^6.0.3", + "deprecation": "^2.0.0", + "once": "^1.4.0" + }, + "engines": { + "node": ">= 14" + } + }, "node_modules/@octokit/endpoint": { "version": "6.0.12", "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-6.0.12.tgz", @@ -1466,6 +1538,34 @@ "node": ">= 14" } }, + "node_modules/@probot/octokit-plugin-config": { + "version": "1.1.5", + "resolved": "https://registry.npmjs.org/@probot/octokit-plugin-config/-/octokit-plugin-config-1.1.5.tgz", + "integrity": "sha512-dPrccDkb5QVZYZ3Gq3aDEdfsuqid687iu+z3jBKFI1LwgQuRaUsmihR0ZLHdXKX6HK6rUw/5Jxg5ZUo0OWWUSA==", + "dependencies": { + "@types/js-yaml": "^4.0.5", + "js-yaml": "^4.1.0" + }, + "peerDependencies": { + "@octokit/core": ">=3" + } + }, + "node_modules/@probot/octokit-plugin-config/node_modules/argparse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==" + }, + "node_modules/@probot/octokit-plugin-config/node_modules/js-yaml": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", + "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", + "dependencies": { + "argparse": "^2.0.1" + }, + "bin": { + "js-yaml": "bin/js-yaml.js" + } + }, "node_modules/@sinclair/typebox": { "version": "0.24.20", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.24.20.tgz", @@ -1579,6 +1679,11 @@ "pretty-format": "^28.0.0" } }, + "node_modules/@types/js-yaml": { + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.5.tgz", + "integrity": "sha512-FhpRzf927MNQdRZP0J5DLIdTXhjLYzeUTmLAu69mnVksLH9CJY3IuSeEgbKUki7GQZm0WqDkGzyxju2EZGD2wA==" + }, "node_modules/@types/json-schema": { "version": "7.0.11", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.11.tgz", @@ -1987,6 +2092,11 @@ "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "dev": true }, + "node_modules/before-after-hook": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-2.2.2.tgz", + "integrity": "sha512-3pZEU3NT5BFUo/AD5ERPWOgQOCZITni6iavr5AUw5AUwQjMlI0kzu5btnyD39AF0gUEsDPwJT+oY1ORBJijPjQ==" + }, "node_modules/brace-expansion": { "version": "1.1.11", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", @@ -6375,6 +6485,63 @@ } } }, + "@octokit/auth-token": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-3.0.0.tgz", + "integrity": "sha512-MDNFUBcJIptB9At7HiV7VCvU3NcL4GnfCQaP8C5lrxWrRPMJBnemYtehaKSOlaM7AYxeRyj9etenu8LVpSpVaQ==", + "requires": { + "@octokit/types": "^6.0.3" + } + }, + "@octokit/core": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-4.0.4.tgz", + "integrity": "sha512-sUpR/hc4Gc7K34o60bWC7WUH6Q7T6ftZ2dUmepSyJr9PRF76/qqkWjE2SOEzCqLA5W83SaISymwKtxks+96hPQ==", + "requires": { + "@octokit/auth-token": "^3.0.0", + "@octokit/graphql": "^5.0.0", + "@octokit/request": "^6.0.0", + "@octokit/request-error": "^3.0.0", + "@octokit/types": "^6.0.3", + "before-after-hook": "^2.2.0", + "universal-user-agent": "^6.0.0" + }, + "dependencies": { + "@octokit/endpoint": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-7.0.0.tgz", + "integrity": "sha512-Kz/mIkOTjs9rV50hf/JK9pIDl4aGwAtT8pry6Rpy+hVXkAPhXanNQRxMoq6AeRgDCZR6t/A1zKniY2V1YhrzlQ==", + "requires": { + "@octokit/types": "^6.0.3", + "is-plain-object": "^5.0.0", + "universal-user-agent": "^6.0.0" + } + }, + "@octokit/request": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-6.2.0.tgz", + "integrity": "sha512-7IAmHnaezZrgUqtRShMlByJK33MT9ZDnMRgZjnRrRV9a/jzzFwKGz0vxhFU6i7VMLraYcQ1qmcAOin37Kryq+Q==", + "requires": { + "@octokit/endpoint": "^7.0.0", + "@octokit/request-error": "^3.0.0", + "@octokit/types": "^6.16.1", + "is-plain-object": "^5.0.0", + "node-fetch": "^2.6.7", + "universal-user-agent": "^6.0.0" + } + }, + "@octokit/request-error": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-3.0.0.tgz", + "integrity": "sha512-WBtpzm9lR8z4IHIMtOqr6XwfkGvMOOILNLxsWvDwtzm/n7f5AWuqJTXQXdDtOvPfTDrH4TPhEvW2qMlR4JFA2w==", + "requires": { + "@octokit/types": "^6.0.3", + "deprecation": "^2.0.0", + "once": "^1.4.0" + } + } + } + }, "@octokit/endpoint": { "version": "6.0.12", "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-6.0.12.tgz", @@ -6551,6 +6718,30 @@ "resolved": "https://registry.npmjs.org/@octokit/webhooks-types/-/webhooks-types-6.2.4.tgz", "integrity": "sha512-MlumL1ClswnrebjNWmUFiHjEqpRl2T3Eh6j6R9mP2SNK7SjYw4tXGeCKHOyZiNpLjc5/1+P39BxwLN28zNn8iQ==" }, + "@probot/octokit-plugin-config": { + "version": "1.1.5", + "resolved": "https://registry.npmjs.org/@probot/octokit-plugin-config/-/octokit-plugin-config-1.1.5.tgz", + "integrity": "sha512-dPrccDkb5QVZYZ3Gq3aDEdfsuqid687iu+z3jBKFI1LwgQuRaUsmihR0ZLHdXKX6HK6rUw/5Jxg5ZUo0OWWUSA==", + "requires": { + "@types/js-yaml": "^4.0.5", + "js-yaml": "^4.1.0" + }, + "dependencies": { + "argparse": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==" + }, + "js-yaml": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", + "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", + "requires": { + "argparse": "^2.0.1" + } + } + } + }, "@sinclair/typebox": { "version": "0.24.20", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.24.20.tgz", @@ -6664,6 +6855,11 @@ "pretty-format": "^28.0.0" } }, + "@types/js-yaml": { + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.5.tgz", + "integrity": "sha512-FhpRzf927MNQdRZP0J5DLIdTXhjLYzeUTmLAu69mnVksLH9CJY3IuSeEgbKUki7GQZm0WqDkGzyxju2EZGD2wA==" + }, "@types/json-schema": { "version": "7.0.11", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.11.tgz", @@ -6964,6 +7160,11 @@ "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "dev": true }, + "before-after-hook": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-2.2.2.tgz", + "integrity": "sha512-3pZEU3NT5BFUo/AD5ERPWOgQOCZITni6iavr5AUw5AUwQjMlI0kzu5btnyD39AF0gUEsDPwJT+oY1ORBJijPjQ==" + }, "brace-expansion": { "version": "1.1.11", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", diff --git a/package.json b/package.json index ff99959..f1abdda 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,10 @@ "license": "MIT", "dependencies": { "@octokit/auth-app": "^4.0.4", + "@octokit/core": "^4.0.4", "@octokit/graphql": "^5.0.0", - "@octokit/webhooks": "^10.0.9" + "@octokit/webhooks": "^10.0.9", + "@probot/octokit-plugin-config": "^1.1.5" }, "devDependencies": { "@types/jest": "^28.1.6", From 388f276944e36ebc3a52ea6e0f744e3219902928 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 26 Jul 2022 20:57:38 +0100 Subject: [PATCH 02/17] Do not use require --- app/router.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/router.js b/app/router.js index 166f9fb..4267ee7 100644 --- a/app/router.js +++ b/app/router.js @@ -20,8 +20,7 @@ import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; import { labelEnabled, transferEnabled } from "./command-enabled.js"; import { defaultConfig } from "./default-config.js"; -const { Octokit } = require("@octokit/core"); -const { config, composeConfigGet } = require("@probot/octokit-plugin-config"); +import { Octokit } from "@octokit/core"; export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; From 79daafeb2f0274a8de0621770c661f3bea2ffcf6 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 26 Jul 2022 21:16:20 +0100 Subject: [PATCH 03/17] Basics are working --- app/command-enabled.js | 4 +++- app/default-config.js | 3 ++- app/router.js | 6 +++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/command-enabled.js b/app/command-enabled.js index 8b113e3..c267d53 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -18,7 +18,9 @@ export function labelEnabled(octokit, config, labels) { if (!labels.includes(labelConfig.allowed_labels[0])) { return { enabled: false, - error: `${labels} doesn't match the allowed labels ${labelConfig.allowed_labels}`, + error: `${labels} doesn't match the allowed labels \`${labelConfig.allowed_labels.join( + "," + )}\``, }; } diff --git a/app/default-config.js b/app/default-config.js index e374cde..07ca681 100644 --- a/app/default-config.js +++ b/app/default-config.js @@ -2,7 +2,8 @@ export const defaultConfig = { commands: { label: { permission: "author-or-member", - allowed_labels: ["*"], + // TODO wildcard + allowed_labels: ["enhancement"], enabled: true, }, "remove-label": { diff --git a/app/router.js b/app/router.js index 4267ee7..7b27da6 100644 --- a/app/router.js +++ b/app/router.js @@ -21,6 +21,7 @@ import { labelEnabled, transferEnabled } from "./command-enabled.js"; import { defaultConfig } from "./default-config.js"; import { Octokit } from "@octokit/core"; +import { config as octoKitConfig } from "@probot/octokit-plugin-config"; export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; @@ -28,7 +29,8 @@ export async function router(auth, id, payload, verbose) { const actorRequest = `as requested by ${payload.sender.login}`; const authToken = await getAuthToken(auth, payload.installation.id); - const octokit = new Octokit({ auth: authToken }); + const OctokitConfig = Octokit.plugin(octoKitConfig); + const octokit = new OctokitConfig({ auth: authToken }); // TODO validate against schema const { config } = await octokit.config.get({ @@ -38,6 +40,8 @@ export async function router(auth, id, payload, verbose) { defaults: defaultConfig, }); + console.log("jsonConfig", JSON.stringify(config)); + if (transferMatches) { const enabled = await transferEnabled(octokit, config); From 80824431ececbacb3d1e091171049065ac4f4c84 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 27 Jul 2022 08:59:40 +0100 Subject: [PATCH 04/17] Merge config --- app/command-enabled.js | 8 ++++++-- app/default-config.js | 6 ++---- app/router.js | 8 ++++---- package-lock.json | 7 +++---- package.json | 3 ++- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/command-enabled.js b/app/command-enabled.js index c267d53..412c2d2 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -11,11 +11,15 @@ export function labelEnabled(octokit, config, labels) { if (!labelConfig.enabled) { return { enabled: false, - error: "/label is not enabled for this repository", + error: "The label command is not enabled for this repository", }; } - if (!labels.includes(labelConfig.allowed_labels[0])) { + // TODO set intersection + if ( + labelConfig.allowed_labels.length > 0 && + !labels.includes(labelConfig.allowed_labels[0]) + ) { return { enabled: false, error: `${labels} doesn't match the allowed labels \`${labelConfig.allowed_labels.join( diff --git a/app/default-config.js b/app/default-config.js index 07ca681..00a50bc 100644 --- a/app/default-config.js +++ b/app/default-config.js @@ -2,14 +2,12 @@ export const defaultConfig = { commands: { label: { permission: "author-or-member", - // TODO wildcard - allowed_labels: ["enhancement"], + allowed_labels: [], enabled: true, }, "remove-label": { permission: "none", - // TODO wildcard - allowed_labels: ["enhancement"], + allowed_labels: [], enabled: true, }, reopen: { diff --git a/app/router.js b/app/router.js index 7b27da6..7ae3852 100644 --- a/app/router.js +++ b/app/router.js @@ -11,8 +11,8 @@ import { closeIssue, removeLabel, reopenIssue, - requestReviewers, reportError, + requestReviewers, transferIssue, } from "./github.js"; import { getAuthToken } from "./auth.js"; @@ -23,6 +23,8 @@ import { defaultConfig } from "./default-config.js"; import { Octokit } from "@octokit/core"; import { config as octoKitConfig } from "@probot/octokit-plugin-config"; +import deepmerge from "deepmerge"; + export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; const transferMatches = transferMatcher(payload.comment.body); @@ -37,11 +39,9 @@ export async function router(auth, id, payload, verbose) { owner: payload.repository.owner.login, repo: sourceRepo, path: ".github/comment-ops.yml", - defaults: defaultConfig, + defaults: (configs) => deepmerge.all([defaultConfig, ...configs]), }); - console.log("jsonConfig", JSON.stringify(config)); - if (transferMatches) { const enabled = await transferEnabled(octokit, config); diff --git a/package-lock.json b/package-lock.json index eac6202..91966f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,8 @@ "@octokit/core": "^4.0.4", "@octokit/graphql": "^5.0.0", "@octokit/webhooks": "^10.0.9", - "@probot/octokit-plugin-config": "^1.1.5" + "@probot/octokit-plugin-config": "^1.1.5", + "deepmerge": "^4.2.2" }, "devDependencies": { "@types/jest": "^28.1.6", @@ -2403,7 +2404,6 @@ "version": "4.2.2", "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-4.2.2.tgz", "integrity": "sha512-FJ3UgI4gIl+PHZm53knsuSFpE+nESMr7M4v9QcgB7S63Kj/6WqMiFQJpBBYz1Pt+66bZpP3Q7Lye0Oo9MPKEdg==", - "dev": true, "engines": { "node": ">=0.10.0" } @@ -7399,8 +7399,7 @@ "deepmerge": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-4.2.2.tgz", - "integrity": "sha512-FJ3UgI4gIl+PHZm53knsuSFpE+nESMr7M4v9QcgB7S63Kj/6WqMiFQJpBBYz1Pt+66bZpP3Q7Lye0Oo9MPKEdg==", - "dev": true + "integrity": "sha512-FJ3UgI4gIl+PHZm53knsuSFpE+nESMr7M4v9QcgB7S63Kj/6WqMiFQJpBBYz1Pt+66bZpP3Q7Lye0Oo9MPKEdg==" }, "deprecation": { "version": "2.3.1", diff --git a/package.json b/package.json index f1abdda..1c7d88a 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,8 @@ "@octokit/core": "^4.0.4", "@octokit/graphql": "^5.0.0", "@octokit/webhooks": "^10.0.9", - "@probot/octokit-plugin-config": "^1.1.5" + "@probot/octokit-plugin-config": "^1.1.5", + "deepmerge": "^4.2.2" }, "devDependencies": { "@types/jest": "^28.1.6", From c61163c4984892522374312848e82daf5530eada Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 27 Jul 2022 17:20:48 +0100 Subject: [PATCH 05/17] Bit of refactoring --- app/command-enabled.js | 2 +- app/commands.js | 41 ++++++++++++++++++++++++++++ app/commands.test.js | 20 ++++++++++++++ app/router.js | 62 +++++++++++++++++------------------------- 4 files changed, 87 insertions(+), 38 deletions(-) create mode 100644 app/commands.js create mode 100644 app/commands.test.js diff --git a/app/command-enabled.js b/app/command-enabled.js index 412c2d2..057d2af 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -2,7 +2,7 @@ export function transferEnabled(octokit, config) { const transferConfig = config.commands.transfer; // TODO check permissions - return transferConfig.enabled; + return Promise.resolve(transferConfig.enabled); } export function labelEnabled(octokit, config, labels) { diff --git a/app/commands.js b/app/commands.js new file mode 100644 index 0000000..14c2d79 --- /dev/null +++ b/app/commands.js @@ -0,0 +1,41 @@ +import { + closeMatcher, + labelMatcher, + removeLabelMatcher, + reopenMatcher, + reviewerMatcher, + transferMatcher, +} from "./matchers.js"; +import { labelEnabled, transferEnabled } from "./command-enabled.js"; + +export function getCommands(commentBody) { + return { + transfer: { + matches: transferMatcher(commentBody), + enabled: (config, octokit) => transferEnabled(octokit, config), + }, + close: { + matches: closeMatcher(commentBody), + }, + reopen: { + matches: reopenMatcher(commentBody), + }, + label: { + matches: labelMatcher(commentBody), + enabled: (octokit, config, labels) => + labelEnabled(octokit, config, labels), + }, + "remove-label": { + matches: removeLabelMatcher(commentBody), + }, + reviewer: { + matches: reviewerMatcher(commentBody), + }, + }; +} + +export function noneMatch(commands) { + return Object.keys(commands) + .map((key) => !!commands[key].matches) + .every((element) => element === false); +} diff --git a/app/commands.test.js b/app/commands.test.js new file mode 100644 index 0000000..a76a070 --- /dev/null +++ b/app/commands.test.js @@ -0,0 +1,20 @@ +import { getCommands, noneMatch } from "./commands.js"; + +describe("commands", () => { + test("noneMatch is true when nothing matches", () => { + const commands = getCommands("nothing that would ever match"); + + expect(noneMatch(commands)).toEqual(true); + }); + + test("noneMatch is false when something matches", () => { + const commands = getCommands("/label one"); + + expect(noneMatch(commands)).toEqual(false); + }); + test("noneMatch is false when multiple matches", () => { + const commands = getCommands("/label one /reviewer reviewer"); + + expect(noneMatch(commands)).toEqual(false); + }); +}); diff --git a/app/router.js b/app/router.js index 7ae3852..114b167 100644 --- a/app/router.js +++ b/app/router.js @@ -1,11 +1,3 @@ -import { - closeMatcher, - labelMatcher, - removeLabelMatcher, - reopenMatcher, - reviewerMatcher, - transferMatcher, -} from "./matchers.js"; import { addLabel, closeIssue, @@ -17,19 +9,27 @@ import { } from "./github.js"; import { getAuthToken } from "./auth.js"; import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; -import { labelEnabled, transferEnabled } from "./command-enabled.js"; import { defaultConfig } from "./default-config.js"; import { Octokit } from "@octokit/core"; import { config as octoKitConfig } from "@probot/octokit-plugin-config"; import deepmerge from "deepmerge"; +import { getCommands, noneMatch } from "./commands.js"; export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; - const transferMatches = transferMatcher(payload.comment.body); const actorRequest = `as requested by ${payload.sender.login}`; + const commands = getCommands(payload.comment.body); + + if (noneMatch(commands)) { + if (verbose) { + console.log("No match for", payload.comment.body); + } + return; + } + const authToken = await getAuthToken(auth, payload.installation.id); const OctokitConfig = Octokit.plugin(octoKitConfig); const octokit = new OctokitConfig({ auth: authToken }); @@ -42,8 +42,9 @@ export async function router(auth, id, payload, verbose) { defaults: (configs) => deepmerge.all([defaultConfig, ...configs]), }); - if (transferMatches) { - const enabled = await transferEnabled(octokit, config); + const transferMatches = commands.transfer.matches; + if (commands.transfer.matches) { + const enabled = await commands.transfer.enabled(octokit, config); if (enabled) { const targetRepo = transferMatches[1]; @@ -51,7 +52,7 @@ export async function router(auth, id, payload, verbose) { `${id} Transferring issue ${payload.issue.html_url} to repo ${targetRepo} ${actorRequest}` ); await transferIssue( - await getAuthToken(auth, payload.installation.id), + authToken, payload.repository.owner.login, sourceRepo, targetRepo, @@ -67,7 +68,7 @@ export async function router(auth, id, payload, verbose) { } } - const closeMatches = closeMatcher(payload.comment.body); + const closeMatches = commands.close.matches; if (closeMatches) { const reason = closeMatches.length > 1 && closeMatches[1] === "not-planned" @@ -76,39 +77,30 @@ export async function router(auth, id, payload, verbose) { console.log( `${id} Closing issue ${payload.issue.html_url}, reason: ${reason} ${actorRequest}` ); - await closeIssue( - await getAuthToken(auth, payload.installation.id), - sourceRepo, - payload.issue.node_id, - reason - ); + await closeIssue(authToken, sourceRepo, payload.issue.node_id, reason); return; } - const reopenMatches = reopenMatcher(payload.comment.body); + const reopenMatches = commands.reopen.matches; if (reopenMatches) { console.log( `${id} Re-opening issue ${payload.issue.html_url} ${actorRequest}` ); - await reopenIssue( - await getAuthToken(auth, payload.installation.id), - sourceRepo, - payload.issue.node_id - ); + await reopenIssue(authToken, sourceRepo, payload.issue.node_id); return; } - const labelMatches = labelMatcher(payload.comment.body); + const labelMatches = commands.label.matches; if (labelMatches) { const labels = extractCommaSeparated(labelMatches[1]); - const result = await labelEnabled(octokit, config, labels); + const result = await commands.label.enabled(octokit, config, labels); if (result.enabled) { console.log( `${id} Labeling issue ${payload.issue.html_url} with labels ${labels} ${actorRequest}` ); await addLabel( - await getAuthToken(auth, payload.installation.id), + authToken, payload.repository.owner.login, sourceRepo, payload.issue.node_id, @@ -120,7 +112,7 @@ export async function router(auth, id, payload, verbose) { } } - const removeLabelMatches = removeLabelMatcher(payload.comment.body); + const removeLabelMatches = commands["remove-label"].matches; if (removeLabelMatches) { const labels = extractCommaSeparated(removeLabelMatches[1]); @@ -128,7 +120,7 @@ export async function router(auth, id, payload, verbose) { `${id} Removing label(s) from issue ${payload.issue.html_url}, labels ${labels} ${actorRequest}` ); await removeLabel( - await getAuthToken(auth, payload.installation.id), + authToken, payload.repository.owner.login, sourceRepo, payload.issue.node_id, @@ -137,7 +129,7 @@ export async function router(auth, id, payload, verbose) { return; } - const reviewerMatches = reviewerMatcher(payload.comment.body); + const reviewerMatches = commands.reviewer.matches; if (reviewerMatches) { console.log( `${id} Requesting review for ${reviewerMatches[1]} at ${payload.issue.html_url} ${actorRequest}` @@ -147,7 +139,7 @@ export async function router(auth, id, payload, verbose) { reviewerMatches[1] ); await requestReviewers( - await getAuthToken(auth, payload.installation.id), + authToken, payload.repository.owner.login, sourceRepo, payload.issue.node_id, @@ -156,8 +148,4 @@ export async function router(auth, id, payload, verbose) { ); return; } - - if (verbose) { - console.log("No match for", payload.comment.body); - } } From 9466deb26e78cf75dac4e65a14c0b51bf77915fe Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 27 Jul 2022 17:40:57 +0100 Subject: [PATCH 06/17] Start adding it --- app/command-enabled.js | 28 +++++++++++++++++++++++++++- app/commands.js | 14 +++++++++++++- app/router.js | 30 ++++++++++++++++++------------ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/app/command-enabled.js b/app/command-enabled.js index 057d2af..c2a73ac 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -2,7 +2,9 @@ export function transferEnabled(octokit, config) { const transferConfig = config.commands.transfer; // TODO check permissions - return Promise.resolve(transferConfig.enabled); + return { + enabled: transferConfig.enabled, + }; } export function labelEnabled(octokit, config, labels) { @@ -33,3 +35,27 @@ export function labelEnabled(octokit, config, labels) { enabled: true, }; } + +export function closeEnabled(config, octokit) { + return { + enabled: true, + }; +} + +export function reopenEnabled(config, octokit) { + return { + enabled: true, + }; +} + +export function removeLabelEnabled(config, octokit, labels) { + return { + enabled: true, + }; +} + +export function reviewerEnabled(config, octokit) { + return { + enabled: true, + }; +} diff --git a/app/commands.js b/app/commands.js index 14c2d79..9929fd8 100644 --- a/app/commands.js +++ b/app/commands.js @@ -6,7 +6,14 @@ import { reviewerMatcher, transferMatcher, } from "./matchers.js"; -import { labelEnabled, transferEnabled } from "./command-enabled.js"; +import { + closeEnabled, + labelEnabled, + removeLabelEnabled, + reopenEnabled, + reviewerEnabled, + transferEnabled, +} from "./command-enabled.js"; export function getCommands(commentBody) { return { @@ -16,9 +23,11 @@ export function getCommands(commentBody) { }, close: { matches: closeMatcher(commentBody), + enabled: (config, octokit) => closeEnabled(config, octokit), }, reopen: { matches: reopenMatcher(commentBody), + enabled: (config, octokit) => reopenEnabled(config, octokit), }, label: { matches: labelMatcher(commentBody), @@ -27,9 +36,12 @@ export function getCommands(commentBody) { }, "remove-label": { matches: removeLabelMatcher(commentBody), + enabled: (config, octokit, labels) => + removeLabelEnabled(config, octokit, labels), }, reviewer: { matches: reviewerMatcher(commentBody), + enabled: (config, octokit) => reviewerEnabled(config, octokit), }, }; } diff --git a/app/router.js b/app/router.js index 114b167..fcd4bd9 100644 --- a/app/router.js +++ b/app/router.js @@ -93,7 +93,7 @@ export async function router(auth, id, payload, verbose) { const labelMatches = commands.label.matches; if (labelMatches) { const labels = extractCommaSeparated(labelMatches[1]); - const result = await commands.label.enabled(octokit, config, labels); + const result = commands.label.enabled(octokit, config, labels); if (result.enabled) { console.log( @@ -116,17 +116,23 @@ export async function router(auth, id, payload, verbose) { if (removeLabelMatches) { const labels = extractCommaSeparated(removeLabelMatches[1]); - console.log( - `${id} Removing label(s) from issue ${payload.issue.html_url}, labels ${labels} ${actorRequest}` - ); - await removeLabel( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels - ); - return; + const result = commands["remove-label"].enabled(octokit, config, labels); + + if (result.enabled) { + console.log( + `${id} Removing label(s) from issue ${payload.issue.html_url}, labels ${labels} ${actorRequest}` + ); + await removeLabel( + authToken, + payload.repository.owner.login, + sourceRepo, + payload.issue.node_id, + labels + ); + return; + } else { + await reportError(authToken, payload.issue.node_id, result.error); + } } const reviewerMatches = commands.reviewer.matches; From dcf97d41668996a5e4a8dda862ecdca292f66b60 Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Wed, 27 Jul 2022 19:40:47 +0100 Subject: [PATCH 07/17] Update app/commands.js Co-authored-by: Joseph Petersen --- app/commands.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/commands.js b/app/commands.js index 9929fd8..3e99250 100644 --- a/app/commands.js +++ b/app/commands.js @@ -47,7 +47,5 @@ export function getCommands(commentBody) { } export function noneMatch(commands) { - return Object.keys(commands) - .map((key) => !!commands[key].matches) - .every((element) => element === false); + return !Object.values(commands).some((command) => command.matches); } From 26efe1e7ef3838fbe44587bd4a1a554b794ca69a Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Thu, 28 Jul 2022 08:51:32 +0100 Subject: [PATCH 08/17] Refactor to add run command --- app/commands.js | 119 ++++++++++++++++++++++++++++++++++++++++++++++-- app/router.js | 98 ++++++++------------------------------- 2 files changed, 132 insertions(+), 85 deletions(-) diff --git a/app/commands.js b/app/commands.js index 3e99250..7868106 100644 --- a/app/commands.js +++ b/app/commands.js @@ -14,34 +14,143 @@ import { reviewerEnabled, transferEnabled, } from "./command-enabled.js"; +import { + addLabel, + closeIssue, + removeLabel, + reopenIssue, + requestReviewers, + transferIssue, +} from "./github.js"; +import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; + +function actorRequest(payload) { + return `as requested by ${payload.sender.login}`; +} export function getCommands(commentBody) { return { transfer: { matches: transferMatcher(commentBody), enabled: (config, octokit) => transferEnabled(octokit, config), + run: async (id, payload, authToken, matches) => { + const targetRepo = matches[1]; + const sourceRepo = payload.repository.name; + console.log( + `${id} Transferring issue ${ + payload.issue.html_url + } to repo ${targetRepo} ${actorRequest(payload)}` + ); + await transferIssue( + authToken, + payload.repository.owner.login, + sourceRepo, + targetRepo, + payload.issue.node_id + ); + }, }, close: { matches: closeMatcher(commentBody), enabled: (config, octokit) => closeEnabled(config, octokit), + run: async (id, payload, authToken, closeMatches) => { + const sourceRepo = payload.repository.name; + const reason = + closeMatches.length > 1 && closeMatches[1] === "not-planned" + ? "NOT_PLANNED" + : "COMPLETED"; + console.log( + `${id} Closing issue ${ + payload.issue.html_url + }, reason: ${reason} ${actorRequest(payload)}` + ); + await closeIssue(authToken, sourceRepo, payload.issue.node_id, reason); + }, }, reopen: { matches: reopenMatcher(commentBody), enabled: (config, octokit) => reopenEnabled(config, octokit), + run: async (id, payload, authToken) => { + const sourceRepo = payload.repository.name; + console.log( + `${id} Re-opening issue ${payload.issue.html_url} ${actorRequest( + payload + )}` + ); + await reopenIssue(authToken, sourceRepo, payload.issue.node_id); + }, }, label: { matches: labelMatcher(commentBody), - enabled: (octokit, config, labels) => - labelEnabled(octokit, config, labels), + enabled: (octokit, config, matches) => { + const labels = extractCommaSeparated(matches[1]); + return labelEnabled(octokit, config, labels); + }, + run: async (id, payload, authToken, matches) => { + const labels = extractCommaSeparated(matches[1]); + const sourceRepo = payload.repository.name; + + console.log( + `${id} Labeling issue ${ + payload.issue.html_url + } with labels ${labels} ${actorRequest(payload)}` + ); + await addLabel( + authToken, + payload.repository.owner.login, + sourceRepo, + payload.issue.node_id, + labels + ); + }, }, - "remove-label": { + removeLabel: { matches: removeLabelMatcher(commentBody), - enabled: (config, octokit, labels) => - removeLabelEnabled(config, octokit, labels), + enabled: (config, octokit, matches) => { + const labels = extractCommaSeparated(matches[1]); + return removeLabelEnabled(config, octokit, labels); + }, + run: async (id, payload, authToken, matches) => { + const sourceRepo = payload.repository.name; + const labels = extractCommaSeparated(matches[1]); + console.log( + `${id} Removing label(s) from issue ${ + payload.issue.html_url + }, labels ${labels} ${actorRequest(payload)}` + ); + await removeLabel( + authToken, + payload.repository.owner.login, + sourceRepo, + payload.issue.node_id, + labels + ); + }, }, reviewer: { matches: reviewerMatcher(commentBody), enabled: (config, octokit) => reviewerEnabled(config, octokit), + run: async (id, payload, authToken, matches) => { + const reviewerMatches = matches[1]; + const sourceRepo = payload.repository.name; + console.log( + `${id} Requesting review for ${reviewerMatches} at ${ + payload.issue.html_url + } ${actorRequest(payload)}` + ); + const reviewers = extractUsersAndTeams( + payload.repository.owner.login, + reviewerMatches + ); + await requestReviewers( + authToken, + payload.repository.owner.login, + sourceRepo, + payload.issue.node_id, + reviewers.users, + reviewers.teams + ); + }, }, }; } diff --git a/app/router.js b/app/router.js index fcd4bd9..9d5290e 100644 --- a/app/router.js +++ b/app/router.js @@ -1,14 +1,5 @@ -import { - addLabel, - closeIssue, - removeLabel, - reopenIssue, - reportError, - requestReviewers, - transferIssue, -} from "./github.js"; +import { reportError } from "./github.js"; import { getAuthToken } from "./auth.js"; -import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; import { defaultConfig } from "./default-config.js"; import { Octokit } from "@octokit/core"; @@ -19,7 +10,6 @@ import { getCommands, noneMatch } from "./commands.js"; export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; - const actorRequest = `as requested by ${payload.sender.login}`; const commands = getCommands(payload.comment.body); @@ -44,21 +34,10 @@ export async function router(auth, id, payload, verbose) { const transferMatches = commands.transfer.matches; if (commands.transfer.matches) { - const enabled = await commands.transfer.enabled(octokit, config); + const enabled = commands.transfer.enabled(octokit, config, transferMatches); if (enabled) { - const targetRepo = transferMatches[1]; - console.log( - `${id} Transferring issue ${payload.issue.html_url} to repo ${targetRepo} ${actorRequest}` - ); - await transferIssue( - authToken, - payload.repository.owner.login, - sourceRepo, - targetRepo, - payload.issue.node_id - ); - return; + await commands.transfer.run(id, payload, authToken, transferMatches); } else { await reportError( authToken, @@ -70,66 +49,40 @@ export async function router(auth, id, payload, verbose) { const closeMatches = commands.close.matches; if (closeMatches) { - const reason = - closeMatches.length > 1 && closeMatches[1] === "not-planned" - ? "NOT_PLANNED" - : "COMPLETED"; - console.log( - `${id} Closing issue ${payload.issue.html_url}, reason: ${reason} ${actorRequest}` - ); - await closeIssue(authToken, sourceRepo, payload.issue.node_id, reason); - return; + await commands.close.run(id, payload, authToken, closeMatches); } const reopenMatches = commands.reopen.matches; if (reopenMatches) { - console.log( - `${id} Re-opening issue ${payload.issue.html_url} ${actorRequest}` - ); - await reopenIssue(authToken, sourceRepo, payload.issue.node_id); - return; + await commands.reopen.run(id, payload, authToken, reopenMatches); } const labelMatches = commands.label.matches; if (labelMatches) { - const labels = extractCommaSeparated(labelMatches[1]); - const result = commands.label.enabled(octokit, config, labels); + const result = commands.label.enabled(octokit, config, labelMatches); if (result.enabled) { - console.log( - `${id} Labeling issue ${payload.issue.html_url} with labels ${labels} ${actorRequest}` - ); - await addLabel( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels - ); - return; + await commands.label.run(id, payload, authToken, labelMatches); } else { await reportError(authToken, payload.issue.node_id, result.error); } } - const removeLabelMatches = commands["remove-label"].matches; + const removeLabelMatches = commands.removeLabel.matches; if (removeLabelMatches) { - const labels = extractCommaSeparated(removeLabelMatches[1]); - - const result = commands["remove-label"].enabled(octokit, config, labels); + const result = commands.removeLabel.enabled( + octokit, + config, + removeLabelMatches + ); if (result.enabled) { - console.log( - `${id} Removing label(s) from issue ${payload.issue.html_url}, labels ${labels} ${actorRequest}` - ); - await removeLabel( + await commands.removeLabel.run( + id, + payload, authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels + removeLabelMatches ); - return; } else { await reportError(authToken, payload.issue.node_id, result.error); } @@ -137,21 +90,6 @@ export async function router(auth, id, payload, verbose) { const reviewerMatches = commands.reviewer.matches; if (reviewerMatches) { - console.log( - `${id} Requesting review for ${reviewerMatches[1]} at ${payload.issue.html_url} ${actorRequest}` - ); - const reviewers = extractUsersAndTeams( - payload.repository.owner.login, - reviewerMatches[1] - ); - await requestReviewers( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - reviewers.users, - reviewers.teams - ); - return; + await commands.reviewer.run(id, payload, authToken, reviewerMatches); } } From f34924eeaf16793f1a57bf3d31986d00e3465477 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Thu, 28 Jul 2022 09:06:00 +0100 Subject: [PATCH 09/17] Refactor --- app/router.js | 61 +++++---------------------------------------------- 1 file changed, 6 insertions(+), 55 deletions(-) diff --git a/app/router.js b/app/router.js index 9d5290e..787e3b9 100644 --- a/app/router.js +++ b/app/router.js @@ -32,64 +32,15 @@ export async function router(auth, id, payload, verbose) { defaults: (configs) => deepmerge.all([defaultConfig, ...configs]), }); - const transferMatches = commands.transfer.matches; - if (commands.transfer.matches) { - const enabled = commands.transfer.enabled(octokit, config, transferMatches); - - if (enabled) { - await commands.transfer.run(id, payload, authToken, transferMatches); - } else { - await reportError( - authToken, - payload.issue.node_id, - "/transfer is not enabled for this repository" - ); - } - } - - const closeMatches = commands.close.matches; - if (closeMatches) { - await commands.close.run(id, payload, authToken, closeMatches); - } - - const reopenMatches = commands.reopen.matches; - if (reopenMatches) { - await commands.reopen.run(id, payload, authToken, reopenMatches); - } - - const labelMatches = commands.label.matches; - if (labelMatches) { - const result = commands.label.enabled(octokit, config, labelMatches); - + const runCommands = Object.values(commands).filter( + (command) => command.matches + ); + for (const command of runCommands) { + const result = command.enabled(octokit, config, command.matches); if (result.enabled) { - await commands.label.run(id, payload, authToken, labelMatches); + await command.run(id, payload, authToken, command.matches); } else { await reportError(authToken, payload.issue.node_id, result.error); } } - - const removeLabelMatches = commands.removeLabel.matches; - if (removeLabelMatches) { - const result = commands.removeLabel.enabled( - octokit, - config, - removeLabelMatches - ); - - if (result.enabled) { - await commands.removeLabel.run( - id, - payload, - authToken, - removeLabelMatches - ); - } else { - await reportError(authToken, payload.issue.node_id, result.error); - } - } - - const reviewerMatches = commands.reviewer.matches; - if (reviewerMatches) { - await commands.reviewer.run(id, payload, authToken, reviewerMatches); - } } From ed123bb39d10b2bbc886eed91c043aec5144f788 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Thu, 28 Jul 2022 09:08:34 +0100 Subject: [PATCH 10/17] One more remove label --- app/default-config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/default-config.js b/app/default-config.js index 00a50bc..a3e5f56 100644 --- a/app/default-config.js +++ b/app/default-config.js @@ -5,7 +5,7 @@ export const defaultConfig = { allowed_labels: [], enabled: true, }, - "remove-label": { + removeLabel: { permission: "none", allowed_labels: [], enabled: true, From 5d2b617fdee047f633b65e29e24b5e1a95907b5a Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 09:18:42 +0100 Subject: [PATCH 11/17] Getting there --- app/command-enabled.js | 76 +++++---- app/command-enabled.test.js | 297 ++++++++++++++++++++++++++++++++++++ app/commands.js | 39 ++--- app/default-config.js | 9 +- app/router.js | 11 +- 5 files changed, 368 insertions(+), 64 deletions(-) create mode 100644 app/command-enabled.test.js diff --git a/app/command-enabled.js b/app/command-enabled.js index c2a73ac..1d8c63d 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -1,61 +1,73 @@ -export function transferEnabled(octokit, config) { - const transferConfig = config.commands.transfer; +const enabled = { + enabled: true, +}; - // TODO check permissions +function notEnabled(command) { return { - enabled: transferConfig.enabled, + enabled: false, + error: `The ${command} is not enabled for this repository`, }; } -export function labelEnabled(octokit, config, labels) { +export function transferEnabled(config) { + if (!config.commands.transfer.enabled) { + return notEnabled("transfer"); + } + + return enabled; +} + +export function labelEnabled(config, labels) { const labelConfig = config.commands.label; if (!labelConfig.enabled) { - return { - enabled: false, - error: "The label command is not enabled for this repository", - }; + return notEnabled("label"); } // TODO set intersection if ( - labelConfig.allowed_labels.length > 0 && - !labels.includes(labelConfig.allowed_labels[0]) + labelConfig.allowedLabels.length > 0 && + !labels.includes(labelConfig.allowedLabels[0]) ) { return { enabled: false, - error: `${labels} doesn't match the allowed labels \`${labelConfig.allowed_labels.join( + error: `${labels} doesn't match the allowed labels \`${labelConfig.allowedLabels.join( "," )}\``, }; } - // TODO check permissions - return { - enabled: true, - }; + return enabled; } -export function closeEnabled(config, octokit) { - return { - enabled: true, - }; +export function closeEnabled(config) { + if (!config.commands.close.enabled) { + return notEnabled("close"); + } + + return enabled; } -export function reopenEnabled(config, octokit) { - return { - enabled: true, - }; +export function reopenEnabled(config) { + if (!config.commands.reopen.enabled) { + return notEnabled("reopen"); + } + + return enabled; } -export function removeLabelEnabled(config, octokit, labels) { - return { - enabled: true, - }; +export function removeLabelEnabled(config, labels) { + if (!config.commands.removeLabel.enabled) { + return notEnabled("remove-label"); + } + + return enabled; } -export function reviewerEnabled(config, octokit) { - return { - enabled: true, - }; +export function reviewerEnabled(config) { + if (!config.commands.reviewer.enabled) { + return notEnabled("reviewer"); + } + + return enabled; } diff --git a/app/command-enabled.test.js b/app/command-enabled.test.js new file mode 100644 index 0000000..93bdcad --- /dev/null +++ b/app/command-enabled.test.js @@ -0,0 +1,297 @@ +import { + closeEnabled, + labelEnabled, + removeLabelEnabled, + reopenEnabled, + reviewerEnabled, + transferEnabled, +} from "./command-enabled.js"; + +describe("command-enabled", () => { + describe("transferEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = transferEnabled({ + commands: { + transfer: { + enabled: true, + }, + }, + }); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = transferEnabled({ + commands: { + transfer: { + enabled: false, + }, + }, + }); + + expect(sut.enabled).toEqual(false); + }); + }); + + describe("labelEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: true, + allowedLabels: [], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: false, + allowedLabels: [], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(false); + }); + + test("is enabled when label is in allowedLabels", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: true, + allowedLabels: ["label2", "label1"], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when label is not in allowedLabels", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: true, + allowedLabels: ["label2", "label1"], + }, + }, + }, + ["label4"] + ); + + expect(sut.enabled).toEqual(false); + }); + test("is enabled when all labels are in allowedLabels", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: true, + allowedLabels: ["label2", "label1", "label3", "label4"], + }, + }, + }, + ["label3", "label1", "label2"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when not all labels are in allowedLabels", () => { + const sut = labelEnabled( + { + commands: { + label: { + enabled: true, + allowedLabels: ["label2", "label1", "label3", "label4"], + }, + }, + }, + ["label2", "label1", "label5"] + ); + + expect(sut.enabled).toEqual(false); + }); + }); + + describe("closeEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = closeEnabled({ + commands: { + close: { + enabled: true, + }, + }, + }); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = closeEnabled({ + commands: { + close: { + enabled: false, + }, + }, + }); + + expect(sut.enabled).toEqual(false); + }); + }); + describe("reopenEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = reopenEnabled({ + commands: { + reopen: { + enabled: true, + }, + }, + }); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = reopenEnabled({ + commands: { + reopen: { + enabled: false, + }, + }, + }); + + expect(sut.enabled).toEqual(false); + }); + }); + + describe("removeLabelEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + allowedLabels: [], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: false, + allowedLabels: [], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(false); + }); + + test("is enabled when label is in allowedLabels", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + allowedLabels: ["label2", "label1"], + }, + }, + }, + ["label1"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when label is not in allowedLabels", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + allowedLabels: ["label2", "label1"], + }, + }, + }, + ["label4"] + ); + + expect(sut.enabled).toEqual(false); + }); + test("is enabled when all labels are in allowedLabels", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + allowedLabels: ["label2", "label1", "label3", "label4"], + }, + }, + }, + ["label3", "label1", "label2"] + ); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when not all labels are in allowedLabels", () => { + const sut = removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + allowedLabels: ["label2", "label1", "label3", "label4"], + }, + }, + }, + ["label2", "label1", "label5"] + ); + + expect(sut.enabled).toEqual(false); + }); + }); + + describe("reviewerEnabled", () => { + test("is enabled when config is enabled", () => { + const sut = reviewerEnabled({ + commands: { + reviewer: { + enabled: true, + }, + }, + }); + + expect(sut.enabled).toEqual(true); + }); + test("is disabled when config is disabled", () => { + const sut = reviewerEnabled({ + commands: { + reviewer: { + enabled: false, + }, + }, + }); + + expect(sut.enabled).toEqual(false); + }); + }); +}); diff --git a/app/commands.js b/app/commands.js index 7868106..a81ccd7 100644 --- a/app/commands.js +++ b/app/commands.js @@ -32,9 +32,9 @@ export function getCommands(commentBody) { return { transfer: { matches: transferMatcher(commentBody), - enabled: (config, octokit) => transferEnabled(octokit, config), - run: async (id, payload, authToken, matches) => { - const targetRepo = matches[1]; + enabled: (config) => transferEnabled(config), + run: async (id, payload, authToken) => { + const targetRepo = this.matches[1]; const sourceRepo = payload.repository.name; console.log( `${id} Transferring issue ${ @@ -52,9 +52,10 @@ export function getCommands(commentBody) { }, close: { matches: closeMatcher(commentBody), - enabled: (config, octokit) => closeEnabled(config, octokit), - run: async (id, payload, authToken, closeMatches) => { + enabled: (config) => closeEnabled(config), + run: async (id, payload, authToken) => { const sourceRepo = payload.repository.name; + const closeMatches = this.matches; const reason = closeMatches.length > 1 && closeMatches[1] === "not-planned" ? "NOT_PLANNED" @@ -69,7 +70,7 @@ export function getCommands(commentBody) { }, reopen: { matches: reopenMatcher(commentBody), - enabled: (config, octokit) => reopenEnabled(config, octokit), + enabled: (config) => reopenEnabled(config), run: async (id, payload, authToken) => { const sourceRepo = payload.repository.name; console.log( @@ -82,12 +83,12 @@ export function getCommands(commentBody) { }, label: { matches: labelMatcher(commentBody), - enabled: (octokit, config, matches) => { - const labels = extractCommaSeparated(matches[1]); - return labelEnabled(octokit, config, labels); + enabled: (config) => { + const labels = extractCommaSeparated(this.matches[1]); + return labelEnabled(config, labels); }, - run: async (id, payload, authToken, matches) => { - const labels = extractCommaSeparated(matches[1]); + run: async (id, payload, authToken) => { + const labels = extractCommaSeparated(this.matches[1]); const sourceRepo = payload.repository.name; console.log( @@ -106,13 +107,13 @@ export function getCommands(commentBody) { }, removeLabel: { matches: removeLabelMatcher(commentBody), - enabled: (config, octokit, matches) => { - const labels = extractCommaSeparated(matches[1]); - return removeLabelEnabled(config, octokit, labels); + enabled: (config) => { + const labels = extractCommaSeparated(this.matches[1]); + return removeLabelEnabled(config, labels); }, - run: async (id, payload, authToken, matches) => { + run: async (id, payload, authToken) => { const sourceRepo = payload.repository.name; - const labels = extractCommaSeparated(matches[1]); + const labels = extractCommaSeparated(this.matches[1]); console.log( `${id} Removing label(s) from issue ${ payload.issue.html_url @@ -129,9 +130,9 @@ export function getCommands(commentBody) { }, reviewer: { matches: reviewerMatcher(commentBody), - enabled: (config, octokit) => reviewerEnabled(config, octokit), - run: async (id, payload, authToken, matches) => { - const reviewerMatches = matches[1]; + enabled: (config) => reviewerEnabled(config), + run: async (id, payload, authToken) => { + const reviewerMatches = this.matches[1]; const sourceRepo = payload.repository.name; console.log( `${id} Requesting review for ${reviewerMatches} at ${ diff --git a/app/default-config.js b/app/default-config.js index a3e5f56..b7c9f7b 100644 --- a/app/default-config.js +++ b/app/default-config.js @@ -1,25 +1,20 @@ export const defaultConfig = { commands: { label: { - permission: "author-or-member", - allowed_labels: [], + allowedLabels: [], enabled: true, }, removeLabel: { - permission: "none", - allowed_labels: [], + allowedLabels: [], enabled: true, }, reopen: { - permission: "member", enabled: true, }, reviewer: { - permission: "none", enabled: true, }, transfer: { - permission: "write-single-or-author", enabled: true, }, }, diff --git a/app/router.js b/app/router.js index 787e3b9..6e1c286 100644 --- a/app/router.js +++ b/app/router.js @@ -25,6 +25,7 @@ export async function router(auth, id, payload, verbose) { const octokit = new OctokitConfig({ auth: authToken }); // TODO validate against schema + // noinspection JSUnusedGlobalSymbols const { config } = await octokit.config.get({ owner: payload.repository.owner.login, repo: sourceRepo, @@ -36,11 +37,9 @@ export async function router(auth, id, payload, verbose) { (command) => command.matches ); for (const command of runCommands) { - const result = command.enabled(octokit, config, command.matches); - if (result.enabled) { - await command.run(id, payload, authToken, command.matches); - } else { - await reportError(authToken, payload.issue.node_id, result.error); - } + const result = command.enabled(config); + result.enabled + ? await command.run(id, payload, authToken) + : await reportError(authToken, payload.issue.node_id, result.error); } } From 10ddcbe32a697ea7913e18dff8a10fb57777182c Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 10:22:32 +0100 Subject: [PATCH 12/17] Tests passing --- README.md | 28 ++++++++++++++++++++++++++++ app/command-enabled.js | 20 +++++++++++++++++--- package.json | 4 ++-- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b30e269..dcd4852 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,34 @@ It uses GitHub webhooks to scale across repositories without needing to add a Gi GitHub apps are used for authentication to limit the required permissions. +## Configuration + +The app can be configured with a `.github/comment-ops.yml` file in the main branch of the repository. + +This can also be applied organization wide by creating it in the organization's `.github` repository. + +The order of configuration is: + +`default config` -> `organization` -> `repository` + +Default configuration: + +```yaml +commands: + label: + allowedLabels: [] # any label is allowed + enabled: true + removeLabel: + allowedLabels: [] # any label is allowed + enabled: true + reopen: + enabled: true + reviewer: + enabled: true + transfer: + enabled: true +``` + ## Getting started First you will need to create a GitHub app. Add the permissions required for the commands you are using (see next section), and tick "Subscribe to events" > "Issue comment" diff --git a/app/command-enabled.js b/app/command-enabled.js index 1d8c63d..290204f 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -24,10 +24,10 @@ export function labelEnabled(config, labels) { return notEnabled("label"); } - // TODO set intersection if ( + // if length is = 0 then all labels are allowed labelConfig.allowedLabels.length > 0 && - !labels.includes(labelConfig.allowedLabels[0]) + !labels.every((label) => labelConfig.allowedLabels.includes(label)) ) { return { enabled: false, @@ -57,10 +57,24 @@ export function reopenEnabled(config) { } export function removeLabelEnabled(config, labels) { - if (!config.commands.removeLabel.enabled) { + const labelConfig = config.commands.removeLabel; + if (!labelConfig.enabled) { return notEnabled("remove-label"); } + if ( + // if length is = 0 then all labels are allowed + labelConfig.allowedLabels.length > 0 && + !labels.every((label) => labelConfig.allowedLabels.includes(label)) + ) { + return { + enabled: false, + error: `${labels} doesn't match the allowed labels \`${labelConfig.allowedLabels.join( + "," + )}\``, + }; + } + return enabled; } diff --git a/package.json b/package.json index 1c7d88a..4a283d5 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,8 @@ "scripts": { "start": "node index.js", "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js", - "lint": "eslint .", - "lint:fix": "eslint --fix ." + "lint": "eslint . && prettier --check .", + "lint:fix": "prettier --write . && eslint --fix ." }, "author": "", "type": "module", From 0ea4ea4d427c3fe2530cd5396d5adbcbed5f654e Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 10:23:36 +0100 Subject: [PATCH 13/17] Code formatting --- app/command-enabled.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/command-enabled.js b/app/command-enabled.js index 290204f..fc30fc9 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -5,7 +5,7 @@ const enabled = { function notEnabled(command) { return { enabled: false, - error: `The ${command} is not enabled for this repository`, + error: `The \`${command}\` is not enabled for this repository`, }; } From aeae367f86d8db4a6745954dcfcf2df50fa261d7 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 11:28:08 +0100 Subject: [PATCH 14/17] Convert to class --- app/commands.js | 166 +++-------------------------- app/commands/actorRequest.js | 3 + app/commands/closeCommand.js | 34 ++++++ app/commands/command.js | 20 ++++ app/commands/labelCommand.js | 39 +++++++ app/commands/removeLabelCommand.js | 38 +++++++ app/commands/reopenCommand.js | 29 +++++ app/commands/reviewerCommand.js | 42 ++++++++ app/commands/transferCommand.js | 36 +++++++ app/matchers.js | 4 +- app/router.js | 8 +- docker-compose.yml | 3 - 12 files changed, 260 insertions(+), 162 deletions(-) create mode 100644 app/commands/actorRequest.js create mode 100644 app/commands/closeCommand.js create mode 100644 app/commands/command.js create mode 100644 app/commands/labelCommand.js create mode 100644 app/commands/removeLabelCommand.js create mode 100644 app/commands/reopenCommand.js create mode 100644 app/commands/reviewerCommand.js create mode 100644 app/commands/transferCommand.js diff --git a/app/commands.js b/app/commands.js index a81ccd7..a5188c3 100644 --- a/app/commands.js +++ b/app/commands.js @@ -1,158 +1,18 @@ -import { - closeMatcher, - labelMatcher, - removeLabelMatcher, - reopenMatcher, - reviewerMatcher, - transferMatcher, -} from "./matchers.js"; -import { - closeEnabled, - labelEnabled, - removeLabelEnabled, - reopenEnabled, - reviewerEnabled, - transferEnabled, -} from "./command-enabled.js"; -import { - addLabel, - closeIssue, - removeLabel, - reopenIssue, - requestReviewers, - transferIssue, -} from "./github.js"; -import { extractCommaSeparated, extractUsersAndTeams } from "./converters.js"; +import { TransferCommand } from "./commands/transferCommand.js"; +import { CloseCommand } from "./commands/closeCommand.js"; +import { ReopenCommand } from "./commands/reopenCommand.js"; +import { LabelCommand } from "./commands/labelCommand.js"; +import { RemoveLabelCommand } from "./commands/removeLabelCommand.js"; +import { ReviewerCommand } from "./commands/reviewerCommand.js"; -function actorRequest(payload) { - return `as requested by ${payload.sender.login}`; -} - -export function getCommands(commentBody) { +export function getCommands(id, payload) { return { - transfer: { - matches: transferMatcher(commentBody), - enabled: (config) => transferEnabled(config), - run: async (id, payload, authToken) => { - const targetRepo = this.matches[1]; - const sourceRepo = payload.repository.name; - console.log( - `${id} Transferring issue ${ - payload.issue.html_url - } to repo ${targetRepo} ${actorRequest(payload)}` - ); - await transferIssue( - authToken, - payload.repository.owner.login, - sourceRepo, - targetRepo, - payload.issue.node_id - ); - }, - }, - close: { - matches: closeMatcher(commentBody), - enabled: (config) => closeEnabled(config), - run: async (id, payload, authToken) => { - const sourceRepo = payload.repository.name; - const closeMatches = this.matches; - const reason = - closeMatches.length > 1 && closeMatches[1] === "not-planned" - ? "NOT_PLANNED" - : "COMPLETED"; - console.log( - `${id} Closing issue ${ - payload.issue.html_url - }, reason: ${reason} ${actorRequest(payload)}` - ); - await closeIssue(authToken, sourceRepo, payload.issue.node_id, reason); - }, - }, - reopen: { - matches: reopenMatcher(commentBody), - enabled: (config) => reopenEnabled(config), - run: async (id, payload, authToken) => { - const sourceRepo = payload.repository.name; - console.log( - `${id} Re-opening issue ${payload.issue.html_url} ${actorRequest( - payload - )}` - ); - await reopenIssue(authToken, sourceRepo, payload.issue.node_id); - }, - }, - label: { - matches: labelMatcher(commentBody), - enabled: (config) => { - const labels = extractCommaSeparated(this.matches[1]); - return labelEnabled(config, labels); - }, - run: async (id, payload, authToken) => { - const labels = extractCommaSeparated(this.matches[1]); - const sourceRepo = payload.repository.name; - - console.log( - `${id} Labeling issue ${ - payload.issue.html_url - } with labels ${labels} ${actorRequest(payload)}` - ); - await addLabel( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels - ); - }, - }, - removeLabel: { - matches: removeLabelMatcher(commentBody), - enabled: (config) => { - const labels = extractCommaSeparated(this.matches[1]); - return removeLabelEnabled(config, labels); - }, - run: async (id, payload, authToken) => { - const sourceRepo = payload.repository.name; - const labels = extractCommaSeparated(this.matches[1]); - console.log( - `${id} Removing label(s) from issue ${ - payload.issue.html_url - }, labels ${labels} ${actorRequest(payload)}` - ); - await removeLabel( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - labels - ); - }, - }, - reviewer: { - matches: reviewerMatcher(commentBody), - enabled: (config) => reviewerEnabled(config), - run: async (id, payload, authToken) => { - const reviewerMatches = this.matches[1]; - const sourceRepo = payload.repository.name; - console.log( - `${id} Requesting review for ${reviewerMatches} at ${ - payload.issue.html_url - } ${actorRequest(payload)}` - ); - const reviewers = extractUsersAndTeams( - payload.repository.owner.login, - reviewerMatches - ); - await requestReviewers( - authToken, - payload.repository.owner.login, - sourceRepo, - payload.issue.node_id, - reviewers.users, - reviewers.teams - ); - }, - }, + transfer: new TransferCommand(id, payload), + close: new CloseCommand(id, payload), + reopen: new ReopenCommand(id, payload), + label: new LabelCommand(id, payload), + removeLabel: new RemoveLabelCommand(id, payload), + reviewer: new ReviewerCommand(id, payload), }; } diff --git a/app/commands/actorRequest.js b/app/commands/actorRequest.js new file mode 100644 index 0000000..d9206ce --- /dev/null +++ b/app/commands/actorRequest.js @@ -0,0 +1,3 @@ +export function actorRequest(payload) { + return `as requested by ${payload.sender.login}`; +} diff --git a/app/commands/closeCommand.js b/app/commands/closeCommand.js new file mode 100644 index 0000000..a0bceb6 --- /dev/null +++ b/app/commands/closeCommand.js @@ -0,0 +1,34 @@ +import { Command } from "./command.js"; +import { closeMatcher } from "../matchers.js"; +import { closeEnabled } from "../command-enabled.js"; +import { closeIssue } from "../github.js"; +import { actorRequest } from "./actorRequest.js"; + +export class CloseCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return closeMatcher(this.payload.comment.body); + } + + enabled(config) { + return closeEnabled(config); + } + + async run(authToken) { + const sourceRepo = this.payload.repository.name; + const closeMatches = this.matches(); + const reason = + closeMatches.length > 1 && closeMatches[1] === "not-planned" + ? "NOT_PLANNED" + : "COMPLETED"; + console.log( + `${this.id} Closing issue ${ + this.payload.issue.html_url + }, reason: ${reason} ${actorRequest(this.payload)}` + ); + await closeIssue(authToken, sourceRepo, this.payload.issue.node_id, reason); + } +} diff --git a/app/commands/command.js b/app/commands/command.js new file mode 100644 index 0000000..ec22aa4 --- /dev/null +++ b/app/commands/command.js @@ -0,0 +1,20 @@ +export class Command { + constructor(id, payload) { + this.id = id; + this.payload = payload; + } + + matches() { + throw new Error("matches() must be implemented"); + } + + enabled(config) { + return { + enabled: false, + }; + } + + run(authToken) { + throw new Error("run(authToken) must be implemented"); + } +} diff --git a/app/commands/labelCommand.js b/app/commands/labelCommand.js new file mode 100644 index 0000000..688ce5b --- /dev/null +++ b/app/commands/labelCommand.js @@ -0,0 +1,39 @@ +import { labelMatcher } from "../matchers.js"; +import { labelEnabled } from "../command-enabled.js"; +import { addLabel } from "../github.js"; +import { Command } from "./command.js"; +import { actorRequest } from "./actorRequest.js"; +import { extractCommaSeparated } from "../converters.js"; + +export class LabelCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return labelMatcher(this.payload.comment.body); + } + + enabled(config) { + const labels = extractCommaSeparated(this.matches()[1]); + return labelEnabled(config, labels); + } + + async run(authToken) { + const labels = extractCommaSeparated(this.matches()[1]); + const sourceRepo = this.payload.repository.name; + + console.log( + `${this.id} Labeling issue ${ + this.payload.issue.html_url + } with labels ${labels} ${actorRequest(this.payload)}` + ); + await addLabel( + authToken, + this.payload.repository.owner.login, + sourceRepo, + this.payload.issue.node_id, + labels + ); + } +} diff --git a/app/commands/removeLabelCommand.js b/app/commands/removeLabelCommand.js new file mode 100644 index 0000000..a76c186 --- /dev/null +++ b/app/commands/removeLabelCommand.js @@ -0,0 +1,38 @@ +import { removeLabelMatcher } from "../matchers.js"; +import { removeLabelEnabled } from "../command-enabled.js"; +import { removeLabel } from "../github.js"; +import { Command } from "./command.js"; +import { actorRequest } from "./actorRequest.js"; +import { extractCommaSeparated } from "../converters.js"; + +export class RemoveLabelCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return removeLabelMatcher(this.payload.comment.body); + } + + enabled(config) { + const removeLabels = extractCommaSeparated(this.matches()[1]); + return removeLabelEnabled(config, removeLabels); + } + + async run(authToken) { + const sourceRepo = this.payload.repository.name; + const labels = extractCommaSeparated(this.matches()[1]); + console.log( + `${this.id} Removing label(s) from issue ${ + this.payload.issue.html_url + }, labels ${labels} ${actorRequest(this.payload)}` + ); + await removeLabel( + authToken, + this.payload.repository.owner.login, + sourceRepo, + this.payload.issue.node_id, + labels + ); + } +} diff --git a/app/commands/reopenCommand.js b/app/commands/reopenCommand.js new file mode 100644 index 0000000..f0b6aaa --- /dev/null +++ b/app/commands/reopenCommand.js @@ -0,0 +1,29 @@ +import { reopenMatcher } from "../matchers.js"; +import { reopenEnabled } from "../command-enabled.js"; +import { reopenIssue } from "../github.js"; +import { Command } from "./command.js"; +import { actorRequest } from "./actorRequest.js"; + +export class ReopenCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return reopenMatcher(this.payload.comment.body); + } + + enabled(config) { + return reopenEnabled(config); + } + + async run(authToken) { + const sourceRepo = this.payload.repository.name; + console.log( + `${this.id} Re-opening issue ${ + this.payload.issue.html_url + } ${actorRequest(this.payload)}` + ); + await reopenIssue(authToken, sourceRepo, this.payload.issue.node_id); + } +} diff --git a/app/commands/reviewerCommand.js b/app/commands/reviewerCommand.js new file mode 100644 index 0000000..f311ff3 --- /dev/null +++ b/app/commands/reviewerCommand.js @@ -0,0 +1,42 @@ +import { reviewerMatcher } from "../matchers.js"; +import { reviewerEnabled } from "../command-enabled.js"; +import { requestReviewers } from "../github.js"; +import { Command } from "./command.js"; +import { actorRequest } from "./actorRequest.js"; +import { extractUsersAndTeams } from "../converters.js"; + +export class ReviewerCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return reviewerMatcher(this.payload.comment.body); + } + + enabled(config) { + return reviewerEnabled(config); + } + + async run(authToken) { + const reviewerMatches = this.matches()[1]; + const sourceRepo = this.payload.repository.name; + console.log( + `${this.id} Requesting review for ${reviewerMatches} at ${ + this.payload.issue.html_url + } ${actorRequest(this.payload)}` + ); + const reviewers = extractUsersAndTeams( + this.payload.repository.owner.login, + reviewerMatches + ); + await requestReviewers( + authToken, + this.payload.repository.owner.login, + sourceRepo, + this.payload.issue.node_id, + reviewers.users, + reviewers.teams + ); + } +} diff --git a/app/commands/transferCommand.js b/app/commands/transferCommand.js new file mode 100644 index 0000000..d073da3 --- /dev/null +++ b/app/commands/transferCommand.js @@ -0,0 +1,36 @@ +import { transferMatcher } from "../matchers.js"; +import { transferEnabled } from "../command-enabled.js"; +import { transferIssue } from "../github.js"; +import { Command } from "./command.js"; +import { actorRequest } from "./actorRequest.js"; + +export class TransferCommand extends Command { + constructor(id, payload) { + super(id, payload); + } + + matches() { + return transferMatcher(this.payload.comment.body); + } + + enabled(config) { + return transferEnabled(config); + } + + async run(authToken) { + const targetRepo = this.matches()[1]; + const sourceRepo = this.payload.repository.name; + console.log( + `${this.id} Transferring issue ${ + this.payload.issue.html_url + } to repo ${targetRepo} ${actorRequest(this.payload)}` + ); + await transferIssue( + authToken, + this.payload.repository.owner.login, + sourceRepo, + targetRepo, + this.payload.issue.node_id + ); + } +} diff --git a/app/matchers.js b/app/matchers.js index 76dbab2..5b2b20b 100644 --- a/app/matchers.js +++ b/app/matchers.js @@ -11,11 +11,11 @@ export function reopenMatcher(text) { } export function labelMatcher(text) { - return text.match(/\/label ([\sA-Za-z\d-,]+)/); + return text.match(/\/label ([\sA-Za-z\d-,:/]+)/); } export function removeLabelMatcher(text) { - return text.match(/\/remove-label ([\sA-Za-z\d-,]+)/); + return text.match(/\/remove-label ([\sA-Za-z\d-,:/]+)/); } export function reviewerMatcher(text) { diff --git a/app/router.js b/app/router.js index 6e1c286..4f3c680 100644 --- a/app/router.js +++ b/app/router.js @@ -11,7 +11,7 @@ import { getCommands, noneMatch } from "./commands.js"; export async function router(auth, id, payload, verbose) { const sourceRepo = payload.repository.name; - const commands = getCommands(payload.comment.body); + const commands = getCommands(id, payload); if (noneMatch(commands)) { if (verbose) { @@ -33,13 +33,13 @@ export async function router(auth, id, payload, verbose) { defaults: (configs) => deepmerge.all([defaultConfig, ...configs]), }); - const runCommands = Object.values(commands).filter( - (command) => command.matches + const runCommands = Object.values(commands).filter((command) => + command.matches() ); for (const command of runCommands) { const result = command.enabled(config); result.enabled - ? await command.run(id, payload, authToken) + ? await command.run(authToken) : await reportError(authToken, payload.issue.node_id, result.error); } } diff --git a/docker-compose.yml b/docker-compose.yml index 2d0392d..942255c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,4 @@ services: environment: - GITHUB_APP_ID - GITHUB_APP_PRIVATE_KEY - - GITHUB_APP_INSTALLATION_ID - - SOURCE_OWNER - - TARGET_OWNER - WEBHOOK_SECRET From 1c7c419d0e8d2b8d6c9b3c963ab2863802a363ee Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Fri, 29 Jul 2022 13:52:31 +0100 Subject: [PATCH 15/17] Update app/commands.js Co-authored-by: Joseph Petersen --- app/commands.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/commands.js b/app/commands.js index a5188c3..76c2325 100644 --- a/app/commands.js +++ b/app/commands.js @@ -6,16 +6,16 @@ import { RemoveLabelCommand } from "./commands/removeLabelCommand.js"; import { ReviewerCommand } from "./commands/reviewerCommand.js"; export function getCommands(id, payload) { - return { - transfer: new TransferCommand(id, payload), - close: new CloseCommand(id, payload), - reopen: new ReopenCommand(id, payload), - label: new LabelCommand(id, payload), - removeLabel: new RemoveLabelCommand(id, payload), - reviewer: new ReviewerCommand(id, payload), - }; + return [ + new TransferCommand(id, payload), + new CloseCommand(id, payload), + new ReopenCommand(id, payload), + new LabelCommand(id, payload), + new RemoveLabelCommand(id, payload), + new ReviewerCommand(id, payload), + ]; } export function noneMatch(commands) { - return !Object.values(commands).some((command) => command.matches); + return commands.some((command) => command.matches); } From 124466130467a0262c29571072ed075b2b3b769c Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 14:13:22 +0100 Subject: [PATCH 16/17] Fix up --- app/commands.js | 16 +++++++++------- .../{actorRequest.js => actor-request.js} | 0 .../{closeCommand.js => close-command.js} | 2 +- app/commands/command.js | 2 ++ .../{labelCommand.js => label-command.js} | 2 +- ...veLabelCommand.js => remove-label-command.js} | 2 +- .../{reopenCommand.js => reopen-command.js} | 2 +- .../{reviewerCommand.js => reviewer-command.js} | 2 +- .../{transferCommand.js => transfer-command.js} | 2 +- app/matchers.js | 5 +++-- app/router.js | 5 ++--- 11 files changed, 22 insertions(+), 18 deletions(-) rename app/commands/{actorRequest.js => actor-request.js} (100%) rename app/commands/{closeCommand.js => close-command.js} (94%) rename app/commands/{labelCommand.js => label-command.js} (95%) rename app/commands/{removeLabelCommand.js => remove-label-command.js} (95%) rename app/commands/{reopenCommand.js => reopen-command.js} (93%) rename app/commands/{reviewerCommand.js => reviewer-command.js} (95%) rename app/commands/{transferCommand.js => transfer-command.js} (94%) diff --git a/app/commands.js b/app/commands.js index 76c2325..74acde3 100644 --- a/app/commands.js +++ b/app/commands.js @@ -1,9 +1,9 @@ -import { TransferCommand } from "./commands/transferCommand.js"; -import { CloseCommand } from "./commands/closeCommand.js"; -import { ReopenCommand } from "./commands/reopenCommand.js"; -import { LabelCommand } from "./commands/labelCommand.js"; -import { RemoveLabelCommand } from "./commands/removeLabelCommand.js"; -import { ReviewerCommand } from "./commands/reviewerCommand.js"; +import { TransferCommand } from "./commands/transfer-command.js"; +import { CloseCommand } from "./commands/close-command.js"; +import { ReopenCommand } from "./commands/reopen-command.js"; +import { LabelCommand } from "./commands/label-command.js"; +import { RemoveLabelCommand } from "./commands/remove-label-command.js"; +import { ReviewerCommand } from "./commands/reviewer-command.js"; export function getCommands(id, payload) { return [ @@ -17,5 +17,7 @@ export function getCommands(id, payload) { } export function noneMatch(commands) { - return commands.some((command) => command.matches); + return !commands.some((command) => { + return command.matches() !== null; + }); } diff --git a/app/commands/actorRequest.js b/app/commands/actor-request.js similarity index 100% rename from app/commands/actorRequest.js rename to app/commands/actor-request.js diff --git a/app/commands/closeCommand.js b/app/commands/close-command.js similarity index 94% rename from app/commands/closeCommand.js rename to app/commands/close-command.js index a0bceb6..382bb30 100644 --- a/app/commands/closeCommand.js +++ b/app/commands/close-command.js @@ -2,7 +2,7 @@ import { Command } from "./command.js"; import { closeMatcher } from "../matchers.js"; import { closeEnabled } from "../command-enabled.js"; import { closeIssue } from "../github.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; export class CloseCommand extends Command { constructor(id, payload) { diff --git a/app/commands/command.js b/app/commands/command.js index ec22aa4..988b480 100644 --- a/app/commands/command.js +++ b/app/commands/command.js @@ -8,12 +8,14 @@ export class Command { throw new Error("matches() must be implemented"); } + // eslint-disable-next-line no-unused-vars enabled(config) { return { enabled: false, }; } + // eslint-disable-next-line no-unused-vars run(authToken) { throw new Error("run(authToken) must be implemented"); } diff --git a/app/commands/labelCommand.js b/app/commands/label-command.js similarity index 95% rename from app/commands/labelCommand.js rename to app/commands/label-command.js index 688ce5b..24977b5 100644 --- a/app/commands/labelCommand.js +++ b/app/commands/label-command.js @@ -2,7 +2,7 @@ import { labelMatcher } from "../matchers.js"; import { labelEnabled } from "../command-enabled.js"; import { addLabel } from "../github.js"; import { Command } from "./command.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; import { extractCommaSeparated } from "../converters.js"; export class LabelCommand extends Command { diff --git a/app/commands/removeLabelCommand.js b/app/commands/remove-label-command.js similarity index 95% rename from app/commands/removeLabelCommand.js rename to app/commands/remove-label-command.js index a76c186..f358df9 100644 --- a/app/commands/removeLabelCommand.js +++ b/app/commands/remove-label-command.js @@ -2,7 +2,7 @@ import { removeLabelMatcher } from "../matchers.js"; import { removeLabelEnabled } from "../command-enabled.js"; import { removeLabel } from "../github.js"; import { Command } from "./command.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; import { extractCommaSeparated } from "../converters.js"; export class RemoveLabelCommand extends Command { diff --git a/app/commands/reopenCommand.js b/app/commands/reopen-command.js similarity index 93% rename from app/commands/reopenCommand.js rename to app/commands/reopen-command.js index f0b6aaa..703f1e1 100644 --- a/app/commands/reopenCommand.js +++ b/app/commands/reopen-command.js @@ -2,7 +2,7 @@ import { reopenMatcher } from "../matchers.js"; import { reopenEnabled } from "../command-enabled.js"; import { reopenIssue } from "../github.js"; import { Command } from "./command.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; export class ReopenCommand extends Command { constructor(id, payload) { diff --git a/app/commands/reviewerCommand.js b/app/commands/reviewer-command.js similarity index 95% rename from app/commands/reviewerCommand.js rename to app/commands/reviewer-command.js index f311ff3..e197678 100644 --- a/app/commands/reviewerCommand.js +++ b/app/commands/reviewer-command.js @@ -2,7 +2,7 @@ import { reviewerMatcher } from "../matchers.js"; import { reviewerEnabled } from "../command-enabled.js"; import { requestReviewers } from "../github.js"; import { Command } from "./command.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; import { extractUsersAndTeams } from "../converters.js"; export class ReviewerCommand extends Command { diff --git a/app/commands/transferCommand.js b/app/commands/transfer-command.js similarity index 94% rename from app/commands/transferCommand.js rename to app/commands/transfer-command.js index d073da3..9dca986 100644 --- a/app/commands/transferCommand.js +++ b/app/commands/transfer-command.js @@ -2,7 +2,7 @@ import { transferMatcher } from "../matchers.js"; import { transferEnabled } from "../command-enabled.js"; import { transferIssue } from "../github.js"; import { Command } from "./command.js"; -import { actorRequest } from "./actorRequest.js"; +import { actorRequest } from "./actor-request.js"; export class TransferCommand extends Command { constructor(id, payload) { diff --git a/app/matchers.js b/app/matchers.js index 5b2b20b..137e98b 100644 --- a/app/matchers.js +++ b/app/matchers.js @@ -11,11 +11,12 @@ export function reopenMatcher(text) { } export function labelMatcher(text) { - return text.match(/\/label ([\sA-Za-z\d-,:/]+)/); + return text.match(/\/label ([\s/A-Za-z\d-,:]+)/); } export function removeLabelMatcher(text) { - return text.match(/\/remove-label ([\sA-Za-z\d-,:/]+)/); + // TODO prevent matching across lines + return text.match(/\/remove-label ([\s/A-Za-z\d-,:]+)/); } export function reviewerMatcher(text) { diff --git a/app/router.js b/app/router.js index 4f3c680..d1b418a 100644 --- a/app/router.js +++ b/app/router.js @@ -14,6 +14,7 @@ export async function router(auth, id, payload, verbose) { const commands = getCommands(id, payload); if (noneMatch(commands)) { + console.log("none match"); if (verbose) { console.log("No match for", payload.comment.body); } @@ -33,9 +34,7 @@ export async function router(auth, id, payload, verbose) { defaults: (configs) => deepmerge.all([defaultConfig, ...configs]), }); - const runCommands = Object.values(commands).filter((command) => - command.matches() - ); + const runCommands = commands.filter((command) => command.matches()); for (const command of runCommands) { const result = command.enabled(config); result.enabled From e17986911b0f88dcee64635fd9cbbe65a9dff7dd Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 29 Jul 2022 14:39:36 +0100 Subject: [PATCH 17/17] Lint and test --- app/commands.test.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/app/commands.test.js b/app/commands.test.js index a76a070..af8ed5d 100644 --- a/app/commands.test.js +++ b/app/commands.test.js @@ -2,18 +2,30 @@ import { getCommands, noneMatch } from "./commands.js"; describe("commands", () => { test("noneMatch is true when nothing matches", () => { - const commands = getCommands("nothing that would ever match"); + const commands = getCommands("any", { + comment: { + body: "nothing that would every match", + }, + }); expect(noneMatch(commands)).toEqual(true); }); test("noneMatch is false when something matches", () => { - const commands = getCommands("/label one"); + const commands = getCommands("any", { + comment: { + body: "/label one", + }, + }); expect(noneMatch(commands)).toEqual(false); }); test("noneMatch is false when multiple matches", () => { - const commands = getCommands("/label one /reviewer reviewer"); + const commands = getCommands("any", { + comment: { + body: "/label one \n/reviewer reviewer", + }, + }); expect(noneMatch(commands)).toEqual(false); });