diff options
-rw-r--r-- | bot/converters.py | 35 | ||||
-rw-r--r-- | bot/exts/backend/error_handler.py | 26 | ||||
-rw-r--r-- | bot/exts/info/tags.py | 9 | ||||
-rw-r--r-- | tests/bot/exts/backend/test_error_handler.py | 24 | ||||
-rw-r--r-- | tests/bot/test_converters.py | 29 |
5 files changed, 13 insertions, 110 deletions
diff --git a/bot/converters.py b/bot/converters.py index 2a3943831..038d2a287 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -236,41 +236,6 @@ class Snowflake(IDConverter): return snowflake -class TagNameConverter(Converter): - """ - Ensure that a proposed tag name is valid. - - Valid tag names meet the following conditions: - * All ASCII characters - * Has at least one non-whitespace character - * Not solely numeric - * Shorter than 127 characters - """ - - @staticmethod - async def convert(ctx: Context, tag_name: str) -> str: - """Lowercase & strip whitespace from proposed tag_name & ensure it's valid.""" - tag_name = tag_name.lower().strip() - - # The tag name has at least one invalid character. - if ascii(tag_name)[1:-1] != tag_name: - raise BadArgument("Don't be ridiculous, you can't use that character!") - - # The tag name is either empty, or consists of nothing but whitespace. - elif not tag_name: - raise BadArgument("Tag names should not be empty, or filled with whitespace.") - - # The tag name is longer than 127 characters. - elif len(tag_name) > 127: - raise BadArgument("Are you insane? That's way too long!") - - # The tag name is ascii but does not contain any letters. - elif not any(character.isalpha() for character in tag_name): - raise BadArgument("Tag names must contain at least one letter.") - - return tag_name - - class TagContentConverter(Converter): """Ensure proposed tag content is not empty and contains at least one non-whitespace character.""" diff --git a/bot/exts/backend/error_handler.py b/bot/exts/backend/error_handler.py index 51b6bc660..f2e2a964c 100644 --- a/bot/exts/backend/error_handler.py +++ b/bot/exts/backend/error_handler.py @@ -9,7 +9,6 @@ from sentry_sdk import push_scope from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Colours, Icons, MODERATION_ROLES -from bot.converters import TagNameConverter from bot.errors import InvalidInfractedUser, LockedResourceError from bot.exts.info import tags from bot.utils.checks import ContextCheckFailure @@ -154,23 +153,16 @@ class ErrorHandler(Cog): await self.on_command_error(ctx, tag_error) return - try: - tag_identifier = tags.TagIdentifier.from_string(ctx.message.content) - if tag_identifier.group is not None: - tag_name = await TagNameConverter.convert(ctx, tag_identifier.name) - tag_name_or_group = await TagNameConverter.convert(ctx, tag_identifier.group) - else: - tag_name = None - tag_name_or_group = await TagNameConverter.convert(ctx, tag_identifier.name) - - except errors.BadArgument: - log.debug( - f"{ctx.author} tried to use an invalid command " - f"and the fallback tag failed validation in TagNameConverter." - ) + tag_identifier = tags.TagIdentifier.from_string(ctx.message.content) + if tag_identifier.group is not None: + tag_name = tag_identifier.name + tag_name_or_group = tag_identifier.group else: - if await ctx.invoke(tags_get_command, tag_name_or_group, tag_name): - return + tag_name = None + tag_name_or_group = tag_identifier.name + + if await ctx.invoke(tags_get_command, tag_name_or_group, tag_name): + return if not any(role.id in MODERATION_ROLES for role in ctx.author.roles): await self.send_command_suggestion(ctx, ctx.invoked_with) diff --git a/bot/exts/info/tags.py b/bot/exts/info/tags.py index 909831bc7..0fc6e99d0 100644 --- a/bot/exts/info/tags.py +++ b/bot/exts/info/tags.py @@ -14,7 +14,6 @@ from discord.ext.commands import Cog, Context, group from bot import constants from bot.bot import Bot -from bot.converters import TagNameConverter from bot.pagination import LinePaginator from bot.utils.messages import wait_for_deletion @@ -235,8 +234,8 @@ class Tags(Cog): async def tags_group( self, ctx: Context, - tag_name_or_group: TagNameConverter = None, - tag_name: TagNameConverter = None, + tag_name_or_group: str = None, + tag_name: str = None, ) -> None: """Show all known tags, a single tag, or run a subcommand.""" await self.get_command(ctx, tag_name_or_group=tag_name_or_group, tag_name=tag_name) @@ -356,8 +355,8 @@ class Tags(Cog): @tags_group.command(name="get", aliases=("show", "g")) async def get_command( self, ctx: Context, - tag_name_or_group: TagNameConverter = None, - tag_name: TagNameConverter = None, + tag_name_or_group: str = None, + tag_name: str = None, ) -> bool: """ If a single argument matching a group name is given, list all accessible tags from that group diff --git a/tests/bot/exts/backend/test_error_handler.py b/tests/bot/exts/backend/test_error_handler.py index 4a466c22e..eafcbae6c 100644 --- a/tests/bot/exts/backend/test_error_handler.py +++ b/tests/bot/exts/backend/test_error_handler.py @@ -296,30 +296,6 @@ class TryGetTagTests(unittest.IsolatedAsyncioTestCase): self.assertIsNone(await self.cog.try_get_tag(self.ctx)) self.cog.on_command_error.assert_awaited_once_with(self.ctx, err) - @patch("bot.exts.backend.error_handler.TagNameConverter") - async def test_try_get_tag_convert_success(self, tag_converter): - """Converting tag should successful.""" - self.ctx.message = MagicMock(content="foo") - tag_converter.convert = AsyncMock(return_value="foo") - self.assertIsNone(await self.cog.try_get_tag(self.ctx)) - tag_converter.convert.assert_awaited_once_with(self.ctx, "foo") - self.ctx.invoke.assert_awaited_once() - - self.ctx.reset_mock() - self.ctx.message = MagicMock(content="foo bar") - tag_converter.convert = AsyncMock(return_value="foo bar") - self.assertIsNone(await self.cog.try_get_tag(self.ctx)) - self.assertEqual(tag_converter.convert.call_count, 2) - self.ctx.invoke.assert_awaited_once() - - @patch("bot.exts.backend.error_handler.TagNameConverter") - async def test_try_get_tag_convert_fail(self, tag_converter): - """Converting tag should raise `BadArgument`.""" - self.ctx.reset_mock() - tag_converter.convert = AsyncMock(side_effect=errors.BadArgument()) - self.assertIsNone(await self.cog.try_get_tag(self.ctx)) - self.ctx.invoke.assert_not_awaited() - async def test_try_get_tag_ctx_invoke(self): """Should call `ctx.invoke` with proper args/kwargs.""" test_cases = ( diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 4af84dde5..d0d7af1ba 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -12,7 +12,6 @@ from bot.converters import ( ISODateTime, PackageName, TagContentConverter, - TagNameConverter, ) @@ -50,34 +49,6 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await TagContentConverter.convert(self.context, value) - async def test_tag_name_converter_for_valid(self): - """TagNameConverter should return the correct values for valid tag names.""" - test_values = ( - ('tracebacks', 'tracebacks'), - ('Tracebacks', 'tracebacks'), - (' Tracebacks ', 'tracebacks'), - ) - - for name, expected_conversion in test_values: - with self.subTest(name=name, expected_conversion=expected_conversion): - conversion = await TagNameConverter.convert(self.context, name) - self.assertEqual(conversion, expected_conversion) - - async def test_tag_name_converter_for_invalid(self): - """TagNameConverter should raise the correct exception for invalid tag names.""" - test_values = ( - ('👋', "Don't be ridiculous, you can't use that character!"), - ('', "Tag names should not be empty, or filled with whitespace."), - (' ', "Tag names should not be empty, or filled with whitespace."), - ('42', "Tag names must contain at least one letter."), - ('x' * 128, "Are you insane? That's way too long!"), - ) - - for invalid_name, exception_message in test_values: - with self.subTest(invalid_name=invalid_name, exception_message=exception_message): - with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): - await TagNameConverter.convert(self.context, invalid_name) - async def test_package_name_for_valid(self): """PackageName returns valid package names unchanged.""" test_values = ('foo', 'le_mon', 'num83r') |