[V3 Mod/ModLog] Fix duplicate cases and allow case creation without audit log perms (#1102)

* Use ban/unban queue

* Refactor unban's cmd help

* Better support for no audit log perms

redbot/cogs/mod/mod.py

@@ -1,4 +1,5 @@
 import asyncio
+from datetime import datetime
 from collections import deque, defaultdict
 
 import discord
@@ -51,8 +52,8 @@ class Mod:
         self.settings.register_channel(**self.default_channel_settings)
         self.settings.register_member(**self.default_member_settings)
         self.settings.register_user(**self.default_user_settings)
-        self.current_softban = {}
+        self.ban_queue = []
-        self.ban_type = None
+        self.unban_queue = []
         self.cache = defaultdict(lambda: deque(maxlen=3))
 
         self.bot.loop.create_task(self._casetype_registration())
@@ -355,14 +356,17 @@ class Mod:
         if days < 0 or days > 7:
             await ctx.send(_("Invalid days. Must be between 0 and 7."))
             return
-
+        queue_entry = (guild.id, user.id)
+        self.ban_queue.append(queue_entry)
         try:
             await guild.ban(user, reason=audit_reason, delete_message_days=days)
             log.info("{}({}) banned {}({}), deleting {} days worth of messages".format(
                 author.name, author.id, user.name, user.id, str(days)))
         except discord.Forbidden:
+            self.ban_queue.remove(queue_entry)
             await ctx.send(_("I'm not allowed to do that."))
         except Exception as e:
+            self.ban_queue.remove(queue_entry)
             print(e)
         else:
             await ctx.send(_("Done. It was about time."))
@@ -402,15 +406,18 @@ class Mod:
             user = discord.Object(id=user_id)  # User not in the guild, but
 
         audit_reason = get_audit_reason(author, reason)
-
+        queue_entry = (guild.id, user_id)
+        self.ban_queue.append(queue_entry)
         try:
             await guild.ban(user, reason=audit_reason)
             log.info("{}({}) hackbanned {}"
                      "".format(author.name, author.id, user_id))
         except discord.NotFound:
+            self.ban_queue.remove(queue_entry)
             await ctx.send(_("User not found. Have you provided the "
                              "correct user ID?"))
         except discord.Forbidden:
+            self.ban_queue.remove(queue_entry)
             await ctx.send(_("I lack the permissions to do this."))
         else:
             await ctx.send(_("Done. The user will not be able to join this "
@@ -452,7 +459,7 @@ class Mod:
             invite = ""
 
         if can_ban:
-            self.current_softban[str(guild.id)] = user
+            queue_entry = (guild.id, user.id)
             try:  # We don't want blocked DMs preventing us from banning
                 msg = await user.send(
                     _("You have been banned and "
@@ -460,17 +467,26 @@ class Mod:
                       "You can now join the guild again. {}").format(invite))
             except discord.HTTPException:
                 msg = None
+            self.ban_queue.append(queue_entry)
             try:
                 await guild.ban(
                     user, reason=audit_reason, delete_message_days=1)
-                await guild.unban(user)
             except discord.errors.Forbidden:
+                self.ban_queue.remove(queue_entry)
                 await ctx.send(
                     _("My role is not high enough to softban that user."))
                 if msg is not None:
                     await msg.delete()
                 return
             except discord.HTTPException as e:
+                self.ban_queue.remove(queue_entry)
+                print(e)
+                return
+            self.unban_queue.append(queue_entry)
+            try:
+                await guild.unban(user)
+            except discord.HTTPException as e:
+                self.unban_queue.remove(queue_entry)
                 print(e)
                 return
             else:
@@ -490,9 +506,6 @@ class Mod:
                         channel=None)
                 except RuntimeError as e:
                     await ctx.send(e)
-            finally:
-                await asyncio.sleep(5)
-                self.current_softban = None
         else:
             await ctx.send(_("I'm not allowed to do that."))
 
@@ -501,11 +514,12 @@ class Mod:
     @checks.admin_or_permissions(ban_members=True)
     @commands.bot_has_permissions(ban_members=True)
     async def unban(self, ctx: RedContext, user_id: int, *, reason: str = None):
-        """Unbans the target user. Requires specifying the target user's ID
+        """Unbans the target user.
-        (which can be found in the mod log channel (if logging was enabled for
+        
-        the casetype associated with the command used to ban the user) or (if
+        Requires specifying the target user's ID. To find this, you may either:
-        developer mode is enabled) by looking in Bans in guild settings,
+         1. Copy it from the mod log case (if one was created), or
-        finding the user, right-clicking, and selecting 'Copy ID'"""
+         2. enable developer mode, go to Bans in this server's settings, right-
+        click the user and select 'Copy ID'."""
         guild = ctx.guild
         author = ctx.author
         user = self.bot.get_user_info(user_id)
@@ -518,10 +532,12 @@ class Mod:
         if user not in bans:
             await ctx.send(_("It seems that user isn't banned!"))
             return
-
+        queue_entry = (guild.id, user.id)
+        self.unban_queue.append(queue_entry)
         try:
             await guild.unban(user, reason=reason)
         except discord.HTTPException:
+            self.unban_queue.remove(queue_entry)
             await ctx.send(_("Something went wrong while attempting to unban that user"))
             return
         else:
@@ -1149,130 +1165,100 @@ class Mod:
             deleted = await self.check_mention_spam(message)
 
     async def on_member_ban(self, guild: discord.Guild, member: discord.Member):
-        if str(guild.id) in self.current_softban and \
+        if (guild.id, member.id) in self.ban_queue:
-                self.current_softban[str(guild.id)] == member:
+            self.ban_queue.remove((guild.id, member.id))
-            return  # softban in progress, so a case will be created
+            return
         try:
-            mod_ch = await modlog.get_modlog_channel(guild)
+            await modlog.get_modlog_channel(guild)
         except RuntimeError:
             return  # No modlog channel so no point in continuing
-        audit_case = None
+        mod, reason, date = await self.get_audit_entry_info(
-        permissions = guild.me.guild_permissions
+            guild, discord.AuditLogAction.ban, member)
-        modlog_cases = await modlog.get_all_cases(guild, self.bot)
+        if date is None:
-        if permissions.view_audit_log:
+            date = datetime.now()
-            async for entry in guild.audit_logs(action=discord.AuditLogAction.ban):
-                if entry.target == member:
-                    audit_case = entry
-                    break
-
-            if audit_case:
-                mod = audit_case.user
-                reason = audit_case.reason
-                for case in sorted(modlog_cases, key=lambda x: x.case_number, reverse=True):
-                    if mod == guild.me and case.user == member\
-                            and case.action_type in ["ban", "hackban"]:
-                        log.info("Case already exists for ban of {}".format(member.name))
-                        break
-                else:  # no ban, softban, or hackban case with the mod and user combo
         try:
-                        await modlog.create_case(guild, audit_case.created_at, "ban",
+            await modlog.create_case(guild, date,
-                                                member, mod, reason if reason else None)
+                                     "ban", member, mod,
+                                     reason if reason else None)
         except RuntimeError as e:
             print(e)
-            else:
-                return
-        else:  # No permissions to view audit logs, so message the guild owner
-            owner = guild.owner
-            try:
-                await owner.send(
-                    _("Hi, I noticed that someone in your server "
-                    "(the server named {}) banned {}#{} (user ID {}). "
-                    "However, I don't have permissions to view audit logs, "
-                    "so I could not determine if a mod log case was created "
-                    "for that ban, meaning I could not create a case in "
-                    "the mod log. If you want me to be able to add cases "
-                    "to the mod log for bans done manually, I need the "
-                    "`View Audit Logs` permission.").format(
-                        guild.name,
-                        member.name,
-                        member.discriminator,
-                        member.id
-                    )
-                )
-            except discord.Forbidden:
-                log.warning(
-                    "I attempted to inform a guild owner of a lack of the "
-                    "'View Audit Log' permission but I am unable to send "
-                    "the guild owner the message!"
-                )
-            except discord.HTTPException:
-                log.warning(
-                    "Something else went wrong while attempting to "
-                    "message a guild owner."
-                )
 
     async def on_member_unban(self, guild: discord.Guild, user: discord.User):
-        if str(guild.id) in self.current_softban and \
+        if (guild.id, user.id) in self.unban_queue:
-                self.current_softban[str(guild.id)] == user:
+            self.unban_queue.remove((guild.id, user.id))
-            return  # softban in progress, so a case will be created
+            return
         try:
-            mod_ch = await modlog.get_modlog_channel(guild)
+            await modlog.get_modlog_channel(guild)
         except RuntimeError:
             return  # No modlog channel so no point in continuing
-        audit_case = None
+        mod, reason, date = await self.get_audit_entry_info(
-        permissions = guild.me.guild_permissions
+            guild, discord.AuditLogAction.unban, user)
-        if permissions.view_audit_log:
+        if date is None:
-            async for entry in guild.audit_logs(action=discord.AuditLogAction.unban):
+            date = datetime.now()
-                if entry.target == user:
-                    audit_case = entry
-                    break
-            else:
-                return
-            if audit_case:
-                mod = audit_case.user
-                reason = audit_case.reason
-
-                cases = await modlog.get_all_cases(guild, self.bot)
-                for case in sorted(cases, key=lambda x: x.case_number, reverse=True):
-                    if mod == guild.me and case.user == user\
-                            and case.action_type == "unban":
-                        log.info("Case already exists for unban of {}".format(user.name))
-                        break
-                else:
         try:
-                        await modlog.create_case(guild, audit_case.created_at, "unban",
+            await modlog.create_case(guild, date, "unban",
-                                                 user, mod, reason if reason else None)
+                                     user, mod, reason)
         except RuntimeError as e:
             print(e)
-        else:  # No permissions to view audit logs, so message the guild owner
+
-            owner = guild.owner
+    async def get_audit_entry_info(self,
+                                   guild: discord.Guild,
+                                   action: int,
+                                   target):
+        """Get info about an audit log entry.
+
+        Parameters
+        ----------
+        guild : discord.Guild
+            Same as ``guild`` in `get_audit_log_entry`.
+        action : int
+            Same as ``action`` in `get_audit_log_entry`.
+        target : `discord.User` or `discord.Member`
+            Same as ``target`` in `get_audit_log_entry`.
+
+        Returns
+        -------
+        tuple
+            A tuple in the form``(mod: discord.Member, reason: str,
+            date_created: datetime.datetime)``. Returns ``(None, None, None)``
+            if the audit log entry could not be found.
+        """
         try:
-                await owner.send(
+            entry = await self.get_audit_log_entry(
-                    _("Hi, I noticed that someone in your server "
+                guild, action=action, target=target)
-                    "(the server named {}) unbanned {}#{} (user ID {}). "
-                    "However, I don't have permissions to view audit logs, "
-                    "so I could not determine if a mod log case was created "
-                    "for that unban, meaning I could not create a case in "
-                    "the mod log. If you want me to be able to add cases "
-                    "to the mod log for unbans done manually, I need the "
-                    "`View Audit Logs` permission.").format(
-                        guild.name,
-                        user.name,
-                        user.discriminator,
-                        user.id
-                    )
-                )
-            except discord.Forbidden:
-                log.warning(
-                    "I attempted to inform a guild owner of a lack of the "
-                    "'View Audit Log' permission but I am unable to send "
-                    "the guild owner the message!"
-                )
         except discord.HTTPException:
-                log.warning(
+            entry = None
-                    "Something else went wrong while attempting to "
+        if entry is None:
-                    "message a guild owner."
+            return None, None, None
-                )
+        return entry.user, entry.reason, entry.created_at
+
+    async def get_audit_log_entry(self,
+                                  guild: discord.Guild,
+                                  action: int,
+                                  target):
+        """Get an audit log entry.
+
+        Any exceptions encountered when looking through the audit log will be
+        propogated out of this function.
+
+        Parameters
+        ----------
+        guild : discord.Guild
+            The guild for the audit log.
+        action : int
+            The audit log action (see `discord.AuditLogAction`).
+        target : `discord.Member` or `discord.User`
+            The target of the audit log action.
+
+        Returns
+        -------
+        discord.AuditLogEntry
+            The audit log entry. Returns ``None`` if not found.
+
+        """
+        async for entry in guild.audit_logs(action=action):
+            if entry.target == target:
+                return entry
 
     async def on_member_update(self, before, after):
         if before.name != after.name:

redbot/core/modlog.py

@@ -107,7 +107,7 @@ class Case:
         user = "{}#{} ({})\n".format(
             self.user.name, self.user.discriminator, self.user.id)
         emb.set_author(name=user, icon_url=self.user.avatar_url)
-
+        if self.moderator is not None:
             moderator = "{}#{} ({})\n".format(
                 self.moderator.name,
                 self.moderator.discriminator,
@@ -153,13 +153,17 @@ class Case:
             The case in the form of a dict
 
         """
+        if self.moderator is not None:
+            mod = self.moderator.id
+        else:
+            mod = None
         data = {
             "case_number": self.case_number,
             "action_type": self.action_type,
             "guild": self.guild.id,
             "created_at": self.created_at,
             "user": self.user.id,
-            "moderator": self.moderator.id,
+            "moderator": mod,
             "reason": self.reason,
             "until": self.until,
             "channel": self.channel.id if hasattr(self.channel, "id") else None,
@@ -373,7 +377,7 @@ async def get_all_cases(guild: discord.Guild, bot: Red) -> List[Case]:
 
 async def create_case(guild: discord.Guild, created_at: datetime, action_type: str,
                       user: Union[discord.User, discord.Member],
-                      moderator: discord.Member, reason: str=None,
+                      moderator: discord.Member=None, reason: str=None,
                       until: datetime=None, channel: discord.TextChannel=None
                       ) -> Union[Case, None]:
     """