Merge pull request #1126 from python-discord/feat/info/1125/user-cmd-whitelist-modmail

Whitelist the modmail category for displaying verbose infraction info from the user command

5 files changed, 22 insertions, 37 deletions

diff --git a/bot/constants.py b/bot/constants.py
index 0cb076d5c..c710e2dff 100644
--- a/bot/constants.py
+++ b/bot/constants.py
@@ -469,7 +469,6 @@ class Guild(metaclass=YAMLGetter):
     moderation_roles: List[int]
     modlog_blacklist: List[int]
     reminder_whitelist: List[int]
-    staff_channels: List[int]
     staff_roles: List[int]
 
 
@@ -623,7 +622,6 @@ MODERATION_ROLES = Guild.moderation_roles
 STAFF_ROLES = Guild.staff_roles
 
 # Channel combinations
-STAFF_CHANNELS = Guild.staff_channels
 MODERATION_CHANNELS = Guild.moderation_channels
 
 # Bot replies
diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py
index 581b3a227..156dfec35 100644
--- a/bot/exts/info/information.py
+++ b/bot/exts/info/information.py
@@ -15,7 +15,7 @@ from bot import constants
 from bot.bot import Bot
 from bot.decorators import in_whitelist
 from bot.pagination import LinePaginator
-from bot.utils.checks import InWhitelistCheckFailure, cooldown_with_role_bypass, has_no_roles_check
+from bot.utils.checks import cooldown_with_role_bypass, has_no_roles_check, in_whitelist_check
 from bot.utils.time import time_since
 
 log = logging.getLogger(__name__)
@@ -202,14 +202,10 @@ class Information(Cog):
             await ctx.send("You may not use this command on users other than yourself.")
             return
 
-        # Non-staff may only do this in #bot-commands
-        if await has_no_roles_check(ctx, *constants.STAFF_ROLES):
-            if not ctx.channel.id == constants.Channels.bot_commands:
-                raise InWhitelistCheckFailure(constants.Channels.bot_commands)
-
-        embed = await self.create_user_embed(ctx, user)
-
-        await ctx.send(embed=embed)
+        # Will redirect to #bot-commands if it fails.
+        if in_whitelist_check(ctx, roles=constants.STAFF_ROLES):
+            embed = await self.create_user_embed(ctx, user)
+            await ctx.send(embed=embed)
 
     async def create_user_embed(self, ctx: Context, user: Member) -> Embed:
         """Creates an embed containing information on the `user`."""
@@ -278,8 +274,14 @@ class Information(Cog):
             )
         ]
 
+        # Use getattr to future-proof for commands invoked via DMs.
+        show_verbose = (
+            ctx.channel.id in constants.MODERATION_CHANNELS
+            or getattr(ctx.channel, "category_id", None) == constants.Categories.modmail
+        )
+
         # Show more verbose output in moderation channels for infractions and nominations
-        if ctx.channel.id in constants.MODERATION_CHANNELS:
+        if show_verbose:
             fields.append(await self.expanded_user_infraction_counts(user))
             fields.append(await self.user_nomination_counts(user))
         else:
diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py
index 05a6ac03f..814b17830 100644
--- a/bot/exts/moderation/infraction/_scheduler.py
+++ b/bot/exts/moderation/infraction/_scheduler.py
@@ -12,7 +12,7 @@ from discord.ext.commands import Context
 from bot import constants
 from bot.api import ResponseCodeError
 from bot.bot import Bot
-from bot.constants import Colours, STAFF_CHANNELS
+from bot.constants import Colours, MODERATION_CHANNELS
 from bot.exts.moderation.infraction import _utils
 from bot.exts.moderation.infraction._utils import UserSnowflake
 from bot.exts.moderation.modlog import ModLog
@@ -136,9 +136,9 @@ class InfractionScheduler:
             )
             if reason:
                 end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})"
-        elif ctx.channel.id not in STAFF_CHANNELS:
+        elif ctx.channel.id not in MODERATION_CHANNELS:
             log.trace(
-                f"Infraction #{id_} context is not in a staff channel; omitting infraction count."
+                f"Infraction #{id_} context is not in a mod channel; omitting infraction count."
             )
         else:
             log.trace(f"Fetching total infraction count for {user}.")
