Merge pull request #1 from kevo1ution/optimize.paginateMilestones

Fixing pagination in teams
repository-settings · Nov 13, 2019 · 2ef97e8 · 2ef97e8
2 parents 511b54d + aa9becf
commit 2ef97e8
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 31 deletions.
diff --git a/lib/plugins/teams.js b/lib/plugins/teams.js
@@ -22,7 +22,8 @@ module.exports = class Teams extends Diffable {
   add (attrs) {
     // There is not a way to resolve a team slug to an id without fetching all
     // teams for an organization.
-    return this.allTeams.then(teams => {
+    const options = this.github.teams.list.endpoint.merge({ per_page: 100, org: this.repo.owner })
+    this.github.paginate(options).then(teams => {
       const existing = teams.find(team => this.comparator(team, attrs))
 
       return this.github.teams.addOrUpdateRepo(this.toParams(existing, attrs))
@@ -43,29 +44,4 @@ module.exports = class Teams extends Diffable {
       permission: attrs.permission
     }
   }
-
-  // Lazy getter to fetch all teams for the organization
-  get allTeams () {
-    const getter = this.github.teams.list({ org: this.repo.owner })
-      .then(this.paginate.bind(this))
-      .then(responses => {
-        return responses.reduce((teams, res) => {
-          return teams.concat(res.data)
-        }, [])
-      })
-    Object.defineProperty(this, 'allTeams', getter)
-    return getter
-  }
-
-  // Paginator will keep fetching the next page until there are no more.
-  paginate (res, records = []) {
-    records = records.concat(res)
-    if (res.meta && this.github.hasNextPage(res)) {
-      return this.github.getNextPage(res).then(next => {
-        return this.paginate(next, records)
-      })
-    } else {
-      return Promise.resolve(records)
-    }
-  }
 }
diff --git a/test/lib/plugins/teams.test.js b/test/lib/plugins/teams.test.js
@@ -9,6 +9,7 @@ describe('Teams', () => {
 
   beforeEach(() => {
     github = {
+      paginate: jest.fn().mockImplementation(() => Promise.resolve()),
       repos: {
         listTeams: jest.fn().mockImplementation(() => Promise.resolve({
           data: [
@@ -20,18 +21,21 @@ describe('Teams', () => {
       },
       teams: {
         addOrUpdateRepo: jest.fn().mockImplementation(() => Promise.resolve()),
-        list: jest.fn().mockImplementation(() => Promise.resolve({
-          data: [
-            { id: 4, slug: 'added' }
-          ]
-        })),
+        list: {
+          endpoint: {
+            merge: jest.fn().mockImplementation(() => {})
+          }
+        },
         removeRepo: jest.fn().mockImplementation(() => Promise.resolve())
       }
     }
   })
 
   describe('sync', () => {
     it('syncs teams', () => {
+      github.paginate.mockReturnValueOnce(Promise.resolve([
+        { id: 4, slug: 'added' }
+      ]))
       const plugin = configure([
         { name: 'unchanged', permission: 'push' },
         { name: 'updated-permission', permission: 'admin' },