Empty (but not null) entries should clear teams and collaborators

lib/mergeDeep.js

@@ -109,7 +109,7 @@ class MergeDeep {
                 }
                 // Add username attribute to the modifications to make it look better ; it won't be added otherwise as it would be the same
                 if (!this.isEmpty(modifications[key][modifications[key].length - 1])) {
-                  Object.assign(modifications[key][modifications[key].length - 1], { name: a.username })
+                  Object.assign(modifications[key][modifications[key].length - 1], { username: a.username })
                 }
                 delete visited[a.username]
                 continue

lib/plugins/collaborators.js

@@ -20,35 +20,28 @@ module.exports = class Collaborators extends Diffable {
       this.github.repos.listCollaborators({ repo: this.repo.repo, owner: this.repo.owner, affiliation: 'outside' }),
       this.github.repos.listInvitations({ repo: this.repo.repo, owner: this.repo.owner })])
       .then(res => {
-        const results0 = res[0].data.map(user => {
+        const mapCollaborator = user => {
           return {
-          // Force all usernames to lowercase to avoid comparison issues.
-            username: user.login.toLowerCase(),
-            pendinginvite: false,
-            permission: (user.permissions.admin && 'admin') ||
-            (user.permissions.push && 'push') ||
-            (user.permissions.pull && 'pull')
-          }
-        })
-        const results1 = res[1].data.map(user => {
-          return {
-          // Force all usernames to lowercase to avoid comparison issues.
+            // Force all usernames to lowercase to avoid comparison issues.
             username: user.login.toLowerCase(),
             pendinginvite: false,
             permission: (user.permissions.admin && 'admin') ||
             (user.permissions.push && 'push') ||
             (user.permissions.pull && 'pull')
           }
-        })
-        const results2 = res[2].data.map(invite => {
+        }
+
+        const results0 = (res[0].data || []).map(mapCollaborator)
+        const results1 = (res[1].data || []).map(mapCollaborator)
+        const results2 = (res[2].data || []).map(invite => {
           return {
           // Force all usernames to lowercase to avoid comparison issues.
             username: invite.invitee.login.toLowerCase(),
             pendinginvite: true,
             invitation_id: invite.id,
             permission: (invite.permissions === 'admin' && 'admin') ||
-          (invite.permissions === 'read' && 'pull') ||
-          (invite.permissions === 'write' && 'push')
+            (invite.permissions === 'read' && 'pull') ||
+            (invite.permissions === 'write' && 'push')
           }
         })
         return results0.concat(results1).concat(results2)
@@ -89,8 +82,8 @@ module.exports = class Collaborators extends Diffable {
     const data = Object.assign({
       invitation_id: invitation_id,
       permissions: (permissions === 'admin' && 'admin') ||
-    (permissions === 'pull' && 'read') ||
-    (permissions === 'push' && 'write')
+      (permissions === 'pull' && 'read') ||
+      (permissions === 'push' && 'write')
     }, this.repo)
     if (this.nop) {
       return Promise.resolve([
@@ -102,26 +95,23 @@ module.exports = class Collaborators extends Diffable {
 
   remove (existing) {
     if (existing.pendinginvite) {
+      const data = Object.assign({ invitation_id: existing.invitation_id }, this.repo)
       if (this.nop) {
         return Promise.resolve([
-          new NopCommand(this.constructor.name, this.repo, this.github.repos.deleteInvitation.endpoint(
-            Object.assign(this.repo, { invitation_id: existing.invitation_id })), 'Delete Invitation')
+          new NopCommand(this.constructor.name, this.repo, this.github.repos.deleteInvitation.endpoint(data),
+            'Delete Invitation')
         ])
       }
-      return this.github.repos.deleteInvitation(
-        Object.assign(this.repo, { invitation_id: existing.invitation_id })
-      )
+      return this.github.repos.deleteInvitation(data)
     } else {
+      const data = Object.assign({ username: existing.username }, this.repo)
       if (this.nop) {
         return Promise.resolve([
-          new NopCommand(this.constructor.name, this.repo, this.github.repos.removeCollaborator(
-            Object.assign(this.repo, { username: existing.username })
-          ), 'Remove Collaborator')
+          new NopCommand(this.constructor.name, this.repo, this.github.repos.removeCollaborator(data),
+            'Remove Collaborator')
         ])
       }
-      return this.github.repos.removeCollaborator(
-        Object.assign(this.repo, { username: existing.username })
-      )
+      return this.github.repos.removeCollaborator(data)
     }
   }
 }

lib/plugins/diffable.js

@@ -63,15 +63,7 @@ module.exports = class Diffable {
       }
     })
     filteredEntries = filteredEntries.map(e => {
-      const o = {}
-      for (const key in e) {
-        if (key !== 'include' && key !== 'exclude') {
-          Object.assign(o, {
-            [key]: e[key]
-          })
-        }
-      }
-
+      const { exclude, include, ...o } = e
       return o
     })
     return filteredEntries
@@ -86,7 +78,8 @@ module.exports = class Diffable {
         const compare = mergeDeep.compareDeep(existingRecords, filteredEntries)
         const results = JSON.stringify(compare, null, 2)
         this.log.debug(`Results of comparing ${this.constructor.name} diffable target ${JSON.stringify(existingRecords)} with source ${JSON.stringify(filteredEntries)} is ${results}`)
-        if (!compare.hasChanges) {
+
+        if (!compare.hasChanges && existingRecords.length === filteredEntries.length) {
           this.log.debug(`There are no changes for ${this.constructor.name} for repo ${this.repo}. Skipping changes`)
           if (this.nop) {
             return Promise.resolve([(new NopCommand(this.constructor.name, this.repo, null, `${this.constructor.name} settings has ${compare.additions.length} additions and ${compare.modifications.length} modifications`))])
@@ -123,7 +116,7 @@ module.exports = class Diffable {
         }
         return Promise.all(changes)
       }).catch(e => {
-        this.log.error(`error calling find for ${this.constructor.name} ${e} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}`)
+        this.log.error(`error calling find for ${this.constructor.name} for repo: ${JSON.stringify(this.repo)} entries ${JSON.stringify(this.entries)}\n`, e)
       })
     }
   }

lib/plugins/teams.js

@@ -5,7 +5,7 @@ const NopCommand = require('../nopcommand')
 const teamRepoEndpoint = '/teams/:team_id/repos/:owner/:repo'
 module.exports = class Teams extends Diffable {
   async find () {
-    this.log(`Finding teams for ${this.repo.owner}/${this.repo.repo}`)
+    this.log.debug(`Finding teams for ${this.repo.owner}/${this.repo.repo}`)
     return this.github.repos.listTeams(this.repo).then(res => res.data).catch(e => { return [] })
   }
 
@@ -28,19 +28,19 @@ module.exports = class Teams extends Diffable {
 
   add (attrs) {
     let existing = { team_id: 1 }
-    this.log(`Getting team with the parms ${JSON.stringify(attrs)}`)
+    this.log.debug(`Getting team with the parms ${JSON.stringify(attrs)}`)
     return this.github.teams.getByName({ org: this.repo.owner, team_slug: attrs.name }).then(res => {
       existing = res.data
-      this.log(`adding team ${attrs.name} to repo ${this.repo.repo}`)
+      this.log.debug(`adding team ${attrs.name} to repo ${this.repo.repo}`)
       if (this.nop) {
         return Promise.resolve([
           new NopCommand(this.constructor.name, this.repo, this.github.teams.addOrUpdateRepoPermissionsInOrg.endpoint(this.toParams(existing, attrs)), 'Add Teams to Repo')
         ])
       }
       return this.github.teams.addOrUpdateRepoPermissionsInOrg(this.toParams(existing, attrs)).then(res => {
-        this.log(`team added ${res}`)
+        this.log.debug(`team added ${res}`)
       }).catch(e => {
-        this.log(`Error adding team to repo ${JSON.stringify(e)} with parms ${JSON.stringify(this.toParams(existing, attrs))}`)
+        this.log.error(`Error adding team to repo ${JSON.stringify(e)} with parms ${JSON.stringify(this.toParams(existing, attrs))}:\n`, e)
       })
     }).catch(e => {
       if (e.status === 404) {
@@ -51,19 +51,19 @@ module.exports = class Teams extends Diffable {
         if (attrs.privacy) {
           createParam.privacy = attrs.privacy
         }
-        this.log(`Creating teams ${JSON.stringify(createParam)}`)
+        this.log.debug(`Creating teams ${JSON.stringify(createParam)}`)
         if (this.nop) {
           return Promise.resolve([
             new NopCommand(this.constructor.name, this.repo, this.github.teams.create.endpoint(createParam), 'Create Team')
           ])
         }
         return this.github.teams.create(createParam).then(res => {
-          this.log(`team ${createParam.name} created`)
+          this.log.debug(`team ${createParam.name} created`)
           existing = res.data
-          this.log(`adding team ${attrs.name} to repo ${this.repo.repo}`)
+          this.log.debug(`adding team ${attrs.name} to repo ${this.repo.repo}`)
           return this.github.teams.addOrUpdateRepoPermissionsInOrg(this.toParams(existing, attrs))
         }).catch(e => {
-          this.log(`Error adding team ${e}`)
+          this.log.error('Error adding team: ', e)
         })
       }
     })