diff --git a/config-default.yml b/config-default.yml
index c809a7340..e7669e6db 100644
--- a/config-default.yml
+++ b/config-default.yml
@@ -197,15 +197,6 @@ guild:
         big_brother_logs:   &BB_LOGS        468507907357409333
         talent_pool:        &TALENT_POOL    534321732593647616
 
-    staff_channels:
-        - *ADMINS
-        - *ADMIN_SPAM
-        - *DEFCON
-        - *HELPERS
-        - *MODS
-        - *MOD_SPAM
-        - *ORGANISATION
-
     moderation_channels:
         - *ADMINS
         - *ADMIN_SPAM
diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py
index ba8d5d608..d3f2995fb 100644
--- a/tests/bot/exts/info/test_information.py
+++ b/tests/bot/exts/info/test_information.py
@@ -532,10 +532,13 @@ class UserCommandTests(unittest.TestCase):
         self.moderator = helpers.MockMember(id=2, name="riffautae", roles=[self.moderator_role])
         self.target = helpers.MockMember(id=3, name="__fluzz__")
 
+        # There's no way to mock the channel constant without deferring imports. The constant is
+        # used as a default value for a parameter, which gets defined upon import.
+        self.bot_command_channel = helpers.MockTextChannel(id=constants.Channels.bot_commands)
+
     def test_regular_member_cannot_target_another_member(self, constants):
         """A regular user should not be able to use `!user` targeting another user."""
         constants.MODERATION_ROLES = [self.moderator_role.id]
-
         ctx = helpers.MockContext(author=self.author)
 
         asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target))
@@ -546,8 +549,6 @@ class UserCommandTests(unittest.TestCase):
         """A regular user should not be able to use this command outside of bot-commands."""
         constants.MODERATION_ROLES = [self.moderator_role.id]
         constants.STAFF_ROLES = [self.moderator_role.id]
-        constants.Channels.bot_commands = 50
-
         ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100))
 
         msg = "Sorry, but you may only use this command within <#50>."
@@ -558,9 +559,7 @@ class UserCommandTests(unittest.TestCase):
     def test_regular_user_may_use_command_in_bot_commands_channel(self, create_embed, constants):
         """A regular user should be allowed to use `!user` targeting themselves in bot-commands."""
         constants.STAFF_ROLES = [self.moderator_role.id]
-        constants.Channels.bot_commands = 50
-
-        ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50))
+        ctx = helpers.MockContext(author=self.author, channel=self.bot_command_channel)
 
         asyncio.run(self.cog.user_info.callback(self.cog, ctx))
 
@@ -568,12 +567,10 @@ class UserCommandTests(unittest.TestCase):
         ctx.send.assert_called_once()
 
     @unittest.mock.patch("bot.exts.info.information.Information.create_user_embed")
-    def test_regular_user_can_explicitly_target_themselves(self, create_embed, constants):
+    def test_regular_user_can_explicitly_target_themselves(self, create_embed, _):
         """A user should target itself with `!user` when a `user` argument was not provided."""
         constants.STAFF_ROLES = [self.moderator_role.id]
-        constants.Channels.bot_commands = 50
-
-        ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50))
+        ctx = helpers.MockContext(author=self.author, channel=self.bot_command_channel)
 
         asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author))
 
@@ -584,8 +581,6 @@ class UserCommandTests(unittest.TestCase):
     def test_staff_members_can_bypass_channel_restriction(self, create_embed, constants):
         """Staff members should be able to bypass the bot-commands channel restriction."""
         constants.STAFF_ROLES = [self.moderator_role.id]
-        constants.Channels.bot_commands = 50
-
         ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200))
 
         asyncio.run(self.cog.user_info.callback(self.cog, ctx))
@@ -598,7 +593,6 @@ class UserCommandTests(unittest.TestCase):
         """A moderator should be able to use `!user` targeting another user."""
         constants.MODERATION_ROLES = [self.moderator_role.id]
         constants.STAFF_ROLES = [self.moderator_role.id]
-
         ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=50))
 
         asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target))