refactor

all-contributors · Jan 17, 2019 · 865b982 · 865b982
1 parent b03b402
commit 865b982
Show file tree

Hide file tree

Showing 6 changed files with 143 additions and 79 deletions.
diff --git a/src/Repository/index.js b/src/Repository/index.js
@@ -1,16 +1,25 @@
-const { ResourceNotFoundError } = require('../utils/errors')
+const {
+    ResourceNotFoundError,
+    AllContributorBotError,
+} = require('../utils/errors')
 
 class Repository {
-    constructor({ repo, owner, github }) {
+    constructor({ repo, owner, github, defaultBranch }) {
         this.github = github
         this.repo = repo
         this.owner = owner
+        this.defaultBranch = defaultBranch
+        this.basedBranch
     }
 
     getFullname() {
         return `${this.owner}/${this.repo}`
     }
 
+    setBasedBranch(branchName) {
+        this.basedBranch = branchName
+    }
+
     async getFile(filePath) {
         // https://octokit.github.io/rest.js/#api-Repos-getContents
         let file
@@ -19,6 +28,7 @@ class Repository {
                 owner: this.owner,
                 repo: this.repo,
                 path: filePath,
+                ref: this.basedBranch,
             })
         } catch (error) {
             if (error.code === 404) {
@@ -61,25 +71,33 @@ class Repository {
         return multipleFilesByPath
     }
 
-    async getHeadRef(defaultBranch) {
+    async getHeadRef(branchName) {
         const result = await this.github.git.getRef({
             owner: this.owner,
             repo: this.repo,
-            ref: `heads/${defaultBranch}`,
+            ref: `heads/${branchName}`,
         })
         return result.data.object.sha
     }
 
-    async createBranch({ branchName, defaultBranch }) {
-        const fromSha = await this.getHeadRef(defaultBranch)
+    async createBranch(branchName) {
+        const fromSha = await this.getHeadRef(this.defaultBranch)
 
-        // https://octokit.github.io/rest.js/#api-Git-createRef
-        await this.github.git.createRef({
-            owner: this.owner,
-            repo: this.repo,
-            ref: `refs/heads/${branchName}`,
-            sha: fromSha,
-        })
+        try {
+            // https://octokit.github.io/rest.js/#api-Git-createRef
+            await this.github.git.createRef({
+                owner: this.owner,
+                repo: this.repo,
+                ref: `refs/heads/${branchName}`,
+                sha: fromSha,
+            })
+
+            return false
+        } catch (error) {
+            // branch already exists
+            if (error.code === 422) return true
+            throw error
+        }
     }
 
     async updateFile({ filePath, content, branchName, originalSha }) {
@@ -140,40 +158,44 @@ class Repository {
         await Promise.all(createOrUpdateFilesMultiple)
     }
 
-    async createPullRequest({ title, body, branchName, defaultBranch }) {
-        const result = await this.github.pulls.create({
-            owner: this.owner,
-            repo: this.repo,
-            title,
-            body,
-            head: branchName,
-            base: defaultBranch,
-            maintainer_can_modify: true,
-        })
-        return result.data.html_url
+    async createPullRequest({ title, body, branchName }) {
+        try {
+            const result = await this.github.pulls.create({
+                owner: this.owner,
+                repo: this.repo,
+                title,
+                body,
+                head: branchName,
+                base: this.defaultBranch,
+                maintainer_can_modify: true,
+            })
+            return { pullRequestURL: result.data.html_url, result: true }
+        } catch (error) {
+            // pull request is already open
+            if (error.code === 422) return { pullRequestURL: '', result: false }
+            throw error
+        }
     }
 
-    async createPullRequestFromFiles({
-        title,
-        body,
-        filesByPath,
-        branchName,
-        defaultBranch,
-    }) {
-        await this.createBranch({ branchName, defaultBranch })
+    async createPullRequestFromFiles({ title, body, filesByPath, branchName }) {
+        if (this.basedBranch === this.defaultBranch)
+            this.createBranch(branchName)
 
         await this.createOrUpdateFiles({
             filesByPath,
             branchName,
         })
 
-        const pullRequestURL = await this.createPullRequest({
+        const { pullRequestURL, result } = await this.createPullRequest({
             title,
             body,
             branchName,
-            defaultBranch,
         })
 
+        // TODO
+        if (!result)
+            throw new AllContributorBotError('Pull request is already open')
+
         return pullRequestURL
     }
 }

diff --git a/src/processIssueComment.js b/src/processIssueComment.js
@@ -20,7 +20,6 @@ async function processAddContributor({
     optionsConfig,
     who,
     contributions,
-    defaultBranch,
 }) {
     const { name, avatar_url, profile } = await getUserDetails({
         github: context.github,
@@ -56,7 +55,6 @@ async function processAddContributor({
         )}.\n\nThis was requested by ${commentReply.replyingToWho()} [in this comment](${commentReply.replyingToWhere()})`,
         filesByPath: filesByPathToUpdate,
         branchName: `all-contributors/add-${who}`,
-        defaultBranch,
     })
 
     commentReply.reply(
@@ -65,14 +63,29 @@ async function processAddContributor({
 }
 
 async function processIssueComment({ context, commentReply }) {
+    const commentBody = context.payload.comment.body
+    const { who, action, contributions } = parseComment(commentBody)
+    const branchName = `all-contributors/add-${who}`
+    const defaultBranch = context.payload.repository.default_branch
     const repository = new Repository({
         ...context.repo(),
         github: context.github,
+        defaultBranch,
     })
+
+    try {
+        await repository.getHeadRef(branchName)
+        repository.setBasedBranch(branchName)
+    } catch (error) {
+        if (error.code !== 404) throw error
+        repository.setBasedBranch(defaultBranch)
+    }
+
     const optionsConfig = new OptionsConfig({
         repository,
         commentReply,
     })
+
     try {
         await optionsConfig.fetch()
     } catch (error) {
@@ -83,18 +96,14 @@ async function processIssueComment({ context, commentReply }) {
         }
     }
 
-    const commentBody = context.payload.comment.body
-    const parsedComment = parseComment(commentBody)
-
-    if (parsedComment.action === 'add') {
+    if (action === 'add') {
         await processAddContributor({
             context,
             commentReply,
             repository,
             optionsConfig,
-            who: parsedComment.who,
-            contributions: parsedComment.contributions,
-            defaultBranch: context.payload.repository.default_branch,
+            who,
+            contributions,
         })
         return
     }

diff --git a/test/Repository/__snapshots__/index.test.js.snap b/test/Repository/__snapshots__/index.test.js.snap
@@ -9,13 +9,6 @@ Object {
 `;
 
 exports[`Repository createPullRequest with files 1`] = `
-Object {
-  "ref": "refs/heads/all-contributors/add-jakebolam",
-  "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd",
-}
-`;
-
-exports[`Repository createPullRequest with files 2`] = `
 Object {
   "branch": "all-contributors/add-jakebolam",
   "content": "eyJ0ZXN0IjoidGVzdCBjb250ZW50In0=",
@@ -24,7 +17,7 @@ Object {
 }
 `;
 
-exports[`Repository createPullRequest with files 3`] = `
+exports[`Repository createPullRequest with files 2`] = `
 Object {
   "branch": "all-contributors/add-jakebolam",
   "content": "VXBkYXRlZCBsaXN0",
@@ -33,7 +26,7 @@ Object {
 }
 `;
 
-exports[`Repository createPullRequest with files 4`] = `
+exports[`Repository createPullRequest with files 3`] = `
 Object {
   "branch": "all-contributors/add-jakebolam",
   "content": "VXBkYXRlZCBsaXN0IGluIG5lc3RlZCBmb2xkZXI=",
@@ -42,6 +35,13 @@ Object {
 }
 `;
 
+exports[`Repository createPullRequest with files 4`] = `
+Object {
+  "ref": "refs/heads/all-contributors/add-jakebolam",
+  "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd",
+}
+`;
+
 exports[`Repository createPullRequest with files 5`] = `
 Object {
   "base": "master",

diff --git a/test/Repository/index.test.js b/test/Repository/index.test.js
@@ -13,6 +13,7 @@ describe('Repository', () => {
         repo: 'all-contributors-bot',
         owner: 'all-contributors',
         github: mockGithub,
+        defaultBranch: 'master',
     })
 
     const verifyBody = body => {
@@ -36,9 +37,11 @@ describe('Repository', () => {
     })
 
     test('createPullRequest with files', async () => {
+        const basedBranch = 'master'
+        repository.setBasedBranch(basedBranch)
         nock('https://api.github.com')
             .get(
-                `/repos/all-contributors/all-contributors-bot/git/refs/heads/master`,
+                `/repos/all-contributors/all-contributors-bot/git/refs/heads/${basedBranch}`,
             )
             .reply(200, gitGetRefdata)
 
@@ -95,7 +98,6 @@ describe('Repository', () => {
                 },
             },
             branchName: 'all-contributors/add-jakebolam',
-            defaultBranch: 'master',
         })
         expect(pullRequestNumber).toEqual(
             'https://github.com/all-contributors/all-contributors-bot/pull/1347',