refactor: github_api/repos (#538)

* refactor: github_api/repos

* fix typo

* ran lint

Author: shine2lay

__tests__/unit/github/api.test.js

@@ -485,3 +485,78 @@ describe('projectListCards', () => {
     }
   })
 })
+
+describe('listCollaborators', () => {
+  test('return correct data if no error', async () => {
+    const collaborators = [
+      { login: 'member1' },
+      { login: 'member2' }
+    ]
+
+    const res = await GithubAPI.listCollaborators(Helper.mockContext({ collaborators }))
+    expect(res).toEqual(['member1', 'member2'])
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.repos.listCollaborators = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.listCollaborators(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})
+
+describe('getAllTopics', () => {
+  test('return correct data if no error', async () => {
+    const topics = [
+      'topic 1',
+      'topic 2'
+    ]
+
+    const res = await GithubAPI.getAllTopics(Helper.mockContext({ repoTopics: topics }))
+    expect(res).toEqual(topics)
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.repos.getAllTopics = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.getAllTopics(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})
+
+describe('compareCommits', () => {
+  test('return correct data if no error', async () => {
+    const diff = [
+      'file 1',
+      'file 2'
+    ]
+
+    const res = await GithubAPI.compareCommits(Helper.mockContext({ compareCommits: diff }))
+    expect(res.files).toEqual(diff)
+  })
+
+  test('that error are re-thrown', async () => {
+    const context = Helper.mockContext()
+    context.octokit.repos.compareCommits = jest.fn().mockRejectedValue({ status: 402 })
+
+    try {
+      await GithubAPI.compareCommits(context)
+      // Fail test if above expression doesn't throw anything.
+      expect(true).toBe(false)
+    } catch (e) {
+      expect(e.status).toBe(402)
+    }
+  })
+})

lib/actions/request_review.js

@@ -21,12 +21,6 @@ const createRequestReview = async (context, number, reviewers, actionObj) => {
   return res
 }
 
-const fetchCollaborators = async (context) => {
-  return context.octokit.repos.listCollaborators(
-    context.repo()
-  )
-}
-
 class RequestReview extends Action {
   constructor () {
     super('request_review')
@@ -52,8 +46,7 @@ class RequestReview extends Action {
     const prNumber = payload.number
 
     // get Collaborators
-    const rawCollaboratorsResult = await fetchCollaborators(context)
-    const collaborators = rawCollaboratorsResult.data.map(user => user.login)
+    const collaborators = await this.githubAPI.listCollaborators(context, context.repo())
 
     // remove anyone in the array that is not a collaborator
     const collaboratorsToRequest = _.intersection(reviewerToRequest, collaborators)

lib/filters/options_processor/topics.js

@@ -1,5 +1,6 @@
 const Options = require('./options')
 const CacheManager = require('../../cache/cache')
+const GithubAPI = require('../../github/api')
 
 // Setup the cache manager
 const cacheManager = new CacheManager()
@@ -23,15 +24,15 @@ const repoTopics = async (context) => {
     }
   }
 
-  return context.octokit.repos.getAllTopics({
+  const response = GithubAPI.getAllTopics(context, {
     owner: context.payload.repository.owner.login,
     repo: context.payload.repository.name
-  }).then(response => {
-    if (globalSettings.use_config_cache !== undefined && globalSettings.use_config_cache === true) {
-      cacheManager.set(`${repo.owner}/${repo.repo}/topics`, response.data.names)
-    }
-    return response.data.names
   })
+
+  if (globalSettings.use_config_cache !== undefined && globalSettings.use_config_cache === true) {
+    cacheManager.set(`${repo.owner}/${repo.repo}/topics`, response)
+  }
+  return response
 }
 
 module.exports = Topics

lib/github/api.js

@@ -326,6 +326,50 @@ class GithubAPI {
       return checkCommonError(err, context, callFn)
     }
   }
+
+  static async listCollaborators (context, callParam) {
+    const callFn = 'repos.listCollaborators'
+
+    debugLog(context, callFn)
+
+    try {
+      const res = await context.octokit.repos.listCollaborators(
+        callParam
+      )
+
+      return res.data.map(user => user.login)
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
+
+  static async getAllTopics (context, callParam) {
+    const callFn = 'repos.getAllTopics'
+
+    debugLog(context, callFn)
+
+    try {
+      const res = await context.octokit.repos.getAllTopics(callParam)
+
+      return res.data.names
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
+
+  static async compareCommits (context, callParam) {
+    const callFn = 'repos.compareCommits'
+
+    debugLog(context, callFn)
+
+    try {
+      const res = await context.octokit.repos.compareCommits(callParam)
+
+      return res.data
+    } catch (err) {
+      return checkCommonError(err, context, callFn)
+    }
+  }
 }
 
 module.exports = GithubAPI

lib/validators/options_processor/owners.js

@@ -11,12 +11,12 @@ class Owner {
 
     const owners = gitPattern.parseOwnerFile(CODEOWNERS)
 
-    const compare = await context.octokit.repos.compareCommits(context.repo({
+    const compare = await GithubAPI.compareCommits(context, context.repo({
       base: payload.base.sha,
       head: payload.head.sha
     }))
 
-    const paths = compare.data.files.map(file => file.filename)
+    const paths = compare.files.map(file => file.filename)
 
     let ownerList = []