Skip to content
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

First pull/push #169

Closed
wants to merge 16 commits into from
Closed

Conversation

F9Alejandro
Copy link
Collaborator

Changed admin.js to have userRole for request #70

@F9Alejandro F9Alejandro linked an issue Nov 3, 2020 that may be closed by this pull request
@F9Alejandro
Copy link
Collaborator Author

So many commits just to get back to original lol.

fixed issue with args[] missing split from arg.
fixed issue with role var in .find clashed with role var with role.id (renamed to roleid)
fixed users own role being removed instead of mentioned user.
.gitignore Outdated Show resolved Hide resolved
plugins/Admin/admin.js Outdated Show resolved Hide resolved
plugins/Admin/admin.js Outdated Show resolved Hide resolved
 Changes to be committed:
	modified:   plugins/Admin/admin.js
Made statements easier to read and understand, slight check of if msg came from a server or not though if statement, should add some sort of error message that is based on if the command is ran in a dm or if the sender doesn't have guild permission of ROLE
@F9Alejandro
Copy link
Collaborator Author

Tested new check of permission and works perfectly, typically no one would give manage roles perm unless they are an admin or higher.

var args = arg.split(" ");
var user, role;
if(msg.member.hasPermission("MANAGE_ROLES")){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to check if the bot has MANAGE_ROLES so that it can fail gracefully if it doesn't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could also do another try catch for it as well, or a quick change to add & to the if statement

@F9Alejandro F9Alejandro closed this Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

toggle role of user.
3 participants