Merge pull request #741 from Denayder/information-refactor

Refactor information.py with lots of changes that makes the overall code cleaner and faster

2 files changed, 50 insertions, 73 deletions

diff --git a/bot/cogs/information.py b/bot/cogs/information.py
index 125d7ce24..13c8aabaa 100644
--- a/bot/cogs/information.py
+++ b/bot/cogs/information.py
@@ -2,14 +2,12 @@ import colorsys
 import logging
 import pprint
 import textwrap
-import typing
-from collections import defaultdict
-from typing import Any, Mapping, Optional
-
-import discord
-from discord import CategoryChannel, Colour, Embed, Member, Role, TextChannel, VoiceChannel, utils
-from discord.ext import commands
-from discord.ext.commands import BucketType, Cog, Context, command, group
+from collections import Counter, defaultdict
+from string import Template
+from typing import Any, Mapping, Optional, Union
+
+from discord import Colour, Embed, Member, Message, Role, Status, utils
+from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group
 from discord.utils import escape_markdown
 
 from bot import constants
@@ -32,8 +30,7 @@ class Information(Cog):
     async def roles_info(self, ctx: Context) -> None:
         """Returns a list of all roles and their corresponding IDs."""
         # Sort the roles alphabetically and remove the @everyone role
-        roles = sorted(ctx.guild.roles, key=lambda role: role.name)
-        roles = [role for role in roles if role.name != "@everyone"]
+        roles = sorted(ctx.guild.roles[1:], key=lambda role: role.name)
 
         # Build a string
         role_string = ""
@@ -46,20 +43,20 @@ class Information(Cog):
             colour=Colour.blurple(),
             description=role_string
         )
-
         embed.set_footer(text=f"Total roles: {len(roles)}")
 
         await ctx.send(embed=embed)
 
     @with_role(*constants.MODERATION_ROLES)
     @command(name="role")
-    async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None:
+    async def role_info(self, ctx: Context, *roles: Union[Role, str]) -> None:
         """
         Return information on a role or list of roles.
 
         To specify multiple roles just add to the arguments, delimit roles with spaces in them using quotation marks.
         """
         parsed_roles = []
+        failed_roles = []
 
         for role_name in roles:
             if isinstance(role_name, Role):
@@ -70,29 +67,29 @@ class Information(Cog):
             role = utils.find(lambda r: r.name.lower() == role_name.lower(), ctx.guild.roles)
 
             if not role:
-                await ctx.send(f":x: Could not convert `{role_name}` to a role")
+                failed_roles.append(role_name)
                 continue
 
             parsed_roles.append(role)
 
+        if failed_roles:
+            await ctx.send(
+                ":x: I could not convert the following role names to a role: \n- "
+                "\n- ".join(failed_roles)
+            )
+
         for role in parsed_roles:
+            h, s, v = colorsys.rgb_to_hsv(*role.colour.to_rgb())
+
             embed = Embed(
                 title=f"{role.name} info",
                 colour=role.colour,
             )
-
             embed.add_field(name="ID", value=role.id, inline=True)
-
             embed.add_field(name="Colour (RGB)", value=f"#{role.colour.value:0>6x}", inline=True)
-
-            h, s, v = colorsys.rgb_to_hsv(*role.colour.to_rgb())
-
             embed.add_field(name="Colour (HSV)", value=f"{h:.2f} {s:.2f} {v}", inline=True)
-
             embed.add_field(name="Member count", value=len(role.members), inline=True)
-
             embed.add_field(name="Position", value=role.position)
-
             embed.add_field(name="Permission code", value=role.permissions.value, inline=True)
 
             await ctx.send(embed=embed)
@@ -104,40 +101,23 @@ class Information(Cog):
         features = ", ".join(ctx.guild.features)
         region = ctx.guild.region
 
-        # How many of each type of channel?
         roles = len(ctx.guild.roles)
-        channels = ctx.guild.channels
-        text_channels = 0
-        category_channels = 0
-        voice_channels = 0
-        for channel in channels:
-            if type(channel) == TextChannel:
-                text_channels += 1
-            elif type(channel) == CategoryChannel:
-                category_channels += 1
-            elif type(channel) == VoiceChannel:
-                voice_channels += 1
-
-        # How many of each user status?
         member_count = ctx.guild.member_count
-        members = ctx.guild.members
-        online = 0
-        dnd = 0
-        idle = 0
-        offline = 0
-        for member in members:
-            if str(member.status) == "online":
-                online += 1
-            elif str(member.status) == "offline":
-                offline += 1
-            elif str(member.status) == "idle":
-                idle += 1
-            elif str(member.status) == "dnd":
-                dnd += 1
 
