Merge pull request #5 from microsoft/codeowners

Allows codeowners to say a 'merge on green'-like catchphrase to merge a PR

TypeScriptRepoPullRequestWebhook/index.ts

@@ -4,6 +4,7 @@ import sign = require("@octokit/webhooks/sign");
 
 import { handlePullRequestPayload } from "../src/typeScriptHandlePullRequest";
 import { anyRepoHandleStatusUpdate } from "../src/anyRepoHandleStatusUpdate";
+import { anyRepoHandleIssueCommentPayload } from "../src/anyRepoHandleIssueComment";
 
 // The goal of these functions is to validate the call is real, then as quickly as possible get out of the azure
 // context and into the `src` directory, where work can be done against tests instead requiring changes to happen
@@ -37,7 +38,7 @@ const httpTrigger: AzureFunction = async function(context: Context, req: HttpReq
       break;
 
     case "issue_comment":
-      // NOOP for now
+      await anyRepoHandleIssueCommentPayload(req.body, context)
       break;
   }
 

package.json

@@ -15,14 +15,17 @@
     "@azure/functions": "^1.0.2-beta2",
     "@octokit/rest": "^16.35.0",
     "@octokit/webhooks": "^6.3.2",
+    "@types/minimatch": "^3.0.3",
+    "minimatch": "^3.0.4",
     "parse-diff": "^0.6.0"
   },
   "devDependencies": {
     "@types/jest": "^24.0.23",
     "jest": "^24.9.0",
     "ts-jest": "^24.2.0",
     "ts-node": "^8.5.4",
-    "typescript": "3.8.0-beta"
+    "typescript": "3.8.0-beta",
+    "codeowners": "^4.1.1"
   },
   "jest": {
     "preset": "ts-jest",

src/anyRepoHandleIssueComment.ts

@@ -0,0 +1,29 @@
+import { WebhookPayloadIssueComment } from "@octokit/webhooks";
+import { Context, Logger } from "@azure/functions";
+import { createGitHubClient } from "./util/createGitHubClient";
+import Octokit = require("@octokit/rest");
+import {sha} from "./sha"
+import { mergeThroughCodeOwners } from "./checks/mergeThroughCodeOwners";
+
+
+export const anyRepoHandleIssueCommentPayload = async (payload: WebhookPayloadIssueComment, context: Context) => {
+  const api = createGitHubClient();
+  const ran = [] as string[]
+
+  const run = (name: string, fn: (api: Octokit, payload: WebhookPayloadIssueComment, logger: Logger) => Promise<void>) => {
+    context.log.info(`\n\n## ${name}\n`)
+    ran.push(name)
+    return fn(api, payload, context.log)
+  }
+
+  // Making this one whitelisted to the website for now
+  if (payload.repository.name === "TypeScript-Website") {
+    await run("Checking if we should merge from codeowners", mergeThroughCodeOwners)
+  }
+
+  context.res = {
+    status: 200,
+    headers: { sha: sha },
+    body: `Success, ran: ${ran.join(", ")}`
+  };
+};

src/checks/mergeThroughCodeOwners.test.ts

@@ -0,0 +1,63 @@
+jest.mock("../util/getContents");
+
+import { createMockGitHubClient, getPRFixture } from "../util/tests/createMockGitHubClient";
+import { getFakeLogger } from "../util/tests/createMockContext";
+
+import { mergeThroughCodeOwners, mergePhrase } from "./mergeThroughCodeOwners";
+import { getContents } from "../util/getContents";
+
+const genericWebhook =   {
+  comment: { 
+    body: mergePhrase, 
+    user: {
+      login: "orta"
+    }
+  },
+  issue: {
+    number: 1234
+  },
+  repository: {
+    owner: {
+      login: "microsoft"
+    },
+    name: "TypeScript-Website"
+  }
+}
+
+describe("for handling merging when green", () => {
+  it("bails when the keyword aren't used", async () => {
+    const { api } = createMockGitHubClient();
+    const logger = getFakeLogger();
+
+    await mergeThroughCodeOwners(api, { comment: { body: "Good Joke"} } as any, logger);
+    expect(logger.info).toBeCalledWith("No included message, not trying to merge through code owners");
+  });
+
+  it("handles the phrase in an issue", async () => {
+    const { api, mockAPI } = createMockGitHubClient();
+    mockAPI.pulls.get.mockRejectedValue(new Error("not found"))
+    const logger = getFakeLogger();
+
+    await mergeThroughCodeOwners(api, genericWebhook as any, logger);
+    expect(logger.info).toBeCalledWith("Comment was in an issue");
+  });
+
+  it("handles the phrase in an pr", async () => {
+    const { api, mockAPI } = createMockGitHubClient();
+    const logger = getFakeLogger();
+    const pr = getPRFixture("api-pr-closed")
+    // @ts-ignore
+    getContents.mockReturnValue("/hello.txt @orta")
+
+    // Getting the PR form the API
+    mockAPI.pulls.get.mockResolvedValue({ data: pr })
+
+    // Getting the files
+    mockAPI.pulls.listFiles.endpoint.merge.mockResolvedValue({})
+    mockAPI.paginate.mockResolvedValue([{ filename: "/hello.txt"}])
+
+    await mergeThroughCodeOwners(api, genericWebhook as any, logger);
+
+    expect(logger.info).toBeCalledWith("Accepting as reasonable to merge");
+  });
+});

src/checks/mergeThroughCodeOwners.ts

@@ -0,0 +1,34 @@
+import { WebhookPayloadIssueComment } from "@octokit/webhooks";
+import * as Octokit from "@octokit/rest";
+import { Logger } from "@azure/functions";
+import { hasAccessToMergePRs } from "../pr_meta/hasAccessToMergePR";
+import { mergeOrAddMergeLabel } from "../pr_meta/mergeOrAddMergeLabel";
+
+export const mergePhrase = "Ready to merge"
+
+/**
+ * Allow someone to declare a PR should be merged if they have access rights via code owners
+ */
+export const mergeThroughCodeOwners = async (api: Octokit, payload: WebhookPayloadIssueComment, logger: Logger) => {
+  if (!payload.comment.body.includes(mergePhrase)) {
+    return logger.info(`No included message, not trying to merge through code owners`)
+  }
+
+  // Grab the correlating PR
+  let pull: Octokit.Response<Octokit.PullsGetResponse>["data"]
+
+  try {
+    const response = await api.pulls.get({ owner: payload.repository.owner.login, repo: payload.repository.name, pull_number: payload.issue.number, })
+    pull = response.data
+  } catch (error) {
+    return logger.info(`Comment was in an issue`)
+  }
+
+  const canMerge = await hasAccessToMergePRs(payload.comment.user.login, api, pull, logger)
+  if (!canMerge) {
+    return // it logs in the function above
+  }
+
+  logger.info("Looks good to merge")
+  mergeOrAddMergeLabel(api, {  number: pull.number, repo: pull.base.repo.name, owner: pull.base.repo.owner.login }, pull.head.sha, logger)
+}

src/pr_meta/hasAccessToMergePR.test.ts

@@ -0,0 +1,45 @@
+import { getFilesNotOwnedByCodeOwner, getCodeOwnersInfo } from "./hasAccessToMergePR"
+
+it("matches simple files", () => {
+  const codeOwnersText = 
+`
+/a/b @orta
+`
+  const files = ["/a/b"]
+
+  const result = getFilesNotOwnedByCodeOwner("orta", files, getCodeOwnersInfo(codeOwnersText))
+  expect(result).toEqual([])
+})
+
+it("matches simple files", () => {
+  const codeOwnersText = 
+`
+/a/b/c/* @orta
+/a/b/d/* @orta
+/b/c @hayes
+`
+  const files = ["/a/b", "/a/b/c/deff", "/b/c"]
+
+  const result = getFilesNotOwnedByCodeOwner("orta", files, getCodeOwnersInfo(codeOwnersText))
+  expect(result).toEqual([
+    "/a/b",
+    "/b/c"
+  ])
+})
+
+
+it("matches simple files", () => {
+  const codeOwnersText = 
+`
+/packages/**/ja/** @hayes
+/a/b/d/* @orta
+/b/c @hayes
+`
+  const files = [
+    "/packages/playground-examples/copy/ja/TypeScript/Type Primitives/Built-in Utility Types.ts ",
+    "/b/c"
+  ]
+
+  const result = getFilesNotOwnedByCodeOwner("hayes", files, getCodeOwnersInfo(codeOwnersText))
+  expect(result).toEqual([])
+})