-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Slack usergroup adapter #300
Fix Slack usergroup adapter #300
Conversation
I've fixed the linting for the Slack usergroup adapter but the version of |
06e054a
to
48b581a
Compare
@mike-gregory-ovo I have rebased and force-pushed, should be okay for a re-review now 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @chris-brindley! 🚀
This PR fixes a bug in the Slack usergroup adapter that was causing issues when adding and removing users from a group in a single operation.
The
Add
andRemove
methods rely on internal state of the adapter to understand what user IDs are to be sent to the API. This internal state is only updated on theGet
method, not theAdd
orRemove
methods. Thus, when callingAdd
and subsequentlyRemove
(or vice-versa), the state is wrong and the adapter sends an incorrect set of IDs to the Slack API. This resulted is buggy behaviour causing users to be added and removed in quick succession.