Merge infraction edit commands

Both the duration and the reason can be edited with the new command.

* Remove try-except; the default error handler is already adequate
* Remove the new reason from the confirmation message
* Simplify humanisation of the timestamp in the confirmation message
* Add a converter to support permanent durations

1 files changed, 62 insertions, 108 deletions

diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py
index 2ad5b96f4..2b3a21dc9 100644
--- a/bot/cogs/moderation.py
+++ b/bot/cogs/moderation.py
@@ -44,6 +44,15 @@ def proxy_user(user_id: str) -> Object:
     return user
 
 
+def permanent_duration(expires_at: str) -> str:
+    """Only allow an expiration to be 'permanent' if it is a string."""
+    expires_at = expires_at.lower()
+    if expires_at != "permanent":
+        raise BadArgument
+    else:
+        return expires_at
+
+
 UserTypes = Union[Member, User, proxy_user]
 
 
@@ -875,121 +884,68 @@ class Moderation(Scheduler, Cog):
         await ctx.invoke(self.bot.get_command("help"), "infraction")
 
     @with_role(*MODERATION_ROLES)
-    @infraction_group.group(name='edit', invoke_without_command=True)
-    async def infraction_edit_group(self, ctx: Context) -> None:
-        """Infraction editing commands."""
-        await ctx.invoke(self.bot.get_command("help"), "infraction", "edit")
-
-    @with_role(*MODERATION_ROLES)
-    @infraction_edit_group.command(name="duration")
-    async def edit_duration(
-            self, ctx: Context,
-            infraction_id: int, expires_at: Union[Duration, str]
+    @infraction_group.command(name='edit')
+    async def infraction_edit(
+        self,
+        ctx: Context,
+        infraction_id: int,
+        expires_at: Union[Duration, permanent_duration, None],
+        *,
+        reason: str = None
     ) -> None:
         """
-        Sets the duration of the given infraction, relative to the time of updating.
+        Edit the duration and/or the reason of an infraction.
 
-        Duration strings are parsed per: http://strftime.org/, use "permanent" to mark the infraction as permanent.
+        Durations are relative to the time of updating.
+        Use "permanent" to mark the infraction as permanent.
         """
-        if isinstance(expires_at, str) and expires_at != 'permanent':
-            raise BadArgument(
-                "If `expires_at` is given as a non-datetime, "
-                "it must be `permanent`."
-            )
-        if expires_at == 'permanent':
-            expires_at = None
-
-        try:
-            previous_infraction = await self.bot.api_client.get(
-                'bot/infractions/' + str(infraction_id)
-            )
-
-            # check the current active infraction
-            infraction = await self.bot.api_client.patch(
-                'bot/infractions/' + str(infraction_id),
-                json={
-                    'expires_at': (
-                        expires_at.isoformat()
-                        if expires_at is not None
-                        else None
-                    )
-                }
-            )
-
-            # Re-schedule
-            self.cancel_task(infraction['id'])
-            loop = asyncio.get_event_loop()
-            self.schedule_task(loop, infraction['id'], infraction)
-
-            if expires_at is None:
-                await ctx.send(f":ok_hand: Updated infraction: marked as permanent.")
-            else:
-                human_expiry = (
-                    datetime
-                    .fromisoformat(infraction['expires_at'][:-1])
-                    .strftime('%c')
-                )
-                await ctx.send(
-                    ":ok_hand: Updated infraction: set to expire on "
-                    f"{human_expiry}."
-                )
-
-        except Exception:
-            log.exception("There was an error updating an infraction.")
-            await ctx.send(":x: There was an error updating the infraction.")
-            return
-
-        # Get information about the infraction's user
-        user_id = infraction["user"]
-        user = ctx.guild.get_member(user_id)
-
-        if user:
-            member_text = f"{user.mention} (`{user.id}`)"
-            thumbnail = user.avatar_url_as(static_format="png")
-        else:
-            member_text = f"`{user_id}`"
-            thumbnail = None
-
-        # The infraction's actor
-        actor_id = infraction["actor"]
-        actor = ctx.guild.get_member(actor_id) or f"`{actor_id}`"
+        if expires_at is None and reason is None:
+            # Unlike UserInputError, the error handler will show a specified message for BadArgument
+            raise BadArgument("Neither a new expiry nor a new reason was specified.")
+
+        # Retrieve the previous infraction for its information.
+        old_infraction = await self.bot.api_client.get(f'bot/infractions/{infraction_id}')
+
+        request_data = {}
+        confirm_messages = []
+        log_text = ""
+
+        if expires_at == "permanent":
+            request_data['expires_at'] = None
+            confirm_messages.append("marked as permanent")
+        elif expires_at is not None:
+            request_data['expires_at'] = expires_at.isoformat()
+            confirm_messages.append(f"set to expire on {expires_at.strftime('%c')}")
+
+        if reason:
+            request_data['reason'] = reason
+            confirm_messages.append("set a new reason")
+            log_text += f"""
+                Previous reason: {old_infraction['reason']}
+                New reason: {reason}
+            """.rstrip()
 
-        await self.mod_log.send_log_message(
-            icon_url=Icons.pencil,
-            colour=Colour.blurple(),
-            title="Infraction edited",
-            thumbnail=thumbnail,
-            text=textwrap.dedent(f"""
-                Member: {member_text}
-                Actor: {actor}
-                Edited by: {ctx.message.author}
-                Previous expiry: {previous_infraction['expires_at']}
-                New expiry: {infraction['expires_at']}
-            """)
+        # Update the infraction
+        new_infraction = await self.bot.api_client.patch(
+            f'bot/infractions/{infraction_id}',
+            json=request_data,
         )
 
-    @with_role(*MODERATION_ROLES)
-    @infraction_edit_group.command(name="reason")
-    async def edit_reason(self, ctx: Context, infraction_id: int, *, reason: str) -> None:
-        """Edit the reason of the given infraction."""
-        try:
-            old_infraction = await self.bot.api_client.get(
-                'bot/infractions/' + str(infraction_id)
-            )
+        # Re-schedule infraction if the expiration has been updated
+        if 'expires_at' in request_data:
+            self.cancel_task(new_infraction['id'])
+            loop = asyncio.get_event_loop()
+            self.schedule_task(loop, new_infraction['id'], new_infraction)
 
-            updated_infraction = await self.bot.api_client.patch(
-                'bot/infractions/' + str(infraction_id),
-                json={'reason': reason}
-            )
-            await ctx.send(f":ok_hand: Updated infraction: set reason to \"{reason}\".")
+            log_text += f"""
+                Previous expiry: {old_infraction['expires_at']}
+                New expiry: {new_infraction['expires_at']}
+            """.rstrip()
 
-        except Exception:
-            log.exception("There was an error updating an infraction.")
-            await ctx.send(":x: There was an error updating the infraction.")
-            return
+        await ctx.send(f":ok_hand: Updated infraction: {' & '.join(confirm_messages)}")
 
         # Get information about the infraction's user
-        user_id = updated_infraction['user']
+        user_id = new_infraction['user']
         user = ctx.guild.get_member(user_id)
 
         if user:
@@ -1000,7 +956,7 @@ class Moderation(Scheduler, Cog):
             thumbnail = None
 
         # The infraction's actor
-        actor_id = updated_infraction['actor']
+        actor_id = new_infraction['actor']
         actor = ctx.guild.get_member(actor_id) or f"`{actor_id}`"
 
         await self.mod_log.send_log_message(
@@ -1011,9 +967,7 @@ class Moderation(Scheduler, Cog):
             text=textwrap.dedent(f"""
                 Member: {user_text}
                 Actor: {actor}
-                Edited by: {ctx.message.author}
-                Previous reason: {old_infraction['reason']}
-                New reason: {updated_infraction['reason']}
+                Edited by: {ctx.message.author}{log_text}
             """)
         )