-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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] Avoid last admin deactivate itself #22949
Conversation
…ket.Chat into lastAdminDeactivateItself
This pull request introduces 1 alert when merging 60ae3e9 into e764914 - view on LGTM.com new alerts:
|
@@ -39,6 +39,15 @@ export function setUserActiveStatus(userId, active, confirmRelinquish = false) { | |||
return false; | |||
} | |||
|
|||
const userAdmin = Users.findOneAdmin(userId.count); |
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.
This should only be called if you're actually trying to disable the user, like inside the following if statement
Co-authored-by: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com>
method: 'removeUserFromRole', | ||
action: 'Remove_last_admin', | ||
}); | ||
} |
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.
We should add this check to removing role if role is admin. Some people seem to keep removing the last admin. 🙈
Proposed changes (including videos or screenshots)
Co-authored-by: @Kartik18g
Issue(s)
Steps to test or reproduce
Further comments