Merge branch 'master' into reminder_missing_users

6 files changed, 101 insertions, 120 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/bot/cogs/tags.py b/bot/cogs/tags.py
index 54a51921c..b6360dfae 100644
--- a/bot/cogs/tags.py
+++ b/bot/cogs/tags.py
@@ -116,8 +116,10 @@ class Tags(Cog):
 
         if _command_on_cooldown(tag_name):
             time_left = Cooldowns.tags - (time.time() - self.tag_cooldowns[tag_name]["time"])
-            log.warning(f"{ctx.author} tried to get the '{tag_name}' tag, but the tag is on cooldown. "
-                        f"Cooldown ends in {time_left:.1f} seconds.")
+            log.info(
+                f"{ctx.author} tried to get the '{tag_name}' tag, but the tag is on cooldown. "
+                f"Cooldown ends in {time_left:.1f} seconds."
+            )
             return
 
         await self._get_tags()
diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py
index 988e0d49a..582237374 100644
--- a/bot/cogs/verification.py
+++ b/bot/cogs/verification.py
@@ -1,7 +1,8 @@
 import logging
+from contextlib import suppress
 from datetime import datetime
 
-from discord import Colour, Message, NotFound, Object
+from discord import Colour, Forbidden, Message, NotFound, Object
 from discord.ext import tasks
 from discord.ext.commands import Cog, Context, command
 
@@ -92,19 +93,21 @@ class Verification(Cog):
                 ping_everyone=Filter.ping_everyone,
             )
 
-        ctx = await self.bot.get_context(message)  # type: Context
-
+        ctx: Context = await self.bot.get_context(message)
         if ctx.command is not None and ctx.command.name == "accept":
-            return  # They used the accept command
+            return
 
-        for role in ctx.author.roles:
-            if role.id == Roles.verified:
-                log.warning(f"{ctx.author} posted '{ctx.message.content}' "
-                            "in the verification channel, but is already verified.")
-                return  # They're already verified
+        if any(r.id == Roles.verified for r in ctx.author.roles):
+            log.info(
+                f"{ctx.author} posted '{ctx.message.content}' "
+                "in the verification channel, but is already verified."
+            )
+            return
 
-        log.debug(f"{ctx.author} posted '{ctx.message.content}' in the verification "
-                  "channel. We are providing instructions how to verify.")
+        log.debug(
+            f"{ctx.author} posted '{ctx.message.content}' in the verification "
+            "channel. We are providing instructions how to verify."
+        )
         await ctx.send(
             f"{ctx.author.mention} Please type `!accept` to verify that you accept our rules, "
             f"and gain access to the rest of the server.",
@@ -112,11 +115,8 @@ class Verification(Cog):
         )
 
         log.trace(f"Deleting the message posted by {ctx.author}")
-
-        try:
+        with suppress(NotFound):
             await ctx.message.delete()
-        except NotFound:
-            log.trace("No message found, it must have been deleted by another bot.")
 
     @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True)
     @without_role(Roles.verified)
@@ -127,17 +127,13 @@ class Verification(Cog):
         await ctx.author.add_roles(Object(Roles.verified), reason="Accepted the rules")
         try:
             await ctx.author.send(WELCOME_MESSAGE)
-        except Exception:
-            # Catch the exception, in case they have DMs off or something
-            log.exception(f"Unable to send welcome message to user {ctx.author}.")
-
-        log.trace(f"Deleting the message posted by {ctx.author}.")
-
-        try:
-            self.mod_log.ignore(Event.message_delete, ctx.message.id)
-            await ctx.message.delete()
-        except NotFound:
-            log.trace("No message found, it must have been deleted by another bot.")
+        except Forbidden:
+            log.info(f"Sending welcome message failed for {ctx.author}.")
+        finally:
+            log.trace(f"Deleting accept message by {ctx.author}.")
+            with suppress(NotFound):
+                self.mod_log.ignore(Event.message_delete, ctx.message.id)
+                await ctx.message.delete()
 
     @command(name='subscribe')
     @in_channel(Channels.bot)
diff --git a/bot/pagination.py b/bot/pagination.py
index e82763912..90c8f849c 100644
--- a/bot/pagination.py
+++ b/bot/pagination.py
@@ -1,8 +1,9 @@
 import asyncio
 import logging
-from typing import Iterable, List, Optional, Tuple
+import typing as t
+from contextlib import suppress
 
-from discord import Embed, Member, Message, Reaction
+import discord
 from discord.abc import User
 from discord.ext.commands import Context, Paginator
 
@@ -14,7 +15,7 @@ RIGHT_EMOJI = "\u27A1"   # [:arrow_right:]
 LAST_EMOJI = "\u23ED"    # [:track_next:]
 DELETE_EMOJI = constants.Emojis.trashcan  # [:trashcan:]
 
