Repository: probot/settings
Pull request #209 (2020-03-05 07:27:49)

fix(teams): migrated away from legacy teams apis


Patch:
diff --git a/README.md b/README.md
index 1c63812..07f8301 100644
--- a/README.md
+++ b/README.md
@@ -97,8 +97,6 @@ collaborators:
   - username: hubot
     permission: pull
 
-# NOTE: The APIs needed for teams are not supported yet by GitHub Apps
-# https://developer.github.com/v3/apps/available-endpoints/
 teams:
   - name: core
     permission: admin
diff --git a/lib/plugins/teams.js b/lib/plugins/teams.js
index 0d4929d..bba2d7d 100644
--- a/lib/plugins/teams.js
+++ b/lib/plugins/teams.js
@@ -1,5 +1,3 @@
-// NOTE: The APIs needed for this plugin are not supported yet by GitHub Apps
-// https://developer.github.com/v3/apps/available-endpoints/
 const Diffable = require('./diffable')
 
 module.exports = class Teams extends Diffable {
@@ -16,29 +14,23 @@ module.exports = class Teams extends Diffable {
   }
 
   update (existing, attrs) {
-    return this.github.teams.addOrUpdateRepo(this.toParams(existing, attrs))
+    return this.github.teams.addOrUpdateRepoInOrg(this.toParams(attrs))
   }
 
   add (attrs) {
-    // There is not a way to resolve a team slug to an id without fetching all
-    // teams for an organization.
-    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))
-    })
+    return this.github.teams.addOrUpdateRepoInOrg(this.toParams(attrs))
   }
 
   remove (existing) {
-    return this.github.teams.removeRepo({ team_id: existing.id, ...this.repo })
+    return this.github.teams.removeRepoInOrg({ team_slug: existing.slug, ...this.repo, org: this.repo.owner })
   }
 
-  toParams (existing, attrs) {
+  toParams (attrs) {
     return {
-      team_id: existing.id,
+      team_slug: attrs.name,
       owner: this.repo.owner,
       repo: this.repo.repo,
+      org: this.repo.owner,
       permission: attrs.permission
     }
   }


Test patch:
diff --git a/test/fixtures/teams-config.yml b/test/fixtures/teams-config.yml
new file mode 100644
index 0000000..b31264c
--- /dev/null
+++ b/test/fixtures/teams-config.yml
@@ -0,0 +1,5 @@
+teams:
+  - name: probot
+    permission: admin
+  - name: greenkeeper-keeper
+    permission: push
diff --git a/test/integration/push.test.js b/test/integration/push.test.js
index 22ab5ac..10aad30 100644
--- a/test/integration/push.test.js
+++ b/test/integration/push.test.js
@@ -102,4 +102,49 @@ describe('push', function () {
       }
     })
   })
+
+  it('syncs teams', async () => {
+    const pathToConfig = path.resolve(__dirname, '..', 'fixtures', 'teams-config.yml')
+    const configFile = Buffer.from(fs.readFileSync(pathToConfig, 'utf8'))
+    const encodedConfig = configFile.toString('base64')
+    githubScope
+      .get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
+      .reply(OK, { content: encodedConfig, name: 'settings.yml', type: 'file' })
+    githubScope
+      .get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
+      .reply(OK, { content: encodedConfig, name: 'settings.yml', type: 'file' })
+    githubScope
+      .get(`/repos/${repository.owner.name}/${repository.name}/teams`)
+      .reply(
+        OK,
+        [
+          { slug: 'greenkeeper-keeper', permission: 'pull' },
+          { slug: 'form8ion', permission: 'push' }
+        ]
+      )
+    githubScope
+      .put(`/orgs/${repository.owner.name}/teams/probot/repos/${repository.owner.name}/${repository.name}`, body => {
+        expect(body).toMatchObject({ permission: 'admin' })
+        return true
+      })
+      .reply(CREATED)
+    githubScope
+      .put(`/orgs/${repository.owner.name}/teams/greenkeeper-keeper/repos/${repository.owner.name}/${repository.name}`, body => {
+        expect(body).toMatchObject({ permission: 'push' })
+        return true
+      })
+      .reply(OK)
+    githubScope
+      .delete(`/orgs/${repository.owner.name}/teams/form8ion/repos/${repository.owner.name}/${repository.name}`)
+      .reply(NO_CONTENT)
+
+    await probot.receive({
+      name: 'push',
+      payload: {
+        ref: 'refs/heads/master',
+        repository,
+        commits: [{ modified: [settings.FILE_NAME], added: [] }]
+      }
+    })
+  })
 })
diff --git a/test/unit/lib/plugins/teams.test.js b/test/unit/lib/plugins/teams.test.js
index 2c8c917..02b5bb6 100644
--- a/test/unit/lib/plugins/teams.test.js
+++ b/test/unit/lib/plugins/teams.test.js
@@ -20,22 +20,14 @@ describe('Teams', () => {
         }))
       },
       teams: {
-        addOrUpdateRepo: jest.fn().mockImplementation(() => Promise.resolve()),
-        list: {
-          endpoint: {
-            merge: jest.fn().mockImplementation(() => {})
-          }
-        },
-        removeRepo: jest.fn().mockImplementation(() => Promise.resolve())
+        addOrUpdateRepoInOrg: jest.fn().mockImplementation(() => Promise.resolve()),
+        removeRepoInOrg: 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' },
@@ -43,29 +35,32 @@ describe('Teams', () => {
       ])
 
       return plugin.sync().then(() => {
-        expect(github.teams.addOrUpdateRepo).toHaveBeenCalledWith({
+        expect(github.teams.addOrUpdateRepoInOrg).toHaveBeenCalledWith({
+          org: 'bkeepers',
           owner: 'bkeepers',
           repo: 'test',
-          team_id: 3,
+          team_slug: 'updated-permission',
           permission: 'admin'
         })
 
-        expect(github.teams.addOrUpdateRepo).toHaveBeenCalledWith({
+        expect(github.teams.addOrUpdateRepoInOrg).toHaveBeenCalledWith({
+          org: 'bkeepers',
           owner: 'bkeepers',
           repo: 'test',
-          team_id: 4,
+          team_slug: 'added',
           permission: 'pull'
         })
 
-        expect(github.teams.addOrUpdateRepo).toHaveBeenCalledTimes(2)
+        expect(github.teams.addOrUpdateRepoInOrg).toHaveBeenCalledTimes(2)
 
-        expect(github.teams.removeRepo).toHaveBeenCalledWith({
+        expect(github.teams.removeRepoInOrg).toHaveBeenCalledWith({
+          org: 'bkeepers',
           owner: 'bkeepers',
           repo: 'test',
-          team_id: 2
+          team_slug: 'removed'
         })
 
-        expect(github.teams.removeRepo).toHaveBeenCalledTimes(1)
+        expect(github.teams.removeRepoInOrg).toHaveBeenCalledTimes(1)
       })
     })
   })