[tempban] Allow tempbans by `UserID` instead of `Member`
Type of feature request
Command
Description of the feature you're suggesting
- Command:
tempban - Cog:
Mod
Hi!
By default, the tempban command checks for the user to be present in the server before banning them temporarily. That means you cannot ban any user who's not present in the server. Which you should be able to.
I don't think it's discord limitation since the you can do the same with other bots and even you can
banusers with thebanfrom the same cog, regardless of their presence in the server.
So it'd be great to have that functionality on red, allowing tempban to check for UserID instead of member so that we can ban people after they leave the server temporarily.
Thank you!
Anything else?
No response
Seems reasonable, I can't see a reason not to allow this.
One thing we need to consider is the behavior when a user is already banned - should we send a failure message or should we downgrade a ban to a tempban?
One thing we need to consider is the behavior when a user is already banned - should we send a failure message or should we downgrade a ban to a tempban?
I think downgrading the ban to a tempban would be more appropriate.
I think sending a failure message when user already banned is a better choose tbh.
May as well just split the difference and have a confirmation menu before downgrading the ban to a tempban.
Sending a failure msg, saying "User is already banned" would be a better option (when anyone tries to ban/tempban the same user who's banned already) and having two separate cmds, (ban and tempban) also gives you more room to play with, plus it doesn't make the ban command complicated.
At least for me the main reason for using a bot like red is to automate tedious administrative tasks. Wanting to downgrade a ban to a tempban is one of those. You would have to either remember to unban the user or unban and then re-(temp-)ban them. Both sound pretty annoying if we're being honest.
I think it is fairly reasonable to say that a typical order of events would be:
- A user is being naughty
- A mod quickly comes to the rescue and bans through discord or with the quicker to type ban command
- The mod wants to issue a tempban instead because while the user was naughty, they were not very naughty
TLDR: Tempban should probably downgrade a ban.
That being said, I also like the Zeph way of life. More choices more better.
Yeah, from usability standpoint it would probably make more sense to allow a downgrade. I like the prompt idea, though I'll wait with the decision until another org member or maybe few more community members chime in to make the choice easier.
For potential contributors, I suggest going with the prompt idea if another decision isn't made by tomorrow.
A prompt would be much better for usability and not adding additional commands.
If the point is to support automation, I'd suggest a [override_ban=False] argument, where when False it just displays
That user is already banned! Use
<command>to override that ban with a temporary ban.
This allows aliass to be configured for bot owners who always want to override, and allow cogs that invoke other commands to more easily hook into this without it being a required part of the command. If the prompt idea is chosen, it should at least support ctx.assume_yes.
I think that's overly complicated and trying to appease everyone for the sake of it. We should definitely support ctx.assume_yes though.
That's already what [p]bankset global does. If a user does not want to override temp bans, it will fail for them and they can ignore the command recommendation. If they want to be prompted to override temp bans, that's a built in prompting system for them to confirm. If they know they will always want to override, they can either remember to pass in the arg (like I do for bankset global) or add an alias with that pre-filled in.
I don't know how that's any more complicated than a similar prompt that always happens, with the one difference between the two options being that in a prompt-only approach, the only way to always get it to override is to make a separate command that calls it with assume_yes. Otherwise, the UX and code complexity is essentially the same unless I'm missing something.
- UX of having to copy the command, paste it and edit it to add a bool value before sending again is worse than being able to just accept the yes/no prompt.
[p]bankset globalis otherwise argumentless which makes it less odd to put argument like that there. In case of tempban, we would have to put it at the beginning or somewhere else before the reason argument which is IMO going to be harder to use and understand in comparison to the same case in bankset. It also could introduce ambiguity between someone passing number to the days argument vs passing it to the new bool. This will be even more confusing to existing users of the command.
I hadn't considered the existing arguments of the commands, that's a valid concern. ~~If only arg parse wasn't confusing to users...~~
Alright, seems like we agree on this after more concerns about the [override_ban=False] approach have been listed here.
To summarize:
[p]tempbanshould allow banning by a user ID usingUnion[discord.Member, RawUserIdConverter]- if using the
[p]tempbancommand would cause a "downgrade" from a permanent ban to a temporary ban, the command should show a prompt asking if the command invoker wants to proceed - the prompt should be skipped (and assumed to be agreed to) if
ctx.assume_yesevaluates toTrue
PRs welcome!