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

Only able to use queue modifying commands if you have the DJ role #56

Closed
wants to merge 21 commits into from

Conversation

Antonio-Bennett
Copy link
Contributor

@Antonio-Bennett Antonio-Bennett commented Dec 14, 2021

- -
Issue #47
Dependencies
Decisions Added a new utility function that checks if the user has the DJ role. Also added a new string to use with send_simple_message utility. I did not create an add_role and delete_role command because that is already built into discord server settings really well. A moderator can just click the user's name and add the role. I can add it if requested :)

@aquelemiguel
Copy link
Owner

Hi @Antonio-Bennett, always good seeing you back here. 😊

I quickly glanced at your code, but when writing #47 I thought about locking the commands via a macro.
Please take a look at #[allowed_roles(roles)] and see whether this simplifies your code. 🙂

@Antonio-Bennett
Copy link
Contributor Author

@aquelemiguel Hey I love working on Parrot :) Yeah, wouldn't that literally mean I just have to add that Macro to the commands.... Wow lol did not see that in the docs. Just 1 line 😭

@joao-conde
Copy link
Collaborator

As @aquelemiguel said, @Antonio-Bennett swap out for the macro usage and then tag us again for review! Thank you!

@Antonio-Bennett
Copy link
Contributor Author

Antonio-Bennett commented Dec 17, 2021

@joao-conde Hey yeah I will do it later today. Sorry for the MIA yesterday was the last day of school so final projects and stuff :) I'll swap out later today and tag for review 🙂

@Antonio-Bennett
Copy link
Contributor Author

Hey @joao-conde @aquelemiguel I might be misinterpreting the docs for allowed roles I add this macro after the command macro but doesn't seem to do anything for me
image

@aquelemiguel
Copy link
Owner

@Antonio-Bennett Huh, that's odd, your usage looks correct. I'm assuming you didn't forget to give yourself the "DJ" role, right?
I'll try it later today on my side and report back. 🤔

@Antonio-Bennett
Copy link
Contributor Author

@aquelemiguel The problem is it doesn't stop a user from using the command without the Role. So even with the macro, I can use the !play command with or without the DJ role. It's weird for sure.

@aquelemiguel
Copy link
Owner

@Antonio-Bennett Interesting, do you also have the Administrator role by any chance? Because it might be overlapping with the DJ role?

@Antonio-Bennett
Copy link
Contributor Author

Hey @aquelemiguel I'm testing on a server that I made. Does the owner have an Administrator role by default?

@aquelemiguel
Copy link
Owner

@Antonio-Bennett Looks like it!

image

@Antonio-Bennett
Copy link
Contributor Author

@aquelemiguel So what step do you want to take regarding this? I don't think it's possible to take away that role from owners. Also I guess your testing worked fine with the macro?

@aquelemiguel
Copy link
Owner

@aquelemiguel So what step do you want to take regarding this? I don't think it's possible to take away that role from owners. Also I guess your testing worked fine with the macro?

Just tested it with @joao-conde, yeah looks like we're right! (Ignore the orange parrot, that's our deployed instance)
I locked !stop behind the DJ role, like you did. I am administrator so my commands always went through. When giving him the role, he was able to use it again.

image

So I'd say you can freely add the macro to the commands you see fit. 😊

@Antonio-Bennett
Copy link
Contributor Author

Ok great will add it to the rest of commands now then :)

@Antonio-Bennett
Copy link
Contributor Author

@aquelemiguel wait I actually have one last question 😅 how exactly would we display the message that informs the user that they lack permission as stated in the issue? The macro kind of just blocks the use right? Is there a way to catch if it blocked it?

@joao-conde
Copy link
Collaborator

@Antonio-Bennett we have no idea as well, that is a matter of searching a bit. Try to find some resources online and bring them for discussion here.

Copy link
Collaborator

@joao-conde joao-conde left a comment

Choose a reason for hiding this comment

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

Use the macro approach to block, we don't really care about a message informing the blocked user. However, search a bit and if you find an easy enough solution to do so, apply it.

@joao-conde
Copy link
Collaborator

@Antonio-Bennett I did a quick search and found this:

Please read it and follow a similar approach.

Also, new PRs entered master so make sure to fix conflicts and then re-assign to me or any other maintainer.

@aquelemiguel aquelemiguel linked an issue Jan 4, 2022 that may be closed by this pull request
@joao-conde
Copy link
Collaborator

Closing this due to inactivity and taking the issue for myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict queue modifying commands to a DJ role
4 participants