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

[Warnings] Permissions for `[p]warnings` command can't be modified #2900

Closed
jack1142 opened this issue Jul 24, 2019 · 3 comments
Closed

[Warnings] Permissions for `[p]warnings` command can't be modified #2900

jack1142 opened this issue Jul 24, 2019 · 3 comments

Comments

@jack1142
Copy link
Member

@jack1142 jack1142 commented Jul 24, 2019

Command bugs

Command name

[p]warnings

What cog is this command from?

Warnings

What were you expecting to happen?

Permissions cog to be able to change required permissions for [p]warnings command - in this case, allow mods to use it.

What actually happened?

The command still couldn't be used by mods. Reason for this is that we use a is_admin_or_superior function inside the command instead of using a check. This was done like this to allow users to check their own warnings without any permissions, but because of this, there's no way to use the command for people without admin role, even when changing the required permissions with Permissions cog. I never had to check if args is given within a check before, so I'm not sure if it's possible to do using commands.check, but IMO Permissions cog should allow to change required permissions for this command.

How can we reproduce this issue?

[p]permissions addserverrule allow warnings Mods

@mikeshardmind

This comment has been minimized.

Copy link
Member

@mikeshardmind mikeshardmind commented Oct 26, 2019

My comment from the recent duplicate applies here:

Red's permission model would require these two separate behaviors to be split into separate commands. A command anyone could run by default to view their own warnings, and a command which used a normal permissions override-able check (at the same admin default level) to view others.

To further clarify, permissions doesn't support variable allow/deny based on the intent of the command. It can't know, only restrict or unrestrict the command.

@Dav-Git

This comment has been minimized.

Copy link
Contributor

@Dav-Git Dav-Git commented Oct 28, 2019

The general idea was that server admins especially on bigger servers can entrust a specific role with the ability to check a users warnings and proceed with different disciplinary actions as a result of the outcome of the warnings check.

Whether that would be done in a seperate command or not doesn't really matter, but I think the option should be there, especially since every other aspect of the warnings cog is controllable using permissions.

@mikeshardmind mikeshardmind removed this from the 3.2.0 milestone Nov 8, 2019
@Dav-Git

This comment has been minimized.

Copy link
Contributor

@Dav-Git Dav-Git commented Jan 10, 2020

Since the PR working on this issue got closed I'm not sure if the In progress status still applies.

Drapersniper added a commit to Drapersniper/Red-DiscordBot that referenced this issue Jan 16, 2020
…rs#3327)

* new code

Added the admin check to warnings and removed the part where the user could check themselves.

Added new mywarns which replaces part of the old behaviour of warn

* Update warnings.py

* Create 2900.enhance.rst

* Fixed command name

Because appearently I can't remember a command for 10 seconds

* Commands in backticks

Put command names in changelog in double backticks after being advised to do so in discord

* made user not optional, and the other thing sinbad requested

* switched parts. magic

resolves Cog-Creators#2900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.