From debe73add8bf5c5f9b33e32201f1ce212758c8e4 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:44:56 +0100 Subject: [PATCH 01/15] No longer check if every role is @everyone; just skip the first element in the list --- bot/cogs/information.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 125d7ce24d..bc2deae8f5 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -32,8 +32,7 @@ def __init__(self, bot: Bot): 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 = "" @@ -202,7 +201,7 @@ async def create_user_embed(self, ctx: Context, user: Member) -> Embed: 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""" From 766331588ebad2ac74ccde572d241803db77f70c Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:45:18 +0100 Subject: [PATCH 02/15] Roles cannot return None because everyone has the Developer role by default, and non-verified users cannot use this command. --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index bc2deae8f5..21e3cfc394 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -212,7 +212,7 @@ async def create_user_embed(self, ctx: Context, user: Member) -> Embed: {custom_status} **Member Information** Joined: {joined} - Roles: {roles or None} + Roles: {roles} """).strip() ] From 515b490fca1f151e654ff72a9fcf8f3113f239c3 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:46:45 +0100 Subject: [PATCH 03/15] Remove some a lot of unneccesary newlines that arguably make it harder to read --- bot/cogs/information.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 21e3cfc394..68614d2c4d 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -45,7 +45,6 @@ async def roles_info(self, ctx: Context) -> None: colour=Colour.blurple(), description=role_string ) - embed.set_footer(text=f"Total roles: {len(roles)}") await ctx.send(embed=embed) @@ -75,23 +74,17 @@ async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None parsed_roles.append(role) 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) From 5f799b68316e03cd0a565af484f7dee3f79ed35e Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:48:26 +0100 Subject: [PATCH 04/15] Refactor how channels and statuses are counted; using Counter() - way cleaner. --- bot/cogs/information.py | 52 ++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 68614d2c4d..412447835c 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -3,7 +3,7 @@ import pprint import textwrap import typing -from collections import defaultdict +from collections import Counter, defaultdict from typing import Any, Mapping, Optional import discord @@ -96,36 +96,18 @@ async def server_info(self, ctx: Context) -> None: 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 + member_count = ctx.guild.member_count + + # How many of each type of channel? + channels = Counter({TextChannel: 0, VoiceChannel: 0, CategoryChannel: 0}) + for channel in ctx.guild.channels: + channels[channel.__class__] += 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 + statuses = Counter({status.value: 0 for status in Status}) + for member in ctx.guild.members: + statuses[member.status.value] += 1 embed = Embed( colour=Colour.blurple(), @@ -138,15 +120,15 @@ async def server_info(self, ctx: Context) -> None: **Counts** Members: {member_count:,} Roles: {roles} - Text: {text_channels} - Voice: {voice_channels} - Channel categories: {category_channels} + Text Channels: {channels[TextChannel]} + Voice Channels: {channels[VoiceChannel]} + Channel categories: {channels[CategoryChannel]} **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['online']} + {constants.Emojis.status_idle} {statuses['idle']} + {constants.Emojis.status_dnd} {statuses['dnd']} + {constants.Emojis.status_offline} {statuses['offline']} """) ) From 79fb50772065e23827396911682da51e24440787 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:48:39 +0100 Subject: [PATCH 05/15] Update tests to reflect status changes --- tests/bot/cogs/test_information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 4496a2ae02..519d2622b2 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -125,10 +125,10 @@ def test_server_info_command(self, time_since_patch): ) ], 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', From 90dd064f2a8cfe66e5cefbe7b679dac38f6f7845 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:50:10 +0100 Subject: [PATCH 06/15] Instead of sending a message everytime a role can't be converted, append it to a list, and then send them it at once (less spammy) --- bot/cogs/information.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 412447835c..bc67ab5c24 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -58,6 +58,7 @@ async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None 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): @@ -68,11 +69,17 @@ async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None 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()) From 8f1f8055383e5cbf017f4f2cec7074518dab95fd Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:59:46 +0100 Subject: [PATCH 07/15] Fix up imports a bit; there's no need to import all of discord and typing for just 1 or 2 uses (e.g. Union, and Message). --- bot/cogs/information.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index bc67ab5c24..3b8a88309e 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -2,14 +2,11 @@ import logging import pprint import textwrap -import typing from collections import Counter, defaultdict -from typing import Any, Mapping, Optional +from typing import Any, Mapping, Optional, Union -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 discord import CategoryChannel, Colour, Embed, Member, Message, Role, Status, TextChannel, VoiceChannel, utils +from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown from bot import constants @@ -51,7 +48,7 @@ async def roles_info(self, ctx: Context) -> None: @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. @@ -337,13 +334,13 @@ def format_fields(self, mapping: Mapping[str, Any], field_width: Optional[int] = @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') @@ -371,7 +368,7 @@ def add_content(title: str, content: str) -> None: 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) From 4653dadbbe929055355892b322e4b0cfd3e09ab6 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 22:00:13 +0100 Subject: [PATCH 08/15] Change if statement to elif; if the first if statement returns true, the second cannot be true making it unneccesary to check --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 3b8a88309e..e1a68ee635 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -147,7 +147,7 @@ async def user_info(self, ctx: Context, user: Member = None) -> None: 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 From 24136095302d192b25d83430a1b9607f05f6059c Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 22:37:03 +0100 Subject: [PATCH 09/15] Fix some of the testing for information.py; I think this should be it. (hopefully). --- tests/bot/cogs/test_information.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 519d2622b2..296c3c5563 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -153,8 +153,8 @@ def test_server_info_command(self, time_since_patch): **Counts** Members: {self.ctx.guild.member_count:,} Roles: {len(self.ctx.guild.roles)} - Text: 1 - Voice: 1 + Text Channels: 1 + Voice Channels: 1 Channel categories: 1 **Members** From b0fe5841f58d72a12b3f3ddfd5de7b648770fd58 Mon Sep 17 00:00:00 2001 From: Deniz Date: Sat, 8 Feb 2020 18:48:32 +0100 Subject: [PATCH 10/15] Use the enum itself instead of its string value --- bot/cogs/information.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index e1a68ee635..efe6608516 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -109,9 +109,9 @@ async def server_info(self, ctx: Context) -> None: channels[channel.__class__] += 1 # How many of each user status? - statuses = Counter({status.value: 0 for status in Status}) + statuses = Counter({status: 0 for status in Status}) for member in ctx.guild.members: - statuses[member.status.value] += 1 + statuses[member.status] += 1 embed = Embed( colour=Colour.blurple(), @@ -129,10 +129,10 @@ async def server_info(self, ctx: Context) -> None: Channel categories: {channels[CategoryChannel]} **Members** - {constants.Emojis.status_online} {statuses['online']} - {constants.Emojis.status_idle} {statuses['idle']} - {constants.Emojis.status_dnd} {statuses['dnd']} - {constants.Emojis.status_offline} {statuses['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]} """) ) From bc932aa09848bc10683d66b7e7d9f6054e9958c6 Mon Sep 17 00:00:00 2001 From: Deniz Date: Wed, 12 Feb 2020 20:33:31 +0100 Subject: [PATCH 11/15] Use collections.Counter properly. Use the ChannelType enum instead of the __class__ attribute, and re-add the None check for !user roles. --- bot/cogs/information.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index efe6608516..2461cc749c 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -5,7 +5,7 @@ from collections import Counter, defaultdict from typing import Any, Mapping, Optional, Union -from discord import CategoryChannel, Colour, Embed, Member, Message, Role, Status, TextChannel, VoiceChannel, utils +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 @@ -104,14 +104,11 @@ async def server_info(self, ctx: Context) -> None: member_count = ctx.guild.member_count # How many of each type of channel? - channels = Counter({TextChannel: 0, VoiceChannel: 0, CategoryChannel: 0}) - for channel in ctx.guild.channels: - channels[channel.__class__] += 1 + 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({status: 0 for status in Status}) - for member in ctx.guild.members: - statuses[member.status] += 1 + statuses = Counter(member.status for member in ctx.guild.members) embed = Embed( colour=Colour.blurple(), @@ -124,9 +121,7 @@ async def server_info(self, ctx: Context) -> None: **Counts** Members: {member_count:,} Roles: {roles} - Text Channels: {channels[TextChannel]} - Voice Channels: {channels[VoiceChannel]} - Channel categories: {channels[CategoryChannel]} + {channel_counts} **Members** {constants.Emojis.status_online} {statuses[Status.online]} @@ -135,7 +130,6 @@ async def server_info(self, ctx: Context) -> None: {constants.Emojis.status_offline} {statuses[Status.offline]} """) ) - embed.set_thumbnail(url=ctx.guild.icon_url) await ctx.send(embed=embed) @@ -191,7 +185,7 @@ async def create_user_embed(self, ctx: Context, user: Member) -> Embed: {custom_status} **Member Information** Joined: {joined} - Roles: {roles} + Roles: {roles or None} """).strip() ] From 6703e3b7db972017764f91232a82c163be2cd588 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 13 Feb 2020 17:35:34 +0100 Subject: [PATCH 12/15] Update the tests accordingly to reflect the new changes --- tests/bot/cogs/test_information.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 296c3c5563..deae7ebad3 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -153,9 +153,9 @@ def test_server_info_command(self, time_since_patch): **Counts** Members: {self.ctx.guild.member_count:,} Roles: {len(self.ctx.guild.roles)} - Text Channels: 1 - Voice Channels: 1 - Channel categories: 1 + Category channels: 1 + Text channels: 1 + Voice channels: 1 **Members** {constants.Emojis.status_online} 2 From b26122d72c1a41a4919c95642c6bc16e82d535a3 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 13 Feb 2020 17:35:56 +0100 Subject: [PATCH 13/15] Add thousand separators to Members count, closes #744 --- bot/cogs/information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 2461cc749c..c8b5eb5adf 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -124,10 +124,10 @@ async def server_info(self, ctx: Context) -> None: {channel_counts} **Members** - {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]} + {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]:,} """) ) embed.set_thumbnail(url=ctx.guild.icon_url) From 7b619a2dd17e00464d93b56717391fe53e0fe6bb Mon Sep 17 00:00:00 2001 From: Deniz Date: Fri, 21 Feb 2020 17:48:10 +0100 Subject: [PATCH 14/15] Use the code provided by sco1 to fix the checks failing. --- bot/cogs/information.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index c8b5eb5adf..fd49e28281 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -3,6 +3,7 @@ import pprint import textwrap 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 @@ -109,10 +110,14 @@ async def server_info(self, ctx: Context) -> None: # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) + embed = Embed(colour=Colour.blurple()) - embed = Embed( - colour=Colour.blurple(), - description=textwrap.dedent(f""" + # 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} @@ -121,7 +126,7 @@ async def server_info(self, ctx: Context) -> None: **Counts** Members: {member_count:,} Roles: {roles} - {channel_counts} + $channel_counts **Members** {constants.Emojis.status_online} {statuses[Status.online]:,} @@ -129,7 +134,7 @@ async def server_info(self, ctx: Context) -> None: {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) From c46498ad20c463d72e6d5da05852371f9ab20e6c Mon Sep 17 00:00:00 2001 From: Deniz Date: Fri, 21 Feb 2020 17:57:36 +0100 Subject: [PATCH 15/15] Remove the space that makes the test fail --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index fd49e28281..13c8aabaad 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -106,7 +106,7 @@ async def server_info(self, ctx: Context) -> None: # 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() + 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)