-PAGINATION_EMOJI = [FIRST_EMOJI, LEFT_EMOJI, RIGHT_EMOJI, LAST_EMOJI, DELETE_EMOJI]
+PAGINATION_EMOJI = (FIRST_EMOJI, LEFT_EMOJI, RIGHT_EMOJI, LAST_EMOJI, DELETE_EMOJI)
 
 log = logging.getLogger(__name__)
 
@@ -89,12 +90,12 @@ class LinePaginator(Paginator):
     @classmethod
     async def paginate(
         cls,
-        lines: Iterable[str],
+        lines: t.List[str],
         ctx: Context,
-        embed: Embed,
+        embed: discord.Embed,
         prefix: str = "",
         suffix: str = "",
-        max_lines: Optional[int] = None,
+        max_lines: t.Optional[int] = None,
         max_size: int = 500,
         empty: bool = True,
         restrict_to_user: User = None,
@@ -102,7 +103,7 @@ class LinePaginator(Paginator):
         footer_text: str = None,
         url: str = None,
         exception_on_empty_embed: bool = False
-    ) -> Optional[Message]:
+    ) -> t.Optional[discord.Message]:
         """
         Use a paginator and set of reactions to provide pagination over a set of lines.
 
@@ -114,11 +115,11 @@ class LinePaginator(Paginator):
         Pagination will also be removed automatically if no reaction is added for five minutes (300 seconds).
 
         Example:
-        >>> embed = Embed()
+        >>> embed = discord.Embed()
         >>> embed.set_author(name="Some Operation", url=url, icon_url=icon)
-        >>> await LinePaginator.paginate((line for line in lines), ctx, embed)
+        >>> await LinePaginator.paginate([line for line in lines], ctx, embed)
         """
-        def event_check(reaction_: Reaction, user_: Member) -> bool:
+        def event_check(reaction_: discord.Reaction, user_: discord.Member) -> bool:
             """Make sure that this reaction is what we want to operate on."""
             no_restrictions = (
                 # Pagination is not restricted
@@ -281,8 +282,9 @@ class LinePaginator(Paginator):
 
                 await message.edit(embed=embed)
 
-        log.debug("Ending pagination and removing all reactions...")
-        await message.clear_reactions()
+        log.debug("Ending pagination and clearing reactions.")
+        with suppress(discord.NotFound):
+            await message.clear_reactions()
 
 
 class ImagePaginator(Paginator):
@@ -299,6 +301,7 @@ class ImagePaginator(Paginator):
         self._current_page = [prefix]
         self.images = []
         self._pages = []
+        self._count = 0
 
     def add_line(self, line: str = '', *, empty: bool = False) -> None:
         """Adds a line to each page."""
@@ -316,13 +319,13 @@ class ImagePaginator(Paginator):
     @classmethod
     async def paginate(
         cls,
-        pages: List[Tuple[str, str]],
-        ctx: Context, embed: Embed,
+        pages: t.List[t.Tuple[str, str]],
+        ctx: Context, embed: discord.Embed,
         prefix: str = "",
         suffix: str = "",
         timeout: int = 300,
         exception_on_empty_embed: bool = False
-    ) -> Optional[Message]:
+    ) -> t.Optional[discord.Message]:
         """
         Use a paginator and set of reactions to provide pagination over a set of title/image pairs.
 
@@ -334,11 +337,11 @@ class ImagePaginator(Paginator):
         Note: Pagination will be removed automatically if no reaction is added for five minutes (300 seconds).
 
         Example:
-        >>> embed = Embed()
+        >>> embed = discord.Embed()
         >>> embed.set_author(name="Some Operation", url=url, icon_url=icon)
         >>> await ImagePaginator.paginate(pages, ctx, embed)
         """
-        def check_event(reaction_: Reaction, member: Member) -> bool:
+        def check_event(reaction_: discord.Reaction, member: discord.Member) -> bool:
             """Checks each reaction added, if it matches our conditions pass the wait_for."""
             return all((
                 # Reaction is on the same message sent
@@ -445,5 +448,6 @@ class ImagePaginator(Paginator):
 
             await message.edit(embed=embed)
 
-        log.debug("Ending pagination and removing all reactions...")
-        await message.clear_reactions()
+        log.debug("Ending pagination and clearing reactions.")
+        with suppress(discord.NotFound):
+            await message.clear_reactions()
diff --git a/docker-compose.yml b/docker-compose.yml
index 7281c7953..11deceae8 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -23,6 +23,7 @@ services:
           - staff.web
     ports:
       - "127.0.0.1:8000:8000"
+    tty: true
     depends_on:
       - postgres
     environment:
@@ -37,6 +38,7 @@ services:
     volumes:
       - ./logs:/bot/logs
       - .:/bot:ro
+    tty: true
     depends_on:
       - web
     environment:
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