fix: Remove summary for teams/collaborators if only (noop) deletions (#…

…18)
eeveebank · Feb 14, 2024 · a9270ab · a9270ab
2 parents 3c6517b + f57861c
commit a9270ab
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 10 deletions.
diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js
@@ -81,7 +81,7 @@ module.exports = class Diffable extends ErrorStash {
         this.log.debug(` ${JSON.stringify(existingRecords, null, 2)} \n\n ${JSON.stringify(filteredEntries, null, 2)} `)
 
         const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields)
-        const compare = this._removeDeletionsIfPartialSync(mergeDeep.compareDeep(existingRecords, filteredEntries))
+        const compare = this._fixComparisonIfPartialSync(mergeDeep.compareDeep(existingRecords, filteredEntries))
         const results = { msg: 'Changes found', additions: compare.additions, modifications: compare.modifications, deletions: compare.deletions }
         this.log.debug(`Results of comparing ${this.constructor.name} diffable target ${JSON.stringify(existingRecords)} with source ${JSON.stringify(filteredEntries)} is ${results}`)
         if (!compare.hasChanges) {
@@ -170,8 +170,17 @@ module.exports = class Diffable extends ErrorStash {
 
   // Remove deletions from the summaries if the plugin is overridden
   // to implement isPartialSync
-  _removeDeletionsIfPartialSync (comparison) {
+  _fixComparisonIfPartialSync (comparison) {
     if (this.isPartialSync) {
+      if (isFalsyOrEmpty(comparison.additions) && isFalsyOrEmpty(comparison.modifications)) {
+        return {
+          additions: {},
+          modifications: {},
+          deletions: {},
+          hasChanges: false
+        }
+      }
+
       return {
         ...comparison,
         deletions: {}
@@ -181,3 +190,5 @@ module.exports = class Diffable extends ErrorStash {
     return comparison
   }
 }
+
+const isFalsyOrEmpty = (changes) => !changes || !Object.keys(changes).length
diff --git a/test/unit/lib/plugins/branches.test.js b/test/unit/lib/plugins/branches.test.js
@@ -205,7 +205,7 @@ describe('Branches', () => {
   })
 
   describe.skip('return values', () => {
-    it.only('returns updateBranchProtection Promise', () => {
+    it('returns updateBranchProtection Promise', () => {
       const plugin = configure(
         [{
           name: 'master',

diff --git a/test/unit/lib/plugins/collaboratorsPartial.test.js b/test/unit/lib/plugins/collaboratorsPartial.test.js
@@ -98,4 +98,19 @@ describe('CollaboratorsPartial', () => {
       );
     })
   })
+
+  it('doesnt add summary if no additions/modifications', async () => {
+    const plugin = configure([
+      { username: 'bkeepers', permission: 'admin' },
+    ], true)
+
+    github.repos.listCollaborators.mockResolvedValueOnce({
+      data: [
+        { login: 'bkeepers', permissions: { admin: true, push: true, pull: true } },
+        { login: 'removed-user', permissions: { admin: false, push: true, pull: true } },
+      ]
+    })
+
+    expect(await plugin.sync()).toBe(undefined)
+  })
 })
diff --git a/test/unit/lib/plugins/repository.test.js b/test/unit/lib/plugins/repository.test.js
@@ -59,7 +59,7 @@ describe('Repository', () => {
       })
     })
 
-    it.only('syncs topics', () => {
+    it('syncs topics', () => {
       const plugin = configure({
         topics: ['foo', 'bar']
       })

diff --git a/test/unit/lib/plugins/teamsPartial.test.js b/test/unit/lib/plugins/teamsPartial.test.js
@@ -38,9 +38,9 @@ describe('TeamsPartial', () => {
       repos: {
         listTeams: jest.fn().mockResolvedValue({
           data: [
-            { id: unchangedTeamId, slug: unchangedTeamName, permission: 'push' },
-            { id: removedTeamId, slug: removedTeamName, permission: 'push' },
-            { id: updatedTeamId, slug: updatedTeamName, permission: 'pull' }
+            { id: unchangedTeamId, slug: unchangedTeamName, name: unchangedTeamName, permission: 'push' },
+            { id: removedTeamId, slug: removedTeamName, name: removedTeamName, permission: 'push' },
+            { id: updatedTeamId, slug: updatedTeamName, name: updatedTeamName, permission: 'pull' }
           ]
         })
       },
@@ -104,10 +104,10 @@ describe('TeamsPartial', () => {
       jest.clearAllMocks()
     })
 
-    it.only('doesnt add deletions in summaries', async () => {
+    it('doesnt add deletions in summaries', async () => {
       const plugin = configure([
         { name: unchangedTeamName, permission: 'push' },
-        { name: updatedTeamName, permission: 'admin' },
+        { name: updatedTeamName, permission: 'pull' },
         { name: addedTeamName, permission: 'pull' }
       ], true)
 
@@ -118,10 +118,19 @@ describe('TeamsPartial', () => {
         { owner: 'bkeepers', repo: 'test' },
         null,
         expect.objectContaining({
-          deletions: {}
+          deletions: {},
         }),
         'INFO'
       );
     })
+
+    it('doesnt add summary if no additions/modifications', async () => {
+      const plugin = configure([
+        { name: unchangedTeamName, permission: 'push' },
+        { name: updatedTeamName, permission: 'pull' }
+      ], true)
+
+      expect(await plugin.sync()).toBe(undefined)
+    })
   })
 })