Ask for confirmation when banning members with elevated roles #2316
Conversation
Robin5605
requested review from
mbaruh,
Den4200,
ks129 and
jb3
as code owners
Co-authored-by: Preocts <preocts@preocts.com>
There was a problem hiding this comment.
Tested, LGTM. Also, I dedicate this PR in honor of @AbooMinister25
|
The linter error needs fixing |
wookie184
added
a: moderation
ichard26
changed the title
ichard26
added
s: needs review
wookie184
added
s: waiting for author
|
Screenshots of the new behaviour:
|
ichard26
dismissed
ChrisLovering’s stale review
I would like more 👀 on my changes given I am not familiar with Discord.py
hedyhli
added
s: needs review
| await interaction.response.send_message("Cancelled infraction.") | ||
| self.stop() | ||
|
|
||
| async def on_timeout(self) -> None: |
There was a problem hiding this comment.
Good place to use the typing.override decorator
https://docs.python.org/3/library/typing.html#typing.override
There was a problem hiding this comment.
We're not on 3.12 yet, so can't use it yet (we were blocked until very recently waiting for dependencies to update).
| class StaffBanConfirmationView(interactions.ViewWithUserAndRoleCheck): | ||
| """The confirmation view that is sent when a moderator attempts to ban a staff member.""" | ||
|
|
||
| confirmed = False |
There was a problem hiding this comment.
Shouldn't this be an instance attribute? Maybe this is fine if the instance is not re-used, but it's not clear this class's instances are not intended to be re-used nor would we have a good way to enforce that.
There was a problem hiding this comment.
I'm not sure if making it an instance attribute nets us anything as the confirmed flag wouldn't be reset to false either way? But sure, why not.
There was a problem hiding this comment.
I didn't correctly word what I was thinking. If it's a class attribute, then the state is persisted across all instances. This means confirmed will remain True for subsequent command invocations, right?
You're correct that as an instance attribute, the instance cannot be re-used because it's never reset. But I think an instance attribute solves the above case.
There was a problem hiding this comment.
ichard26@asus-ubuntu:~$ cat test.py
class A:
flag = False
a = A()
a2 = A()
a.flag = True
print(a.flag, a2.flag)
ichard26@asus-ubuntu:~$ python test.py
True FalseAs far as I understand, unless the class attribute is directly modified via the class, state won't persist across all instances.
There was a problem hiding this comment.
Okay yeah, that makes sense now that I see it. Thanks for clearing that up - was not intuitive to me at a glance.
|
I'm not really happy with how long this PR and review revisions has taken; I sincerely hope this is close to be able to be merged. (Albeit if you have any more nits, please send them over.) |
Closes #2314
Created a subclass of
botcore.utils.Interactions.ViewWithUserAndRoleCheckthat will be sent when a moderator attempts to ban staff (helper role or above) inbot/exts/moderation/infraction/_utils.py. Only the!bancommand author and moderators are allowed to interact with this view. If cancelled, the view is deleted (NOTE: not the entire message). Similarly, if the timeout (10 seconds) is reached, the view will be deleted (once again, not the entire message). If confirmed, the user will be banned as usual.Waiting for input:
After timeout or cancel:
