Merge pull request #164 from artsy/squash-on-green-fix

Squash on Green

org/markAsMergeOnGreen.ts

@@ -2,6 +2,23 @@ import { danger } from "danger"
 import { IssueComment, PullRequestReview } from "github-webhook-event-types"
 import { IssueCommentIssue } from "github-webhook-event-types/source/IssueComment"
 
+export const labelMap = {
+  "#mergeongreen": {
+    name: "Merge On Green",
+    color: "247A38",
+    description: "A label to indicate that Peril should merge this PR when all statuses are green",
+    mergeMethod: "merge",
+    commitGenerator: (prNumber: number) => `Merge pull request #${prNumber} by Peril`,
+  },
+  "#squashongreen": {
+    name: "Squash On Green",
+    color: "247A38",
+    description: "A label to indicate that Peril should squash-merge this PR when all statuses are green",
+    mergeMethod: "squash",
+    commitGenerator: (prNumber: number) => undefined, // defaults to GitHub's generated commit messages
+  },
+} as const
+
 /** If a comment to an issue contains "Merge on Green", apply a label for it to be merged when green. */
 export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestReview) => {
   const api = danger.github.api
@@ -42,15 +59,17 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe
   }
 
   // Don't do any work unless we have to
-  const keywords = ["#mergeongreen"]
-  const match = keywords.find(k => text.toLowerCase().includes(k))
+  const keywords = Object.keys(labelMap)
+  const match = keywords.find((k) => text.toLowerCase().includes(k)) as keyof typeof labelMap | undefined
   if (!match) {
     return console.log(`Did not find any of the merging phrases in the comment beginning ${text.substring(0, 12)}.`)
   }
 
+  const label = labelMap[match]
+
   // Check to see if the label has already been set
-  if (issue.labels.find(l => l.name === "Merge On Green")) {
-    return console.log("Already has Merge on Green")
+  if (issue.labels.find((l) => l.name === label.name)) {
+    return console.log("Already has Merge on Green-type label")
   }
 
   // Check for org access, so that some rando doesn't
@@ -64,12 +83,6 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe
     return console.log("Sender does not have permission to merge")
   }
 
-  // Let's people know that it will be merged
-  const label = {
-    name: "Merge On Green",
-    color: "247A38",
-    description: "A label to indicate that Peril should merge this PR when all statuses are green",
-  }
   const repo = {
     owner: org,
     repo: issueCommentOrPrReview.repository.name,
@@ -78,7 +91,7 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe
 
   console.log("Adding the label:", repo)
   await danger.github.utils.createOrAddLabel(label, repo)
-  console.log("Updated the PR with a Merge on Green label")
+  console.log("Updated the PR with a Merge on Green-type label")
 }
 
 export default rfc10

org/mergeOnGreen.ts

@@ -1,6 +1,7 @@
 import { danger } from "danger"
 import { Status } from "github-webhook-event-types"
-import { LabelLabel } from "github-webhook-event-types/source/Label"
+import { labelMap } from "./markAsMergeOnGreen"
+import { capitalize } from "lodash"
 
 export const rfc10 = async (status: Status) => {
   const api = danger.github.api
@@ -30,12 +31,14 @@ export const rfc10 = async (status: Status) => {
     const issue = await api.issues.get({ owner, repo, number })
 
     // Get the PR combined status
-    const mergeLabel = issue.data.labels.find((l: LabelLabel) => l.name === "Merge On Green")
+    const issueLabelNames = issue.data.labels.map((l) => l.name)
+    const mergeLabel = Object.values(labelMap).find((label) => issueLabelNames.includes(label.name))
+
     if (!mergeLabel) {
-      return console.log("PR does not have Merge on Green")
+      return console.log("PR does not have Merge on Green-type label")
     }
 
-    let commitTitle = `Merge pull request #${number} by Peril`
+    let commitTitle = mergeLabel.commitGenerator(number)
 
     if (issue.data.title) {
       // Strip any "@user =>" prefixes from the pr title
@@ -44,7 +47,7 @@ export const rfc10 = async (status: Status) => {
     }
 
     // Merge the PR
-    await api.pulls.merge({ owner, repo, number, commit_title: commitTitle })
+    await api.pulls.merge({ owner, repo, number, commit_title: commitTitle, merge_method: mergeLabel.mergeMethod })
     console.log(`Merged Pull Request ${number}`)
   }
 }

package.json

@@ -22,20 +22,20 @@
     "@types/jest": "^23.3.9",
     "@types/jira-client": "^6.4.0",
     "@types/lodash": "^4.14.121",
-    "@types/node": "^10.12.9",
+    "@types/node": "^14.6.4",
     "@types/node-fetch": "^2.1.3",
     "danger": "^7.0.9",
     "danger-plugin-spellcheck": "^1.2.3",
     "danger-plugin-yarn": "^1.3.0",
     "github-webhook-event-types": "^1.2.1",
     "husky": "^0.14.3",
-    "jest": "^24.3.0",
+    "jest": "^26.4.2",
     "jira-client": "^6.4.1",
     "lint-staged": "^7.2.2",
-    "prettier": "^1.15.2",
-    "ts-jest": "^24.0.0",
+    "prettier": "^2.1.1",
+    "ts-jest": "^26.3.0",
     "ts-node": "^7.0.1",
-    "typescript": "^3.1.6"
+    "typescript": "3.4.1"
   },
   "prettier": {
     "printWidth": 120,

tests/rfc_10.test.ts

@@ -67,7 +67,16 @@ describe("for adding the label", () => {
         issue: { labels: [{ name: "Merge On Green" }], pull_request: {} },
         ...repo,
       } as any)
-      expect(console.log).toBeCalledWith("Already has Merge on Green")
+      expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
+    })
+
+    it("bails when the issue already has merge on green", async () => {
+      await markAsMergeOnGreen({
+        comment: { body: "#squashongreen", user: { login: "danger" } },
+        issue: { labels: [{ name: "Squash On Green" }], pull_request: {} },
+        ...repo,
+      } as any)
+      expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
     })
   })
 
