-
Notifications
You must be signed in to change notification settings - Fork 639
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
Disabled users inside robot brain #184
Comments
or at least we should have a property for disabled users. |
Agreed. I'd actually rather have a property for them. |
I started #229 to track most of the data coming from Slack about a user in the hubot brain. That should help here. That said, I don't know if it's a good idea to keep disabled users in the brain like you said. |
@technicalpickles As far as I know, removing users is not implemented in slack since a user can be a member of more than one organization. So, disabling a user is actually removing him/her from an organization. I vote for not keeping disabled users in the brain! |
That's a really good point. I think that means we should skip populating users that are disabled into the brain. Only other question remains is: should disabled users be actively removed from the brain? That is, say I'm a user, and I'm disabled, does all the data with my user go away? Or should it stick around? I'm inclined to leave it in the brain for now, as that's what most other adapters do by convention. |
I agree. Let's keep it in the brain. |
It should stick around—we keep the user around in Slack because maybe they come back into the org (think summer interns that you later re-hire)—you'd like them to be able to pick up where they left off. The Hubot brain should mirror what Slack is doing, I think. |
How far does #229 go to fixing this issue, BTW? |
Should I look in |
It would be in the |
Plus #229 only updates users on receiving a |
This should be resolved in #381. Reopen if not. |
I see disabled slack users inside robot brain which is not desired. Isn't it better to ignore them?
The text was updated successfully, but these errors were encountered: