Remove TagNameConverter

The converter was now only used to restrict requested names which
can be handled by not matching a tag in the cog and not displaying output,
this allows us to be a bit more generous with tag names during the command
fallback when a name with invalid symbols is parsed after a group

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')