@@ -96,10 +105,26 @@ describe("for adding the label", () => {
         pull_request: pull_request,
         ...repo,
       } as any)
-      expect(console.log).toBeCalledWith("Already has Merge on Green")
+      expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
     })
   })
 
+  it("creates the label when the label doesn't exist on the repo", async () => {
+    dm.danger.github.api.orgs.checkMembership.mockReturnValueOnce(Promise.resolve({ data: {} }))
+    dm.danger.github.api.issues.getLabels.mockReturnValueOnce(Promise.resolve({ data: [] }))
+
+    await markAsMergeOnGreen({
+      comment: {
+        body: "#squashongreen",
+        user: { sender: { login: "orta" } },
+      },
+      issue: { labels: [], pull_request: {} },
+      ...repo,
+    } as any)
+
+    expect(dm.danger.github.utils.createOrAddLabel).toBeCalled()
+  })
+
   it("creates the label when the label doesn't exist on the repo", async () => {
     dm.danger.github.api.orgs.checkMembership.mockReturnValueOnce(Promise.resolve({ data: {} }))
     dm.danger.github.api.issues.getLabels.mockReturnValueOnce(Promise.resolve({ data: [] }))
@@ -157,10 +182,10 @@ describe("for handling merging when green", () => {
       commit: { sha: "123abc" },
     } as any)
 
-    expect(console.log).toBeCalledWith("PR does not have Merge on Green")
+    expect(console.log).toBeCalledWith("PR does not have Merge on Green-type label")
   })
 
-  it("triggers a PR merge when there is a merge on green label", async () => {
+  it("triggers a PR merge when there is a Merge on Green label", async () => {
     // Has the right status
     dm.danger.github.api.repos.getCombinedStatusForRef.mockReturnValueOnce(
       Promise.resolve({ data: { state: "success" } })
@@ -185,6 +210,36 @@ describe("for handling merging when green", () => {
       number: 1,
       owner: "danger",
       repo: "doggo",
+      merge_method: "merge",
+    })
+  })
+
+  it("triggers a PR squash when there is a Squash on Green label", async () => {
+    // Has the right status
+    dm.danger.github.api.repos.getCombinedStatusForRef.mockReturnValueOnce(
+      Promise.resolve({ data: { state: "success" } })
+    )
+
+    // Gets a corresponding issue
+    dm.danger.github.api.search.issues.mockReturnValueOnce(Promise.resolve({ data: { items: [{ number: 1 }] } }))
+
+    // Returns an issue without the merge on green label
+    dm.danger.github.api.issues.get.mockReturnValueOnce(
+      Promise.resolve({ data: { labels: [{ name: "Squash On Green" }] } })
+    )
+
+    await mergeOnGreen({
+      state: "success",
+      repository: { owner: { login: "danger" }, name: "doggo" },
+      commit: { sha: "123abc" },
+    } as any)
+
+    expect(dm.danger.github.api.pulls.merge).toBeCalledWith({
+      commit_title: undefined,
+      number: 1,
+      owner: "danger",
+      repo: "doggo",
+      merge_method: "squash",
     })
   })
 })