Really postpone deleting old messages to the end

Author: mbaruh

bot/exts/moderation/clean.py

@@ -307,14 +307,16 @@ async def _delete_messages_individually(self, channel_messages: dict[TextChannel
                     deleted.append(message)
         return deleted
 
-    async def _delete_found(
+    async def _delete_bulk(
         self, message_mappings: dict[TextChannel, list[Message]], executor:AsyncExecutor
-    ) -> list[Message]:
+    ) -> tuple[list[Message], dict[TextChannel, 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.
+
+        The function returns the deleted messages. Additionally, messages older than 14d are returned to
+        be deleted separately.
         If cleaning was cancelled in the middle, return messages already deleted.
         """
         deleted = []
@@ -325,7 +327,7 @@ async def _delete_found(
             for current_index, message in enumerate(messages):
                 if not self.cleaning:
                     # Means that the cleaning was canceled
-                    return deleted
+                    return deleted, {}
 
                 if self.is_older_than_14d(message):
                     # Further messages are too old to be deleted in bulk
@@ -341,21 +343,15 @@ async def _delete_found(
                     to_delete = []
 
             if not self.cleaning:
-                return deleted
+                return deleted, {}
             if len(to_delete) > 0:
                 # Deleting any leftover messages if there are any
                 executor.submit(channel.delete_messages(to_delete))
                 deleted.extend(to_delete)
 
         await executor.gather(return_exceptions=True)
 
-        if old_messages:
-            if not self.cleaning:
-                return deleted
-            old_deleted = await self._delete_messages_individually(old_messages)
-            deleted.extend(old_deleted)
-
-        return deleted
+        return deleted, old_messages
 
     async def _modlog_cleaned_messages(
         self,
@@ -440,14 +436,15 @@ async def _clean_messages(
             executor.submit(self._delete_invocation(ctx))
 
         deleted_messages = []
+        old_messages = {channel: [] for channel in deletion_channels}
 
         if self._use_cache(second_limit):
             log.trace(f"Messages for cleaning by {ctx.author.id} will be searched in the cache.")
             message_mappings, message_ids = self._get_messages_from_cache(
                 channels=deletion_channels, to_delete=predicate, lower_limit=first_limit
             )
             self.mod_log.ignore(Event.message_delete, *message_ids)
-            deleted_messages = await self._delete_found(message_mappings, executor)
+            deleted_messages, old_messages = await self._delete_bulk(message_mappings, executor)
             second_limit = self._earliest_cache_datetime()
 
         if self._use_api(first_limit):
@@ -459,12 +456,17 @@ async def _clean_messages(
                 before=second_limit
             )
             self.mod_log.ignore(Event.message_delete, *message_ids)
-            api_deleted_messages = await self._delete_found(message_mappings, executor)
+            api_deleted_messages, api_old_messages = await self._delete_bulk(message_mappings, executor)
             deleted_messages.extend(api_deleted_messages)
+            for channel, messages in api_old_messages.items():
+                old_messages[channel].extend(messages)
 
         if not self.cleaning:
             # Means that the cleaning was canceled
             return None
+        if old_messages:
+            old_deleted = await self._delete_messages_individually(old_messages)
+            deleted_messages.extend(old_deleted)
 
         self.cleaning = False
         log.trace("Cleaning completed, wrapping up")

tests/bot/exts/moderation/test_clean.py

@@ -1,6 +1,8 @@
 import unittest
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import arrow
+
 from bot.exts.moderation.clean import Clean
 from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockMessage, MockRole, MockTextChannel
 
@@ -20,7 +22,8 @@ def setUp(self):
         self.cog._modlog_cleaned_messages = AsyncMock(return_value=self.log_url)
 
         self.cog._use_cache = MagicMock(return_value=True)
-        self.cog._delete_found = AsyncMock(return_value=[42, 84])
+        self.cog._use_api = MagicMock(return_value=False)
+        self.cog._delete_bulk = AsyncMock(return_value=([42, 84], {}))
 
     @patch("bot.exts.moderation.clean.is_mod_channel")
     async def test_clean_deletes_invocation_in_non_mod_channel(self, mod_channel_check):
@@ -51,7 +54,7 @@ async def test_clean_doesnt_attempt_deletion_when_attempt_delete_invocation_is_f
             await self.cog._clean_messages(
                 self.ctx,
                 None,
-                first_limit=MockMessage(),
+                first_limit=arrow.utcnow().datetime,
                 attempt_delete_invocation=False,
             ),
             self.log_url,