-        embed = Embed(
-            colour=Colour.blurple(),
-            description=textwrap.dedent(f"""
+        # How many of each type of channel?
+        channels = Counter(c.type for c in ctx.guild.channels)
+        channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]}\n" for ch in channels)).strip()
+
+        # How many of each user status?
+        statuses = Counter(member.status for member in ctx.guild.members)
+        embed = Embed(colour=Colour.blurple())
+
+        # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the
+        # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting
+        # without joining a tuple of strings we can use a Template string to insert the already-formatted channel_counts
+        # after the dedent is made.
+        embed.description = Template(
+            textwrap.dedent(f"""
                 **Server information**
                 Created: {created}
                 Voice region: {region}
@@ -146,18 +126,15 @@ class Information(Cog):
                 **Counts**
                 Members: {member_count:,}
                 Roles: {roles}
-                Text: {text_channels}
-                Voice: {voice_channels}
-                Channel categories: {category_channels}
+                $channel_counts
 
                 **Members**
-                {constants.Emojis.status_online} {online}
-                {constants.Emojis.status_idle} {idle}
-                {constants.Emojis.status_dnd} {dnd}
-                {constants.Emojis.status_offline} {offline}
+                {constants.Emojis.status_online} {statuses[Status.online]:,}
+                {constants.Emojis.status_idle} {statuses[Status.idle]:,}
+                {constants.Emojis.status_dnd} {statuses[Status.dnd]:,}
+                {constants.Emojis.status_offline} {statuses[Status.offline]:,}
             """)
-        )
-
+        ).substitute({"channel_counts": channel_counts})
         embed.set_thumbnail(url=ctx.guild.icon_url)
 
         await ctx.send(embed=embed)
@@ -169,7 +146,7 @@ class Information(Cog):
             user = ctx.author
 
         # Do a role check if this is being executed on someone other than the caller
-        if user != ctx.author and not with_role_check(ctx, *constants.MODERATION_ROLES):
+        elif user != ctx.author and not with_role_check(ctx, *constants.MODERATION_ROLES):
             await ctx.send("You may not use this command on users other than yourself.")
             return
 
@@ -202,7 +179,7 @@ class Information(Cog):
             name = f"{user.nick} ({name})"
 
         joined = time_since(user.joined_at, precision="days")
-        roles = ", ".join(role.mention for role in user.roles if role.name != "@everyone")
+        roles = ", ".join(role.mention for role in user.roles[1:])
 
         description = [
             textwrap.dedent(f"""
@@ -356,13 +333,13 @@ 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, bypass_roles=constants.STAFF_ROLES)
-    async def raw(self, ctx: Context, *, message: discord.Message, json: bool = False) -> None:
+    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
         # doing this extra request is also much easier than trying to convert everything back into a dictionary again
         raw_data = await ctx.bot.http.get_message(message.channel.id, message.id)
 
-        paginator = commands.Paginator()
+        paginator = Paginator()
 
         def add_content(title: str, content: str) -> None:
             paginator.add_line(f'== {title} ==\n')
@@ -390,7 +367,7 @@ class Information(Cog):
             await ctx.send(page)
 
     @raw.command()
-    async def json(self, ctx: Context, message: discord.Message) -> None:
+    async def json(self, ctx: Context, message: Message) -> None:
         """Shows information about the raw API response in a copy-pasteable Python format."""
         await ctx.invoke(self.raw, message=message, json=True)
 
diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py
index 4496a2ae0..deae7ebad 100644
--- a/tests/bot/cogs/test_information.py
+++ b/tests/bot/cogs/test_information.py
@@ -125,10 +125,10 @@ class InformationCogTests(unittest.TestCase):
                 )
             ],
             members=[
-                *(helpers.MockMember(status='online') for _ in range(2)),
-                *(helpers.MockMember(status='idle') for _ in range(1)),
-                *(helpers.MockMember(status='dnd') for _ in range(4)),
-                *(helpers.MockMember(status='offline') for _ in range(3)),
+                *(helpers.MockMember(status=discord.Status.online) for _ in range(2)),
+                *(helpers.MockMember(status=discord.Status.idle) for _ in range(1)),
+                *(helpers.MockMember(status=discord.Status.dnd) for _ in range(4)),
+                *(helpers.MockMember(status=discord.Status.offline) for _ in range(3)),
             ],
             member_count=1_234,
             icon_url='a-lemon.jpg',
@@ -153,9 +153,9 @@ class InformationCogTests(unittest.TestCase):
                 **Counts**
                 Members: {self.ctx.guild.member_count:,}
                 Roles: {len(self.ctx.guild.roles)}
-                Text: 1
-                Voice: 1
-                Channel categories: 1
+                Category channels: 1
+                Text channels: 1
+                Voice channels: 1
 
                 **Members**
                 {constants.Emojis.status_online} 2