Addressing Review Changes

- Changed docstring explanation and function name of `get_active_infractions` to `get_active_infraction()` to better convey that only one infraction is returned.  Also changed all relevant uses to reflect that change.

- Added explanation of parameter `send_msg` to the doc strings of `pardon_infraction()` and `get_active_infraction()`

- Adjusted placement of `log.trace()` in `pardon_infraction()`

- Adjusted logic in `apply_ban()` to remove redundant check.

- Adjusted logic in `apply_ban()` to be consistent with other checks.

Signed-off-by: Daniel Brown <[email protected]>

4 files changed, 31 insertions, 25 deletions

diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py
index 29b4db20e..89f72ade7 100644
--- a/bot/cogs/moderation/infractions.py
+++ b/bot/cogs/moderation/infractions.py
@@ -199,7 +199,7 @@ class Infractions(InfractionScheduler, commands.Cog):
 
     async def apply_mute(self, ctx: Context, user: Member, reason: str, **kwargs) -> None:
         """Apply a mute infraction with kwargs passed to `post_infraction`."""
-        if await utils.get_active_infractions(ctx, user, "mute"):
+        if await utils.get_active_infraction(ctx, user, "mute"):
             return
 
         infraction = await utils.post_infraction(ctx, user, "mute", reason, active=True, **kwargs)
@@ -236,28 +236,23 @@ class Infractions(InfractionScheduler, commands.Cog):
         Will also remove the banned user from the Big Brother watch list if applicable.
         """
         # In the case of a permanent ban, we don't need get_active_infractions to tell us if one is active
-        send_msg = "expires_at" in kwargs
-        active_infraction = await utils.get_active_infractions(ctx, user, "ban", send_msg)
+        send_msg = kwargs.get("expires_at") is None
+        active_infraction = await utils.get_active_infraction(ctx, user, "ban", send_msg)
 
         if active_infraction:
             log.trace("Active infractions found.")
-            if (
-                active_infraction.get('expires_at') is not None
-                and kwargs.get('expires_at') is None
-            ):
-                log.trace("Active ban is a temporary and being called by a perma.  Removing temporary.")
-                await self.pardon_infraction(ctx, "ban", user, send_msg)
-
-            elif (
-                active_infraction.get('expires_at') is None
-                and kwargs.get('expires_at') is None
-            ):
-                log.trace("Active ban is a perma ban and being called by a perma.  Send bounce back message.")
-                await ctx.send(
-                    f":x: According to my records, this user is already permanently banned. "
-                    f"See infraction **#{active_infraction['id']}**."
-                )
-                return
+            if kwargs.get('expires_at') is None:
+                if active_infraction.get('expires_at') is not None:
+                    log.trace("Active ban is a temporary and being called by a perma.  Removing temporary.")
+                    await self.pardon_infraction(ctx, "ban", user, send_msg)
+
+                elif active_infraction.get('expires_at') is None:
+                    log.trace("Active ban is a perma ban and being called by a perma.  Send bounce back message.")
+                    await ctx.send(
+                        f":x: According to my records, this user is already permanently banned. "
+                        f"See infraction **#{active_infraction['id']}**."
+                    )
+                    return
             else:
                 log.trace("Active ban is a temp ban being called by a temp or a perma being called by a temp. Ignore.")
                 return
diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index 413717fb6..dc42bee2e 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -197,7 +197,12 @@ class InfractionScheduler(Scheduler):
             user: UserSnowflake,
             send_msg: bool = True
     ) -> None:
-        """Prematurely end an infraction for a user and log the action in the mod log."""
+        """
+        Prematurely end an infraction for a user and log the action in the mod log.
+
+        If `send_msg` is True, then a pardoning confirmation message will be sent to
+        the context channel.  Otherwise, no such message will be sent.
+        """
         log.trace(f"Pardoning {infr_type} infraction for {user}.")
 
         # Check the current active infraction
@@ -282,8 +287,8 @@ class InfractionScheduler(Scheduler):
             log.info(f"Pardoned {infr_type} infraction #{id_} for {user}.")
 
         # Send a confirmation message to the invoking context.
-        log.trace(f"Sending infraction #{id_} pardon confirmation message.")
         if send_msg:
+            log.trace(f"Sending infraction #{id_} pardon confirmation message.")
             await ctx.send(
                 f"{dm_emoji}{confirm_msg} infraction **{infr_type}** for {user.mention}. "
                 f"{log_text.get('Failure', '')}"
diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py
index 272f7c4f0..29855c325 100644
--- a/bot/cogs/moderation/superstarify.py
+++ b/bot/cogs/moderation/superstarify.py
@@ -130,7 +130,7 @@ class Superstarify(InfractionScheduler, Cog):
         An optional reason can be provided. If no reason is given, the original name will be shown
         in a generated reason.
         """
-        if await utils.get_active_infractions(ctx, member, "superstar"):
+        if await utils.get_active_infraction(ctx, member, "superstar"):
             return
 
         # Post the infraction to the API
diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py
index 406f9d08a..e4e0f1ec2 100644
--- a/bot/cogs/moderation/utils.py
+++ b/bot/cogs/moderation/utils.py
@@ -97,13 +97,19 @@ async def post_infraction(
                 return
 
 
-async def get_active_infractions(
+async def get_active_infraction(
         ctx: Context,
         user: UserSnowflake,
         infr_type: str,
         send_msg: bool = True
 ) -> t.Optional[dict]:
-    """Retrieves active infractions of the given type for the user."""
+    """
+    Retrieves an active infraction of the given type for the user.
+
+    If `send_msg` is True and the user has an active infraction matching the `infr_type` parameter,
+    then a message for the moderator will be sent to the context channel letting them know.
+    Otherwise, no message will be sent.
+    """
     log.trace(f"Checking if {user} has active infractions of type {infr_type}.")
 
     active_infractions = await ctx.bot.api_client.get(