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

[Mod] Enhanced [p]hackban #2164

Merged
merged 7 commits into from
Jan 5, 2019
Merged

[Mod] Enhanced [p]hackban #2164

merged 7 commits into from
Jan 5, 2019

Conversation

Twentysix26
Copy link
Member

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

This PR makes use of the recently introduced Greedy and Optional to enhance the [p]hackban command, allowing multiple user IDs to be passed. It also adds the optional "message deletion days" parameter.

Work in progress and not completely tested yet (especially the modlog's case creation)
Feedback welcome

Kowlin
Kowlin previously requested changes Oct 1, 2018
Copy link
Member

@Kowlin Kowlin left a comment

Choose a reason for hiding this comment

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

Black went on its own way and said that our int's aren't good enough.

@@ -39,7 +68,7 @@ class Mod:

def __init__(self, bot: Red):
self.bot = bot
self.settings = Config.get_conf(self, 4961522000, force_registration=True)
self.settings = Config.get_conf(self, 4_961_522_000, force_registration=True)
Copy link
Member

Choose a reason for hiding this comment

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

Black went a bit nuts here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it does that for readability but it's not really useful for us in this case. I guess we have to tweak it a bit.

special_date = datetime(2016, 1, 10, 6, 8, 4, 443000)
is_special = user.id == 96130341705637888 and guild.id == 133049272517001216
special_date = datetime(2016, 1, 10, 6, 8, 4, 443_000)
is_special = user.id == 96_130_341_705_637_888 and guild.id == 133_049_272_517_001_216
Copy link
Member

Choose a reason for hiding this comment

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

And here too.

if user is not None:
# Instead of replicating all that handling... gets attr from decorator
try:
await ctx.invoke(self.ban, user, days, reason=reason)
Copy link
Member Author

Choose a reason for hiding this comment

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

Edge case still to be fixed here: at the moment if the invoked ban fails, the banning is still successful as far as the hackban command is concerned.

@Tobotimus Tobotimus added Type: Enhancement Something meant to enhance existing Red features. QA: Needed labels Oct 2, 2018
@Kowlin Kowlin dismissed their stale review October 2, 2018 20:51

Outdated review

@Twentysix26
Copy link
Member Author

I think I covered all the edge cases after the last commit. Should be ready to be tested fully.

@Twentysix26 Twentysix26 changed the title [WIP] [Mod] Enhanced [p]hackban [Mod] Enhanced [p]hackban Oct 2, 2018
@Kowlin
Copy link
Member

Kowlin commented Oct 2, 2018

Gonna give it a shot.

@aikaterna
Copy link
Member

If you hackban someone and the mod-log channel is not set to a channel, you get this error:

[05/10/2018 15:28] INFO mod hackban 525: aikaterna(15449707214864****) hackbanned 49561788060847****
[05/10/2018 15:28] ERROR events on_error 191: Exception in modlog_case_create
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/discord/client.py", line 225, in _run_event
    await coro(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/redbot/cogs/mod/mod.py", line 1620, in on_modlog_case_create
    mod_channel = await modlog.get_modlog_channel(case.guild)
  File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/redbot/core/modlog.py", line 694, in get_modlog_channel
    raise RuntimeError("Failed to get the mod log channel!")
RuntimeError: Failed to get the mod log channel!

I'm not sure if this is the case on brand new installs, this was on a bot that had a mod-log channel set and then removed and when that removal occurs the mod-log channel saved in the settings.json is set as "null" for that guild.

@Twentysix26
Copy link
Member Author

@aikaterna Is this related to what this PR introduces? I'm doing the same call (and error handling) as the other commands so if this is a separate problem it might be worth opening a dedicated issue

@Twentysix26
Copy link
Member Author

Twentysix26 commented Oct 14, 2018

I put the whole [p]ban logic into its own function, so that it can be used by both commands. This was necessary to handle the [p]ban invocation from [p]hackban cleanly: before, if any error occurred, it would send a separate message for it. So, for example, if 9 bans out of 10 failed, you would get the [p]hackban's "results" message plus 9 separate messages coming from [p]ban.

@mikeshardmind
Copy link
Contributor

I'd like to take a look at this later, but I'm in favor of anything that de-duplicates shared logic, and moves as much out of command objects as is sane to do (increases ability for things written to be used by other things sanely).

@Twentysix26 Twentysix26 merged commit 937d2fe into V3/develop Jan 5, 2019
@Twentysix26 Twentysix26 deleted the V3/greedy_mod branch January 18, 2019 23:38
@Jackenmen Jackenmen added this to the 3.1.0 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants