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

Add role changing for custom commands #292

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cactusbot/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ async def update_count(self, command, action):
self.api.patch("/user/{token}/command/{command}/count".format(
token=self.api.token, command=command), data=json.dumps(data)))

async def update(self, command, value):
"""Update command attributes."""

return await self.api.patch("/user/{token}/command/{command}".format(
token=self.api.token, command=command), data=json.dumps(value))
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if the type signature was based more on values, rather than just json.dumpsing value.

For example, something like:

async def update(self, command, name=None, role=None):

Copy link
Member Author

Choose a reason for hiding this comment

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

No - I copied this update function from configs. Keeping it the way it is incase we ever want to update something else about the command, say the name.



class Alias(CactusAPIBucket):
"""CactusAPI /alias bucket."""
Expand Down
53 changes: 53 additions & 0 deletions cactusbot/commands/magic/command.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
"""Manage commands."""

from . import Command
from ..command import ROLES

_ROLE_NAMES = [ROLES[role].lower() for role in ROLES]
_REVERSED_ROLES = {v: k for k, v in ROLES.items()}


def _role_id(role_name):
return _REVERSED_ROLES[role_name] if role_name in _REVERSED_ROLES else -1
Copy link
Member

Choose a reason for hiding this comment

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

I would use None rather than -1 here, in case -1 becomes an actual thing at some point™. (Global banned, or muted, or whatever.)



def _is_role(role):
return role.lower() in _ROLE_NAMES


async def _update_role(api, command, role):
"""Update the role of a command."""

data = {
"attributes": {
"response": {
"role": role
}
}
}

return await api.command.update(command, data)


class Meta(Command):
Expand Down Expand Up @@ -91,3 +117,30 @@ async def count(self, command: r'?command',
response = await self.api.command.update_count(command, action_string)
if response.status == 200:
return "Count updated."

@Command.command(role="moderator")
async def permission(self, command: r'?command', role=None):
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we want to use the rename syntax (in discussion on #249), rather than permission?

!command rename stuff things
!command rename test +test

Copy link
Member Author

Choose a reason for hiding this comment

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

We did, but not using that for permissions. That becomes a problem of having to readd the command to change both, and right now I would prefer if the two were separated.

"""Change the role of a command."""

if role is None:
Copy link
Member

Choose a reason for hiding this comment

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

Good call. If we switch to rename, we should do something similar.

> !command rename highfive
< Command !highfive (User): "You get a highfive, %ARGS%!"

# Didn't get a value, return the role of the command
response = await self.api.command.get(command)

if response.status == 200:
data = await response.json()
minimum_role = data["data"]["attributes"]["response"]["role"]
return "Minimum role for !{command} is '{role}'".format(
command=command, role=ROLES[minimum_role])

valid_role = _is_role(role)
if not valid_role:
# not a real role, tell the user.
return "'{role}' is not a valid role!".format(role=role)
# Valid role, update it.
role_id = _role_id(role)
if role_id == -1:
return "Invalid role {}".format(role)
response = await _update_role(self.api, command, role)
if response.status == 200:
return "Updated role for !{command} to '{role}'".format(
command=command, role=role)
18 changes: 18 additions & 0 deletions docs/user/command.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,21 @@ Otherwise, `action` may begin with either a `+` or `-`, to increase or decrease
[MindlessPuppetz] !command count derp +12
[CactusBot] Count updated.
```

## `!command role <command> [role]`
Copy link
Member

Choose a reason for hiding this comment

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

Yay, docs! (Nice choices for usernames, BTW.)


Retrieve or set the `role` of a custom command.

If `role` is not supplied, the current `role` is returned.

```
[EvilNotion] !command role magical
[CactusBot] Minimum role for !magical is 'Moderator'
```

Otherwise, the role is set:

```
[HauntedKnight] !command role magical User
[CactusBot] Updated role for !magical to 'User'
```