From b2367b20e9f73d269224a6dbee20e23e6b6de6b7 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Sun, 22 Nov 2020 12:31:14 +0100 Subject: [PATCH 01/53] rework clean to fully use `delete_messages` instead of `purge` --- bot/exts/utils/clean.py | 86 ++++++++++++++++++++++++++++------------- 1 file changed, 60 insertions(+), 26 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index bf25cb4c24..d6dd2401f2 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -1,9 +1,10 @@ import logging import random import re -from typing import Iterable, Optional +import time +from typing import Dict, Iterable, List, Optional -from discord import Colour, Embed, Message, TextChannel, User +from discord import Colour, Embed, Message, NotFound, TextChannel, User from discord.ext import commands from discord.ext.commands import Cog, Context, group, has_any_role @@ -36,6 +37,14 @@ def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") + async def _delete_messages_individually(self, messages: List[Message]) -> None: + for message in messages: + try: + await message.delete() + except NotFound: + # message doesn't exist or was already deleted + continue + async def _clean_messages( self, amount: int, @@ -107,7 +116,7 @@ def predicate_regex(message: Message) -> bool: elif regex: predicate = predicate_regex # Delete messages that match regex else: - predicate = None # Delete all messages + predicate = lambda m: True # Delete all messages # noqa: E731 # Default to using the invoking context's channel if not channels: @@ -117,19 +126,28 @@ def predicate_regex(message: Message) -> bool: self.mod_log.ignore(Event.message_delete, ctx.message.id) await ctx.message.delete() - messages = [] + # we need Channel to Message mapping for easier deletion via TextChannel.delete_messages + message_mappings: Dict[TextChannel, List[Message]] = {} message_ids = [] self.cleaning = True # Find the IDs of the messages to delete. IDs are needed in order to ignore mod log events. for channel in channels: + + messages = [] + async for message in channel.history(limit=amount): # If at any point the cancel command is invoked, we should stop. if not self.cleaning: return - # If we are looking for specific message. + # If the message passes predicate, let's save it. + if predicate(message): + messages.append(message) + message_ids.append(message) + + # if we are looking for specific message if until_message: # we could use ID's here however in case if the message we are looking for gets deleted, @@ -138,33 +156,49 @@ def predicate_regex(message: Message) -> bool: # means we have found the message until which we were supposed to be deleting. break - # Since we will be using `delete_messages` method of a TextChannel and we need message objects to - # use it as well as to send logs we will start appending messages here instead adding them from - # purge. - messages.append(message) - - # If the message passes predicate, let's save it. - if predicate is None or predicate(message): - message_ids.append(message.id) + if len(messages) > 0: + # we don't want to create mappings of TextChannel to empty list + message_mappings[channel] = messages self.cleaning = False # Now let's delete the actual messages with purge. self.mod_log.ignore(Event.message_delete, *message_ids) - for channel in channels: - if until_message: - for i in range(0, len(messages), 100): - # while purge automatically handles the amount of messages - # delete_messages only allows for up to 100 messages at once - # thus we need to paginate the amount to always be <= 100 - await channel.delete_messages(messages[i:i + 100]) - else: - messages += await channel.purge(limit=amount, check=predicate) - # Reverse the list to restore chronological order - if messages: - messages = reversed(messages) - log_url = await self.mod_log.upload_log(messages, ctx.author.id) + # Creates ID like int object that would represent an object that is exactly 14 days old + minimum_time = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 + + for channel, messages in message_mappings.items(): + + to_delete = [] + + for current_index, message in enumerate(messages): + + if message.id < minimum_time: + # further messages are too old to be deleted in bulk + await self._delete_messages_individually(messages[current_index:]) + break + + to_delete.append(message) + + if len(to_delete) == 100: + # we can only delete up to 100 messages in a bulk + await channel.delete_messages(to_delete) + to_delete.clear() + + if len(to_delete) > 0: + # deleting any leftover messages if there are any + await channel.delete_messages(to_delete) + + log_messages = [] + + for messages in message_mappings.values(): + log_messages.extend(messages) + + if log_messages: + # Reverse the list to restore chronological order + log_messages = reversed(log_messages) + log_url = await self.mod_log.upload_log(log_messages, ctx.author.id) else: # Can't build an embed, nothing to clean! embed = Embed( From 7bcc74fb600c8e64fa136d8273fea6fbb3834339 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Sun, 22 Nov 2020 12:32:08 +0100 Subject: [PATCH 02/53] rename command `messages` to `until` new name should be more selfexplanatory --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index d6dd2401f2..7ee0287fdd 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -277,9 +277,9 @@ async def clean_regex( """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" await self._clean_messages(amount, ctx, regex=regex, channels=channels) - @clean_group.command(name="message", aliases=["messages"]) + @clean_group.command(name="until") @has_any_role(*MODERATION_ROLES) - async def clean_message(self, ctx: Context, message: Message) -> None: + async def clean_until(self, ctx: Context, message: Message) -> None: """Delete all messages until certain message, stop cleaning after hitting the `message`.""" await self._clean_messages( CleanMessages.message_limit, From 3797474cabac3fae94a381c0e00998d563efdc5a Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Tue, 23 Feb 2021 14:50:35 +0100 Subject: [PATCH 03/53] Introduce cache to cleaning as well as fix cancel --- bot/exts/utils/clean.py | 125 ++++++++++++++++++++++++++++------------ 1 file changed, 89 insertions(+), 36 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 7ee0287fdd..6301ade04d 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -2,7 +2,8 @@ import random import re import time -from typing import Dict, Iterable, List, Optional +from collections import defaultdict +from typing import Callable, DefaultDict, Iterable, List, Optional from discord import Colour, Embed, Message, NotFound, TextChannel, User from discord.ext import commands @@ -16,6 +17,9 @@ log = logging.getLogger(__name__) +# Type alias for checks +CheckHint = Callable[[Message], bool] + class Clean(Cog): """ @@ -39,18 +43,74 @@ def mod_log(self) -> ModLog: async def _delete_messages_individually(self, messages: List[Message]) -> None: for message in messages: + # Ensure that deletion was not canceled + if not self.cleaning: + return try: await message.delete() except NotFound: - # message doesn't exist or was already deleted + # Message doesn't exist or was already deleted continue + def _get_messages_from_cache(self, amount: int, check: CheckHint) -> List[DefaultDict, List[int]]: + """Helper function for getting messages from the cache.""" + message_mappings = defaultdict(lambda: []) + message_ids = [] + for message in self.bot.cached_messages: + if not self.cleaning: + # Cleaning was canceled + return (message_mappings, message_ids) + + if check(message): + message_mappings[message.channel].append(message) + message_ids.append(message.id) + + if len(message_ids) == amount: + # We've got the requested amount of messages + return message_mappings, message_ids + + # Amount exceeds amount of messages matching the check + return message_mappings, message_ids + + async def _get_messages_from_channels( + self, + amount: int, + channels: Iterable[TextChannel], + check: CheckHint, + until_message: Optional[Message] = None + ) -> DefaultDict: + message_mappings = defaultdict(lambda: []) + message_ids = [] + + for channel in channels: + + async for message in channel.history(amount=amount): + + if not self.cleaning: + # Cleaning was canceled + return (message_mappings, message_ids) + + if check(message): + message_mappings[message.channel].append(message) + message_ids.append(message.id) + + if until_message: + + # We could use ID's here however in case if the message we are looking for gets deleted, + # We won't have a way to figure that out thus checking for datetime should be more reliable + if message.created_at < until_message.created_at: + # Means we have found the message until which we were supposed to be deleting. + break + + return message_mappings, message_ids + async def _clean_messages( self, amount: int, ctx: Context, channels: Iterable[TextChannel], bots_only: bool = False, + use_cache: bool = False, user: User = None, regex: Optional[str] = None, until_message: Optional[Message] = None, @@ -126,41 +186,21 @@ def predicate_regex(message: Message) -> bool: self.mod_log.ignore(Event.message_delete, ctx.message.id) await ctx.message.delete() - # we need Channel to Message mapping for easier deletion via TextChannel.delete_messages - message_mappings: Dict[TextChannel, List[Message]] = {} - message_ids = [] self.cleaning = True - # Find the IDs of the messages to delete. IDs are needed in order to ignore mod log events. - for channel in channels: - - messages = [] - - async for message in channel.history(limit=amount): - - # If at any point the cancel command is invoked, we should stop. - if not self.cleaning: - return - - # If the message passes predicate, let's save it. - if predicate(message): - messages.append(message) - message_ids.append(message) - - # if we are looking for specific message - if until_message: - - # we could use ID's here however in case if the message we are looking for gets deleted, - # we won't have a way to figure that out thus checking for datetime should be more reliable - if message.created_at < until_message.created_at: - # means we have found the message until which we were supposed to be deleting. - break - - if len(messages) > 0: - # we don't want to create mappings of TextChannel to empty list - message_mappings[channel] = messages + if use_cache: + message_mappings, message_ids = self._get_messages_from_cache(amount, predicate) + else: + message_mappings, message_ids = await self._get_messages_from_channels( + amount=amount, + channels=channels, + check=predicate, + until_message=until_message + ) - self.cleaning = False + if not self.cleaning: + # Means that the cleaning was canceled + return # Now let's delete the actual messages with purge. self.mod_log.ignore(Event.message_delete, *message_ids) @@ -174,9 +214,16 @@ def predicate_regex(message: Message) -> bool: for current_index, message in enumerate(messages): + if not self.cleaning: + # Means that the cleaning was canceled + return + if message.id < minimum_time: # further messages are too old to be deleted in bulk await self._delete_messages_individually(messages[current_index:]) + if not self.cleaning: + # Means that deletion was canceled while deleting the individual messages + return break to_delete.append(message) @@ -241,7 +288,10 @@ async def clean_user( channels: commands.Greedy[TextChannel] = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, user=user, channels=channels) + use_cache = True + if channels: + use_cache = False + await self._clean_messages(amount, ctx, user=user, channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) @has_any_role(*MODERATION_ROLES) @@ -275,7 +325,10 @@ async def clean_regex( channels: commands.Greedy[TextChannel] = None ) -> None: """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, regex=regex, channels=channels) + use_cache = True + if channels: + use_cache = False + await self._clean_messages(amount, ctx, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") @has_any_role(*MODERATION_ROLES) From 0a7c7283af42e2b2062a4a555781b24508c6ad38 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Tue, 23 Feb 2021 18:48:54 +0100 Subject: [PATCH 04/53] Implement range clean command --- bot/exts/utils/clean.py | 63 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 6301ade04d..0788eed1d1 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -52,7 +52,7 @@ async def _delete_messages_individually(self, messages: List[Message]) -> None: # Message doesn't exist or was already deleted continue - def _get_messages_from_cache(self, amount: int, check: CheckHint) -> List[DefaultDict, List[int]]: + def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> List[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(lambda: []) message_ids = [] @@ -61,7 +61,7 @@ def _get_messages_from_cache(self, amount: int, check: CheckHint) -> List[Defaul # Cleaning was canceled return (message_mappings, message_ids) - if check(message): + if predicate(message): message_mappings[message.channel].append(message) message_ids.append(message.id) @@ -76,7 +76,7 @@ async def _get_messages_from_channels( self, amount: int, channels: Iterable[TextChannel], - check: CheckHint, + predicate: CheckHint, until_message: Optional[Message] = None ) -> DefaultDict: message_mappings = defaultdict(lambda: []) @@ -90,7 +90,7 @@ async def _get_messages_from_channels( # Cleaning was canceled return (message_mappings, message_ids) - if check(message): + if predicate(message): message_mappings[message.channel].append(message) message_ids.append(message.id) @@ -114,6 +114,7 @@ async def _clean_messages( user: User = None, regex: Optional[str] = None, until_message: Optional[Message] = None, + after_message: Optional[Message] = None, ) -> None: """A helper function that does the actual message cleaning.""" def predicate_bots_only(message: Message) -> bool: @@ -148,6 +149,10 @@ def predicate_regex(message: Message) -> bool: else: return bool(re.search(regex.lower(), content.lower())) + def predicate_range(message: Message) -> bool: + """Check if message is older than message provided in after_message but younger than until_message.""" + return message.created_at > after_message.created_at and message.created_at < until_message.created_at + # Is this an acceptable amount of messages to clean? if amount > CleanMessages.message_limit: embed = Embed( @@ -158,6 +163,38 @@ def predicate_regex(message: Message) -> bool: await ctx.send(embed=embed) return + if after_message: + + # Ensure that until_message is specified. + if not until_message: + embed = Embed( + color=Colour(Colours.soft_red), + title=random.choice(NEGATIVE_REPLIES), + description="`until_message` must be specified if `after_message` is specified." + ) + await ctx.send(embed=embed) + return + + # Check if the messages are not in same channel + if after_message.channel != until_message.channel: + embed = Embed( + color=Colour(Colours.soft_red), + title=random.choice(NEGATIVE_REPLIES), + description="You cannot do range clean across different channel." + ) + await ctx.send(embed=embed) + return + + # Ensure that after_message is younger than until_message + if after_message.created_at >= until_message.created_at: + embed = Embed( + color=Colour(Colours.soft_red), + title=random.choice(NEGATIVE_REPLIES), + description="`after` message must be younger than `until` message" + ) + await ctx.send(embed=embed) + return + # Are we already performing a clean? if self.cleaning: embed = Embed( @@ -175,6 +212,8 @@ def predicate_regex(message: Message) -> bool: predicate = predicate_specific_user # Delete messages from specific user elif regex: predicate = predicate_regex # Delete messages that match regex + elif after_message: + predicate = predicate_range # Delete messages older than specific message else: predicate = lambda m: True # Delete all messages # noqa: E731 @@ -189,12 +228,12 @@ def predicate_regex(message: Message) -> bool: self.cleaning = True if use_cache: - message_mappings, message_ids = self._get_messages_from_cache(amount, predicate) + message_mappings, message_ids = self._get_messages_from_cache(amount=amount, predicate=predicate) else: message_mappings, message_ids = await self._get_messages_from_channels( amount=amount, channels=channels, - check=predicate, + predicate=predicate, until_message=until_message ) @@ -341,6 +380,18 @@ async def clean_until(self, ctx: Context, message: Message) -> None: until_message=message ) + @clean_group.command(name="from-to", aliases=["after-until", "range"]) + @has_any_role(*MODERATION_ROLES) + async def clean_from_to(self, ctx: Context, after_message: Message, until_message: Message) -> None: + """Delete all messages within range of messages.""" + await self._clean_messages( + CleanMessages.message_limit, + ctx, + channels=[until_message.channel], + until_message=until_message, + after_message=after_message, + ) + @clean_group.command(name="stop", aliases=["cancel", "abort"]) @has_any_role(*MODERATION_ROLES) async def clean_cancel(self, ctx: Context) -> None: From bb26ed30a27a1fc5951059ceed064422210df91a Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Wed, 24 Feb 2021 06:35:43 +0100 Subject: [PATCH 05/53] set `self.cleaning` to False once done cleaning --- bot/exts/utils/clean.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 0788eed1d1..b572f70a76 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -276,6 +276,8 @@ def predicate_range(message: Message) -> bool: # deleting any leftover messages if there are any await channel.delete_messages(to_delete) + self.cleaning = False + log_messages = [] for messages in message_mappings.values(): From 26c60c13219cdc3db80480016f61cdf90db1a187 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Thu, 4 Mar 2021 10:26:08 +0100 Subject: [PATCH 06/53] Change typing, remove `range` alias --- bot/exts/utils/clean.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index f98e5c255f..925d424831 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -3,7 +3,7 @@ import re import time from collections import defaultdict -from typing import Callable, DefaultDict, Iterable, List, Optional +from typing import Callable, DefaultDict, Iterable, List, Optional, Tuple from discord import Colour, Embed, Message, NotFound, TextChannel, User from discord.ext import commands @@ -52,7 +52,7 @@ async def _delete_messages_individually(self, messages: List[Message]) -> None: # Message doesn't exist or was already deleted continue - def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> List[DefaultDict, List[int]]: + def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(lambda: []) message_ids = [] @@ -382,7 +382,7 @@ async def clean_until(self, ctx: Context, message: Message) -> None: until_message=message ) - @clean_group.command(name="from-to", aliases=["after-until", "range"]) + @clean_group.command(name="from-to", aliases=["after-until"]) @has_any_role(*MODERATION_ROLES) async def clean_from_to(self, ctx: Context, after_message: Message, until_message: Message) -> None: """Delete all messages within range of messages.""" From 7e74bb3608af5f1f96216db9472ccf5960c9124e Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Thu, 15 Apr 2021 18:44:49 +0200 Subject: [PATCH 07/53] swap predicate save order for correct deletion --- bot/exts/utils/clean.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 925d424831..e080f7caa0 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -90,10 +90,6 @@ async def _get_messages_from_channels( # Cleaning was canceled return (message_mappings, message_ids) - if predicate(message): - message_mappings[message.channel].append(message) - message_ids.append(message.id) - if until_message: # We could use ID's here however in case if the message we are looking for gets deleted, @@ -102,6 +98,10 @@ async def _get_messages_from_channels( # Means we have found the message until which we were supposed to be deleting. break + if predicate(message): + message_mappings[message.channel].append(message) + message_ids.append(message.id) + return message_mappings, message_ids async def _clean_messages( From 11be1666fce272d13e72467563fde53eae0f8419 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:09:36 +0200 Subject: [PATCH 08/53] replace lambda with list in defaultdict --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 117d636324..be92c4994e 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -54,7 +54,7 @@ async def _delete_messages_individually(self, messages: List[Message]) -> None: def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" - message_mappings = defaultdict(lambda: []) + message_mappings = defaultdict(list) message_ids = [] for message in self.bot.cached_messages: if not self.cleaning: @@ -79,7 +79,7 @@ async def _get_messages_from_channels( predicate: CheckHint, until_message: Optional[Message] = None ) -> DefaultDict: - message_mappings = defaultdict(lambda: []) + message_mappings = defaultdict(list) message_ids = [] for channel in channels: From 5bf42c7d3c4927dae2a130a95d83295db746338d Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:10:48 +0200 Subject: [PATCH 09/53] Use correct kwarg for channel.history --- bot/exts/utils/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index be92c4994e..d0abd67844 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -84,7 +84,7 @@ async def _get_messages_from_channels( for channel in channels: - async for message in channel.history(amount=amount): + async for message in channel.history(limit=amount): if not self.cleaning: # Cleaning was canceled From add078121aba4630fef86a44b261211de89f4f95 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:13:38 +0200 Subject: [PATCH 10/53] simplify use_cache var --- bot/exts/utils/clean.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index d0abd67844..e41164edcb 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -333,9 +333,7 @@ async def clean_user( channels: commands.Greedy[TextChannel] = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `amount` messages.""" - use_cache = True - if channels: - use_cache = False + use_cache = not channels await self._clean_messages(amount, ctx, user=user, channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) @@ -370,9 +368,7 @@ async def clean_regex( channels: commands.Greedy[TextChannel] = None ) -> None: """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" - use_cache = True - if channels: - use_cache = False + use_cache = not channels await self._clean_messages(amount, ctx, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") From 477810f86387c85ae6da0b80ffccb40dac60e26e Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:16:17 +0200 Subject: [PATCH 11/53] Naming changes for better self documentation --- bot/exts/utils/clean.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index e41164edcb..62d9f2dbe6 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -18,7 +18,7 @@ log = logging.getLogger(__name__) # Type alias for checks -CheckHint = Callable[[Message], bool] +Predicate = Callable[[Message], bool] class Clean(Cog): @@ -52,7 +52,7 @@ async def _delete_messages_individually(self, messages: List[Message]) -> None: # Message doesn't exist or was already deleted continue - def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> Tuple[DefaultDict, List[int]]: + def _get_messages_from_cache(self, amount: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) message_ids = [] @@ -61,7 +61,7 @@ def _get_messages_from_cache(self, amount: int, predicate: CheckHint) -> Tuple[D # Cleaning was canceled return (message_mappings, message_ids) - if predicate(message): + if to_delete(message): message_mappings[message.channel].append(message) message_ids.append(message.id) @@ -76,7 +76,7 @@ async def _get_messages_from_channels( self, amount: int, channels: Iterable[TextChannel], - predicate: CheckHint, + to_delete: Predicate, until_message: Optional[Message] = None ) -> DefaultDict: message_mappings = defaultdict(list) @@ -98,7 +98,7 @@ async def _get_messages_from_channels( # Means we have found the message until which we were supposed to be deleting. break - if predicate(message): + if to_delete(message): message_mappings[message.channel].append(message) message_ids.append(message.id) From 708063c70bcea2cdeecb1eb66c703817e0195042 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:17:00 +0200 Subject: [PATCH 12/53] make predicate_range inclusive --- bot/exts/utils/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 62d9f2dbe6..af94056960 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -151,7 +151,7 @@ def predicate_regex(message: Message) -> bool: def predicate_range(message: Message) -> bool: """Check if message is older than message provided in after_message but younger than until_message.""" - return message.created_at > after_message.created_at and message.created_at < until_message.created_at + return message.created_at >= after_message.created_at and message.created_at <= until_message.created_at # Is this an acceptable amount of messages to clean? if amount > CleanMessages.message_limit: From c2705492ec13984aa75f4c31532ca18b1756121d Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 07:18:48 +0200 Subject: [PATCH 13/53] Better response wording, added alias --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index af94056960..985025afe6 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -180,7 +180,7 @@ def predicate_range(message: Message) -> bool: embed = Embed( color=Colour(Colours.soft_red), title=random.choice(NEGATIVE_REPLIES), - description="You cannot do range clean across different channel." + description="You cannot do range clean across several channel." ) await ctx.send(embed=embed) return @@ -382,7 +382,7 @@ async def clean_until(self, ctx: Context, message: Message) -> None: until_message=message ) - @clean_group.command(name="from-to", aliases=["after-until"]) + @clean_group.command(name="from-to", aliases=["after-until", "between"]) @has_any_role(*MODERATION_ROLES) async def clean_from_to(self, ctx: Context, after_message: Message, until_message: Message) -> None: """Delete all messages within range of messages.""" From 6c9e4f55a26f9903532a9b72f69fcab84c1a3370 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 11:41:33 +0200 Subject: [PATCH 14/53] Don't delete invocation in mod channel --- bot/exts/utils/clean.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 985025afe6..d9164738a1 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -14,6 +14,7 @@ Channels, CleanMessages, Colours, Event, Icons, MODERATION_ROLES, NEGATIVE_REPLIES ) from bot.exts.moderation.modlog import ModLog +from bot.utils.channel import is_mod_channel log = logging.getLogger(__name__) @@ -221,13 +222,15 @@ def predicate_range(message: Message) -> bool: if not channels: channels = [ctx.channel] - # Delete the invocation first - self.mod_log.ignore(Event.message_delete, ctx.message.id) - try: - await ctx.message.delete() - except errors.NotFound: - # Invocation message has already been deleted - log.info("Tried to delete invocation message, but it was already deleted.") + if not is_mod_channel(ctx.channel): + + # Delete the invocation first + self.mod_log.ignore(Event.message_delete, ctx.message.id) + try: + await ctx.message.delete() + except errors.NotFound: + # Invocation message has already been deleted + log.info("Tried to delete invocation message, but it was already deleted.") self.cleaning = True From e5e4343435c31f53e4d58cf8cbd180bfccd94023 Mon Sep 17 00:00:00 2001 From: Senjan21 <53477086+Senjan21@users.noreply.github.com> Date: Fri, 16 Apr 2021 13:03:13 +0200 Subject: [PATCH 15/53] document snowflake check better --- bot/exts/utils/clean.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index d9164738a1..e08be79fe8 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -105,6 +105,17 @@ async def _get_messages_from_channels( return message_mappings, message_ids + def is_older_than_14d(self, message: Message) -> bool: + """ + Precisely checks if message is older than 14 days, bulk deletion limit. + + Inspired by how purge works internally. + Comparison on message age could possibly be less accurate which in turn would resort in problems + with message deletion if said messages are very close to the 14d mark. + """ + two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 + return message.id < two_weeks_old_snowflake + async def _clean_messages( self, amount: int, @@ -251,9 +262,6 @@ def predicate_range(message: Message) -> bool: # Now let's delete the actual messages with purge. self.mod_log.ignore(Event.message_delete, *message_ids) - # Creates ID like int object that would represent an object that is exactly 14 days old - minimum_time = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 - for channel, messages in message_mappings.items(): to_delete = [] @@ -264,7 +272,7 @@ def predicate_range(message: Message) -> bool: # Means that the cleaning was canceled return - if message.id < minimum_time: + if self.is_older_than_14d(message): # further messages are too old to be deleted in bulk await self._delete_messages_individually(messages[current_index:]) if not self.cleaning: From 12f3c40954db931300ea606fd03d329f16395f19 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 26 Jul 2021 15:27:44 +0100 Subject: [PATCH 16/53] Update _get_messages_from_channels return type --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index e08be79fe8..529dd9ee64 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -3,7 +3,7 @@ import re import time from collections import defaultdict -from typing import Callable, DefaultDict, Iterable, List, Optional, Tuple +from typing import Any, Callable, DefaultDict, Iterable, List, Optional, Tuple from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors from discord.ext import commands @@ -79,7 +79,7 @@ async def _get_messages_from_channels( channels: Iterable[TextChannel], to_delete: Predicate, until_message: Optional[Message] = None - ) -> DefaultDict: + ) -> tuple[defaultdict[Any, list], list]: message_mappings = defaultdict(list) message_ids = [] From 02b3c8af0268239050d52db0becd856bc8ab9863 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 26 Jul 2021 15:31:55 +0100 Subject: [PATCH 17/53] Make is_older_than_14d a static method --- bot/exts/utils/clean.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 529dd9ee64..a1a9eafe49 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -42,6 +42,18 @@ def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") + @staticmethod + def is_older_than_14d(message: Message) -> bool: + """ + Precisely checks if message is older than 14 days, bulk deletion limit. + + Inspired by how purge works internally. + Comparison on message age could possibly be less accurate which in turn would resort in problems + with message deletion if said messages are very close to the 14d mark. + """ + two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 + return message.id < two_weeks_old_snowflake + async def _delete_messages_individually(self, messages: List[Message]) -> None: for message in messages: # Ensure that deletion was not canceled @@ -105,17 +117,6 @@ async def _get_messages_from_channels( return message_mappings, message_ids - def is_older_than_14d(self, message: Message) -> bool: - """ - Precisely checks if message is older than 14 days, bulk deletion limit. - - Inspired by how purge works internally. - Comparison on message age could possibly be less accurate which in turn would resort in problems - with message deletion if said messages are very close to the 14d mark. - """ - two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 - return message.id < two_weeks_old_snowflake - async def _clean_messages( self, amount: int, From 0cc135de21c8fe8a85b2c42b95b04779b3af7baa Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 28 Jul 2021 17:21:36 +0100 Subject: [PATCH 18/53] Return empty containers if clean is cancelled --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index a1a9eafe49..3aabe42f7f 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -100,8 +100,8 @@ async def _get_messages_from_channels( async for message in channel.history(limit=amount): if not self.cleaning: - # Cleaning was canceled - return (message_mappings, message_ids) + # Cleaning was canceled, return empty containers + return defaultdict(list), [] if until_message: From 770528c70ff38b739c963c88b89ec6401d687d16 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 28 Jul 2021 17:22:24 +0100 Subject: [PATCH 19/53] simplify range predicate for clean command --- bot/exts/utils/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 3aabe42f7f..847ac5c860 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -164,7 +164,7 @@ def predicate_regex(message: Message) -> bool: def predicate_range(message: Message) -> bool: """Check if message is older than message provided in after_message but younger than until_message.""" - return message.created_at >= after_message.created_at and message.created_at <= until_message.created_at + return after_message.created_at <= message.created_at <= until_message.created_at # Is this an acceptable amount of messages to clean? if amount > CleanMessages.message_limit: From ed352272a67224182178bbd5583746053ec912a6 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 28 Jul 2021 17:59:04 +0100 Subject: [PATCH 20/53] Rely on error handler for sending input errors to user --- bot/exts/utils/clean.py | 54 +++++++---------------------------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 847ac5c860..7514c7a643 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -1,5 +1,4 @@ import logging -import random import re import time from collections import defaultdict @@ -8,10 +7,11 @@ from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors from discord.ext import commands from discord.ext.commands import Cog, Context, group, has_any_role +from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached, MissingRequiredArgument from bot.bot import Bot from bot.constants import ( - Channels, CleanMessages, Colours, Event, Icons, MODERATION_ROLES, NEGATIVE_REPLIES + Channels, CleanMessages, Colours, Event, Icons, MODERATION_ROLES ) from bot.exts.moderation.modlog import ModLog from bot.utils.channel import is_mod_channel @@ -168,55 +168,24 @@ def predicate_range(message: Message) -> bool: # Is this an acceptable amount of messages to clean? if amount > CleanMessages.message_limit: - embed = Embed( - color=Colour(Colours.soft_red), - title=random.choice(NEGATIVE_REPLIES), - description=f"You cannot clean more than {CleanMessages.message_limit} messages." - ) - await ctx.send(embed=embed) - return + raise BadArgument(f"You cannot clean more than {CleanMessages.message_limit} messages.") if after_message: - # Ensure that until_message is specified. if not until_message: - embed = Embed( - color=Colour(Colours.soft_red), - title=random.choice(NEGATIVE_REPLIES), - description="`until_message` must be specified if `after_message` is specified." - ) - await ctx.send(embed=embed) - return + raise MissingRequiredArgument("`until_message` must be specified if `after_message` is specified.") - # Check if the messages are not in same channel + # Messages are not in same channel if after_message.channel != until_message.channel: - embed = Embed( - color=Colour(Colours.soft_red), - title=random.choice(NEGATIVE_REPLIES), - description="You cannot do range clean across several channel." - ) - await ctx.send(embed=embed) - return + raise BadArgument("You cannot do range clean across several channel.") # Ensure that after_message is younger than until_message if after_message.created_at >= until_message.created_at: - embed = Embed( - color=Colour(Colours.soft_red), - title=random.choice(NEGATIVE_REPLIES), - description="`after` message must be younger than `until` message" - ) - await ctx.send(embed=embed) - return + raise BadArgument("`after` message must be younger than `until` message") # Are we already performing a clean? if self.cleaning: - embed = Embed( - color=Colour(Colours.soft_red), - title=random.choice(NEGATIVE_REPLIES), - description="Please wait for the currently ongoing clean operation to complete." - ) - await ctx.send(embed=embed) - return + raise MaxConcurrencyReached("Please wait for the currently ongoing clean operation to complete.") # Set up the correct predicate if bots_only: @@ -305,12 +274,7 @@ def predicate_range(message: Message) -> bool: log_url = await self.mod_log.upload_log(log_messages, ctx.author.id) else: # Can't build an embed, nothing to clean! - embed = Embed( - color=Colour(Colours.soft_red), - description="No matching messages could be found." - ) - await ctx.send(embed=embed, delete_after=10) - return + raise BadArgument("No matching messages could be found.") # Build the embed and send it target_channels = ", ".join(channel.mention for channel in channels) From b2e76ddc6f4d3ccd327f48d9333eb977ddfb72d2 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 28 Jul 2021 18:04:13 +0100 Subject: [PATCH 21/53] Fix references to kwarg after renaming in clean command --- bot/exts/utils/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/utils/clean.py b/bot/exts/utils/clean.py index 7514c7a643..25582165a0 100644 --- a/bot/exts/utils/clean.py +++ b/bot/exts/utils/clean.py @@ -216,12 +216,12 @@ def predicate_range(message: Message) -> bool: self.cleaning = True if use_cache: - message_mappings, message_ids = self._get_messages_from_cache(amount=amount, predicate=predicate) + message_mappings, message_ids = self._get_messages_from_cache(amount=amount, to_delete=predicate) else: message_mappings, message_ids = await self._get_messages_from_channels( amount=amount, channels=channels, - predicate=predicate, + to_delete=predicate, until_message=until_message ) From 97aa87a2d3d262cfa06e922fa93889cc26b7c2cb Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 14:49:29 +0300 Subject: [PATCH 22/53] Moved clean cog to moderation ext The cog is moderation related and all commands are exclusive to moderators. --- bot/exts/{utils => moderation}/clean.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename bot/exts/{utils => moderation}/clean.py (100%) diff --git a/bot/exts/utils/clean.py b/bot/exts/moderation/clean.py similarity index 100% rename from bot/exts/utils/clean.py rename to bot/exts/moderation/clean.py From 2b5a5311110f52328651e5a19a186f4e3552ee84 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 17:21:18 +0300 Subject: [PATCH 23/53] Move clean logging to a helper function --- bot/exts/moderation/clean.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 25582165a0..e198dde9cd 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -2,6 +2,7 @@ import re import time from collections import defaultdict +from itertools import chain from typing import Any, Callable, DefaultDict, Iterable, List, Optional, Tuple from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors @@ -263,25 +264,24 @@ def predicate_range(message: Message) -> bool: self.cleaning = False - log_messages = [] + await self._log_clean(list(chain.from_iterable(message_mappings.values())), channels, ctx.author) - for messages in message_mappings.values(): - log_messages.extend(messages) - - if log_messages: - # Reverse the list to restore chronological order - log_messages = reversed(log_messages) - log_url = await self.mod_log.upload_log(log_messages, ctx.author.id) - else: + async def _log_clean(self, messages: list[Message], channels: Iterable[TextChannel], invoker: User) -> None: + """Log the deleted messages to the modlog.""" + if not messages: # Can't build an embed, nothing to clean! raise BadArgument("No matching messages could be found.") + # Reverse the list to restore chronological order + log_messages = reversed(messages) + log_url = await self.mod_log.upload_log(log_messages, invoker.id) + # Build the embed and send it target_channels = ", ".join(channel.mention for channel in channels) message = ( - f"**{len(message_ids)}** messages deleted in {target_channels} by " - f"{ctx.author.mention}\n\n" + f"**{len(messages)}** messages deleted in {target_channels} by " + f"{invoker.mention}\n\n" f"A log of the deleted messages can be found [here]({log_url})." ) From 841a148f45bfe265815eac1aa9f22d84e332f548 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 17:31:54 +0300 Subject: [PATCH 24/53] Move setting cleaning flag to correct line Between the concurrency check and setting the cleaning flag to True there was an await statement, which could potentially cause race conditions.The setting of the flag was moved to right below the concurrency check. --- bot/exts/moderation/clean.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index e198dde9cd..2e3f9ac77a 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -187,6 +187,7 @@ def predicate_range(message: Message) -> bool: # Are we already performing a clean? if self.cleaning: raise MaxConcurrencyReached("Please wait for the currently ongoing clean operation to complete.") + self.cleaning = True # Set up the correct predicate if bots_only: @@ -214,8 +215,6 @@ def predicate_range(message: Message) -> bool: # Invocation message has already been deleted log.info("Tried to delete invocation message, but it was already deleted.") - self.cleaning = True - if use_cache: message_mappings, message_ids = self._get_messages_from_cache(amount=amount, to_delete=predicate) else: From 8aeec5fc96bc5dcb8db1dbdbad870ade55ef5540 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 17:33:07 +0300 Subject: [PATCH 25/53] Correct logging comment --- bot/exts/moderation/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 2e3f9ac77a..19f64e0e7f 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -271,7 +271,7 @@ async def _log_clean(self, messages: list[Message], channels: Iterable[TextChann # Can't build an embed, nothing to clean! raise BadArgument("No matching messages could be found.") - # Reverse the list to restore chronological order + # Reverse the list to have reverse chronological order log_messages = reversed(messages) log_url = await self.mod_log.upload_log(log_messages, invoker.id) From 33e05017bfb0530a736fe1473f5e2b3c275f18f0 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 17:37:09 +0300 Subject: [PATCH 26/53] Change `from-to` primary name to `between` --- bot/exts/moderation/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 19f64e0e7f..007aba317e 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -357,9 +357,9 @@ async def clean_until(self, ctx: Context, message: Message) -> None: until_message=message ) - @clean_group.command(name="from-to", aliases=["after-until", "between"]) + @clean_group.command(name="between", aliases=["after-until", "from-to"]) @has_any_role(*MODERATION_ROLES) - async def clean_from_to(self, ctx: Context, after_message: Message, until_message: Message) -> None: + async def clean_between(self, ctx: Context, after_message: Message, until_message: Message) -> None: """Delete all messages within range of messages.""" await self._clean_messages( CleanMessages.message_limit, From f9d2e6919ab746b046c510daab2133e4b53bda6d Mon Sep 17 00:00:00 2001 From: mbaruh Date: Fri, 27 Aug 2021 17:52:13 +0300 Subject: [PATCH 27/53] Don't delete clean cancel embed in mod channel --- bot/exts/moderation/clean.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 007aba317e..504ecccd19 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -379,7 +379,10 @@ async def clean_cancel(self, ctx: Context) -> None: color=Colour.blurple(), description="Clean interrupted." ) - await ctx.send(embed=embed, delete_after=10) + delete_after = 10 + if is_mod_channel(ctx.channel): + delete_after = None + await ctx.send(embed=embed, delete_after=delete_after) def setup(bot: Bot) -> None: From d81b550594d02f25fcc310eb22cda5bd930fd197 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 28 Aug 2021 17:12:34 +0300 Subject: [PATCH 28/53] Change cache usage The cache is used only when all channels are used, as before. Unlike before, using all channels requires using "*" in the channels argument. Before all channels would be used if use_cache was set to True. Using all channels uses the cache by default. To traverse every single text channel in the server, setting use_cache to False is required in the command. --- bot/exts/moderation/clean.py | 64 ++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 504ecccd19..15a48ea757 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -3,11 +3,11 @@ import time from collections import defaultdict from itertools import chain -from typing import Any, Callable, DefaultDict, Iterable, List, Optional, Tuple +from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors -from discord.ext import commands -from discord.ext.commands import Cog, Context, group, has_any_role +from discord.ext.commands import Cog, Context, Converter, group, has_any_role +from discord.ext.commands.converter import TextChannelConverter from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached, MissingRequiredArgument from bot.bot import Bot @@ -23,6 +23,22 @@ Predicate = Callable[[Message], bool] +class CleanChannels(Converter): + """A converter that turns the given string to a list of channels to clean, or the literal `*` for all channels.""" + + _channel_converter = TextChannelConverter() + + async def convert(self, ctx: Context, argument: str) -> Union[Literal["*"], list[TextChannel]]: + """Converts a string to a list of channels to clean, or the literal `*` for all channels.""" + if argument == "*": + return "*" + return [await self._channel_converter.convert(ctx, channel) for channel in argument.split()] + + +if TYPE_CHECKING: + CleanChannels = Union[Literal["*"], list[TextChannel]] # noqa: F811 + + class Clean(Cog): """ A cog that allows messages to be deleted in bulk, while applying various filters. @@ -122,13 +138,13 @@ async def _clean_messages( self, amount: int, ctx: Context, - channels: Iterable[TextChannel], + channels: CleanChannels, bots_only: bool = False, - use_cache: bool = False, user: User = None, regex: Optional[str] = None, until_message: Optional[Message] = None, after_message: Optional[Message] = None, + use_cache: Optional[bool] = True ) -> None: """A helper function that does the actual message cleaning.""" def predicate_bots_only(message: Message) -> bool: @@ -215,12 +231,15 @@ def predicate_range(message: Message) -> bool: # Invocation message has already been deleted log.info("Tried to delete invocation message, but it was already deleted.") - if use_cache: + if channels == "*" and use_cache: message_mappings, message_ids = self._get_messages_from_cache(amount=amount, to_delete=predicate) else: + deletion_channels = channels + if channels == "*": + deletion_channels = [channel for channel in ctx.guild.channels if isinstance(channel, TextChannel)] message_mappings, message_ids = await self._get_messages_from_channels( amount=amount, - channels=channels, + channels=deletion_channels, to_delete=predicate, until_message=until_message ) @@ -265,7 +284,7 @@ def predicate_range(message: Message) -> bool: await self._log_clean(list(chain.from_iterable(message_mappings.values())), channels, ctx.author) - async def _log_clean(self, messages: list[Message], channels: Iterable[TextChannel], invoker: User) -> None: + async def _log_clean(self, messages: list[Message], channels: CleanChannels, invoker: User) -> None: """Log the deleted messages to the modlog.""" if not messages: # Can't build an embed, nothing to clean! @@ -276,7 +295,10 @@ async def _log_clean(self, messages: list[Message], channels: Iterable[TextChann log_url = await self.mod_log.upload_log(log_messages, invoker.id) # Build the embed and send it - target_channels = ", ".join(channel.mention for channel in channels) + if channels == "*": + target_channels = "all channels" + else: + target_channels = ", ".join(channel.mention for channel in channels) message = ( f"**{len(messages)}** messages deleted in {target_channels} by " @@ -305,10 +327,11 @@ async def clean_user( ctx: Context, user: User, amount: Optional[int] = 10, - channels: commands.Greedy[TextChannel] = None + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `amount` messages.""" - use_cache = not channels await self._clean_messages(amount, ctx, user=user, channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) @@ -317,10 +340,12 @@ async def clean_all( self, ctx: Context, amount: Optional[int] = 10, - channels: commands.Greedy[TextChannel] = None + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None ) -> None: """Delete all messages, regardless of poster, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, channels=channels) + await self._clean_messages(amount, ctx, channels=channels, use_cache=use_cache) @clean_group.command(name="bots", aliases=["bot"]) @has_any_role(*MODERATION_ROLES) @@ -328,22 +353,25 @@ async def clean_bots( self, ctx: Context, amount: Optional[int] = 10, - channels: commands.Greedy[TextChannel] = None + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None ) -> None: """Delete all messages posted by a bot, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, bots_only=True, channels=channels) + await self._clean_messages(amount, ctx, bots_only=True, channels=channels, use_cache=use_cache) - @clean_group.command(name="regex", aliases=["word", "expression"]) + @clean_group.command(name="regex", aliases=["word", "expression", "pattern"]) @has_any_role(*MODERATION_ROLES) async def clean_regex( self, ctx: Context, regex: str, amount: Optional[int] = 10, - channels: commands.Greedy[TextChannel] = None + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None ) -> None: """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" - use_cache = not channels await self._clean_messages(amount, ctx, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") From 4334988a664bbb516760a6046a2e8106e9777eab Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 28 Aug 2021 17:30:14 +0300 Subject: [PATCH 29/53] Rename "amount" argument to "traverse" This name as been confusing moderators for a while now. "amount" sounds like this is the amount of messages that the bot will try to delete, and keep going until it reaches that number. In reality it's the amount of latest messages per channel the bot will traverse. Hopefully the new name conveys that better. --- bot/exts/moderation/clean.py | 48 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 15a48ea757..5954672fee 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -82,7 +82,7 @@ async def _delete_messages_individually(self, messages: List[Message]) -> None: # Message doesn't exist or was already deleted continue - def _get_messages_from_cache(self, amount: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: + def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) message_ids = [] @@ -95,16 +95,16 @@ def _get_messages_from_cache(self, amount: int, to_delete: Predicate) -> Tuple[D message_mappings[message.channel].append(message) message_ids.append(message.id) - if len(message_ids) == amount: - # We've got the requested amount of messages + if len(message_ids) == traverse: + # We traversed the requested amount of messages. return message_mappings, message_ids - # Amount exceeds amount of messages matching the check + # There are fewer messages in the cache than the number requested to traverse. return message_mappings, message_ids async def _get_messages_from_channels( self, - amount: int, + traverse: int, channels: Iterable[TextChannel], to_delete: Predicate, until_message: Optional[Message] = None @@ -114,7 +114,7 @@ async def _get_messages_from_channels( for channel in channels: - async for message in channel.history(limit=amount): + async for message in channel.history(limit=traverse): if not self.cleaning: # Cleaning was canceled, return empty containers @@ -136,7 +136,7 @@ async def _get_messages_from_channels( async def _clean_messages( self, - amount: int, + traverse: int, ctx: Context, channels: CleanChannels, bots_only: bool = False, @@ -183,9 +183,9 @@ def predicate_range(message: Message) -> bool: """Check if message is older than message provided in after_message but younger than until_message.""" return after_message.created_at <= message.created_at <= until_message.created_at - # Is this an acceptable amount of messages to clean? - if amount > CleanMessages.message_limit: - raise BadArgument(f"You cannot clean more than {CleanMessages.message_limit} messages.") + # Is this an acceptable amount of messages to traverse? + if traverse > CleanMessages.message_limit: + raise BadArgument(f"You cannot traverse more than {CleanMessages.message_limit} messages.") if after_message: # Ensure that until_message is specified. @@ -232,13 +232,13 @@ def predicate_range(message: Message) -> bool: log.info("Tried to delete invocation message, but it was already deleted.") if channels == "*" and use_cache: - message_mappings, message_ids = self._get_messages_from_cache(amount=amount, to_delete=predicate) + message_mappings, message_ids = self._get_messages_from_cache(traverse=traverse, to_delete=predicate) else: deletion_channels = channels if channels == "*": deletion_channels = [channel for channel in ctx.guild.channels if isinstance(channel, TextChannel)] message_mappings, message_ids = await self._get_messages_from_channels( - amount=amount, + traverse=traverse, channels=deletion_channels, to_delete=predicate, until_message=until_message @@ -326,39 +326,39 @@ async def clean_user( self, ctx: Context, user: User, - amount: Optional[int] = 10, + traverse: Optional[int] = 10, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: - """Delete messages posted by the provided user, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, user=user, channels=channels, use_cache=use_cache) + """Delete messages posted by the provided user, stop cleaning after traversing `traverse` messages.""" + await self._clean_messages(traverse, ctx, user=user, channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) @has_any_role(*MODERATION_ROLES) async def clean_all( self, ctx: Context, - amount: Optional[int] = 10, + traverse: Optional[int] = 10, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: - """Delete all messages, regardless of poster, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, channels=channels, use_cache=use_cache) + """Delete all messages, regardless of poster, stop cleaning after traversing `traverse` messages.""" + await self._clean_messages(traverse, ctx, channels=channels, use_cache=use_cache) @clean_group.command(name="bots", aliases=["bot"]) @has_any_role(*MODERATION_ROLES) async def clean_bots( self, ctx: Context, - amount: Optional[int] = 10, + traverse: Optional[int] = 10, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: - """Delete all messages posted by a bot, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, bots_only=True, channels=channels, use_cache=use_cache) + """Delete all messages posted by a bot, stop cleaning after traversing `traverse` messages.""" + await self._clean_messages(traverse, ctx, bots_only=True, channels=channels, use_cache=use_cache) @clean_group.command(name="regex", aliases=["word", "expression", "pattern"]) @has_any_role(*MODERATION_ROLES) @@ -366,13 +366,13 @@ async def clean_regex( self, ctx: Context, regex: str, - amount: Optional[int] = 10, + traverse: Optional[int] = 10, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: - """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages.""" - await self._clean_messages(amount, ctx, regex=regex, channels=channels, use_cache=use_cache) + """Delete all messages that match a certain regex, stop cleaning after traversing `traverse` messages.""" + await self._clean_messages(traverse, ctx, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") @has_any_role(*MODERATION_ROLES) From be9bce46ab5479d3fdbb8e7baa26f1ad947685f6 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 28 Aug 2021 23:09:20 +0300 Subject: [PATCH 30/53] Refactor code, correct logging This commit further splits the bulky _clean_messages function, and all its helper functions are grouped together in the same region. Additionally, this commit fixes logging by logging only the messages that have been successfully deleted, before being possibly interrupted by the cancel command. --- bot/exts/moderation/clean.py | 229 ++++++++++++++++++++--------------- 1 file changed, 133 insertions(+), 96 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 5954672fee..455d28faa6 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -2,7 +2,6 @@ import re import time from collections import defaultdict -from itertools import chain from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors @@ -59,28 +58,35 @@ def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") + # region: Helper functions + @staticmethod - def is_older_than_14d(message: Message) -> bool: - """ - Precisely checks if message is older than 14 days, bulk deletion limit. + def _validate_input( + traverse: int, + channels: CleanChannels, + bots_only: bool, + user: User, + until_message: Message, + after_message: Message, + use_cache: bool + ) -> None: + """Raise errors if an argument value or a combination of values is invalid.""" + # Is this an acceptable amount of messages to traverse? + if traverse > CleanMessages.message_limit: + raise BadArgument(f"You cannot traverse more than {CleanMessages.message_limit} messages.") - Inspired by how purge works internally. - Comparison on message age could possibly be less accurate which in turn would resort in problems - with message deletion if said messages are very close to the 14d mark. - """ - two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 - return message.id < two_weeks_old_snowflake + if after_message: + # Ensure that until_message is specified. + if not until_message: + raise MissingRequiredArgument("`until_message` must be specified if `after_message` is specified.") - async def _delete_messages_individually(self, messages: List[Message]) -> None: - for message in messages: - # Ensure that deletion was not canceled - if not self.cleaning: - return - try: - await message.delete() - except NotFound: - # Message doesn't exist or was already deleted - continue + # Messages are not in same channel + if after_message.channel != until_message.channel: + raise BadArgument("You cannot do range clean across several channel.") + + # Ensure that after_message is younger than until_message + if after_message.created_at >= until_message.created_at: + raise BadArgument("`after` message must be younger than `until` message") def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" @@ -134,6 +140,107 @@ async def _get_messages_from_channels( return message_mappings, message_ids + @staticmethod + def is_older_than_14d(message: Message) -> bool: + """ + Precisely checks if message is older than 14 days, bulk deletion limit. + + Inspired by how purge works internally. + Comparison on message age could possibly be less accurate which in turn would resort in problems + with message deletion if said messages are very close to the 14d mark. + """ + two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 + return message.id < two_weeks_old_snowflake + + async def _delete_messages_individually(self, messages: List[Message]) -> list[Message]: + """Delete each message in the list unless cleaning is cancelled. Return the deleted messages.""" + deleted = [] + for message in messages: + # Ensure that deletion was not canceled + if not self.cleaning: + return deleted + try: + await message.delete() + except NotFound: + # Message doesn't exist or was already deleted + continue + else: + deleted.append(message) + return deleted + + async def _delete_found(self, message_mappings: dict[TextChannel, list[Message]]) -> list[Message]: + """ + Delete the detected messages. + + Deletion is made in bulk per channel for messages less than 14d old. + The function returns the deleted messages. + If cleaning was cancelled in the middle, return messages already deleted. + """ + deleted = [] + for channel, messages in message_mappings.items(): + to_delete = [] + + for current_index, message in enumerate(messages): + if not self.cleaning: + # Means that the cleaning was canceled + return deleted + + if self.is_older_than_14d(message): + # further messages are too old to be deleted in bulk + deleted_remaining = await self._delete_messages_individually(messages[current_index:]) + deleted.extend(deleted_remaining) + if not self.cleaning: + # Means that deletion was canceled while deleting the individual messages + return deleted + break + + to_delete.append(message) + + if len(to_delete) == 100: + # we can only delete up to 100 messages in a bulk + await channel.delete_messages(to_delete) + deleted.extend(to_delete) + to_delete.clear() + + if len(to_delete) > 0: + # deleting any leftover messages if there are any + await channel.delete_messages(to_delete) + deleted.extend(to_delete) + + return deleted + + async def _log_clean(self, messages: list[Message], channels: CleanChannels, invoker: User) -> None: + """Log the deleted messages to the modlog.""" + if not messages: + # Can't build an embed, nothing to clean! + raise BadArgument("No matching messages could be found.") + + # Reverse the list to have reverse chronological order + log_messages = reversed(messages) + log_url = await self.mod_log.upload_log(log_messages, invoker.id) + + # Build the embed and send it + if channels == "*": + target_channels = "all channels" + else: + target_channels = ", ".join(channel.mention for channel in channels) + + message = ( + f"**{len(messages)}** messages deleted in {target_channels} by " + f"{invoker.mention}\n\n" + f"A log of the deleted messages can be found [here]({log_url})." + ) + + await self.mod_log.send_log_message( + icon_url=Icons.message_bulk_delete, + colour=Colour(Colours.soft_red), + title="Bulk message delete", + text=message, + channel_id=Channels.mod_log, + ) + + # endregion + async def _clean_messages( self, traverse: int, @@ -183,22 +290,7 @@ def predicate_range(message: Message) -> bool: """Check if message is older than message provided in after_message but younger than until_message.""" return after_message.created_at <= message.created_at <= until_message.created_at - # Is this an acceptable amount of messages to traverse? - if traverse > CleanMessages.message_limit: - raise BadArgument(f"You cannot traverse more than {CleanMessages.message_limit} messages.") - - if after_message: - # Ensure that until_message is specified. - if not until_message: - raise MissingRequiredArgument("`until_message` must be specified if `after_message` is specified.") - - # Messages are not in same channel - if after_message.channel != until_message.channel: - raise BadArgument("You cannot do range clean across several channel.") - - # Ensure that after_message is younger than until_message - if after_message.created_at >= until_message.created_at: - raise BadArgument("`after` message must be younger than `until` message") + self._validate_input(traverse, channels, bots_only, user, until_message, after_message, use_cache) # Are we already performing a clean? if self.cleaning: @@ -250,69 +342,12 @@ def predicate_range(message: Message) -> bool: # Now let's delete the actual messages with purge. self.mod_log.ignore(Event.message_delete, *message_ids) - - for channel, messages in message_mappings.items(): - - to_delete = [] - - for current_index, message in enumerate(messages): - - if not self.cleaning: - # Means that the cleaning was canceled - return - - if self.is_older_than_14d(message): - # further messages are too old to be deleted in bulk - await self._delete_messages_individually(messages[current_index:]) - if not self.cleaning: - # Means that deletion was canceled while deleting the individual messages - return - break - - to_delete.append(message) - - if len(to_delete) == 100: - # we can only delete up to 100 messages in a bulk - await channel.delete_messages(to_delete) - to_delete.clear() - - if len(to_delete) > 0: - # deleting any leftover messages if there are any - await channel.delete_messages(to_delete) - + deleted_messages = await self._delete_found(message_mappings) self.cleaning = False - await self._log_clean(list(chain.from_iterable(message_mappings.values())), channels, ctx.author) - - async def _log_clean(self, messages: list[Message], channels: CleanChannels, invoker: User) -> None: - """Log the deleted messages to the modlog.""" - if not messages: - # Can't build an embed, nothing to clean! - raise BadArgument("No matching messages could be found.") - - # Reverse the list to have reverse chronological order - log_messages = reversed(messages) - log_url = await self.mod_log.upload_log(log_messages, invoker.id) + await self._log_clean(deleted_messages, channels, ctx.author) - # Build the embed and send it - if channels == "*": - target_channels = "all channels" - else: - target_channels = ", ".join(channel.mention for channel in channels) - - message = ( - f"**{len(messages)}** messages deleted in {target_channels} by " - f"{invoker.mention}\n\n" - f"A log of the deleted messages can be found [here]({log_url})." - ) - - await self.mod_log.send_log_message( - icon_url=Icons.message_bulk_delete, - colour=Colour(Colours.soft_red), - title="Bulk message delete", - text=message, - channel_id=Channels.mod_log, - ) + # region: Commands @group(invoke_without_command=True, name="clean", aliases=["clear", "purge"]) @has_any_role(*MODERATION_ROLES) @@ -412,6 +447,8 @@ async def clean_cancel(self, ctx: Context) -> None: delete_after = None await ctx.send(embed=embed, delete_after=delete_after) + # endregion + def setup(bot: Bot) -> None: """Load the Clean cog.""" From 675630a620afe9dee4772bc659a16289be9665d7 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 28 Aug 2021 23:22:28 +0300 Subject: [PATCH 31/53] Add checkmark after command completes in mod channels --- bot/exts/moderation/clean.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 455d28faa6..f8526b1b9b 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -11,7 +11,7 @@ from bot.bot import Bot from bot.constants import ( - Channels, CleanMessages, Colours, Event, Icons, MODERATION_ROLES + Channels, CleanMessages, Colours, Emojis, Event, Icons, MODERATION_ROLES ) from bot.exts.moderation.modlog import ModLog from bot.utils.channel import is_mod_channel @@ -347,6 +347,9 @@ def predicate_range(message: Message) -> bool: await self._log_clean(deleted_messages, channels, ctx.author) + if is_mod_channel(ctx.channel): + await ctx.message.add_reaction(Emojis.check_mark) + # region: Commands @group(invoke_without_command=True, name="clean", aliases=["clear", "purge"]) From 7b0cb52bc05261200a03428a51a48813eb3ccf0b Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 28 Aug 2021 23:33:32 +0300 Subject: [PATCH 32/53] Send message when no messages found This commit changes the clean command to send a message instead of raising BadArgument when no messages are found. Not finding messages is not an error, and doesn't necessarily require the help embed to spring up, just that the parameters might need tweaking. --- bot/exts/moderation/clean.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index f8526b1b9b..1d323fa0bb 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -209,15 +209,17 @@ async def _delete_found(self, message_mappings: dict[TextChannel, list[Message]] return deleted - async def _log_clean(self, messages: list[Message], channels: CleanChannels, invoker: User) -> None: - """Log the deleted messages to the modlog.""" + async def _log_clean(self, messages: list[Message], channels: CleanChannels, ctx: Context) -> bool: + """Log the deleted messages to the modlog. Return True if logging was successful.""" if not messages: # Can't build an embed, nothing to clean! - raise BadArgument("No matching messages could be found.") + delete_after = None if is_mod_channel(ctx.channel) else 5 + await ctx.send(":x: No matching messages could be found.", delete_after=delete_after) + return False # Reverse the list to have reverse chronological order log_messages = reversed(messages) - log_url = await self.mod_log.upload_log(log_messages, invoker.id) + log_url = await self.mod_log.upload_log(log_messages, ctx.author.id) # Build the embed and send it if channels == "*": @@ -227,7 +229,7 @@ async def _log_clean(self, messages: list[Message], channels: CleanChannels, inv message = ( f"**{len(messages)}** messages deleted in {target_channels} by " - f"{invoker.mention}\n\n" + f"{ctx.author.mention}\n\n" f"A log of the deleted messages can be found [here]({log_url})." ) @@ -239,6 +241,8 @@ async def _log_clean(self, messages: list[Message], channels: CleanChannels, inv channel_id=Channels.mod_log, ) + return True + # endregion async def _clean_messages( @@ -345,9 +349,9 @@ def predicate_range(message: Message) -> bool: deleted_messages = await self._delete_found(message_mappings) self.cleaning = False - await self._log_clean(deleted_messages, channels, ctx.author) + logged = await self._log_clean(deleted_messages, channels, ctx) - if is_mod_channel(ctx.channel): + if logged and is_mod_channel(ctx.channel): await ctx.message.add_reaction(Emojis.check_mark) # region: Commands From 13308200ff62784832ba9f9084b69cd3a214b966 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sun, 29 Aug 2021 02:10:43 +0300 Subject: [PATCH 33/53] `until` and `between` overhaul - The two subcommands can now accept a time delta and an ISO date time in addition to messages. - The two limits are now exclusive. Meaning cleaning until a message will not delete that message. - Added a separate predicate for the `until` case, as the combination of that command and cache usage would result in incorrect behavior. Additionally, deleting from cache now correctly traverses only `traverse` messages, rather than trying to delete `traverse` messages. --- bot/converters.py | 19 +++++ bot/exts/moderation/clean.py | 145 ++++++++++++++++++++++------------- 2 files changed, 111 insertions(+), 53 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 0118cc48af..546f6e8f49 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -388,6 +388,24 @@ async def convert(self, ctx: Context, duration: str) -> datetime: raise BadArgument(f"`{duration}` results in a datetime outside the supported range.") +class Age(DurationDelta): + """Convert duration strings into UTC datetime.datetime objects.""" + + async def convert(self, ctx: Context, duration: str) -> datetime: + """ + Converts a `duration` string to a datetime object that's `duration` in the past. + + The converter supports the same symbols for each unit of time as its parent class. + """ + delta = await super().convert(ctx, duration) + now = datetime.utcnow() + + try: + return now - delta + except (ValueError, OverflowError): + raise BadArgument(f"`{duration}` results in a datetime outside the supported range.") + + class OffTopicName(Converter): """A converter that ensures an added off-topic name is valid.""" @@ -554,6 +572,7 @@ async def convert(self, ctx: Context, arg: str) -> t.Optional[dict]: SourceConverter = SourceType # noqa: F811 DurationDelta = relativedelta # noqa: F811 Duration = datetime # noqa: F811 + Age = datetime # noqa: F811 OffTopicName = str # noqa: F811 ISODateTime = datetime # noqa: F811 HushDurationConverter = int # noqa: F811 diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 1d323fa0bb..90f7f3e034 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -2,17 +2,20 @@ import re import time from collections import defaultdict +from datetime import datetime +from itertools import islice from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors from discord.ext.commands import Cog, Context, Converter, group, has_any_role from discord.ext.commands.converter import TextChannelConverter -from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached, MissingRequiredArgument +from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached from bot.bot import Bot from bot.constants import ( Channels, CleanMessages, Colours, Emojis, Event, Icons, MODERATION_ROLES ) +from bot.converters import Age, ISODateTime from bot.exts.moderation.modlog import ModLog from bot.utils.channel import is_mod_channel @@ -21,6 +24,8 @@ # Type alias for checks Predicate = Callable[[Message], bool] +CleanLimit = Union[Message, Age, ISODateTime] + class CleanChannels(Converter): """A converter that turns the given string to a list of channels to clean, or the literal `*` for all channels.""" @@ -66,46 +71,40 @@ def _validate_input( channels: CleanChannels, bots_only: bool, user: User, - until_message: Message, - after_message: Message, + first_limit: CleanLimit, + second_limit: CleanLimit, use_cache: bool ) -> None: """Raise errors if an argument value or a combination of values is invalid.""" # Is this an acceptable amount of messages to traverse? if traverse > CleanMessages.message_limit: - raise BadArgument(f"You cannot traverse more than {CleanMessages.message_limit} messages.") + raise BadArgument(f"Cannot traverse more than {CleanMessages.message_limit} messages.") - if after_message: - # Ensure that until_message is specified. - if not until_message: - raise MissingRequiredArgument("`until_message` must be specified if `after_message` is specified.") + if (isinstance(first_limit, Message) or isinstance(first_limit, Message)) and channels: + raise BadArgument("Both a message limit and channels specified.") - # Messages are not in same channel - if after_message.channel != until_message.channel: - raise BadArgument("You cannot do range clean across several channel.") + if isinstance(first_limit, Message) and isinstance(second_limit, Message): + # Messages are not in same channel. + if first_limit.channel != second_limit.channel: + raise BadArgument("Message limits are in different channels.") - # Ensure that after_message is younger than until_message - if after_message.created_at >= until_message.created_at: - raise BadArgument("`after` message must be younger than `until` message") + # This is an implementation error rather than user error. + if second_limit and not first_limit: + raise ValueError("Second limit specified without the first.") def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) message_ids = [] - for message in self.bot.cached_messages: + for message in islice(self.bot.cached_messages, traverse): if not self.cleaning: # Cleaning was canceled - return (message_mappings, message_ids) + return message_mappings, message_ids if to_delete(message): message_mappings[message.channel].append(message) message_ids.append(message.id) - if len(message_ids) == traverse: - # We traversed the requested amount of messages. - return message_mappings, message_ids - - # There are fewer messages in the cache than the number requested to traverse. return message_mappings, message_ids async def _get_messages_from_channels( @@ -113,27 +112,19 @@ async def _get_messages_from_channels( traverse: int, channels: Iterable[TextChannel], to_delete: Predicate, - until_message: Optional[Message] = None + before: Optional[datetime] = None, + after: Optional[datetime] = None ) -> tuple[defaultdict[Any, list], list]: message_mappings = defaultdict(list) message_ids = [] for channel in channels: - - async for message in channel.history(limit=traverse): + async for message in channel.history(limit=traverse, before=before, after=after): if not self.cleaning: - # Cleaning was canceled, return empty containers + # Cleaning was canceled, return empty containers. return defaultdict(list), [] - if until_message: - - # We could use ID's here however in case if the message we are looking for gets deleted, - # We won't have a way to figure that out thus checking for datetime should be more reliable - if message.created_at < until_message.created_at: - # Means we have found the message until which we were supposed to be deleting. - break - if to_delete(message): message_mappings[message.channel].append(message) message_ids.append(message.id) @@ -253,8 +244,8 @@ async def _clean_messages( bots_only: bool = False, user: User = None, regex: Optional[str] = None, - until_message: Optional[Message] = None, - after_message: Optional[Message] = None, + first_limit: Optional[CleanLimit] = None, + second_limit: Optional[CleanLimit] = None, use_cache: Optional[bool] = True ) -> None: """A helper function that does the actual message cleaning.""" @@ -291,10 +282,14 @@ def predicate_regex(message: Message) -> bool: return bool(re.search(regex.lower(), content.lower())) def predicate_range(message: Message) -> bool: - """Check if message is older than message provided in after_message but younger than until_message.""" - return after_message.created_at <= message.created_at <= until_message.created_at + """Check if the message age is between the two limits.""" + return first_limit <= message.created_at <= second_limit - self._validate_input(traverse, channels, bots_only, user, until_message, after_message, use_cache) + def predicate_after(message: Message) -> bool: + """Check if the message is older than the first limit.""" + return message.created_at >= first_limit + + self._validate_input(traverse, channels, bots_only, user, first_limit, second_limit, use_cache) # Are we already performing a clean? if self.cleaning: @@ -308,17 +303,31 @@ def predicate_range(message: Message) -> bool: predicate = predicate_specific_user # Delete messages from specific user elif regex: predicate = predicate_regex # Delete messages that match regex - elif after_message: - predicate = predicate_range # Delete messages older than specific message + elif second_limit: + predicate = predicate_range # Delete messages in the specified age range + elif first_limit: + predicate = predicate_after # Delete messages older than specific message else: predicate = lambda m: True # Delete all messages # noqa: E731 - # Default to using the invoking context's channel + # Default to using the invoking context's channel or the channel of the message limit(s). if not channels: - channels = [ctx.channel] + # At this point second_limit is guaranteed to not exist, be a datetime, or a message in the same channel. + if isinstance(first_limit, Message): + channels = [first_limit.channel] + elif isinstance(second_limit, Message): + channels = [second_limit.channel] + else: + channels = [ctx.channel] - if not is_mod_channel(ctx.channel): + if isinstance(first_limit, Message): + first_limit = first_limit.created_at + if isinstance(second_limit, Message): + second_limit = second_limit.created_at + if first_limit and second_limit: + first_limit, second_limit = sorted([first_limit, second_limit]) + if not is_mod_channel(ctx.channel): # Delete the invocation first self.mod_log.ignore(Event.message_delete, ctx.message.id) try: @@ -337,7 +346,8 @@ def predicate_range(message: Message) -> bool: traverse=traverse, channels=deletion_channels, to_delete=predicate, - until_message=until_message + before=second_limit, + after=first_limit # Remember first is the earlier datetime. ) if not self.cleaning: @@ -418,25 +428,54 @@ async def clean_regex( @clean_group.command(name="until") @has_any_role(*MODERATION_ROLES) - async def clean_until(self, ctx: Context, message: Message) -> None: - """Delete all messages until certain message, stop cleaning after hitting the `message`.""" + async def clean_until( + self, + ctx: Context, + until: CleanLimit, + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None) -> None: + """ + Delete all messages until a certain limit. + + A limit can be either a message, and ISO date-time string, or a time delta. + If a message is specified, `channels` cannot be specified. + """ await self._clean_messages( CleanMessages.message_limit, ctx, - channels=[message.channel], - until_message=message + channels=channels, + first_limit=until, + use_cache=use_cache ) @clean_group.command(name="between", aliases=["after-until", "from-to"]) @has_any_role(*MODERATION_ROLES) - async def clean_between(self, ctx: Context, after_message: Message, until_message: Message) -> None: - """Delete all messages within range of messages.""" + async def clean_between( + self, + ctx: Context, + first_limit: CleanLimit, + second_limit: CleanLimit, + use_cache: Optional[bool] = True, + *, + channels: Optional[CleanChannels] = None + ) -> None: + """ + Delete all messages within range. + + The range is specified through two limits. + A limit can be either a message, and ISO date-time string, or a time delta. + + If two messages are specified, they both must be in the same channel. + If a message is specified, `channels` cannot be specified. + """ await self._clean_messages( CleanMessages.message_limit, ctx, - channels=[until_message.channel], - until_message=until_message, - after_message=after_message, + channels=channels, + first_limit=first_limit, + second_limit=second_limit, + use_cache=use_cache ) @clean_group.command(name="stop", aliases=["cancel", "abort"]) From 9f124b9eefd24bd1e3bc7210361fe927e8f9eeba Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sun, 29 Aug 2021 13:47:37 +0300 Subject: [PATCH 34/53] Restrict until and between to a single channel The subcommands should stay simple and answer the most common use cases. Deleting all messages within a time range across many channels seems esoteric and gives just more room for mistakes. --- bot/exts/moderation/clean.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 90f7f3e034..6c7f3c22de 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -432,21 +432,19 @@ async def clean_until( self, ctx: Context, until: CleanLimit, - use_cache: Optional[bool] = True, - *, - channels: Optional[CleanChannels] = None) -> None: + channel: Optional[TextChannel] = None + ) -> None: """ Delete all messages until a certain limit. A limit can be either a message, and ISO date-time string, or a time delta. - If a message is specified, `channels` cannot be specified. + If a message is specified, `channel` cannot be specified. """ await self._clean_messages( CleanMessages.message_limit, ctx, - channels=channels, + channels=[channel] if channel else None, first_limit=until, - use_cache=use_cache ) @clean_group.command(name="between", aliases=["after-until", "from-to"]) @@ -456,9 +454,7 @@ async def clean_between( ctx: Context, first_limit: CleanLimit, second_limit: CleanLimit, - use_cache: Optional[bool] = True, - *, - channels: Optional[CleanChannels] = None + channel: Optional[TextChannel] = None ) -> None: """ Delete all messages within range. @@ -467,15 +463,14 @@ async def clean_between( A limit can be either a message, and ISO date-time string, or a time delta. If two messages are specified, they both must be in the same channel. - If a message is specified, `channels` cannot be specified. + If a message is specified, `channel` cannot be specified. """ await self._clean_messages( CleanMessages.message_limit, ctx, - channels=channels, + channels=[channel] if channel else None, first_limit=first_limit, second_limit=second_limit, - use_cache=use_cache ) @clean_group.command(name="stop", aliases=["cancel", "abort"]) From ec8f06312d756325fff31d7735ea56465440ed57 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sun, 29 Aug 2021 13:53:06 +0300 Subject: [PATCH 35/53] Use a cog-wide role check --- bot/exts/moderation/clean.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 6c7f3c22de..950c0c82e1 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -367,13 +367,11 @@ def predicate_after(message: Message) -> bool: # region: Commands @group(invoke_without_command=True, name="clean", aliases=["clear", "purge"]) - @has_any_role(*MODERATION_ROLES) async def clean_group(self, ctx: Context) -> None: """Commands for cleaning messages in channels.""" await ctx.send_help(ctx.command) @clean_group.command(name="user", aliases=["users"]) - @has_any_role(*MODERATION_ROLES) async def clean_user( self, ctx: Context, @@ -387,7 +385,6 @@ async def clean_user( await self._clean_messages(traverse, ctx, user=user, channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) - @has_any_role(*MODERATION_ROLES) async def clean_all( self, ctx: Context, @@ -400,7 +397,6 @@ async def clean_all( await self._clean_messages(traverse, ctx, channels=channels, use_cache=use_cache) @clean_group.command(name="bots", aliases=["bot"]) - @has_any_role(*MODERATION_ROLES) async def clean_bots( self, ctx: Context, @@ -413,7 +409,6 @@ async def clean_bots( await self._clean_messages(traverse, ctx, bots_only=True, channels=channels, use_cache=use_cache) @clean_group.command(name="regex", aliases=["word", "expression", "pattern"]) - @has_any_role(*MODERATION_ROLES) async def clean_regex( self, ctx: Context, @@ -427,7 +422,6 @@ async def clean_regex( await self._clean_messages(traverse, ctx, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") - @has_any_role(*MODERATION_ROLES) async def clean_until( self, ctx: Context, @@ -448,7 +442,6 @@ async def clean_until( ) @clean_group.command(name="between", aliases=["after-until", "from-to"]) - @has_any_role(*MODERATION_ROLES) async def clean_between( self, ctx: Context, @@ -474,7 +467,6 @@ async def clean_between( ) @clean_group.command(name="stop", aliases=["cancel", "abort"]) - @has_any_role(*MODERATION_ROLES) async def clean_cancel(self, ctx: Context) -> None: """If there is an ongoing cleaning process, attempt to immediately cancel it.""" self.cleaning = False @@ -490,6 +482,10 @@ async def clean_cancel(self, ctx: Context) -> None: # endregion + async def cog_check(self, ctx: Context) -> bool: + """Only allow moderators to invoke the commands in this cog.""" + return await has_any_role(*MODERATION_ROLES).predicate(ctx) + def setup(bot: Bot) -> None: """Load the Clean cog.""" From fee4c0c8be7d0e7d0bcb8358bb11255feb3f66b8 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sun, 29 Aug 2021 15:06:50 +0300 Subject: [PATCH 36/53] Handle reacted message being deleted --- bot/exts/moderation/clean.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 950c0c82e1..6fb33c6924 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -2,6 +2,7 @@ import re import time from collections import defaultdict +from contextlib import suppress from datetime import datetime from itertools import islice from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union @@ -362,7 +363,8 @@ def predicate_after(message: Message) -> bool: logged = await self._log_clean(deleted_messages, channels, ctx) if logged and is_mod_channel(ctx.channel): - await ctx.message.add_reaction(Emojis.check_mark) + with suppress(NotFound): # Can happen if the invoker deleted their own messages. + await ctx.message.add_reaction(Emojis.check_mark) # region: Commands From ab155fb20ea77c4c7ab60e6368b76733662b93d7 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sun, 29 Aug 2021 17:47:27 +0300 Subject: [PATCH 37/53] Added master command The subcommands are kept simple and with few arguments, as they deal with most cases and their usage shouldn't be cumbersome. However we might to clean by criteria of several functionalities offered by the subcommands, for example delete a specific user's messages but only those that contain a certain pattern. For this reason the top-level command can now accept all arguments available in any of the subcommands, and will combine the criteria. Because the channels list has to be the last argument in order to accept either a list of channel or "*", I had to force a specific pattern in the regex argument for it to not consume the first channel specified. The regex argument must now have an "r" prefix and be enclosed in single quotes. For example: r'\d+'. For patterns with spaces the whole thing still needs to be enclosed in double quotes. For consistency the `clean regex` subcommand was changed to require the same. --- bot/exts/moderation/clean.py | 230 ++++++++++++++++++++++++----------- 1 file changed, 156 insertions(+), 74 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 6fb33c6924..bf018e8aa7 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -8,7 +8,7 @@ from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors -from discord.ext.commands import Cog, Context, Converter, group, has_any_role +from discord.ext.commands import Cog, Context, Converter, Greedy, group, has_any_role from discord.ext.commands.converter import TextChannelConverter from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached @@ -22,6 +22,8 @@ log = logging.getLogger(__name__) +DEFAULT_TRAVERSE = 10 + # Type alias for checks Predicate = Callable[[Message], bool] @@ -40,8 +42,17 @@ async def convert(self, ctx: Context, argument: str) -> Union[Literal["*"], list return [await self._channel_converter.convert(ctx, channel) for channel in argument.split()] +class Regex(Converter): + """A converter that takes a string in the form r'.+' and strips the 'r' prefix and the single quotes.""" + + async def convert(self, ctx: Context, argument: str) -> str: + """Strips the 'r' prefix and the enclosing single quotes from the string.""" + return re.match(r"r'(.+?)'", argument).group(1) + + if TYPE_CHECKING: CleanChannels = Union[Literal["*"], list[TextChannel]] # noqa: F811 + Regex = str # noqa: F811 class Clean(Cog): @@ -71,10 +82,9 @@ def _validate_input( traverse: int, channels: CleanChannels, bots_only: bool, - user: User, + users: list[User], first_limit: CleanLimit, second_limit: CleanLimit, - use_cache: bool ) -> None: """Raise errors if an argument value or a combination of values is invalid.""" # Is this an acceptable amount of messages to traverse? @@ -89,10 +99,85 @@ def _validate_input( if first_limit.channel != second_limit.channel: raise BadArgument("Message limits are in different channels.") + if users and bots_only: + raise BadArgument("Marked as bots only, but users were specified.") + # This is an implementation error rather than user error. if second_limit and not first_limit: raise ValueError("Second limit specified without the first.") + @staticmethod + def _build_predicate( + bots_only: bool = False, + users: list[User] = None, + regex: Optional[str] = None, + first_limit: Optional[datetime] = None, + second_limit: Optional[datetime] = None, + ) -> Predicate: + """Return the predicate that decides whether to delete a given message.""" + def predicate_bots_only(message: Message) -> bool: + """Return True if the message was sent by a bot.""" + return message.author.bot + + def predicate_specific_users(message: Message) -> bool: + """Return True if the message was sent by the user provided in the _clean_messages call.""" + return message.author in users + + def predicate_regex(message: Message) -> bool: + """Check if the regex provided in _clean_messages matches the message content or any embed attributes.""" + content = [message.content] + + # Add the content for all embed attributes + for embed in message.embeds: + content.append(embed.title) + content.append(embed.description) + content.append(embed.footer.text) + content.append(embed.author.name) + for field in embed.fields: + content.append(field.name) + content.append(field.value) + + # Get rid of empty attributes and turn it into a string + content = [attr for attr in content if attr] + content = "\n".join(content) + + # Now let's see if there's a regex match + if not content: + return False + else: + return bool(re.search(regex.lower(), content.lower())) + + def predicate_range(message: Message) -> bool: + """Check if the message age is between the two limits.""" + return first_limit <= message.created_at <= second_limit + + def predicate_after(message: Message) -> bool: + """Check if the message is older than the first limit.""" + return message.created_at >= first_limit + + predicates = [] + # Set up the correct predicate + if bots_only: + predicates.append(predicate_bots_only) # Delete messages from bots + if users: + predicates.append(predicate_specific_users) # Delete messages from specific user + if regex: + predicates.append(predicate_regex) # Delete messages that match regex + # Add up to one of the following: + if second_limit: + predicates.append(predicate_range) # Delete messages in the specified age range + elif first_limit: + predicates.append(predicate_after) # Delete messages older than specific message + + if not predicates: + predicate = lambda m: True # Delete all messages # noqa: E731 + elif len(predicates) == 1: + predicate = predicates[0] + else: + predicate = lambda m: all(pred(m) for pred in predicates) # noqa: E731 + + return predicate + def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) @@ -239,78 +324,24 @@ async def _log_clean(self, messages: list[Message], channels: CleanChannels, ctx async def _clean_messages( self, - traverse: int, ctx: Context, + traverse: int, channels: CleanChannels, bots_only: bool = False, - user: User = None, + users: list[User] = None, regex: Optional[str] = None, first_limit: Optional[CleanLimit] = None, second_limit: Optional[CleanLimit] = None, use_cache: Optional[bool] = True ) -> None: """A helper function that does the actual message cleaning.""" - def predicate_bots_only(message: Message) -> bool: - """Return True if the message was sent by a bot.""" - return message.author.bot - - def predicate_specific_user(message: Message) -> bool: - """Return True if the message was sent by the user provided in the _clean_messages call.""" - return message.author == user - - def predicate_regex(message: Message) -> bool: - """Check if the regex provided in _clean_messages matches the message content or any embed attributes.""" - content = [message.content] - - # Add the content for all embed attributes - for embed in message.embeds: - content.append(embed.title) - content.append(embed.description) - content.append(embed.footer.text) - content.append(embed.author.name) - for field in embed.fields: - content.append(field.name) - content.append(field.value) - - # Get rid of empty attributes and turn it into a string - content = [attr for attr in content if attr] - content = "\n".join(content) - - # Now let's see if there's a regex match - if not content: - return False - else: - return bool(re.search(regex.lower(), content.lower())) - - def predicate_range(message: Message) -> bool: - """Check if the message age is between the two limits.""" - return first_limit <= message.created_at <= second_limit - - def predicate_after(message: Message) -> bool: - """Check if the message is older than the first limit.""" - return message.created_at >= first_limit - - self._validate_input(traverse, channels, bots_only, user, first_limit, second_limit, use_cache) + self._validate_input(traverse, channels, bots_only, users, first_limit, second_limit) # Are we already performing a clean? if self.cleaning: raise MaxConcurrencyReached("Please wait for the currently ongoing clean operation to complete.") self.cleaning = True - # Set up the correct predicate - if bots_only: - predicate = predicate_bots_only # Delete messages from bots - elif user: - predicate = predicate_specific_user # Delete messages from specific user - elif regex: - predicate = predicate_regex # Delete messages that match regex - elif second_limit: - predicate = predicate_range # Delete messages in the specified age range - elif first_limit: - predicate = predicate_after # Delete messages older than specific message - else: - predicate = lambda m: True # Delete all messages # noqa: E731 - # Default to using the invoking context's channel or the channel of the message limit(s). if not channels: # At this point second_limit is guaranteed to not exist, be a datetime, or a message in the same channel. @@ -328,6 +359,9 @@ def predicate_after(message: Message) -> bool: if first_limit and second_limit: first_limit, second_limit = sorted([first_limit, second_limit]) + # Needs to be called after standardizing the input. + predicate = self._build_predicate(bots_only, users, regex, first_limit, second_limit) + if not is_mod_channel(ctx.channel): # Delete the invocation first self.mod_log.ignore(Event.message_delete, ctx.message.id) @@ -369,9 +403,51 @@ def predicate_after(message: Message) -> bool: # region: Commands @group(invoke_without_command=True, name="clean", aliases=["clear", "purge"]) - async def clean_group(self, ctx: Context) -> None: - """Commands for cleaning messages in channels.""" - await ctx.send_help(ctx.command) + async def clean_group( + self, + ctx: Context, + traverse: Optional[int] = None, + users: Greedy[User] = None, + first_limit: Optional[CleanLimit] = None, + second_limit: Optional[CleanLimit] = None, + use_cache: Optional[bool] = None, + bots_only: Optional[bool] = False, + regex: Optional[Regex] = None, + *, + channels: Optional[CleanChannels] = None + ) -> None: + """ + Commands for cleaning messages in channels. + + If arguments are provided, will act as a master command from which all subcommands can be derived. + `traverse`: The number of messages to look at in each channel. + `users`: A series of user mentions, ID's, or names. + `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. + If a message is provided, cleaning will happen in that channel, and channels cannot be provided. + If only one of them is provided, acts as `clean until`. If both are provided, acts as `clean between`. + `use_cache`: Whether to use the message cache. + If not provided, will default to False unless an asterisk is used for the channels. + `bots_only`: Whether to delete only bots. If specified, users cannot be specified. + `regex`: A regex pattern the message must contain to be deleted. + The pattern must be provided with an "r" prefix and enclosed in single quotes. + If the pattern contains spaces, it still needs to be enclosed in double quotes on top of that. + `channels`: A series of channels to delete in, or an asterisk to delete from all channels. + """ + if not any([traverse, users, first_limit, second_limit, regex]): + await ctx.send_help(ctx.command) + return + + if not traverse: + if first_limit: + traverse = CleanMessages.message_limit + else: + traverse = DEFAULT_TRAVERSE + if not use_cache: + use_cache = channels == "*" + + await self._clean_messages( + ctx, traverse, channels, bots_only, users, regex, first_limit, second_limit, use_cache + ) @clean_group.command(name="user", aliases=["users"]) async def clean_user( @@ -384,44 +460,50 @@ async def clean_user( channels: Optional[CleanChannels] = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `traverse` messages.""" - await self._clean_messages(traverse, ctx, user=user, channels=channels, use_cache=use_cache) + await self._clean_messages(ctx, traverse, users=[user], channels=channels, use_cache=use_cache) @clean_group.command(name="all", aliases=["everything"]) async def clean_all( self, ctx: Context, - traverse: Optional[int] = 10, + traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: """Delete all messages, regardless of poster, stop cleaning after traversing `traverse` messages.""" - await self._clean_messages(traverse, ctx, channels=channels, use_cache=use_cache) + await self._clean_messages(ctx, traverse, channels=channels, use_cache=use_cache) @clean_group.command(name="bots", aliases=["bot"]) async def clean_bots( self, ctx: Context, - traverse: Optional[int] = 10, + traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: """Delete all messages posted by a bot, stop cleaning after traversing `traverse` messages.""" - await self._clean_messages(traverse, ctx, bots_only=True, channels=channels, use_cache=use_cache) + await self._clean_messages(ctx, traverse, bots_only=True, channels=channels, use_cache=use_cache) @clean_group.command(name="regex", aliases=["word", "expression", "pattern"]) async def clean_regex( self, ctx: Context, - regex: str, - traverse: Optional[int] = 10, + regex: Regex, + traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, channels: Optional[CleanChannels] = None ) -> None: - """Delete all messages that match a certain regex, stop cleaning after traversing `traverse` messages.""" - await self._clean_messages(traverse, ctx, regex=regex, channels=channels, use_cache=use_cache) + """ + Delete all messages that match a certain regex, stop cleaning after traversing `traverse` messages. + + The pattern must be provided with an "r" prefix and enclosed in single quotes. + If the pattern contains spaces, and still needs to be enclosed in double quotes on top of that. + For example: r'[0-9]+' + """ + await self._clean_messages(ctx, traverse, regex=regex, channels=channels, use_cache=use_cache) @clean_group.command(name="until") async def clean_until( @@ -437,8 +519,8 @@ async def clean_until( If a message is specified, `channel` cannot be specified. """ await self._clean_messages( - CleanMessages.message_limit, ctx, + CleanMessages.message_limit, channels=[channel] if channel else None, first_limit=until, ) @@ -461,8 +543,8 @@ async def clean_between( If a message is specified, `channel` cannot be specified. """ await self._clean_messages( - CleanMessages.message_limit, ctx, + CleanMessages.message_limit, channels=[channel] if channel else None, first_limit=first_limit, second_limit=second_limit, From f2fb9f3dd449d58162471525ecaccc6db7d721f0 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 31 Aug 2021 20:49:15 +0300 Subject: [PATCH 38/53] Disallow time range cleaning in multiple channels Cleaning in the same time range across several channels seems like an arbitrary decision. --- bot/exts/moderation/clean.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index bf018e8aa7..1148b3eb56 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -91,6 +91,9 @@ def _validate_input( if traverse > CleanMessages.message_limit: raise BadArgument(f"Cannot traverse more than {CleanMessages.message_limit} messages.") + if first_limit and channels and (channels == "*" or len(channels) > 1): + raise BadArgument("Message or time range specified across multiple channels.") + if (isinstance(first_limit, Message) or isinstance(first_limit, Message)) and channels: raise BadArgument("Both a message limit and channels specified.") @@ -424,6 +427,7 @@ async def clean_group( `users`: A series of user mentions, ID's, or names. `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. If a message is provided, cleaning will happen in that channel, and channels cannot be provided. + If a limit is provided, multiple channels cannot be provided. If only one of them is provided, acts as `clean until`. If both are provided, acts as `clean between`. `use_cache`: Whether to use the message cache. If not provided, will default to False unless an asterisk is used for the channels. From 3ccb533686d464e11bf330ac19900a9f6cfc4366 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 31 Aug 2021 22:06:53 +0300 Subject: [PATCH 39/53] Changed regex formatting to wrapped in backticks After discussion, backticks seems like the preferrable formatting as it also cancels Discord's formatting. Additionally removed the Optionals from the last args in the commands, to not silently ignore incorrect input. --- bot/exts/moderation/clean.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 1148b3eb56..5b64693ccf 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -43,11 +43,11 @@ async def convert(self, ctx: Context, argument: str) -> Union[Literal["*"], list class Regex(Converter): - """A converter that takes a string in the form r'.+' and strips the 'r' prefix and the single quotes.""" + """A converter that takes a string in the form `.+` and returns the contents of the inline code.""" async def convert(self, ctx: Context, argument: str) -> str: - """Strips the 'r' prefix and the enclosing single quotes from the string.""" - return re.match(r"r'(.+?)'", argument).group(1) + """Strips the backticks from the string.""" + return re.fullmatch(r"`(.+?)`", argument).group(1) if TYPE_CHECKING: @@ -417,7 +417,7 @@ async def clean_group( bots_only: Optional[bool] = False, regex: Optional[Regex] = None, *, - channels: Optional[CleanChannels] = None + channels: CleanChannels = None ) -> None: """ Commands for cleaning messages in channels. @@ -433,11 +433,11 @@ async def clean_group( If not provided, will default to False unless an asterisk is used for the channels. `bots_only`: Whether to delete only bots. If specified, users cannot be specified. `regex`: A regex pattern the message must contain to be deleted. - The pattern must be provided with an "r" prefix and enclosed in single quotes. + The pattern must be provided enclosed in backticks. If the pattern contains spaces, it still needs to be enclosed in double quotes on top of that. `channels`: A series of channels to delete in, or an asterisk to delete from all channels. """ - if not any([traverse, users, first_limit, second_limit, regex]): + if not any([traverse, users, first_limit, second_limit, regex, channels]): await ctx.send_help(ctx.command) return @@ -461,7 +461,7 @@ async def clean_user( traverse: Optional[int] = 10, use_cache: Optional[bool] = True, *, - channels: Optional[CleanChannels] = None + channels: CleanChannels = None ) -> None: """Delete messages posted by the provided user, stop cleaning after traversing `traverse` messages.""" await self._clean_messages(ctx, traverse, users=[user], channels=channels, use_cache=use_cache) @@ -473,7 +473,7 @@ async def clean_all( traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, - channels: Optional[CleanChannels] = None + channels: CleanChannels = None ) -> None: """Delete all messages, regardless of poster, stop cleaning after traversing `traverse` messages.""" await self._clean_messages(ctx, traverse, channels=channels, use_cache=use_cache) @@ -485,7 +485,7 @@ async def clean_bots( traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, - channels: Optional[CleanChannels] = None + channels: CleanChannels = None ) -> None: """Delete all messages posted by a bot, stop cleaning after traversing `traverse` messages.""" await self._clean_messages(ctx, traverse, bots_only=True, channels=channels, use_cache=use_cache) @@ -498,14 +498,14 @@ async def clean_regex( traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, - channels: Optional[CleanChannels] = None + channels: CleanChannels = None ) -> None: """ Delete all messages that match a certain regex, stop cleaning after traversing `traverse` messages. - The pattern must be provided with an "r" prefix and enclosed in single quotes. + The pattern must be provided enclosed in backticks. If the pattern contains spaces, and still needs to be enclosed in double quotes on top of that. - For example: r'[0-9]+' + For example: `[0-9]` """ await self._clean_messages(ctx, traverse, regex=regex, channels=channels, use_cache=use_cache) @@ -514,7 +514,7 @@ async def clean_until( self, ctx: Context, until: CleanLimit, - channel: Optional[TextChannel] = None + channel: TextChannel = None ) -> None: """ Delete all messages until a certain limit. @@ -535,7 +535,7 @@ async def clean_between( ctx: Context, first_limit: CleanLimit, second_limit: CleanLimit, - channel: Optional[TextChannel] = None + channel: TextChannel = None ) -> None: """ Delete all messages within range. From f5d7a006e4a0ebc1be0fd79be76eaf3501c6521a Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 7 Sep 2021 02:13:57 +0300 Subject: [PATCH 40/53] Code and comments polish Co-authored-by: Shivansh-007 --- bot/exts/moderation/clean.py | 46 ++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 5b64693ccf..a9f936d88f 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -5,7 +5,7 @@ from contextlib import suppress from datetime import datetime from itertools import islice -from typing import Any, Callable, DefaultDict, Iterable, List, Literal, Optional, TYPE_CHECKING, Tuple, Union +from typing import Any, Callable, DefaultDict, Iterable, Literal, Optional, TYPE_CHECKING, Union from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors from discord.ext.commands import Cog, Context, Converter, Greedy, group, has_any_role @@ -57,7 +57,7 @@ async def convert(self, ctx: Context, argument: str) -> str: class Clean(Cog): """ - A cog that allows messages to be deleted in bulk, while applying various filters. + A cog that allows messages to be deleted in bulk while applying various filters. You can delete messages sent by a specific user, messages sent by bots, all messages, or messages that match a specific regular expression. @@ -94,7 +94,7 @@ def _validate_input( if first_limit and channels and (channels == "*" or len(channels) > 1): raise BadArgument("Message or time range specified across multiple channels.") - if (isinstance(first_limit, Message) or isinstance(first_limit, Message)) and channels: + if (isinstance(first_limit, Message) or isinstance(second_limit, Message)) and channels: raise BadArgument("Both a message limit and channels specified.") if isinstance(first_limit, Message) and isinstance(second_limit, Message): @@ -141,8 +141,7 @@ def predicate_regex(message: Message) -> bool: content.append(field.value) # Get rid of empty attributes and turn it into a string - content = [attr for attr in content if attr] - content = "\n".join(content) + content = "\n".join(attr for attr in content if attr) # Now let's see if there's a regex match if not content: @@ -173,15 +172,12 @@ def predicate_after(message: Message) -> bool: predicates.append(predicate_after) # Delete messages older than specific message if not predicates: - predicate = lambda m: True # Delete all messages # noqa: E731 - elif len(predicates) == 1: - predicate = predicates[0] - else: - predicate = lambda m: all(pred(m) for pred in predicates) # noqa: E731 - - return predicate + return lambda m: True + if len(predicates) == 1: + return predicates[0] + return lambda m: all(pred(m) for pred in predicates) - def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> Tuple[DefaultDict, List[int]]: + def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> tuple[DefaultDict, list[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) message_ids = [] @@ -232,7 +228,7 @@ def is_older_than_14d(message: Message) -> bool: two_weeks_old_snowflake = int((time.time() - 14 * 24 * 60 * 60) * 1000.0 - 1420070400000) << 22 return message.id < two_weeks_old_snowflake - async def _delete_messages_individually(self, messages: List[Message]) -> list[Message]: + async def _delete_messages_individually(self, messages: list[Message]) -> list[Message]: """Delete each message in the list unless cleaning is cancelled. Return the deleted messages.""" deleted = [] for message in messages: @@ -289,7 +285,7 @@ async def _delete_found(self, message_mappings: dict[TextChannel, list[Message]] return deleted - async def _log_clean(self, messages: list[Message], channels: CleanChannels, ctx: Context) -> bool: + async def _modlog_cleaned_messages(self, messages: list[Message], channels: CleanChannels, ctx: Context) -> bool: """Log the deleted messages to the modlog. Return True if logging was successful.""" if not messages: # Can't build an embed, nothing to clean! @@ -347,7 +343,7 @@ async def _clean_messages( # Default to using the invoking context's channel or the channel of the message limit(s). if not channels: - # At this point second_limit is guaranteed to not exist, be a datetime, or a message in the same channel. + # Input was validated - if first_limit is a message, second_limit won't point at a different channel. if isinstance(first_limit, Message): channels = [first_limit.channel] elif isinstance(second_limit, Message): @@ -397,7 +393,7 @@ async def _clean_messages( deleted_messages = await self._delete_found(message_mappings) self.cleaning = False - logged = await self._log_clean(deleted_messages, channels, ctx) + logged = await self._modlog_cleaned_messages(deleted_messages, channels, ctx) if logged and is_mod_channel(ctx.channel): with suppress(NotFound): # Can happen if the invoker deleted their own messages. @@ -417,25 +413,25 @@ async def clean_group( bots_only: Optional[bool] = False, regex: Optional[Regex] = None, *, - channels: CleanChannels = None + channels: CleanChannels = None # "Optional" with discord.py silently ignores incorrect input. ) -> None: """ Commands for cleaning messages in channels. If arguments are provided, will act as a master command from which all subcommands can be derived. - `traverse`: The number of messages to look at in each channel. - `users`: A series of user mentions, ID's, or names. - `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. + • `traverse`: The number of messages to look at in each channel. + • `users`: A series of user mentions, ID's, or names. + • `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. If a message is provided, cleaning will happen in that channel, and channels cannot be provided. If a limit is provided, multiple channels cannot be provided. If only one of them is provided, acts as `clean until`. If both are provided, acts as `clean between`. - `use_cache`: Whether to use the message cache. + • `use_cache`: Whether to use the message cache. If not provided, will default to False unless an asterisk is used for the channels. - `bots_only`: Whether to delete only bots. If specified, users cannot be specified. - `regex`: A regex pattern the message must contain to be deleted. + • `bots_only`: Whether to delete only bots. If specified, users cannot be specified. + • `regex`: A regex pattern the message must contain to be deleted. The pattern must be provided enclosed in backticks. If the pattern contains spaces, it still needs to be enclosed in double quotes on top of that. - `channels`: A series of channels to delete in, or an asterisk to delete from all channels. + • `channels`: A series of channels to delete in, or an asterisk to delete from all channels. """ if not any([traverse, users, first_limit, second_limit, regex, channels]): await ctx.send_help(ctx.command) From ed30eae8b29ad9863a297db541bb1b9fdaf9ab1e Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 7 Sep 2021 20:07:02 +0300 Subject: [PATCH 41/53] Fix regex search The regex was lowercased, even though regex patterns are case sensitive. Also adds the DOTALL flag. --- bot/exts/moderation/clean.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index a9f936d88f..ca458f066f 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -144,10 +144,7 @@ def predicate_regex(message: Message) -> bool: content = "\n".join(attr for attr in content if attr) # Now let's see if there's a regex match - if not content: - return False - else: - return bool(re.search(regex.lower(), content.lower())) + return bool(re.search(regex, content, re.IGNORECASE + re.DOTALL)) def predicate_range(message: Message) -> bool: """Check if the message age is between the two limits.""" From c992b6eacd47b67ba731c229ac0e6ab8df63d25f Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 7 Sep 2021 21:43:10 +0300 Subject: [PATCH 42/53] Improve responses - Tells the user if clean cancel was attempted with no ongoing clean. - Fixes MaxConcurrencyReached call bug. There was a missing argument, and it shouldn't invoke the help embed anyway, so it's now a message. - Some code refactoring. --- bot/exts/moderation/clean.py | 55 +++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index ca458f066f..7a24833fe0 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -7,10 +7,10 @@ from itertools import islice from typing import Any, Callable, DefaultDict, Iterable, Literal, Optional, TYPE_CHECKING, Union -from discord import Colour, Embed, Message, NotFound, TextChannel, User, errors +from discord import Colour, Message, NotFound, TextChannel, User, errors from discord.ext.commands import Cog, Context, Converter, Greedy, group, has_any_role from discord.ext.commands.converter import TextChannelConverter -from discord.ext.commands.errors import BadArgument, MaxConcurrencyReached +from discord.ext.commands.errors import BadArgument from bot.bot import Bot from bot.constants import ( @@ -23,6 +23,7 @@ log = logging.getLogger(__name__) DEFAULT_TRAVERSE = 10 +MESSAGE_DELETE_DELAY = 5 # Type alias for checks Predicate = Callable[[Message], bool] @@ -109,6 +110,12 @@ def _validate_input( if second_limit and not first_limit: raise ValueError("Second limit specified without the first.") + @staticmethod + async def _send_expiring_message(ctx: Context, content: str) -> None: + """Send `content` to the context channel. Automatically delete if it's not a mod channel.""" + delete_after = None if is_mod_channel(ctx.channel) else MESSAGE_DELETE_DELAY + await ctx.send(content, delete_after=delete_after) + @staticmethod def _build_predicate( bots_only: bool = False, @@ -174,6 +181,16 @@ def predicate_after(message: Message) -> bool: return predicates[0] return lambda m: all(pred(m) for pred in predicates) + async def _delete_invocation(self, ctx: Context) -> None: + """Delete the command invocation if it's not in a mod channel.""" + if not is_mod_channel(ctx.channel): + self.mod_log.ignore(Event.message_delete, ctx.message.id) + try: + await ctx.message.delete() + except errors.NotFound: + # Invocation message has already been deleted + log.info("Tried to delete invocation message, but it was already deleted.") + def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> tuple[DefaultDict, list[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) @@ -286,8 +303,7 @@ async def _modlog_cleaned_messages(self, messages: list[Message], channels: Clea """Log the deleted messages to the modlog. Return True if logging was successful.""" if not messages: # Can't build an embed, nothing to clean! - delete_after = None if is_mod_channel(ctx.channel) else 5 - await ctx.send(":x: No matching messages could be found.", delete_after=delete_after) + await self._send_expiring_message(ctx, ":x: No matching messages could be found.") return False # Reverse the list to have reverse chronological order @@ -335,7 +351,10 @@ async def _clean_messages( # Are we already performing a clean? if self.cleaning: - raise MaxConcurrencyReached("Please wait for the currently ongoing clean operation to complete.") + await self._send_expiring_message( + ctx, ":x: Please wait for the currently ongoing clean operation to complete." + ) + return self.cleaning = True # Default to using the invoking context's channel or the channel of the message limit(s). @@ -358,14 +377,8 @@ async def _clean_messages( # Needs to be called after standardizing the input. predicate = self._build_predicate(bots_only, users, regex, first_limit, second_limit) - if not is_mod_channel(ctx.channel): - # Delete the invocation first - self.mod_log.ignore(Event.message_delete, ctx.message.id) - try: - await ctx.message.delete() - except errors.NotFound: - # Invocation message has already been deleted - log.info("Tried to delete invocation message, but it was already deleted.") + # Delete the invocation first + await self._delete_invocation(ctx) if channels == "*" and use_cache: message_mappings, message_ids = self._get_messages_from_cache(traverse=traverse, to_delete=predicate) @@ -550,16 +563,14 @@ async def clean_between( @clean_group.command(name="stop", aliases=["cancel", "abort"]) async def clean_cancel(self, ctx: Context) -> None: """If there is an ongoing cleaning process, attempt to immediately cancel it.""" - self.cleaning = False + if not self.cleaning: + message = ":question: There's no cleaning going on." + else: + self.cleaning = False + message = f"{Emojis.check_mark} Clean interrupted." - embed = Embed( - color=Colour.blurple(), - description="Clean interrupted." - ) - delete_after = 10 - if is_mod_channel(ctx.channel): - delete_after = None - await ctx.send(embed=embed, delete_after=delete_after) + await self._send_expiring_message(ctx, message) + await self._delete_invocation(ctx) # endregion From 5b8e16bb9e0226e40173a84da4e103d9960b8839 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Tue, 7 Sep 2021 22:54:43 +0300 Subject: [PATCH 43/53] Fix delete order In case of old messages, it would delete the old messages first, and only then bulk delete the remainder, which affected logging. This commit corrects the deletion order. --- bot/exts/moderation/clean.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 7a24833fe0..c90aff2567 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -270,33 +270,38 @@ async def _delete_found(self, message_mappings: dict[TextChannel, list[Message]] for channel, messages in message_mappings.items(): to_delete = [] - for current_index, message in enumerate(messages): + delete_old = False + for current_index, message in enumerate(messages): # noqa: B007 if not self.cleaning: # Means that the cleaning was canceled return deleted if self.is_older_than_14d(message): - # further messages are too old to be deleted in bulk - deleted_remaining = await self._delete_messages_individually(messages[current_index:]) - deleted.extend(deleted_remaining) - if not self.cleaning: - # Means that deletion was canceled while deleting the individual messages - return deleted + # Further messages are too old to be deleted in bulk + delete_old = True break to_delete.append(message) if len(to_delete) == 100: - # we can only delete up to 100 messages in a bulk + # Only up to 100 messages can be deleted in a bulk await channel.delete_messages(to_delete) deleted.extend(to_delete) to_delete.clear() + if not self.cleaning: + return deleted if len(to_delete) > 0: - # deleting any leftover messages if there are any + # Deleting any leftover messages if there are any await channel.delete_messages(to_delete) deleted.extend(to_delete) + if not self.cleaning: + return deleted + if delete_old: + old_deleted = await self._delete_messages_individually(messages[current_index:]) + deleted.extend(old_deleted) + return deleted async def _modlog_cleaned_messages(self, messages: list[Message], channels: CleanChannels, ctx: Context) -> bool: From e33b4aad936ac052695a45f62bd986d13f2163b0 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Wed, 8 Sep 2021 00:18:23 +0300 Subject: [PATCH 44/53] Switch `users` and `traverse` in main command When providing a user ID it would clash with `traverse` which came first. --- bot/exts/moderation/clean.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index c90aff2567..5f97aae227 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -420,8 +420,8 @@ async def _clean_messages( async def clean_group( self, ctx: Context, - traverse: Optional[int] = None, users: Greedy[User] = None, + traverse: Optional[int] = None, first_limit: Optional[CleanLimit] = None, second_limit: Optional[CleanLimit] = None, use_cache: Optional[bool] = None, @@ -434,8 +434,8 @@ async def clean_group( Commands for cleaning messages in channels. If arguments are provided, will act as a master command from which all subcommands can be derived. - • `traverse`: The number of messages to look at in each channel. • `users`: A series of user mentions, ID's, or names. + • `traverse`: The number of messages to look at in each channel. • `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. If a message is provided, cleaning will happen in that channel, and channels cannot be provided. If a limit is provided, multiple channels cannot be provided. From d9efe01198ae6645d146be2b42c025a47d21bbf4 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Wed, 8 Sep 2021 02:05:22 +0300 Subject: [PATCH 45/53] Fix incorrect cache usage --- bot/exts/moderation/clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 5f97aae227..f12550ab6a 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -457,7 +457,7 @@ async def clean_group( traverse = CleanMessages.message_limit else: traverse = DEFAULT_TRAVERSE - if not use_cache: + if use_cache is None: use_cache = channels == "*" await self._clean_messages( From f4658f8468cbe055e67d795cc6aa4b171c8c0b0f Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 11 Sep 2021 20:15:21 +0300 Subject: [PATCH 46/53] Handle Regex converter errors Handle cases where there are no enclosing backticks, and where the regex pattern is invalid. --- bot/exts/moderation/clean.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index f12550ab6a..af79d5a35c 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -44,16 +44,21 @@ async def convert(self, ctx: Context, argument: str) -> Union[Literal["*"], list class Regex(Converter): - """A converter that takes a string in the form `.+` and returns the contents of the inline code.""" + """A converter that takes a string in the form `.+` and returns the contents of the inline code compiled.""" - async def convert(self, ctx: Context, argument: str) -> str: - """Strips the backticks from the string.""" - return re.fullmatch(r"`(.+?)`", argument).group(1) + async def convert(self, ctx: Context, argument: str) -> re.Pattern: + """Strips the backticks from the string and compiles it to a regex pattern.""" + if not (match := re.fullmatch(r"`(.+?)`", argument)): + raise BadArgument("Regex pattern missing wrapping backticks") + try: + return re.compile(match.group(1), re.IGNORECASE + re.DOTALL) + except re.error as e: + raise BadArgument(f"Regex error: {e.msg}") if TYPE_CHECKING: CleanChannels = Union[Literal["*"], list[TextChannel]] # noqa: F811 - Regex = str # noqa: F811 + Regex = re.Pattern # noqa: F811 class Clean(Cog): @@ -120,7 +125,7 @@ async def _send_expiring_message(ctx: Context, content: str) -> None: def _build_predicate( bots_only: bool = False, users: list[User] = None, - regex: Optional[str] = None, + regex: Optional[re.Pattern] = None, first_limit: Optional[datetime] = None, second_limit: Optional[datetime] = None, ) -> Predicate: @@ -151,7 +156,7 @@ def predicate_regex(message: Message) -> bool: content = "\n".join(attr for attr in content if attr) # Now let's see if there's a regex match - return bool(re.search(regex, content, re.IGNORECASE + re.DOTALL)) + return bool(regex.search(content)) def predicate_range(message: Message) -> bool: """Check if the message age is between the two limits.""" @@ -346,7 +351,7 @@ async def _clean_messages( channels: CleanChannels, bots_only: bool = False, users: list[User] = None, - regex: Optional[str] = None, + regex: Optional[re.Pattern] = None, first_limit: Optional[CleanLimit] = None, second_limit: Optional[CleanLimit] = None, use_cache: Optional[bool] = True From e215fb03552f822e6e702e741956ee17d07f6117 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Sat, 11 Sep 2021 20:34:50 +0300 Subject: [PATCH 47/53] End clean on unexpected errors Added a cog_command_error method that sets cleaning to False when a command ends on an exception. I don't have anything in mind that might cause this, but it will ensure that in any case the cog will still be usable. --- bot/exts/moderation/clean.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index af79d5a35c..3fb2c2870c 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -588,6 +588,10 @@ async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" return await has_any_role(*MODERATION_ROLES).predicate(ctx) + async def cog_command_error(self, ctx: Context, error: Exception) -> None: + """Safely end the cleaning operation on unexpected errors.""" + self.cleaning = False + def setup(bot: Bot) -> None: """Load the Clean cog.""" From c27226504b5d384023074eb070e37464b6c8749a Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 20 Sep 2021 23:11:01 +0300 Subject: [PATCH 48/53] Indentation, type-hint, and documentation fixes --- bot/exts/moderation/clean.py | 63 ++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 3fb2c2870c..d5bfdb485b 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -5,7 +5,7 @@ from contextlib import suppress from datetime import datetime from itertools import islice -from typing import Any, Callable, DefaultDict, Iterable, Literal, Optional, TYPE_CHECKING, Union +from typing import Any, Callable, Iterable, Literal, Optional, TYPE_CHECKING, Union from discord import Colour, Message, NotFound, TextChannel, User, errors from discord.ext.commands import Cog, Context, Converter, Greedy, group, has_any_role @@ -86,11 +86,11 @@ def mod_log(self) -> ModLog: @staticmethod def _validate_input( traverse: int, - channels: CleanChannels, + channels: Optional[CleanChannels], bots_only: bool, - users: list[User], - first_limit: CleanLimit, - second_limit: CleanLimit, + users: Optional[list[User]], + first_limit: Optional[CleanLimit], + second_limit: Optional[CleanLimit], ) -> None: """Raise errors if an argument value or a combination of values is invalid.""" # Is this an acceptable amount of messages to traverse? @@ -124,7 +124,7 @@ async def _send_expiring_message(ctx: Context, content: str) -> None: @staticmethod def _build_predicate( bots_only: bool = False, - users: list[User] = None, + users: Optional[list[User]] = None, regex: Optional[re.Pattern] = None, first_limit: Optional[datetime] = None, second_limit: Optional[datetime] = None, @@ -196,7 +196,7 @@ async def _delete_invocation(self, ctx: Context) -> None: # Invocation message has already been deleted log.info("Tried to delete invocation message, but it was already deleted.") - def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> tuple[DefaultDict, list[int]]: + def _get_messages_from_cache(self, traverse: int, to_delete: Predicate) -> tuple[defaultdict[Any, list], list[int]]: """Helper function for getting messages from the cache.""" message_mappings = defaultdict(list) message_ids = [] @@ -348,9 +348,9 @@ async def _clean_messages( self, ctx: Context, traverse: int, - channels: CleanChannels, + channels: Optional[CleanChannels], bots_only: bool = False, - users: list[User] = None, + users: Optional[list[User]] = None, regex: Optional[re.Pattern] = None, first_limit: Optional[CleanLimit] = None, second_limit: Optional[CleanLimit] = None, @@ -423,24 +423,25 @@ async def _clean_messages( @group(invoke_without_command=True, name="clean", aliases=["clear", "purge"]) async def clean_group( - self, - ctx: Context, - users: Greedy[User] = None, - traverse: Optional[int] = None, - first_limit: Optional[CleanLimit] = None, - second_limit: Optional[CleanLimit] = None, - use_cache: Optional[bool] = None, - bots_only: Optional[bool] = False, - regex: Optional[Regex] = None, - *, - channels: CleanChannels = None # "Optional" with discord.py silently ignores incorrect input. + self, + ctx: Context, + users: Greedy[User] = None, + traverse: Optional[int] = None, + first_limit: Optional[CleanLimit] = None, + second_limit: Optional[CleanLimit] = None, + use_cache: Optional[bool] = None, + bots_only: Optional[bool] = False, + regex: Optional[Regex] = None, + *, + channels: CleanChannels = None # "Optional" with discord.py silently ignores incorrect input. ) -> None: """ Commands for cleaning messages in channels. If arguments are provided, will act as a master command from which all subcommands can be derived. • `users`: A series of user mentions, ID's, or names. - • `traverse`: The number of messages to look at in each channel. + • `traverse`: The number of messages to look at in each channel. If using the cache, will look at the first + `traverse` messages in the cache. • `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. If a message is provided, cleaning will happen in that channel, and channels cannot be provided. If a limit is provided, multiple channels cannot be provided. @@ -474,7 +475,7 @@ async def clean_user( self, ctx: Context, user: User, - traverse: Optional[int] = 10, + traverse: Optional[int] = DEFAULT_TRAVERSE, use_cache: Optional[bool] = True, *, channels: CleanChannels = None @@ -527,10 +528,10 @@ async def clean_regex( @clean_group.command(name="until") async def clean_until( - self, - ctx: Context, - until: CleanLimit, - channel: TextChannel = None + self, + ctx: Context, + until: CleanLimit, + channel: TextChannel = None ) -> None: """ Delete all messages until a certain limit. @@ -547,11 +548,11 @@ async def clean_until( @clean_group.command(name="between", aliases=["after-until", "from-to"]) async def clean_between( - self, - ctx: Context, - first_limit: CleanLimit, - second_limit: CleanLimit, - channel: TextChannel = None + self, + ctx: Context, + first_limit: CleanLimit, + second_limit: CleanLimit, + channel: TextChannel = None ) -> None: """ Delete all messages within range. From aa666737ba0bf3cfcd58a4c9b782382d342632fe Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 25 Oct 2021 21:55:16 +0300 Subject: [PATCH 49/53] Adjust docstring to #1876 --- bot/exts/moderation/clean.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index d5bfdb485b..c01430a043 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -13,9 +13,7 @@ from discord.ext.commands.errors import BadArgument from bot.bot import Bot -from bot.constants import ( - Channels, CleanMessages, Colours, Emojis, Event, Icons, MODERATION_ROLES -) +from bot.constants import Channels, CleanMessages, Colours, Emojis, Event, Icons, MODERATION_ROLES from bot.converters import Age, ISODateTime from bot.exts.moderation.modlog import ModLog from bot.utils.channel import is_mod_channel @@ -439,20 +437,21 @@ async def clean_group( Commands for cleaning messages in channels. If arguments are provided, will act as a master command from which all subcommands can be derived. - • `users`: A series of user mentions, ID's, or names. - • `traverse`: The number of messages to look at in each channel. If using the cache, will look at the first - `traverse` messages in the cache. - • `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. + + \u2003• `users`: A series of user mentions, ID's, or names. + \u2003• `traverse`: The number of messages to look at in each channel. If using the cache, will look at the + first `traverse` messages in the cache. + \u2003• `first_limit` and `second_limit`: A message, a duration delta, or an ISO datetime. If a message is provided, cleaning will happen in that channel, and channels cannot be provided. If a limit is provided, multiple channels cannot be provided. If only one of them is provided, acts as `clean until`. If both are provided, acts as `clean between`. - • `use_cache`: Whether to use the message cache. + \u2003• `use_cache`: Whether to use the message cache. If not provided, will default to False unless an asterisk is used for the channels. - • `bots_only`: Whether to delete only bots. If specified, users cannot be specified. - • `regex`: A regex pattern the message must contain to be deleted. + \u2003• `bots_only`: Whether to delete only bots. If specified, users cannot be specified. + \u2003• `regex`: A regex pattern the message must contain to be deleted. The pattern must be provided enclosed in backticks. If the pattern contains spaces, it still needs to be enclosed in double quotes on top of that. - • `channels`: A series of channels to delete in, or an asterisk to delete from all channels. + \u2003• `channels`: A series of channels to delete in, or an asterisk to delete from all channels. """ if not any([traverse, users, first_limit, second_limit, regex, channels]): await ctx.send_help(ctx.command) @@ -521,7 +520,7 @@ async def clean_regex( Delete all messages that match a certain regex, stop cleaning after traversing `traverse` messages. The pattern must be provided enclosed in backticks. - If the pattern contains spaces, and still needs to be enclosed in double quotes on top of that. + If the pattern contains spaces, it still needs to be enclosed in double quotes on top of that. For example: `[0-9]` """ await self._clean_messages(ctx, traverse, regex=regex, channels=channels, use_cache=use_cache) From b42f148955600d85260c43c50260333fe62b823e Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 25 Oct 2021 22:00:58 +0300 Subject: [PATCH 50/53] Apply requested style changes --- bot/exts/moderation/clean.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index c01430a043..65ffec88be 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -1,3 +1,4 @@ +import contextlib import logging import re import time @@ -46,7 +47,8 @@ class Regex(Converter): async def convert(self, ctx: Context, argument: str) -> re.Pattern: """Strips the backticks from the string and compiles it to a regex pattern.""" - if not (match := re.fullmatch(r"`(.+?)`", argument)): + match = re.fullmatch(r"`(.+?)`", argument) + if not match: raise BadArgument("Regex pattern missing wrapping backticks") try: return re.compile(match.group(1), re.IGNORECASE + re.DOTALL) @@ -252,12 +254,8 @@ async def _delete_messages_individually(self, messages: list[Message]) -> list[M # Ensure that deletion was not canceled if not self.cleaning: return deleted - try: + with contextlib.suppress(NotFound): # Message doesn't exist or was already deleted await message.delete() - except NotFound: - # Message doesn't exist or was already deleted - continue - else: deleted.append(message) return deleted From cae048338aa31a6c9c12a75e2f7f1674d817ce7f Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 25 Oct 2021 22:07:22 +0300 Subject: [PATCH 51/53] Improve documentation of global variables --- bot/exts/moderation/clean.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 65ffec88be..9001b4fe27 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -21,12 +21,14 @@ log = logging.getLogger(__name__) +# Default number of messages to look at in each channel. DEFAULT_TRAVERSE = 10 +# Number of seconds before command invocations and responses are deleted in non-moderation channels. MESSAGE_DELETE_DELAY = 5 -# Type alias for checks +# Type alias for checks for whether a message should be deleted. Predicate = Callable[[Message], bool] - +# Type alias for message lookup ranges. CleanLimit = Union[Message, Age, ISODateTime] @@ -56,7 +58,7 @@ async def convert(self, ctx: Context, argument: str) -> re.Pattern: raise BadArgument(f"Regex error: {e.msg}") -if TYPE_CHECKING: +if TYPE_CHECKING: # Used to allow method resolution in IDEs like in converters.py. CleanChannels = Union[Literal["*"], list[TextChannel]] # noqa: F811 Regex = re.Pattern # noqa: F811 From 37b7a3b5f6424039f11d4ee8d6f087568ebded16 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 25 Oct 2021 22:16:17 +0300 Subject: [PATCH 52/53] Update Age converter to use TZ aware datetime --- bot/converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/converters.py b/bot/converters.py index 0cd06bf5eb..0984fa0a34 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -405,7 +405,7 @@ async def convert(self, ctx: Context, duration: str) -> datetime: The converter supports the same symbols for each unit of time as its parent class. """ delta = await super().convert(ctx, duration) - now = datetime.utcnow() + now = datetime.now(timezone.utc) try: return now - delta From d19824d76c0cbe793b387002bcf1c6932579a668 Mon Sep 17 00:00:00 2001 From: mbaruh Date: Mon, 25 Oct 2021 22:35:12 +0300 Subject: [PATCH 53/53] Remove channel limitation with time range Discussion in the pull request raised some legitimate use cases for supplying a time range for multiple channels (e.g clean the last couple of minutes instead of specifying number of messages to traverse). --- bot/exts/moderation/clean.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bot/exts/moderation/clean.py b/bot/exts/moderation/clean.py index 9001b4fe27..94494b9837 100644 --- a/bot/exts/moderation/clean.py +++ b/bot/exts/moderation/clean.py @@ -99,9 +99,6 @@ def _validate_input( if traverse > CleanMessages.message_limit: raise BadArgument(f"Cannot traverse more than {CleanMessages.message_limit} messages.") - if first_limit and channels and (channels == "*" or len(channels) > 1): - raise BadArgument("Message or time range specified across multiple channels.") - if (isinstance(first_limit, Message) or isinstance(second_limit, Message)) and channels: raise BadArgument("Both a message limit and channels specified.")