refactor: github_api/pulls (#541)

* refactor: github_api/pulls

* ran lint

Author: shine2lay

__fixtures__/unit/helper.js

@@ -179,6 +179,7 @@ module.exports = {
               return { status: 204 }
             }
           },
+          requestReviewers: jest.fn().mockReturnValue(options.requestReviewers || 'request review success'),
           merge: jest.fn().mockReturnValue(options.merge || 'merged'),
           get: jest.fn()
         },

__tests__/unit/github/api.test.js

@@ -604,3 +604,46 @@ describe('mergePR', () => {
     }
   })
 })
+
+describe('requestReviewers', () => {
+  test('return correct data if no error', async () => {
+    const res = await GithubAPI.requestReviewers(Helper.mockContext())
+    expect(res).toEqual('request review success')
+  })
+
+  test('that error are NOT re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.requestReviewers = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.requestReviewers(context)
+    } catch (e) {
+      // fail if it throws error
+      expect(true).toBe(false)
+    }
+  })
+})
+
+describe('getPR', () => {
+  test('return correct data if no error', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.get.mockReturnValue({ data: { number: 12 } })
+
+    const res = await GithubAPI.getPR(context)
+
+    expect(res.data).toEqual({ number: 12 })
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.get = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.getPR(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})

lib/actions/merge.js

@@ -6,7 +6,8 @@ const MERGE_METHOD_OPTIONS = ['merge', 'rebase', 'squash']
 const mergePR = async (context, prNumber, mergeMethod, actionObj) => {
   const isMerged = await actionObj.githubAPI.checkIfMerged(context, prNumber)
   if (!isMerged) {
-    const pullRequest = await context.octokit.pulls.get(context.repo({ pull_number: prNumber }))
+    const pullRequest = await actionObj.githubAPI.getPR(context, prNumber)
+
     if (pullRequest.data.mergeable_state !== 'blocked' && pullRequest.data.state === 'open') {
       await actionObj.githubAPI.mergePR(context, context.repo({ pull_number: prNumber, merge_method: mergeMethod }))
     }

lib/actions/request_review.js

@@ -1,26 +1,6 @@
 const { Action } = require('./action')
-const logger = require('../logger')
 const _ = require('lodash')
 
-const createRequestReview = async (context, number, reviewers, actionObj) => {
-  let res
-  try {
-    res = await context.octokit.pulls.requestReviewers(
-      context.repo({ pull_number: number, reviewers })
-    )
-  } catch (err) {
-    const errorLog = {
-      log_type: logger.logTypes.REQUEST_REVIEW_FAIL_ERROR,
-      eventId: context.eventId,
-      repo: context.payload.repository.full_name,
-      action_name: actionObj.name
-    }
-
-    actionObj.log.info(JSON.stringify(errorLog))
-  }
-  return res
-}
-
 class RequestReview extends Action {
   constructor () {
     super('request_review')
@@ -54,11 +34,10 @@ class RequestReview extends Action {
     if (collaboratorsToRequest.length === 0) {
       return
     }
-    return createRequestReview(
+    return this.githubAPI.requestReviewers(
       context,
       prNumber,
-      reviewerToRequest,
-      this
+      reviewerToRequest
     )
   }
 }

lib/github/api.js

@@ -412,6 +412,34 @@ class GithubAPI {
       }
     }
   }
+
+  static async getPR (context, pullNumber) {
+    const callFn = 'pulls.get'
+
+    debugLog(context, callFn)
+
+    try {
+      return context.octokit.pulls.get(context.repo({ pull_number: pullNumber }))
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
+
+  static async requestReviewers (context, pullNumber, reviewers) {
+    const callFn = 'pulls.requestReview'
+
+    debugLog(context, callFn)
+    try {
+      return await context.octokit.pulls.requestReviewers(
+        context.repo({ pull_number: pullNumber, reviewers })
+      )
+    } catch (err) {
+      const errorLog = createLog(context, { callFn: callFn, errors: err.toString(), logType: logger.logTypes.REQUEST_REVIEW_FAIL_ERROR })
+      const log = logger.create(`GithubAPI/${callFn}`)
+
+      log.info(errorLog)
+    }
+  }
 }
 
 module.exports = GithubAPI