Send infraction DM before applying infraction

I've "reverted" the change that reversed the order of DM'ing a user
about their infraction and applying the actual infraction. A recent PR
reversed the order to stop us from sending DMs when applying the
infraction failed.

However, in order to DM a user, the bot has to share a guild with the
recipient and kicking them off of our server first does not help with
that. That's why I reverted the change and reverted some other minor
changes made in relation to this change.

Note: I did not change the code sending the DM itself; I merely moved it
back to where it belongs and added a comment about the necessity of
doing the DM'ing first.

I couldn't cleanly revert a commit to do this, as changes were spread
out over and included in multiple commits that also contained changes
not related to the `DM->apply infraction` order.

1 files changed, 21 insertions, 20 deletions

diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py
index f0a3ad1b1..b03d89537 100644
--- a/bot/cogs/moderation/scheduler.py
+++ b/bot/cogs/moderation/scheduler.py
@@ -106,6 +106,27 @@ class InfractionScheduler(Scheduler):
         log_content = None
         failed = False
 
+        # DM the user about the infraction if it's not a shadow/hidden infraction.
+        # This needs to happen before we apply the infraction, as the bot cannot
+        # send DMs to user that it doesn't share a guild with. If we were to
+        # apply kick/ban infractions first, this would mean that we'd make it
+        # impossible for us to deliver a DM. See python-discord/bot#982.
+        if not infraction["hidden"]:
+            dm_result = f"{constants.Emojis.failmail} "
+            dm_log_text = "\nDM: **Failed**"
+
+            # Sometimes user is a discord.Object; make it a proper user.
+            try:
+                if not isinstance(user, (discord.Member, discord.User)):
+                    user = await self.bot.fetch_user(user.id)
+            except discord.HTTPException as e:
+                log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})")
+            else:
+                # Accordingly display whether the user was successfully notified via DM.
+                if await utils.notify_infraction(user, infr_type, expiry, reason, icon):
+                    dm_result = ":incoming_envelope: "
+                    dm_log_text = "\nDM: Sent"
+
         if infraction["actor"] == self.bot.user.id:
             log.trace(
                 f"Infraction #{id_} actor is bot; including the reason in the confirmation message."
@@ -150,27 +171,7 @@ class InfractionScheduler(Scheduler):
                     log.exception(log_msg)
                 failed = True
 
-        # DM the user about the infraction if it's not a shadow/hidden infraction.
-        # Don't send DM when applying failed.
-        if not infraction["hidden"] and not failed:
-            dm_result = f"{constants.Emojis.failmail} "
-            dm_log_text = "\nDM: **Failed**"
-
-            # Sometimes user is a discord.Object; make it a proper user.
-            try:
-                if not isinstance(user, (discord.Member, discord.User)):
-                    user = await self.bot.fetch_user(user.id)
-            except discord.HTTPException as e:
-                log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})")
-            else:
-                # Accordingly display whether the user was successfully notified via DM.
-                if await utils.notify_infraction(user, infr_type, expiry, reason, icon):
-                    dm_result = ":incoming_envelope: "
-                    dm_log_text = "\nDM: Sent"
-
         if failed:
-            dm_log_text = "\nDM: **Canceled**"
-            dm_result = f"{constants.Emojis.failmail} "
             log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.")
             try:
                 await self.bot.api_client.delete(f"bot/infractions/{id_}")