From 7434ed3152e6d3f89babe2fef332983925d04434 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 14:45:32 +0300 Subject: (Syncer Tests): Replaced wrong side effect Replaced `TimeoutError` with `asyncio.TimeoutError`. --- tests/bot/cogs/sync/test_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/sync/test_base.py b/tests/bot/cogs/sync/test_base.py index 6ee9dfda6..70aea2bab 100644 --- a/tests/bot/cogs/sync/test_base.py +++ b/tests/bot/cogs/sync/test_base.py @@ -1,3 +1,4 @@ +import asyncio import unittest from unittest import mock @@ -211,7 +212,7 @@ class SyncerConfirmationTests(unittest.IsolatedAsyncioTestCase): subtests = ( (constants.Emojis.check_mark, True, None), ("InVaLiD", False, None), - (None, False, TimeoutError), + (None, False, asyncio.TimeoutError), ) for emoji, ret_val, side_effect in subtests: -- cgit v1.2.3 From 1140e9690644e46196a1c8cad900272ffb3ae09a Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 20 Apr 2020 18:46:30 +0200 Subject: Replace `in_channel` decorator by `in_whitelisted_context` The `in_channel` decorator that served as a factory for `in_channel` checks was replaced by the broaded `in_whitelisted_context` decorator. This means that we can now whitelist commands using channel IDs, category IDs, and/or role IDs. The whitelists will be applied in an "OR" fashion, meaning that as soon as some part of the context happens to be whitelisted, the `predicate` check the decorator produces will return `True`. To reflect that this is now a broader decorator that checks for a whitelisted *context* (as opposed to just whitelisted channels), the exception the predicate raises has been changed to `InWhitelistedContextCheckFailure` to reflect the broader scope of the decorator. I've updated all the commands that used the previous version, `in_channel`, to use the replacement. --- bot/cogs/error_handler.py | 6 +-- bot/cogs/information.py | 10 +++-- bot/cogs/snekbox.py | 11 ++++- bot/cogs/utils.py | 8 +++- bot/cogs/verification.py | 18 +++++--- bot/decorators.py | 84 +++++++++++++++++++++++++------------- tests/bot/cogs/test_information.py | 4 +- 7 files changed, 94 insertions(+), 47 deletions(-) (limited to 'tests') diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index dae283c6a..3f56a9798 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -9,7 +9,7 @@ from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels from bot.converters import TagNameConverter -from bot.decorators import InChannelCheckFailure +from bot.decorators import InWhitelistedContextCheckFailure log = logging.getLogger(__name__) @@ -202,7 +202,7 @@ class ErrorHandler(Cog): * BotMissingRole * BotMissingAnyRole * NoPrivateMessage - * InChannelCheckFailure + * InWhitelistedContextCheckFailure """ bot_missing_errors = ( errors.BotMissingPermissions, @@ -215,7 +215,7 @@ class ErrorHandler(Cog): await ctx.send( f"Sorry, it looks like I don't have the permissions or roles I need to do that." ) - elif isinstance(e, (InChannelCheckFailure, errors.NoPrivateMessage)): + elif isinstance(e, (InWhitelistedContextCheckFailure, errors.NoPrivateMessage)): ctx.bot.stats.incr("errors.wrong_channel_or_dm_error") await ctx.send(e) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 7921a4932..6b3fc0c96 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -12,7 +12,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.decorators import InChannelCheckFailure, in_channel, with_role +from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, with_role from bot.pagination import LinePaginator from bot.utils.checks import cooldown_with_role_bypass, with_role_check from bot.utils.time import time_since @@ -152,7 +152,7 @@ class Information(Cog): # Non-staff may only do this in #bot-commands if not with_role_check(ctx, *constants.STAFF_ROLES): if not ctx.channel.id == constants.Channels.bot_commands: - raise InChannelCheckFailure(constants.Channels.bot_commands) + raise InWhitelistedContextCheckFailure(constants.Channels.bot_commands) embed = await self.create_user_embed(ctx, user) @@ -331,7 +331,11 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_channel(constants.Channels.bot_commands, bypass_roles=constants.STAFF_ROLES) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + whitelisted_roles=constants.STAFF_ROLES, + redirect_channel=constants.Channels.bot_commands, + ) async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None: """Shows information about the raw API response.""" # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 315383b12..8827cb585 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -13,7 +13,7 @@ from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot from bot.constants import Channels, Roles, URLs -from bot.decorators import in_channel +from bot.decorators import in_whitelisted_context from bot.utils.messages import wait_for_deletion log = logging.getLogger(__name__) @@ -38,6 +38,9 @@ RAW_CODE_REGEX = re.compile( ) MAX_PASTE_LEN = 1000 + +# `!eval` command whitelists +EVAL_CHANNELS = (Channels.bot_commands, Channels.esoteric) EVAL_ROLES = (Roles.helpers, Roles.moderators, Roles.admins, Roles.owners, Roles.python_community, Roles.partners) SIGKILL = 9 @@ -265,7 +268,11 @@ class Snekbox(Cog): @command(name="eval", aliases=("e",)) @guild_only() - @in_channel(Channels.bot_commands, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) + @in_whitelisted_context( + whitelisted_channels=EVAL_CHANNELS, + whitelisted_roles=EVAL_ROLES, + redirect_channel=Channels.bot_commands, + ) async def eval_command(self, ctx: Context, *, code: str = None) -> None: """ Run Python code and get the results. diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 3ed471bbf..234ec514d 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -13,7 +13,7 @@ from discord.ext.commands import BadArgument, Cog, Context, command from bot.bot import Bot from bot.constants import Channels, MODERATION_ROLES, Mention, STAFF_ROLES -from bot.decorators import in_channel, with_role +from bot.decorators import in_whitelisted_context, with_role from bot.utils.time import humanize_delta log = logging.getLogger(__name__) @@ -118,7 +118,11 @@ class Utils(Cog): await ctx.message.channel.send(embed=pep_embed) @command() - @in_channel(Channels.bot_commands, bypass_roles=STAFF_ROLES) + @in_whitelisted_context( + whitelisted_channels=(Channels.bot_commands,), + whitelisted_roles=STAFF_ROLES, + redirect_channel=Channels.bot_commands, + ) async def charinfo(self, ctx: Context, *, characters: str) -> None: """Shows you information on up to 25 unicode characters.""" match = re.match(r"<(a?):(\w+):(\d+)>", characters) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index b0a493e68..040f52fbf 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -9,7 +9,7 @@ from discord.ext.commands import Cog, Context, command from bot import constants from bot.bot import Bot from bot.cogs.moderation import ModLog -from bot.decorators import InChannelCheckFailure, in_channel, without_role +from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, without_role from bot.utils.checks import without_role_check log = logging.getLogger(__name__) @@ -122,7 +122,7 @@ class Verification(Cog): @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(constants.Roles.verified) - @in_channel(constants.Channels.verification) + @in_whitelisted_context(whitelisted_channels=(constants.Channels.verification,)) async def accept_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Accept our rules and gain access to the rest of the server.""" log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") @@ -138,7 +138,10 @@ class Verification(Cog): await ctx.message.delete() @command(name='subscribe') - @in_channel(constants.Channels.bot_commands) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + redirect_channel=constants.Channels.bot_commands, + ) async def subscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Subscribe to announcement notifications by assigning yourself the role.""" has_role = False @@ -162,7 +165,10 @@ class Verification(Cog): ) @command(name='unsubscribe') - @in_channel(constants.Channels.bot_commands) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + redirect_channel=constants.Channels.bot_commands, + ) async def unsubscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Unsubscribe from announcement notifications by removing the role from yourself.""" has_role = False @@ -187,8 +193,8 @@ class Verification(Cog): # This cannot be static (must have a __func__ attribute). async def cog_command_error(self, ctx: Context, error: Exception) -> None: - """Check for & ignore any InChannelCheckFailure.""" - if isinstance(error, InChannelCheckFailure): + """Check for & ignore any InWhitelistedContextCheckFailure.""" + if isinstance(error, InWhitelistedContextCheckFailure): error.handled = True @staticmethod diff --git a/bot/decorators.py b/bot/decorators.py index 2d18eaa6a..149564d18 100644 --- a/bot/decorators.py +++ b/bot/decorators.py @@ -3,7 +3,7 @@ import random from asyncio import Lock, sleep from contextlib import suppress from functools import wraps -from typing import Callable, Container, Union +from typing import Callable, Container, Optional, Union from weakref import WeakValueDictionary from discord import Colour, Embed, Member @@ -17,48 +17,74 @@ from bot.utils.checks import with_role_check, without_role_check log = logging.getLogger(__name__) -class InChannelCheckFailure(CheckFailure): - """Raised when a check fails for a message being sent in a whitelisted channel.""" +class InWhitelistedContextCheckFailure(CheckFailure): + """Raised when the `in_whitelist` check fails.""" - def __init__(self, *channels: int): - self.channels = channels - channels_str = ', '.join(f"<#{c_id}>" for c_id in channels) + def __init__(self, redirect_channel: Optional[int] = None): + error_message = "Sorry, but you are not allowed to use that command here." - super().__init__(f"Sorry, but you may only use this command within {channels_str}.") + if redirect_channel: + error_message += f" Please use the <#{redirect_channel}> channel instead." + super().__init__(error_message) + + +def in_whitelisted_context( + *, + whitelisted_channels: Container[int] = (), + whitelisted_categories: Container[int] = (), + whitelisted_roles: Container[int] = (), + redirect_channel: Optional[int] = None, -def in_channel( - *channels: int, - hidden_channels: Container[int] = None, - bypass_roles: Container[int] = None ) -> Callable: """ - Checks that the message is in a whitelisted channel or optionally has a bypass role. + Check if a command was issued in a whitelisted context. + + The whitelists that can be provided are: - Hidden channels are channels which will not be displayed in the InChannelCheckFailure error - message. + - `channels`: a container with channel ids for whitelisted channels + - `categories`: a container with category ids for whitelisted categories + - `roles`: a container with with role ids for whitelisted roles + + An optional `redirect_channel` can be provided to redirect users that are not + authorized to use the command in the current context. If no such channel is + provided, the users are simply told that they are not authorized to use the + command. """ - hidden_channels = hidden_channels or [] - bypass_roles = bypass_roles or [] + if redirect_channel and redirect_channel not in whitelisted_channels: + # It does not make sense for the channel whitelist to not contain the redirection + # channel (if provided). That's why we add the redirection channel to the `channels` + # container if it's not already in it. As we allow any container type to be passed, + # we first create a tuple in order to safely add the redirection channel. + # + # Note: It's possible for the redirect channel to be in a whitelisted category, but + # there's no easy way to check that and as a channel can easily be moved in and out of + # categories, it's probably not wise to rely on its category in any case. + whitelisted_channels = tuple(whitelisted_channels) + (redirect_channel,) def predicate(ctx: Context) -> bool: - """In-channel checker predicate.""" - if ctx.channel.id in channels or ctx.channel.id in hidden_channels: - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The command was used in a whitelisted channel.") + """Check if a command was issued in a whitelisted context.""" + if whitelisted_channels and ctx.channel.id in whitelisted_channels: + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted channel.") return True - if bypass_roles: - if any(r.id in bypass_roles for r in ctx.author.roles): - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The command was not used in a whitelisted channel, " - f"but the author had a role to bypass the in_channel check.") - return True + # Only check the category id if we have a category whitelist and the channel has a `category_id` + if ( + whitelisted_categories + and hasattr(ctx.channel, "category_id") + and ctx.channel.category_id in whitelisted_categories + ): + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted category.") + return True - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The in_channel check failed.") + # Only check the roles whitelist if we have one and ensure the author's roles attribute returns + # an iterable to prevent breakage in DM channels (for if we ever decide to enable commands there). + if whitelisted_roles and any(r.id in whitelisted_roles for r in getattr(ctx.author, "roles", ())): + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they have a whitelisted role.") + return True - raise InChannelCheckFailure(*channels) + log.trace(f"{ctx.author} may not use the `{ctx.command.name}` command within this context.") + raise InWhitelistedContextCheckFailure(redirect_channel) return commands.check(predicate) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 3c26374f5..4a36fe030 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -7,7 +7,7 @@ import discord from bot import constants from bot.cogs import information -from bot.decorators import InChannelCheckFailure +from bot.decorators import InWhitelistedContextCheckFailure from tests import helpers @@ -525,7 +525,7 @@ class UserCommandTests(unittest.TestCase): ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) msg = "Sorry, but you may only use this command within <#50>." - with self.assertRaises(InChannelCheckFailure, msg=msg): + with self.assertRaises(InWhitelistedContextCheckFailure, msg=msg): asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=unittest.mock.AsyncMock) -- cgit v1.2.3 From 00291d7d5f859e4131cb5c94541a90f80f358376 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 20 Apr 2020 18:53:31 +0200 Subject: Remove vestigial kwargs from MockTextChannel.__init__ --- tests/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/helpers.py b/tests/helpers.py index 8e13f0f28..9001deedf 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -315,7 +315,7 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ spec_set = channel_instance - def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: + def __init__(self, **kwargs) -> None: default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} super().__init__(**collections.ChainMap(kwargs, default_kwargs)) -- cgit v1.2.3 From 57e69925af9a941dfe32acc0431a9699eda027f5 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 20 Apr 2020 18:57:12 +0200 Subject: Add tests for `in_whitelisted_context` decorator I have added tests for the new `in_whitelisted_context` decorator. They work by calling the decorator with different kwargs to generate a specific predicate callable. That callable is then called to assess if it comes to the right conclusion. --- tests/bot/test_decorators.py | 115 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 tests/bot/test_decorators.py (limited to 'tests') diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py new file mode 100644 index 000000000..fae7c0c52 --- /dev/null +++ b/tests/bot/test_decorators.py @@ -0,0 +1,115 @@ +import collections +import unittest +import unittest.mock + +from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context +from tests import helpers + + +WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx")) + + +class InWhitelistedContextTests(unittest.TestCase): + """Tests for the `in_whitelisted_context` check.""" + + @classmethod + def setUpClass(cls): + """Set up helpers that only need to be defined once.""" + cls.bot_commands = helpers.MockTextChannel(id=123456789, category_id=123456) + cls.help_channel = helpers.MockTextChannel(id=987654321, category_id=987654) + cls.non_whitelisted_channel = helpers.MockTextChannel(id=666666) + + cls.non_staff_member = helpers.MockMember() + cls.staff_role = helpers.MockRole(id=121212) + cls.staff_member = helpers.MockMember(roles=(cls.staff_role,)) + + cls.whitelisted_channels = (cls.bot_commands.id,) + cls.whitelisted_categories = (cls.help_channel.category_id,) + cls.whitelisted_roles = (cls.staff_role.id,) + + def test_predicate_returns_true_for_whitelisted_context(self): + """The predicate should return `True` if a whitelisted context was passed to it.""" + test_cases = ( + # Commands issued in whitelisted channels by members without whitelisted roles + WhitelistedContextTestCase( + kwargs={"whitelisted_channels": self.whitelisted_channels}, + ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) + ), + # `redirect_channel` should be added implicitly to the `whitelisted_channels` + WhitelistedContextTestCase( + kwargs={"redirect_channel": self.bot_commands.id}, + ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) + ), + + # Commands issued in a whitelisted category by members without whitelisted roles + WhitelistedContextTestCase( + kwargs={"whitelisted_categories": self.whitelisted_categories}, + ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member) + ), + + # Command issued by a staff member in a non-whitelisted channel/category + WhitelistedContextTestCase( + kwargs={"whitelisted_roles": self.whitelisted_roles}, + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member) + ), + + # With all kwargs provided + WhitelistedContextTestCase( + kwargs={ + "whitelisted_channels": self.whitelisted_channels, + "whitelisted_categories": self.whitelisted_categories, + "whitelisted_roles": self.whitelisted_roles, + "redirect_channel": self.bot_commands, + }, + ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member) + ), + ) + + for test_case in test_cases: + # patch `commands.check` with a no-op lambda that just returns the predicate passed to it + # so we can test the predicate that was generated from the specified kwargs. + with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): + predicate = in_whitelisted_context(**test_case.kwargs) + + with self.subTest(test_case=test_case): + self.assertTrue(predicate(test_case.ctx)) + + def test_predicate_raises_exception_for_non_whitelisted_context(self): + """The predicate should raise `InWhitelistedContextCheckFailure` for a non-whitelisted context.""" + test_cases = ( + # Failing check with `redirect_channel` + WhitelistedContextTestCase( + kwargs={ + "whitelisted_categories": self.whitelisted_categories, + "whitelisted_channels": self.whitelisted_channels, + "whitelisted_roles": self.whitelisted_roles, + "redirect_channel": self.bot_commands.id, + }, + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) + ), + + # Failing check without `redirect_channel` + WhitelistedContextTestCase( + kwargs={ + "whitelisted_categories": self.whitelisted_categories, + "whitelisted_channels": self.whitelisted_channels, + "whitelisted_roles": self.whitelisted_roles, + }, + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) + ), + ) + + for test_case in test_cases: + # Create expected exception message based on whether or not a redirect channel was provided + expected_message = "Sorry, but you are not allowed to use that command here." + if test_case.kwargs.get("redirect_channel"): + expected_message += f" Please use the <#{test_case.kwargs['redirect_channel']}> channel instead." + + # patch `commands.check` with a no-op lambda that just returns the predicate passed to it + # so we can test the predicate that was generated from the specified kwargs. + with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): + predicate = in_whitelisted_context(**test_case.kwargs) + + with self.subTest(test_case=test_case): + with self.assertRaises(InWhitelistedContextCheckFailure, msg=expected_message): + predicate(test_case.ctx) -- cgit v1.2.3 From b20bb7471b8d1d01f217f0620f8597bf1bae4456 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Thu, 23 Apr 2020 15:51:58 +0200 Subject: Simplify `in_whitelisted_context` decorator API The API of the `in_whitelisted_context` decorator was a bit clunky: - The long parameter names frequently required multiline decorators - Despite `#bot-commands` being the defacto default, it needed to be passed - The name of the function, `in_whitelisted_context` is fairly long in itself To shorten the call length of the decorator, the parameter names were shortened by dropping the `whitelisted_` prefix. This means that the parameter names are now just `channels`, `categories`, and `roles`. This already means that all current usages of the decorator are reduced to one line. In addition, `#bot-commands` has now been made the default redirect channel for the decorator. This means that if no `redirect` was passed, users will be redirected to `bot-commands` to use the command. If needed, `None` (or any falsey value) can be passed to disable redirection. Passing another channel id will trigger that channel to be used as the redirection target instead of bot-commands. Finally, the name of the decorator was shortened to `in_whitelist`, which already communicates what it is supposed to do. --- bot/cogs/error_handler.py | 6 ++-- bot/cogs/information.py | 10 ++----- bot/cogs/snekbox.py | 9 ++---- bot/cogs/utils.py | 8 ++--- bot/cogs/verification.py | 18 ++++-------- bot/decorators.py | 49 +++++++++++++++---------------- tests/bot/cogs/test_information.py | 4 +-- tests/bot/test_decorators.py | 60 +++++++++++++++++++------------------- 8 files changed, 72 insertions(+), 92 deletions(-) (limited to 'tests') diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 3f56a9798..b2f4c59f6 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -9,7 +9,7 @@ from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels from bot.converters import TagNameConverter -from bot.decorators import InWhitelistedContextCheckFailure +from bot.decorators import InWhitelistCheckFailure log = logging.getLogger(__name__) @@ -202,7 +202,7 @@ class ErrorHandler(Cog): * BotMissingRole * BotMissingAnyRole * NoPrivateMessage - * InWhitelistedContextCheckFailure + * InWhitelistCheckFailure """ bot_missing_errors = ( errors.BotMissingPermissions, @@ -215,7 +215,7 @@ class ErrorHandler(Cog): await ctx.send( f"Sorry, it looks like I don't have the permissions or roles I need to do that." ) - elif isinstance(e, (InWhitelistedContextCheckFailure, errors.NoPrivateMessage)): + elif isinstance(e, (InWhitelistCheckFailure, errors.NoPrivateMessage)): ctx.bot.stats.incr("errors.wrong_channel_or_dm_error") await ctx.send(e) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 6b3fc0c96..4eb36c340 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -12,7 +12,7 @@ from discord.utils import escape_markdown from bot import constants from bot.bot import Bot -from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, with_role +from bot.decorators import InWhitelistCheckFailure, in_whitelist, with_role from bot.pagination import LinePaginator from bot.utils.checks import cooldown_with_role_bypass, with_role_check from bot.utils.time import time_since @@ -152,7 +152,7 @@ class Information(Cog): # Non-staff may only do this in #bot-commands if not with_role_check(ctx, *constants.STAFF_ROLES): if not ctx.channel.id == constants.Channels.bot_commands: - raise InWhitelistedContextCheckFailure(constants.Channels.bot_commands) + raise InWhitelistCheckFailure(constants.Channels.bot_commands) embed = await self.create_user_embed(ctx, user) @@ -331,11 +331,7 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_whitelisted_context( - whitelisted_channels=(constants.Channels.bot_commands,), - whitelisted_roles=constants.STAFF_ROLES, - redirect_channel=constants.Channels.bot_commands, - ) + @in_whitelist(channels=(constants.Channels.bot_commands,), roles=constants.STAFF_ROLES) async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None: """Shows information about the raw API response.""" # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 4999074b6..8d4688114 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -13,7 +13,7 @@ from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot from bot.constants import Categories, Channels, Roles, URLs -from bot.decorators import in_whitelisted_context +from bot.decorators import in_whitelist from bot.utils.messages import wait_for_deletion log = logging.getLogger(__name__) @@ -269,12 +269,7 @@ class Snekbox(Cog): @command(name="eval", aliases=("e",)) @guild_only() - @in_whitelisted_context( - whitelisted_channels=EVAL_CHANNELS, - whitelisted_categories=EVAL_CATEGORIES, - whitelisted_roles=EVAL_ROLES, - redirect_channel=Channels.bot_commands, - ) + @in_whitelist(channels=EVAL_CHANNELS, categories=EVAL_CATEGORIES, roles=EVAL_ROLES) async def eval_command(self, ctx: Context, *, code: str = None) -> None: """ Run Python code and get the results. diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 234ec514d..8023eb962 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -13,7 +13,7 @@ from discord.ext.commands import BadArgument, Cog, Context, command from bot.bot import Bot from bot.constants import Channels, MODERATION_ROLES, Mention, STAFF_ROLES -from bot.decorators import in_whitelisted_context, with_role +from bot.decorators import in_whitelist, with_role from bot.utils.time import humanize_delta log = logging.getLogger(__name__) @@ -118,11 +118,7 @@ class Utils(Cog): await ctx.message.channel.send(embed=pep_embed) @command() - @in_whitelisted_context( - whitelisted_channels=(Channels.bot_commands,), - whitelisted_roles=STAFF_ROLES, - redirect_channel=Channels.bot_commands, - ) + @in_whitelist(channels=(Channels.bot_commands,), roles=STAFF_ROLES) async def charinfo(self, ctx: Context, *, characters: str) -> None: """Shows you information on up to 25 unicode characters.""" match = re.match(r"<(a?):(\w+):(\d+)>", characters) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 040f52fbf..388b7a338 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -9,7 +9,7 @@ from discord.ext.commands import Cog, Context, command from bot import constants from bot.bot import Bot from bot.cogs.moderation import ModLog -from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, without_role +from bot.decorators import InWhitelistCheckFailure, in_whitelist, without_role from bot.utils.checks import without_role_check log = logging.getLogger(__name__) @@ -122,7 +122,7 @@ class Verification(Cog): @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(constants.Roles.verified) - @in_whitelisted_context(whitelisted_channels=(constants.Channels.verification,)) + @in_whitelist(channels=(constants.Channels.verification,)) async def accept_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Accept our rules and gain access to the rest of the server.""" log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") @@ -138,10 +138,7 @@ class Verification(Cog): await ctx.message.delete() @command(name='subscribe') - @in_whitelisted_context( - whitelisted_channels=(constants.Channels.bot_commands,), - redirect_channel=constants.Channels.bot_commands, - ) + @in_whitelist(channels=(constants.Channels.bot_commands,)) async def subscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Subscribe to announcement notifications by assigning yourself the role.""" has_role = False @@ -165,10 +162,7 @@ class Verification(Cog): ) @command(name='unsubscribe') - @in_whitelisted_context( - whitelisted_channels=(constants.Channels.bot_commands,), - redirect_channel=constants.Channels.bot_commands, - ) + @in_whitelist(channels=(constants.Channels.bot_commands,)) async def unsubscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Unsubscribe from announcement notifications by removing the role from yourself.""" has_role = False @@ -193,8 +187,8 @@ class Verification(Cog): # This cannot be static (must have a __func__ attribute). async def cog_command_error(self, ctx: Context, error: Exception) -> None: - """Check for & ignore any InWhitelistedContextCheckFailure.""" - if isinstance(error, InWhitelistedContextCheckFailure): + """Check for & ignore any InWhitelistCheckFailure.""" + if isinstance(error, InWhitelistCheckFailure): error.handled = True @staticmethod diff --git a/bot/decorators.py b/bot/decorators.py index 149564d18..2ee5879f2 100644 --- a/bot/decorators.py +++ b/bot/decorators.py @@ -11,30 +11,34 @@ from discord.errors import NotFound from discord.ext import commands from discord.ext.commands import CheckFailure, Cog, Context -from bot.constants import ERROR_REPLIES, RedirectOutput +from bot.constants import Channels, ERROR_REPLIES, RedirectOutput from bot.utils.checks import with_role_check, without_role_check log = logging.getLogger(__name__) -class InWhitelistedContextCheckFailure(CheckFailure): +class InWhitelistCheckFailure(CheckFailure): """Raised when the `in_whitelist` check fails.""" - def __init__(self, redirect_channel: Optional[int] = None): - error_message = "Sorry, but you are not allowed to use that command here." + def __init__(self, redirect_channel: Optional[int]) -> None: + self.redirect_channel = redirect_channel if redirect_channel: - error_message += f" Please use the <#{redirect_channel}> channel instead." + redirect_message = f" here. Please use the <#{redirect_channel}> channel instead" + else: + redirect_message = "" + + error_message = f"You are not allowed to use that command{redirect_message}." super().__init__(error_message) -def in_whitelisted_context( +def in_whitelist( *, - whitelisted_channels: Container[int] = (), - whitelisted_categories: Container[int] = (), - whitelisted_roles: Container[int] = (), - redirect_channel: Optional[int] = None, + channels: Container[int] = (), + categories: Container[int] = (), + roles: Container[int] = (), + redirect: Optional[int] = Channels.bot_commands, ) -> Callable: """ @@ -46,45 +50,40 @@ def in_whitelisted_context( - `categories`: a container with category ids for whitelisted categories - `roles`: a container with with role ids for whitelisted roles - An optional `redirect_channel` can be provided to redirect users that are not - authorized to use the command in the current context. If no such channel is - provided, the users are simply told that they are not authorized to use the - command. + If the command was invoked in a context that was not whitelisted, the member is either + redirected to the `redirect` channel that was passed (default: #bot-commands) or simply + told that they're not allowed to use this particular command (if `None` was passed). """ - if redirect_channel and redirect_channel not in whitelisted_channels: + if redirect and redirect not in channels: # It does not make sense for the channel whitelist to not contain the redirection - # channel (if provided). That's why we add the redirection channel to the `channels` + # channel (if applicable). That's why we add the redirection channel to the `channels` # container if it's not already in it. As we allow any container type to be passed, # we first create a tuple in order to safely add the redirection channel. # # Note: It's possible for the redirect channel to be in a whitelisted category, but # there's no easy way to check that and as a channel can easily be moved in and out of # categories, it's probably not wise to rely on its category in any case. - whitelisted_channels = tuple(whitelisted_channels) + (redirect_channel,) + channels = tuple(channels) + (redirect,) def predicate(ctx: Context) -> bool: """Check if a command was issued in a whitelisted context.""" - if whitelisted_channels and ctx.channel.id in whitelisted_channels: + if channels and ctx.channel.id in channels: log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted channel.") return True # Only check the category id if we have a category whitelist and the channel has a `category_id` - if ( - whitelisted_categories - and hasattr(ctx.channel, "category_id") - and ctx.channel.category_id in whitelisted_categories - ): + if categories and hasattr(ctx.channel, "category_id") and ctx.channel.category_id in categories: log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted category.") return True # Only check the roles whitelist if we have one and ensure the author's roles attribute returns # an iterable to prevent breakage in DM channels (for if we ever decide to enable commands there). - if whitelisted_roles and any(r.id in whitelisted_roles for r in getattr(ctx.author, "roles", ())): + if roles and any(r.id in roles for r in getattr(ctx.author, "roles", ())): log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they have a whitelisted role.") return True log.trace(f"{ctx.author} may not use the `{ctx.command.name}` command within this context.") - raise InWhitelistedContextCheckFailure(redirect_channel) + raise InWhitelistCheckFailure(redirect) return commands.check(predicate) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 4a36fe030..6dace1080 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -7,7 +7,7 @@ import discord from bot import constants from bot.cogs import information -from bot.decorators import InWhitelistedContextCheckFailure +from bot.decorators import InWhitelistCheckFailure from tests import helpers @@ -525,7 +525,7 @@ class UserCommandTests(unittest.TestCase): ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) msg = "Sorry, but you may only use this command within <#50>." - with self.assertRaises(InWhitelistedContextCheckFailure, msg=msg): + with self.assertRaises(InWhitelistCheckFailure, msg=msg): asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=unittest.mock.AsyncMock) diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py index fae7c0c52..645051fec 100644 --- a/tests/bot/test_decorators.py +++ b/tests/bot/test_decorators.py @@ -2,15 +2,15 @@ import collections import unittest import unittest.mock -from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context +from bot.decorators import InWhitelistCheckFailure, in_whitelist from tests import helpers WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx")) -class InWhitelistedContextTests(unittest.TestCase): - """Tests for the `in_whitelisted_context` check.""" +class InWhitelistTests(unittest.TestCase): + """Tests for the `in_whitelist` check.""" @classmethod def setUpClass(cls): @@ -23,43 +23,43 @@ class InWhitelistedContextTests(unittest.TestCase): cls.staff_role = helpers.MockRole(id=121212) cls.staff_member = helpers.MockMember(roles=(cls.staff_role,)) - cls.whitelisted_channels = (cls.bot_commands.id,) - cls.whitelisted_categories = (cls.help_channel.category_id,) - cls.whitelisted_roles = (cls.staff_role.id,) + cls.channels = (cls.bot_commands.id,) + cls.categories = (cls.help_channel.category_id,) + cls.roles = (cls.staff_role.id,) def test_predicate_returns_true_for_whitelisted_context(self): """The predicate should return `True` if a whitelisted context was passed to it.""" test_cases = ( # Commands issued in whitelisted channels by members without whitelisted roles WhitelistedContextTestCase( - kwargs={"whitelisted_channels": self.whitelisted_channels}, + kwargs={"channels": self.channels}, ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) ), - # `redirect_channel` should be added implicitly to the `whitelisted_channels` + # `redirect` should be added implicitly to the `channels` WhitelistedContextTestCase( - kwargs={"redirect_channel": self.bot_commands.id}, + kwargs={"redirect": self.bot_commands.id}, ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) ), # Commands issued in a whitelisted category by members without whitelisted roles WhitelistedContextTestCase( - kwargs={"whitelisted_categories": self.whitelisted_categories}, + kwargs={"categories": self.categories}, ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member) ), # Command issued by a staff member in a non-whitelisted channel/category WhitelistedContextTestCase( - kwargs={"whitelisted_roles": self.whitelisted_roles}, + kwargs={"roles": self.roles}, ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member) ), # With all kwargs provided WhitelistedContextTestCase( kwargs={ - "whitelisted_channels": self.whitelisted_channels, - "whitelisted_categories": self.whitelisted_categories, - "whitelisted_roles": self.whitelisted_roles, - "redirect_channel": self.bot_commands, + "channels": self.channels, + "categories": self.categories, + "roles": self.roles, + "redirect": self.bot_commands, }, ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member) ), @@ -69,31 +69,31 @@ class InWhitelistedContextTests(unittest.TestCase): # patch `commands.check` with a no-op lambda that just returns the predicate passed to it # so we can test the predicate that was generated from the specified kwargs. with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): - predicate = in_whitelisted_context(**test_case.kwargs) + predicate = in_whitelist(**test_case.kwargs) with self.subTest(test_case=test_case): self.assertTrue(predicate(test_case.ctx)) def test_predicate_raises_exception_for_non_whitelisted_context(self): - """The predicate should raise `InWhitelistedContextCheckFailure` for a non-whitelisted context.""" + """The predicate should raise `InWhitelistCheckFailure` for a non-whitelisted context.""" test_cases = ( - # Failing check with `redirect_channel` + # Failing check with `redirect` WhitelistedContextTestCase( kwargs={ - "whitelisted_categories": self.whitelisted_categories, - "whitelisted_channels": self.whitelisted_channels, - "whitelisted_roles": self.whitelisted_roles, - "redirect_channel": self.bot_commands.id, + "categories": self.categories, + "channels": self.channels, + "roles": self.roles, + "redirect": self.bot_commands.id, }, ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) ), - # Failing check without `redirect_channel` + # Failing check without `redirect` WhitelistedContextTestCase( kwargs={ - "whitelisted_categories": self.whitelisted_categories, - "whitelisted_channels": self.whitelisted_channels, - "whitelisted_roles": self.whitelisted_roles, + "categories": self.categories, + "channels": self.channels, + "roles": self.roles, }, ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) ), @@ -102,14 +102,14 @@ class InWhitelistedContextTests(unittest.TestCase): for test_case in test_cases: # Create expected exception message based on whether or not a redirect channel was provided expected_message = "Sorry, but you are not allowed to use that command here." - if test_case.kwargs.get("redirect_channel"): - expected_message += f" Please use the <#{test_case.kwargs['redirect_channel']}> channel instead." + if test_case.kwargs.get("redirect"): + expected_message += f" Please use the <#{test_case.kwargs['redirect']}> channel instead." # patch `commands.check` with a no-op lambda that just returns the predicate passed to it # so we can test the predicate that was generated from the specified kwargs. with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): - predicate = in_whitelisted_context(**test_case.kwargs) + predicate = in_whitelist(**test_case.kwargs) with self.subTest(test_case=test_case): - with self.assertRaises(InWhitelistedContextCheckFailure, msg=expected_message): + with self.assertRaises(InWhitelistCheckFailure, msg=expected_message): predicate(test_case.ctx) -- cgit v1.2.3 From f5bb251bbfd92bfe67ee9638f2bf6d054eb30502 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 27 Apr 2020 16:01:45 +0200 Subject: Exclude never-run lines from coverage --- tests/bot/cogs/test_cogs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 39f6492cb..fdda59a8f 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -31,7 +31,7 @@ class CommandNameTests(unittest.TestCase): def walk_modules() -> t.Iterator[ModuleType]: """Yield imported modules from the bot.cogs subpackage.""" def on_error(name: str) -> t.NoReturn: - raise ImportError(name=name) + raise ImportError(name=name) # pragma: no cover # The mock prevents asyncio.get_event_loop() from being called. with mock.patch("discord.ext.tasks.loop"): @@ -71,7 +71,7 @@ class CommandNameTests(unittest.TestCase): for name in self.get_qualified_names(cmd): with self.subTest(cmd=func_name, name=name): - if name in all_names: + if name in all_names: # pragma: no cover conflicts = ", ".join(all_names.get(name, "")) self.fail( f"Name '{name}' of the command {func_name} conflicts with {conflicts}." -- cgit v1.2.3 From 167f57b9cc78708b7c6b48f64442d7bddce2f75c Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 27 Apr 2020 16:02:15 +0200 Subject: Add mock for discord.DMChannels --- tests/helpers.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'tests') diff --git a/tests/helpers.py b/tests/helpers.py index 9001deedf..2b79a6c2a 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -323,6 +323,27 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): self.mention = f"#{self.name}" +# Create data for the DMChannel instance +state = unittest.mock.MagicMock() +me = unittest.mock.MagicMock() +dm_channel_data = {"id": 1, "recipients": [unittest.mock.MagicMock()]} +dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data) + + +class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock TextChannel objects. + + Instances of this class will follow the specifications of `discord.TextChannel` instances. For + more information, see the `MockGuild` docstring. + """ + spec_set = dm_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'recipient': MockUser(), "me": MockUser()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + # Create a Message instance to get a realistic MagicMock of `discord.Message` message_data = { 'id': 1, -- cgit v1.2.3 From d21e5962be961a267cef6ffef4f7d4aaf1114a08 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Mon, 27 Apr 2020 16:03:12 +0200 Subject: Add DMChannel tests for in_whitelist decorator The `in_whitelist` decorator should not fail when a decorated command was called in a DMChannel; it should simply conclude that the user is not allowed to use the command. I've added a test case that uses a DMChannel context with User, not Member, objects. In addition, I've opted to display a test case description in the `subTest`: Simply printing the actual arguments and context is messy and does not actually show you the information you'd like. This description is enough to figure out which test is failing and what the gist of the test is. --- tests/bot/test_decorators.py | 94 +++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 31 deletions(-) (limited to 'tests') diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py index 645051fec..a17dd3e16 100644 --- a/tests/bot/test_decorators.py +++ b/tests/bot/test_decorators.py @@ -2,11 +2,12 @@ import collections import unittest import unittest.mock +from bot import constants from bot.decorators import InWhitelistCheckFailure, in_whitelist from tests import helpers -WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx")) +InWhitelistTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx", "description")) class InWhitelistTests(unittest.TestCase): @@ -18,6 +19,7 @@ class InWhitelistTests(unittest.TestCase): cls.bot_commands = helpers.MockTextChannel(id=123456789, category_id=123456) cls.help_channel = helpers.MockTextChannel(id=987654321, category_id=987654) cls.non_whitelisted_channel = helpers.MockTextChannel(id=666666) + cls.dm_channel = helpers.MockDMChannel() cls.non_staff_member = helpers.MockMember() cls.staff_role = helpers.MockRole(id=121212) @@ -30,38 +32,35 @@ class InWhitelistTests(unittest.TestCase): def test_predicate_returns_true_for_whitelisted_context(self): """The predicate should return `True` if a whitelisted context was passed to it.""" test_cases = ( - # Commands issued in whitelisted channels by members without whitelisted roles - WhitelistedContextTestCase( + InWhitelistTestCase( kwargs={"channels": self.channels}, - ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) + ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member), + description="In whitelisted channels by members without whitelisted roles", ), - # `redirect` should be added implicitly to the `channels` - WhitelistedContextTestCase( + InWhitelistTestCase( kwargs={"redirect": self.bot_commands.id}, - ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member) + ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member), + description="`redirect` should be implicitly added to `channels`", ), - - # Commands issued in a whitelisted category by members without whitelisted roles - WhitelistedContextTestCase( + InWhitelistTestCase( kwargs={"categories": self.categories}, - ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member) + ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member), + description="Whitelisted category without whitelisted role", ), - - # Command issued by a staff member in a non-whitelisted channel/category - WhitelistedContextTestCase( + InWhitelistTestCase( kwargs={"roles": self.roles}, - ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member) + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member), + description="Whitelisted role outside of whitelisted channel/category" ), - - # With all kwargs provided - WhitelistedContextTestCase( + InWhitelistTestCase( kwargs={ "channels": self.channels, "categories": self.categories, "roles": self.roles, "redirect": self.bot_commands, }, - ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member) + ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member), + description="Case with all whitelist kwargs used", ), ) @@ -71,45 +70,78 @@ class InWhitelistTests(unittest.TestCase): with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): predicate = in_whitelist(**test_case.kwargs) - with self.subTest(test_case=test_case): + with self.subTest(test_description=test_case.description): self.assertTrue(predicate(test_case.ctx)) def test_predicate_raises_exception_for_non_whitelisted_context(self): """The predicate should raise `InWhitelistCheckFailure` for a non-whitelisted context.""" test_cases = ( - # Failing check with `redirect` - WhitelistedContextTestCase( + # Failing check with explicit `redirect` + InWhitelistTestCase( kwargs={ "categories": self.categories, "channels": self.channels, "roles": self.roles, "redirect": self.bot_commands.id, }, - ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member), + description="Failing check with an explicit redirect channel", + ), + + # Failing check with implicit `redirect` + InWhitelistTestCase( + kwargs={ + "categories": self.categories, + "channels": self.channels, + "roles": self.roles, + }, + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member), + description="Failing check with an implicit redirect channel", ), # Failing check without `redirect` - WhitelistedContextTestCase( + InWhitelistTestCase( + kwargs={ + "categories": self.categories, + "channels": self.channels, + "roles": self.roles, + "redirect": None, + }, + ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member), + description="Failing check without a redirect channel", + ), + + # Command issued in DM channel + InWhitelistTestCase( kwargs={ "categories": self.categories, "channels": self.channels, "roles": self.roles, + "redirect": None, }, - ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member) + ctx=helpers.MockContext(channel=self.dm_channel, author=self.dm_channel.me), + description="Commands issued in DM channel should be rejected", ), ) for test_case in test_cases: - # Create expected exception message based on whether or not a redirect channel was provided - expected_message = "Sorry, but you are not allowed to use that command here." - if test_case.kwargs.get("redirect"): - expected_message += f" Please use the <#{test_case.kwargs['redirect']}> channel instead." + if "redirect" not in test_case.kwargs or test_case.kwargs["redirect"] is not None: + # There are two cases in which we have a redirect channel: + # 1. No redirect channel was passed; the default value of `bot_commands` is used + # 2. An explicit `redirect` is set that is "not None" + redirect_channel = test_case.kwargs.get("redirect", constants.Channels.bot_commands) + redirect_message = f" here. Please use the <#{redirect_channel}> channel instead" + else: + # If an explicit `None` was passed for `redirect`, there is no redirect channel + redirect_message = "" + + exception_message = f"You are not allowed to use that command{redirect_message}." # patch `commands.check` with a no-op lambda that just returns the predicate passed to it # so we can test the predicate that was generated from the specified kwargs. with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate): predicate = in_whitelist(**test_case.kwargs) - with self.subTest(test_case=test_case): - with self.assertRaises(InWhitelistCheckFailure, msg=expected_message): + with self.subTest(test_description=test_case.description): + with self.assertRaisesRegex(InWhitelistCheckFailure, exception_message): predicate(test_case.ctx) -- cgit v1.2.3 From b43379d663a86680f762d20a7bd27a20927d4bfc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 30 Apr 2020 18:35:03 -0700 Subject: Tests: change avatar_url_as assertion to use static_format --- tests/bot/cogs/test_information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 6dace1080..b5f928dd6 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -485,7 +485,7 @@ class UserEmbedTests(unittest.TestCase): user.avatar_url_as.return_value = "avatar url" embed = asyncio.run(self.cog.create_user_embed(ctx, user)) - user.avatar_url_as.assert_called_once_with(format="png") + user.avatar_url_as.assert_called_once_with(static_format="png") self.assertEqual(embed.thumbnail.url, "avatar url") -- cgit v1.2.3 From 4e24e9c43a331ebc0f9b598f4de6c45e04216782 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Sun, 17 May 2020 12:10:56 +0200 Subject: Use `send_help` to invoke command help After the refactoring of the help command, we need to use the built-in method of calling the help command: `Context.send_help`. As an argument, the qualified name (a string containing the full command path, including parents) of the command can be passed. Examples: - await ctx.send_help("reminders edit") This would send a help embed with information on `!reminders edit` to the Context. - await ctx.send_help(ctx.command.qualified_name) This would extract the qualified name of the command, which is the full command path, and send a help embed to Context. - await ctx.send_help() This will send the main "root" help embed to the Context. --- bot/cogs/bot.py | 2 +- bot/cogs/clean.py | 2 +- bot/cogs/defcon.py | 2 +- bot/cogs/error_handler.py | 33 +++++++++++++-------------------- bot/cogs/eval.py | 2 +- bot/cogs/extensions.py | 8 ++++---- bot/cogs/moderation/management.py | 2 +- bot/cogs/off_topic_names.py | 2 +- bot/cogs/reddit.py | 2 +- bot/cogs/reminders.py | 2 +- bot/cogs/site.py | 2 +- bot/cogs/snekbox.py | 2 +- bot/cogs/utils.py | 2 +- bot/cogs/watchchannels/bigbrother.py | 2 +- bot/cogs/watchchannels/talentpool.py | 4 ++-- tests/bot/cogs/test_snekbox.py | 3 +-- 16 files changed, 32 insertions(+), 40 deletions(-) (limited to 'tests') diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index a6929b431..ae829d5c3 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -41,7 +41,7 @@ class BotCog(Cog, name="Bot"): @with_role(Roles.verified) async def botinfo_group(self, ctx: Context) -> None: """Bot informational commands.""" - await ctx.invoke(self.bot.get_command("help"), "bot") + await ctx.send_help("bot") @botinfo_group.command(name='about', aliases=('info',), hidden=True) @with_role(Roles.verified) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 5cdf0b048..e9bdbf510 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -180,7 +180,7 @@ class Clean(Cog): @with_role(*MODERATION_ROLES) async def clean_group(self, ctx: Context) -> None: """Commands for cleaning messages in channels.""" - await ctx.invoke(self.bot.get_command("help"), "clean") + await ctx.send_help("clean") @clean_group.command(name="user", aliases=["users"]) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index 56fca002a..71847a441 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -122,7 +122,7 @@ class Defcon(Cog): @with_role(Roles.admins, Roles.owners) async def defcon_group(self, ctx: Context) -> None: """Check the DEFCON status or run a subcommand.""" - await ctx.invoke(self.bot.get_command("help"), "defcon") + await ctx.send_help("defcon") async def _defcon_action(self, ctx: Context, days: int, action: Action) -> None: """Providing a structured way to do an defcon action.""" diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index b2f4c59f6..2d6cd85e6 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -2,7 +2,7 @@ import contextlib import logging import typing as t -from discord.ext.commands import Cog, Command, Context, errors +from discord.ext.commands import Cog, Context, errors from sentry_sdk import push_scope from bot.api import ResponseCodeError @@ -79,19 +79,13 @@ class ErrorHandler(Cog): f"{e.__class__.__name__}: {e}" ) - async def get_help_command(self, command: t.Optional[Command]) -> t.Tuple: - """Return the help command invocation args to display help for `command`.""" - parent = None - if command is not None: - parent = command.parent - - # Retrieve the help command for the invoked command. - if parent and command: - return self.bot.get_command("help"), parent.name, command.name - elif command: - return self.bot.get_command("help"), command.name - else: - return self.bot.get_command("help") + @staticmethod + def get_help_command(ctx: Context) -> t.Coroutine: + """Return a prepared `help` command invocation coroutine.""" + if ctx.command: + return ctx.send_help(ctx.command.qualified_name) + + return ctx.send_help() async def try_silence(self, ctx: Context) -> bool: """ @@ -165,20 +159,19 @@ class ErrorHandler(Cog): * ArgumentParsingError: send an error message * Other: send an error message and the help command """ - # TODO: use ctx.send_help() once PR #519 is merged. - help_command = await self.get_help_command(ctx.command) + prepared_help_command = self.get_help_command(ctx) if isinstance(e, errors.MissingRequiredArgument): await ctx.send(f"Missing required argument `{e.param.name}`.") - await ctx.invoke(*help_command) + await prepared_help_command self.bot.stats.incr("errors.missing_required_argument") elif isinstance(e, errors.TooManyArguments): await ctx.send(f"Too many arguments provided.") - await ctx.invoke(*help_command) + await prepared_help_command self.bot.stats.incr("errors.too_many_arguments") elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") - await ctx.invoke(*help_command) + await prepared_help_command self.bot.stats.incr("errors.bad_argument") elif isinstance(e, errors.BadUnionArgument): await ctx.send(f"Bad argument: {e}\n```{e.errors[-1]}```") @@ -188,7 +181,7 @@ class ErrorHandler(Cog): self.bot.stats.incr("errors.argument_parsing_error") else: await ctx.send("Something about your input seems off. Check the arguments:") - await ctx.invoke(*help_command) + await prepared_help_command self.bot.stats.incr("errors.other_user_input_error") @staticmethod diff --git a/bot/cogs/eval.py b/bot/cogs/eval.py index 52136fc8d..2d52197e8 100644 --- a/bot/cogs/eval.py +++ b/bot/cogs/eval.py @@ -178,7 +178,7 @@ async def func(): # (None,) -> Any async def internal_group(self, ctx: Context) -> None: """Internal commands. Top secret!""" if not ctx.invoked_subcommand: - await ctx.invoke(self.bot.get_command("help"), "internal") + await ctx.send_help("internal") @internal_group.command(name='eval', aliases=('e',)) @with_role(Roles.admins, Roles.owners) diff --git a/bot/cogs/extensions.py b/bot/cogs/extensions.py index fb6cd9aa3..4493046e1 100644 --- a/bot/cogs/extensions.py +++ b/bot/cogs/extensions.py @@ -65,7 +65,7 @@ class Extensions(commands.Cog): @group(name="extensions", aliases=("ext", "exts", "c", "cogs"), invoke_without_command=True) async def extensions_group(self, ctx: Context) -> None: """Load, unload, reload, and list loaded extensions.""" - await ctx.invoke(self.bot.get_command("help"), "extensions") + await ctx.send_help("extensions") @extensions_group.command(name="load", aliases=("l",)) async def load_command(self, ctx: Context, *extensions: Extension) -> None: @@ -75,7 +75,7 @@ class Extensions(commands.Cog): If '\*' or '\*\*' is given as the name, all unloaded extensions will be loaded. """ # noqa: W605 if not extensions: - await ctx.invoke(self.bot.get_command("help"), "extensions load") + await ctx.send_help("extensions load") return if "*" in extensions or "**" in extensions: @@ -92,7 +92,7 @@ class Extensions(commands.Cog): If '\*' or '\*\*' is given as the name, all loaded extensions will be unloaded. """ # noqa: W605 if not extensions: - await ctx.invoke(self.bot.get_command("help"), "extensions unload") + await ctx.send_help("extensions unload") return blacklisted = "\n".join(UNLOAD_BLACKLIST & set(extensions)) @@ -118,7 +118,7 @@ class Extensions(commands.Cog): If '\*\*' is given as the name, all extensions, including unloaded ones, will be reloaded. """ # noqa: W605 if not extensions: - await ctx.invoke(self.bot.get_command("help"), "extensions reload") + await ctx.send_help("extensions reload") return if "**" in extensions: diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py index 250a24247..5cd59cc07 100644 --- a/bot/cogs/moderation/management.py +++ b/bot/cogs/moderation/management.py @@ -43,7 +43,7 @@ class ModManagement(commands.Cog): @commands.group(name='infraction', aliases=('infr', 'infractions', 'inf'), invoke_without_command=True) async def infraction_group(self, ctx: Context) -> None: """Infraction manipulation commands.""" - await ctx.invoke(self.bot.get_command("help"), "infraction") + await ctx.send_help("infraction") @infraction_group.command(name='edit') async def infraction_edit( diff --git a/bot/cogs/off_topic_names.py b/bot/cogs/off_topic_names.py index 81511f99d..829772f65 100644 --- a/bot/cogs/off_topic_names.py +++ b/bot/cogs/off_topic_names.py @@ -97,7 +97,7 @@ class OffTopicNames(Cog): @with_role(*MODERATION_ROLES) async def otname_group(self, ctx: Context) -> None: """Add or list items from the off-topic channel name rotation.""" - await ctx.invoke(self.bot.get_command("help"), "otname") + await ctx.send_help("otname") @otname_group.command(name='add', aliases=('a',)) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 5a7fa100f..07a2497be 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -245,7 +245,7 @@ class Reddit(Cog): @group(name="reddit", invoke_without_command=True) async def reddit_group(self, ctx: Context) -> None: """View the top posts from various subreddits.""" - await ctx.invoke(self.bot.get_command("help"), "reddit") + await ctx.send_help("reddit") @reddit_group.command(name="top") async def top_command(self, ctx: Context, subreddit: Subreddit = "r/Python") -> None: diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index 8b6457cbb..e2289c75d 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -281,7 +281,7 @@ class Reminders(Scheduler, Cog): @remind_group.group(name="edit", aliases=("change", "modify"), invoke_without_command=True) async def edit_reminder_group(self, ctx: Context) -> None: """Commands for modifying your current reminders.""" - await ctx.invoke(self.bot.get_command("help"), "reminders", "edit") + await ctx.send_help("reminders edit") @edit_reminder_group.command(name="duration", aliases=("time",)) async def edit_reminder_duration(self, ctx: Context, id_: int, expiration: Duration) -> None: diff --git a/bot/cogs/site.py b/bot/cogs/site.py index 853e29568..c17761a2b 100644 --- a/bot/cogs/site.py +++ b/bot/cogs/site.py @@ -21,7 +21,7 @@ class Site(Cog): @group(name="site", aliases=("s",), invoke_without_command=True) async def site_group(self, ctx: Context) -> None: """Commands for getting info about our website.""" - await ctx.invoke(self.bot.get_command("help"), "site") + await ctx.send_help("site") @site_group.command(name="home", aliases=("about",)) async def site_main(self, ctx: Context) -> None: diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 8d4688114..5de978758 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -289,7 +289,7 @@ class Snekbox(Cog): return if not code: # None or empty string - await ctx.invoke(self.bot.get_command("help"), "eval") + await ctx.send_help("eval") return log.info(f"Received code from {ctx.author} for evaluation:\n{code}") diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 89d556f58..7350dc2ba 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -55,7 +55,7 @@ class Utils(Cog): if pep_number.isdigit(): pep_number = int(pep_number) else: - await ctx.invoke(self.bot.get_command("help"), "pep") + await ctx.send_help("pep") return # Handle PEP 0 directly because it's not in .rst or .txt so it can't be accessed like other PEPs. diff --git a/bot/cogs/watchchannels/bigbrother.py b/bot/cogs/watchchannels/bigbrother.py index 903c87f85..37f2d2b9d 100644 --- a/bot/cogs/watchchannels/bigbrother.py +++ b/bot/cogs/watchchannels/bigbrother.py @@ -30,7 +30,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): @with_role(*MODERATION_ROLES) async def bigbrother_group(self, ctx: Context) -> None: """Monitors users by relaying their messages to the Big Brother watch channel.""" - await ctx.invoke(self.bot.get_command("help"), "bigbrother") + await ctx.send_help("bigbrother") @bigbrother_group.command(name='watched', aliases=('all', 'list')) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/watchchannels/talentpool.py b/bot/cogs/watchchannels/talentpool.py index ad0c51fa6..b8473963d 100644 --- a/bot/cogs/watchchannels/talentpool.py +++ b/bot/cogs/watchchannels/talentpool.py @@ -34,7 +34,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): @with_role(*MODERATION_ROLES) async def nomination_group(self, ctx: Context) -> None: """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" - await ctx.invoke(self.bot.get_command("help"), "talentpool") + await ctx.send_help("talentpool") @nomination_group.command(name='watched', aliases=('all', 'list')) @with_role(*MODERATION_ROLES) @@ -173,7 +173,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): @with_role(*MODERATION_ROLES) async def nomination_edit_group(self, ctx: Context) -> None: """Commands to edit nominations.""" - await ctx.invoke(self.bot.get_command("help"), "talentpool", "edit") + await ctx.send_help("talentpool edit") @nomination_edit_group.command(name='reason') @with_role(*MODERATION_ROLES) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 1dec0ccaf..190d41d66 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -209,9 +209,8 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): async def test_eval_command_call_help(self): """Test if the eval command call the help command if no code is provided.""" ctx = MockContext() - ctx.invoke = AsyncMock() await self.cog.eval_command.callback(self.cog, ctx=ctx, code='') - ctx.invoke.assert_called_once_with(self.bot.get_command("help"), "eval") + ctx.send_help.assert_called_once_with("eval") async def test_send_eval(self): """Test the send_eval function.""" -- cgit v1.2.3 From 87cec1a863213aa23a07d29ca928766d382ee732 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Sun, 17 May 2020 12:37:34 +0200 Subject: Use `Command`-object for `send_help` As @mathsman5133 pointed out, it's better to use the `Command`-instance we typically already have in the current context than to rely on parsing the qualified name again. The invocation is now done as: `await ctx.send_help(ctx.command)` --- bot/cogs/bot.py | 2 +- bot/cogs/clean.py | 2 +- bot/cogs/defcon.py | 2 +- bot/cogs/error_handler.py | 2 +- bot/cogs/eval.py | 2 +- bot/cogs/extensions.py | 8 ++++---- bot/cogs/moderation/management.py | 2 +- bot/cogs/off_topic_names.py | 2 +- bot/cogs/reddit.py | 2 +- bot/cogs/reminders.py | 2 +- bot/cogs/site.py | 2 +- bot/cogs/snekbox.py | 2 +- bot/cogs/utils.py | 2 +- bot/cogs/watchchannels/bigbrother.py | 2 +- bot/cogs/watchchannels/talentpool.py | 4 ++-- tests/bot/cogs/test_snekbox.py | 4 ++-- 16 files changed, 21 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index ae829d5c3..f6aea51c5 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -41,7 +41,7 @@ class BotCog(Cog, name="Bot"): @with_role(Roles.verified) async def botinfo_group(self, ctx: Context) -> None: """Bot informational commands.""" - await ctx.send_help("bot") + await ctx.send_help(ctx.command) @botinfo_group.command(name='about', aliases=('info',), hidden=True) @with_role(Roles.verified) diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index e9bdbf510..b5d9132cb 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -180,7 +180,7 @@ class Clean(Cog): @with_role(*MODERATION_ROLES) async def clean_group(self, ctx: Context) -> None: """Commands for cleaning messages in channels.""" - await ctx.send_help("clean") + await ctx.send_help(ctx.command) @clean_group.command(name="user", aliases=["users"]) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index 71847a441..25b0a6ad5 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -122,7 +122,7 @@ class Defcon(Cog): @with_role(Roles.admins, Roles.owners) async def defcon_group(self, ctx: Context) -> None: """Check the DEFCON status or run a subcommand.""" - await ctx.send_help("defcon") + await ctx.send_help(ctx.command) async def _defcon_action(self, ctx: Context, days: int, action: Action) -> None: """Providing a structured way to do an defcon action.""" diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 2d6cd85e6..23d1eed82 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -83,7 +83,7 @@ class ErrorHandler(Cog): def get_help_command(ctx: Context) -> t.Coroutine: """Return a prepared `help` command invocation coroutine.""" if ctx.command: - return ctx.send_help(ctx.command.qualified_name) + return ctx.send_help(ctx.command) return ctx.send_help() diff --git a/bot/cogs/eval.py b/bot/cogs/eval.py index 2d52197e8..eb8bfb1cf 100644 --- a/bot/cogs/eval.py +++ b/bot/cogs/eval.py @@ -178,7 +178,7 @@ async def func(): # (None,) -> Any async def internal_group(self, ctx: Context) -> None: """Internal commands. Top secret!""" if not ctx.invoked_subcommand: - await ctx.send_help("internal") + await ctx.send_help(ctx.command) @internal_group.command(name='eval', aliases=('e',)) @with_role(Roles.admins, Roles.owners) diff --git a/bot/cogs/extensions.py b/bot/cogs/extensions.py index 4493046e1..365f198ff 100644 --- a/bot/cogs/extensions.py +++ b/bot/cogs/extensions.py @@ -65,7 +65,7 @@ class Extensions(commands.Cog): @group(name="extensions", aliases=("ext", "exts", "c", "cogs"), invoke_without_command=True) async def extensions_group(self, ctx: Context) -> None: """Load, unload, reload, and list loaded extensions.""" - await ctx.send_help("extensions") + await ctx.send_help(ctx.command) @extensions_group.command(name="load", aliases=("l",)) async def load_command(self, ctx: Context, *extensions: Extension) -> None: @@ -75,7 +75,7 @@ class Extensions(commands.Cog): If '\*' or '\*\*' is given as the name, all unloaded extensions will be loaded. """ # noqa: W605 if not extensions: - await ctx.send_help("extensions load") + await ctx.send_help(ctx.command) return if "*" in extensions or "**" in extensions: @@ -92,7 +92,7 @@ class Extensions(commands.Cog): If '\*' or '\*\*' is given as the name, all loaded extensions will be unloaded. """ # noqa: W605 if not extensions: - await ctx.send_help("extensions unload") + await ctx.send_help(ctx.command) return blacklisted = "\n".join(UNLOAD_BLACKLIST & set(extensions)) @@ -118,7 +118,7 @@ class Extensions(commands.Cog): If '\*\*' is given as the name, all extensions, including unloaded ones, will be reloaded. """ # noqa: W605 if not extensions: - await ctx.send_help("extensions reload") + await ctx.send_help(ctx.command) return if "**" in extensions: diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py index 5cd59cc07..edfdfd9e2 100644 --- a/bot/cogs/moderation/management.py +++ b/bot/cogs/moderation/management.py @@ -43,7 +43,7 @@ class ModManagement(commands.Cog): @commands.group(name='infraction', aliases=('infr', 'infractions', 'inf'), invoke_without_command=True) async def infraction_group(self, ctx: Context) -> None: """Infraction manipulation commands.""" - await ctx.send_help("infraction") + await ctx.send_help(ctx.command) @infraction_group.command(name='edit') async def infraction_edit( diff --git a/bot/cogs/off_topic_names.py b/bot/cogs/off_topic_names.py index 829772f65..201579a0b 100644 --- a/bot/cogs/off_topic_names.py +++ b/bot/cogs/off_topic_names.py @@ -97,7 +97,7 @@ class OffTopicNames(Cog): @with_role(*MODERATION_ROLES) async def otname_group(self, ctx: Context) -> None: """Add or list items from the off-topic channel name rotation.""" - await ctx.send_help("otname") + await ctx.send_help(ctx.command) @otname_group.command(name='add', aliases=('a',)) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 07a2497be..5f2aec7a5 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -245,7 +245,7 @@ class Reddit(Cog): @group(name="reddit", invoke_without_command=True) async def reddit_group(self, ctx: Context) -> None: """View the top posts from various subreddits.""" - await ctx.send_help("reddit") + await ctx.send_help(ctx.command) @reddit_group.command(name="top") async def top_command(self, ctx: Context, subreddit: Subreddit = "r/Python") -> None: diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index e2289c75d..c242d2920 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -281,7 +281,7 @@ class Reminders(Scheduler, Cog): @remind_group.group(name="edit", aliases=("change", "modify"), invoke_without_command=True) async def edit_reminder_group(self, ctx: Context) -> None: """Commands for modifying your current reminders.""" - await ctx.send_help("reminders edit") + await ctx.send_help(ctx.command) @edit_reminder_group.command(name="duration", aliases=("time",)) async def edit_reminder_duration(self, ctx: Context, id_: int, expiration: Duration) -> None: diff --git a/bot/cogs/site.py b/bot/cogs/site.py index c17761a2b..7fc2a9c34 100644 --- a/bot/cogs/site.py +++ b/bot/cogs/site.py @@ -21,7 +21,7 @@ class Site(Cog): @group(name="site", aliases=("s",), invoke_without_command=True) async def site_group(self, ctx: Context) -> None: """Commands for getting info about our website.""" - await ctx.send_help("site") + await ctx.send_help(ctx.command) @site_group.command(name="home", aliases=("about",)) async def site_main(self, ctx: Context) -> None: diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 5de978758..c2782b9c8 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -289,7 +289,7 @@ class Snekbox(Cog): return if not code: # None or empty string - await ctx.send_help("eval") + await ctx.send_help(ctx.command) return log.info(f"Received code from {ctx.author} for evaluation:\n{code}") diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 7350dc2ba..6b59d37c8 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -55,7 +55,7 @@ class Utils(Cog): if pep_number.isdigit(): pep_number = int(pep_number) else: - await ctx.send_help("pep") + await ctx.send_help(ctx.command) return # Handle PEP 0 directly because it's not in .rst or .txt so it can't be accessed like other PEPs. diff --git a/bot/cogs/watchchannels/bigbrother.py b/bot/cogs/watchchannels/bigbrother.py index 37f2d2b9d..e4fb173e0 100644 --- a/bot/cogs/watchchannels/bigbrother.py +++ b/bot/cogs/watchchannels/bigbrother.py @@ -30,7 +30,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): @with_role(*MODERATION_ROLES) async def bigbrother_group(self, ctx: Context) -> None: """Monitors users by relaying their messages to the Big Brother watch channel.""" - await ctx.send_help("bigbrother") + await ctx.send_help(ctx.command) @bigbrother_group.command(name='watched', aliases=('all', 'list')) @with_role(*MODERATION_ROLES) diff --git a/bot/cogs/watchchannels/talentpool.py b/bot/cogs/watchchannels/talentpool.py index b8473963d..9a85c68c2 100644 --- a/bot/cogs/watchchannels/talentpool.py +++ b/bot/cogs/watchchannels/talentpool.py @@ -34,7 +34,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): @with_role(*MODERATION_ROLES) async def nomination_group(self, ctx: Context) -> None: """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" - await ctx.send_help("talentpool") + await ctx.send_help(ctx.command) @nomination_group.command(name='watched', aliases=('all', 'list')) @with_role(*MODERATION_ROLES) @@ -173,7 +173,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): @with_role(*MODERATION_ROLES) async def nomination_edit_group(self, ctx: Context) -> None: """Commands to edit nominations.""" - await ctx.send_help("talentpool edit") + await ctx.send_help(ctx.command) @nomination_edit_group.command(name='reason') @with_role(*MODERATION_ROLES) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 190d41d66..8490b02ca 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -208,9 +208,9 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): async def test_eval_command_call_help(self): """Test if the eval command call the help command if no code is provided.""" - ctx = MockContext() + ctx = MockContext(command="sentinel") await self.cog.eval_command.callback(self.cog, ctx=ctx, code='') - ctx.send_help.assert_called_once_with("eval") + ctx.send_help.assert_called_once_with("sentinel") async def test_send_eval(self): """Test the send_eval function.""" -- cgit v1.2.3