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

Cannot refresh jail time #271

Closed
robstolarz opened this issue Feb 17, 2019 · 3 comments
Closed

Cannot refresh jail time #271

robstolarz opened this issue Feb 17, 2019 · 3 comments

Comments

@robstolarz
Copy link
Collaborator

robstolarz commented Feb 17, 2019

def check_other_roles(self, member):
has_other, punish_role, _ = self.bot.sql.moderation.get_other_roles(member)
if has_other:
embed = discord.Embed(colour=discord.Colour.red())
role_descr = (
""
if punish_role is None
else f"because they already have {punish_role.mention}"
)
embed.description = (
f"Cannot add a new overriding role to {member.mention} {role_descr}"
)
raise CommandFailed(embed=embed)
, called from
if remove_other:
self.check_other_roles(member)
await self.bot.sql.moderation.remove_other_roles(
member, roles.jail, full_reason
)
, checks to make sure that someone does not already have overriding roles before applying a new set. However, because of this check, jail time cannot easily be refreshed or changed once it is set. Instead, an error like "Cannot add a new overriding role to <user> because they already have <role>" is presented.

Would it make sense to alter this check to see if the new overriding roles are the same as the old ones and, if so, allow the check to pass?

Or perhaps these functions should be merged to attempt to add the overriding roles, and fail if an override already exists and is different than the new one?

@emmiegit
Copy link
Collaborator

Related to #267

@emmiegit
Copy link
Collaborator

Checking to make sure the existing overriding role is None or the same makes sense. However modifying the actual timer is part of the linked issue.

@emmiegit
Copy link
Collaborator

Multiple overriding roles were added in #298. Because the core issue this card is referring to exists in #267, I am closing this as a duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants