feat(collaborators): listen to member changes

[anderssonjohan]

part of fixes #251

Add listeners for repository collaborator events:

• member
• team

Synchronizes settings when users or teams are added, removed, and when permissions for a member is changed.

Also includes minor fix to index.test.js to make the tests run.

Tasks

• add new listeners with tests
• remove excess members

[anderssonjohan]

ping @decyjphr @martinm82

[decyjphr]

@anderssonjohan will work on it tomorrow. Thanks for the ping.

[martinm82]

LGTM

[martinm82]

Should we update as well the README?

[martinm82]

Wouldn't it be possible to use the logger through the context?

Suggested change

	this.log = log
	this.log = context.log

[anderssonjohan]

Ah, great! I wasn't aware of that. I added this because of null references for this.log when trying to run the tests in index.test.js. I'll push a change.

[anderssonjohan]

Also, good catch, I obviously did not update all the places where ConfigManager were created in index.js! Fixup commit pushed (will rebase when review is done)

[anderssonjohan]

I just updated the PR description because these changes currently do not fix #251. It syncs memberships defined as policy, but the collaborators plugin does currently not remove any manually added members (users/teams). Thus, this PR should not auto-close the related issue until that part is added.

[anderssonjohan]

@decyjphr @martinm82 I found the issue with members not being removed. In my config I have only teams and no collaborators 😅 . So for starters, I need to add collaborators: [] to the config to enable the collaborators plugin so the code that can remove excess members is called (Collaborators.remove via Collaborators.sync). But with an empty array we never get hasChanges = true from compareDeep, because the loop over source has zero iterations when it's empty.
Thus, to remove manually added collaborators you need to configure a collaborator to enter that loop.

I'll add a test that verifies that compareDeep returns hasChanges: true when source (config object) is empty but target (entries from GitHub response) is not.

Example debugging session where I added manually added my github account to the repo with config set to collaborators: []. As you can see, the loop won't run.

This also means that users can enable this behavior (removal of manually added collaborators) by activating the plugin. If the want the collaborators to stay there is an option like in my previous config where I didn't use that plugin at all.

[anderssonjohan]

@decyjphr @martinm82 Now the PR is complete in terms of fixing #251.

A small note on the fix for compareDeep: I first made a version where the source and target were swapped if the source was empty. In that way the recursive call would loop through the target instead and produce the same diff as in a case where GitHub "is empty" and config is not. That, however, made one test fail for compareDeep so my commit only ensures that hasChanges is true (additions and modifications are both empty) so the code that removes the unwanted collaborators is reached.

What we had before:

compareDeep({x:1}, {}) => { hasChanges: true, additions: [ {x:1} ], modifications: [] }
compareDeep({}, {x:1}) => { hasChanges: false, additions: [], modifications: [] }

What we have now is:

compareDeep({x:1}, {}) => { hasChanges: true, additions: [ {x:1} ], modifications: [] }
compareDeep({}, {x:1}) => { hasChanges: true, additions: [], modifications: [] }

I even started to look at lodash differenceWith to see if it could be used with a comparator function that would compare by value and username/name attributes, but that's a much larger change that I didn't dare to look into 😅
I hope you find this small change to compareDeep reasonable.

[anderssonjohan]

I think my fixes here for empty list of collaborators also fixes the problem described by #301

[decyjphr]

Thanks. 🚢