refactor: github_api/pulls:lists (#540)

* refactor: github_api/pulls:lists

* add missing awaits

* ran lint

Author: shine2lay

__tests__/unit/github/api.test.js

@@ -647,3 +647,83 @@ describe('getPR', () => {
     }
   })
 })
+
+describe('listPR', () => {
+  test('return correct data if no error', async () => {
+    const res = await GithubAPI.listPR(Helper.mockContext())
+
+    expect(res.data).toEqual([])
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.list = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.listPR(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})
+
+describe('listReviews', () => {
+  test('return correct data if no error', async () => {
+    const reviews = [
+      'review 1',
+      'review 2'
+    ]
+
+    const res = await GithubAPI.listReviews(Helper.mockContext({ reviews }))
+    expect(res).toEqual(reviews)
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.listReviews.endpoint.merge = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.listReviews(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})
+
+describe('listCommits', () => {
+  test('return correct data if no error', async () => {
+    const date = Date.now()
+    const commits = [
+      {
+        commit: {
+          author: {
+            date
+          },
+          message: 'fix: this'
+        }
+      }
+    ]
+
+    const res = await GithubAPI.listCommits(Helper.mockContext({ commits }))
+    expect(res.length).toEqual(1)
+    expect(res[0].date).toEqual(date)
+    expect(res[0].message).toEqual('fix: this')
+  })
+
+  test('that error are NOT re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.pulls.listCommits.endpoint.merge = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.listCommits(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})

lib/github/api.js

@@ -440,6 +440,54 @@ class GithubAPI {
       log.info(errorLog)
     }
   }
+
+  static async listPR (context) {
+    const callFn = 'pulls.list'
+
+    debugLog(context, callFn)
+
+    try {
+      return await context.octokit.pulls.list(context.repo({
+        base: context.payload.ref
+      }))
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
+
+  static async listReviews (context, pullNumber) {
+    const callFn = 'pulls.listReviews'
+
+    debugLog(context, callFn)
+
+    try {
+      return await context.octokit.paginate(
+        context.octokit.pulls.listReviews.endpoint.merge(
+          context.repo({ pull_number: pullNumber })
+        ),
+        res => res.data
+      )
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
+
+  static async listCommits (context, pullNumber) {
+    const callFn = 'pulls.listCommits'
+
+    debugLog(context, callFn)
+
+    try {
+      return await context.octokit.paginate(
+        context.octokit.pulls.listCommits.endpoint.merge(
+          context.repo({ pull_number: pullNumber })
+        ),
+        res => res.data.map(o => ({ message: o.commit.message, date: o.commit.author.date }))
+      )
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
 }
 
 module.exports = GithubAPI

lib/interceptors/interceptor.js

@@ -1,3 +1,4 @@
+const GithubAPI = require('../github/api')
 /**
  * The Interceptor class defines the interface for all inheriting interceptors that
  * mutates the probot context with additional meta data or changes existing property Values
@@ -9,6 +10,10 @@
  * Interceptors are cached instances and should be treated as singletons and is NOT thread safe. Instance variables should be treated as constants.
  */
 class Interceptor {
+  constructor () {
+    this.githubAPI = GithubAPI
+  }
+
   /**
    * All Interceptors should overwrite this method and mutate the context as needed.
    * By default returns the context unchanged.

lib/interceptors/push.js

@@ -29,9 +29,7 @@ class Push extends Interceptor {
 
     const registry = { filters: new Map(), validators: new Map(), actions: new Map() }
 
-    const res = await context.octokit.pulls.list(context.repo({
-      base: context.payload.ref
-    }))
+    const res = await this.githubAPI.listPR(context)
 
     const pulls = res.data
     await Promise.all(pulls.map(pullRequest => {

lib/validators/approvals.js

@@ -66,12 +66,7 @@ class Approvals extends Validator {
       delete validationSettings.limit
     }
 
-    const reviews = await context.octokit.paginate(
-      context.octokit.pulls.listReviews.endpoint.merge(
-        context.repo({ pull_number: this.getPayload(context).number })
-      ),
-      res => res.data
-    )
+    const reviews = await this.githubAPI.listReviews(context, this.getPayload(context).number)
 
     const {
       requiredReviewers,

lib/validators/commit.js

@@ -44,12 +44,7 @@ class Commit extends Validator {
 
     const validatorContext = { name: 'commit' }
 
-    const commits = await context.octokit.paginate(
-      context.octokit.pulls.listCommits.endpoint.merge(
-        context.repo({ pull_number: this.getPayload(context).number })
-      ),
-      res => res.data.map(o => ({ message: o.commit.message, date: o.commit.author.date }))
-    )
+    const commits = await this.githubAPI.listCommits(context, this.getPayload(context).number)
     let orderedCommits = _.orderBy(commits, ['date'], ['asc'])
 
     if (singleCommitOnly && orderedCommits.length !== 1) {