-
-
Notifications
You must be signed in to change notification settings - Fork 209
Feat/add giverole command #1007
Feat/add giverole command #1007
Conversation
src/commands/moderation/giverole.js
Outdated
|
|
||
| async run ({ channel, member, author, t }, targetMember, role) { | ||
| const embed = new SwitchbladeEmbed(author) | ||
| await targetMember.addRole(role).then(modifiedMember => { |
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.
Whenever you use async/await, you don't have to use .then or .catch you can use the simple try/catch. Ex.:
try {
await targetMember.addRole(role)
// do other things
} catch (e) {
// handle error
}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.
Just sent this change
| if (!member.hasPermission('ADMINISTRATOR', false, true, false)) { | ||
| if (member.highestRole.comparePositionTo(role) <= 0) throw new CommandError(t('commands:giverole.roleAboveAuthors')) | ||
| if (member.highestRole.comparePositionTo(member.highestRole) <= 0) throw new CommandError(t('commands:giverole.roleBelow')) | ||
| } |
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 verification process should be made before adding the role to the target member.
And the condition in line 22 is non-sense, you are comparing member.highestRole to itself: member.highestRole.comparePositionTo(member.highestRole)
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.
Oh shit, I just changed the variable names and forgot this one lol.
|
So, how's this going? |
|
@davipatury re-review? |
Closes #1006