diff options
| author | 2020-07-20 13:56:27 -0400 | |
|---|---|---|
| committer | 2020-07-20 13:56:27 -0400 | |
| commit | 30e9571fb87423b28438ea1485ec38903b3fee44 (patch) | |
| tree | 298e0fdc38159fcbbf6f9ab7c70079fd72788699 | |
| parent | Merge branch 'role-reminders' of github.com:python-discord/bot into role-remi... (diff) | |
| parent | Merge pull request #1060 from wookie184/remove-eval-py-formatting (diff) | |
Merge branch 'master' into role-reminders
| -rw-r--r-- | bot/__main__.py | 1 | ||||
| -rw-r--r-- | bot/cogs/clean.py | 35 | ||||
| -rw-r--r-- | bot/cogs/dm_relay.py | 32 | ||||
| -rw-r--r-- | bot/cogs/help.py | 9 | ||||
| -rw-r--r-- | bot/cogs/moderation/__init__.py | 4 | ||||
| -rw-r--r-- | bot/cogs/moderation/incidents.py | 407 | ||||
| -rw-r--r-- | bot/cogs/snekbox.py | 2 | ||||
| -rw-r--r-- | bot/cogs/source.py | 133 | ||||
| -rw-r--r-- | bot/cogs/tags.py | 1 | ||||
| -rw-r--r-- | bot/constants.py | 12 | ||||
| -rw-r--r-- | bot/converters.py | 19 | ||||
| -rw-r--r-- | bot/resources/tags/or-gotcha.md | 2 | ||||
| -rw-r--r-- | config-default.yml | 22 | ||||
| -rw-r--r-- | tests/bot/cogs/moderation/test_incidents.py | 770 | ||||
| -rw-r--r-- | tests/bot/cogs/test_snekbox.py | 6 | 
15 files changed, 1429 insertions, 26 deletions
| diff --git a/bot/__main__.py b/bot/__main__.py index 49388455a..5382f5502 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -66,6 +66,7 @@ bot.load_extension("bot.cogs.reddit")  bot.load_extension("bot.cogs.reminders")  bot.load_extension("bot.cogs.site")  bot.load_extension("bot.cogs.snekbox") +bot.load_extension("bot.cogs.source")  bot.load_extension("bot.cogs.stats")  bot.load_extension("bot.cogs.sync")  bot.load_extension("bot.cogs.tags") diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 368d91c85..f436e531a 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -45,6 +45,7 @@ class Clean(Cog):          bots_only: bool = False,          user: User = None,          regex: Optional[str] = None, +        until_message: Optional[Message] = None,      ) -> None:          """A helper function that does the actual message cleaning."""          def predicate_bots_only(message: Message) -> bool: @@ -129,6 +130,20 @@ class Clean(Cog):                  if not self.cleaning:                      return +                # If we are looking for specific message. +                if until_message: + +                    # we could use ID's here however in case if the message we are looking for gets deleted, +                    # we won't have a way to figure that out thus checking for datetime should be more reliable +                    if message.created_at < until_message.created_at: +                        # means we have found the message until which we were supposed to be deleting. +                        break + +                    # Since we will be using `delete_messages` method of a TextChannel and we need message objects to +                    # use it as well as to send logs we will start appending messages here instead adding them from +                    # purge. +                    messages.append(message) +                  # If the message passes predicate, let's save it.                  if predicate is None or predicate(message):                      message_ids.append(message.id) @@ -138,7 +153,14 @@ class Clean(Cog):          # Now let's delete the actual messages with purge.          self.mod_log.ignore(Event.message_delete, *message_ids)          for channel in channels: -            messages += await channel.purge(limit=amount, check=predicate) +            if until_message: +                for i in range(0, len(messages), 100): +                    # while purge automatically handles the amount of messages +                    # delete_messages only allows for up to 100 messages at once +                    # thus we need to paginate the amount to always be <= 100 +                    await channel.delete_messages(messages[i:i + 100]) +            else: +                messages += await channel.purge(limit=amount, check=predicate)          # Reverse the list to restore chronological order          if messages: @@ -221,6 +243,17 @@ class Clean(Cog):          """Delete all messages that match a certain regex, stop cleaning after traversing `amount` messages."""          await self._clean_messages(amount, ctx, regex=regex, channels=channels) +    @clean_group.command(name="message", aliases=["messages"]) +    @with_role(*MODERATION_ROLES) +    async def clean_message(self, ctx: Context, message: Message) -> None: +        """Delete all messages until certain message, stop cleaning after hitting the `message`.""" +        await self._clean_messages( +            CleanMessages.message_limit, +            ctx, +            channels=[message.channel], +            until_message=message +        ) +      @clean_group.command(name="stop", aliases=["cancel", "abort"])      @with_role(*MODERATION_ROLES)      async def clean_cancel(self, ctx: Context) -> None: diff --git a/bot/cogs/dm_relay.py b/bot/cogs/dm_relay.py index f62d6105e..0d8f340b4 100644 --- a/bot/cogs/dm_relay.py +++ b/bot/cogs/dm_relay.py @@ -1,4 +1,5 @@  import logging +from typing import Optional  import discord  from discord import Color @@ -7,6 +8,8 @@ from discord.ext.commands import Cog  from bot import constants  from bot.bot import Bot +from bot.converters import UserMentionOrID +from bot.utils import RedisCache  from bot.utils.checks import in_whitelist_check, with_role_check  from bot.utils.messages import send_attachments  from bot.utils.webhooks import send_webhook @@ -17,6 +20,9 @@ log = logging.getLogger(__name__)  class DMRelay(Cog):      """Relay direct messages to and from the bot.""" +    # RedisCache[str, t.Union[discord.User.id, discord.Member.id]] +    dm_cache = RedisCache() +      def __init__(self, bot: Bot):          self.bot = bot          self.webhook_id = constants.Webhooks.dm_log @@ -24,11 +30,11 @@ class DMRelay(Cog):          self.bot.loop.create_task(self.fetch_webhook())      @commands.command(aliases=("reply",)) -    async def send_dm(self, ctx: commands.Context, member: discord.Member, *, message: str) -> None: +    async def send_dm(self, ctx: commands.Context, member: Optional[UserMentionOrID], *, message: str) -> None:          """          Allows you to send a DM to a user from the bot. -        A `member` must be provided. +        If `member` is not provided, it will send to the last user who DM'd the bot.          This feature should be used extremely sparingly. Use ModMail if you need to have a serious          conversation with a user. This is just for responding to extraordinary DMs, having a little @@ -36,14 +42,24 @@ class DMRelay(Cog):          NOTE: This feature will be removed if it is overused.          """ -        try: -            await member.send(message) -            await ctx.message.add_reaction("✅") +        if not member: +            user_id = await self.dm_cache.get("last_user") +            member = ctx.guild.get_member(user_id) if user_id else None + +        # If we still don't have a Member at this point, give up +        if not member: +            log.debug("This bot has never gotten a DM, or the RedisCache has been cleared.") +            await ctx.message.add_reaction("❌")              return +        try: +            await member.send(message)          except discord.errors.Forbidden:              log.debug("User has disabled DMs.")              await ctx.message.add_reaction("❌") +        else: +            await ctx.message.add_reaction("✅") +            self.bot.stats.incr("dm_relay.dm_sent")      async def fetch_webhook(self) -> None:          """Fetches the webhook object, so we can post to it.""" @@ -65,9 +81,11 @@ class DMRelay(Cog):              await send_webhook(                  webhook=self.webhook,                  content=message.clean_content, -                username=message.author.display_name, +                username=f"{message.author.display_name} ({message.author.id})",                  avatar_url=message.author.avatar_url              ) +            await self.dm_cache.set("last_user", message.author.id) +            self.bot.stats.incr("dm_relay.dm_received")          # Handle any attachments          if message.attachments: @@ -81,7 +99,7 @@ class DMRelay(Cog):                  await send_webhook(                      webhook=self.webhook,                      embed=e, -                    username=message.author.display_name, +                    username=f"{message.author.display_name} ({message.author.id})",                      avatar_url=message.author.avatar_url                  )              except discord.HTTPException: diff --git a/bot/cogs/help.py b/bot/cogs/help.py index 70e62d590..3d1d6fd10 100644 --- a/bot/cogs/help.py +++ b/bot/cogs/help.py @@ -8,6 +8,7 @@ from typing import List, Union  from discord import Colour, Embed, Member, Message, NotFound, Reaction, User  from discord.ext.commands import Bot, Cog, Command, Context, Group, HelpCommand  from fuzzywuzzy import fuzz, process +from fuzzywuzzy.utils import full_process  from bot import constants  from bot.constants import Channels, Emojis, STAFF_ROLES @@ -145,7 +146,13 @@ class CustomHelpCommand(HelpCommand):          Will return an instance of the `HelpQueryNotFound` exception with the error message and possible matches.          """          choices = await self.get_all_help_choices() -        result = process.extractBests(string, choices, scorer=fuzz.ratio, score_cutoff=60) + +        # Run fuzzywuzzy's processor beforehand, and avoid matching if processed string is empty +        # This avoids fuzzywuzzy from raising a warning on inputs with only non-alphanumeric characters +        if (processed := full_process(string)): +            result = process.extractBests(processed, choices, scorer=fuzz.ratio, score_cutoff=60, processor=None) +        else: +            result = []          return HelpQueryNotFound(f'Query "{string}" not found.', dict(result)) diff --git a/bot/cogs/moderation/__init__.py b/bot/cogs/moderation/__init__.py index a5c1ef362..995187ef0 100644 --- a/bot/cogs/moderation/__init__.py +++ b/bot/cogs/moderation/__init__.py @@ -1,4 +1,5 @@  from bot.bot import Bot +from .incidents import Incidents  from .infractions import Infractions  from .management import ModManagement  from .modlog import ModLog @@ -8,7 +9,8 @@ from .superstarify import Superstarify  def setup(bot: Bot) -> None: -    """Load the Infractions, ModManagement, ModLog, Silence, Slowmode, and Superstarify cogs.""" +    """Load the Incidents, Infractions, ModManagement, ModLog, Silence, Slowmode and Superstarify cogs.""" +    bot.add_cog(Incidents(bot))      bot.add_cog(Infractions(bot))      bot.add_cog(ModLog(bot))      bot.add_cog(ModManagement(bot)) diff --git a/bot/cogs/moderation/incidents.py b/bot/cogs/moderation/incidents.py new file mode 100644 index 000000000..3605ab1d2 --- /dev/null +++ b/bot/cogs/moderation/incidents.py @@ -0,0 +1,407 @@ +import asyncio +import logging +import typing as t +from datetime import datetime +from enum import Enum + +import discord +from discord.ext.commands import Cog + +from bot.bot import Bot +from bot.constants import Channels, Colours, Emojis, Guild, Webhooks +from bot.utils.messages import sub_clyde + +log = logging.getLogger(__name__) + +# Amount of messages for `crawl_task` to process at most on start-up - limited to 50 +# as in practice, there should never be this many messages, and if there are, +# something has likely gone very wrong +CRAWL_LIMIT = 50 + +# Seconds for `crawl_task` to sleep after adding reactions to a message +CRAWL_SLEEP = 2 + + +class Signal(Enum): +    """ +    Recognized incident status signals. + +    This binds emoji to actions. The bot will only react to emoji linked here. +    All other signals are seen as invalid. +    """ + +    ACTIONED = Emojis.incident_actioned +    NOT_ACTIONED = Emojis.incident_unactioned +    INVESTIGATING = Emojis.incident_investigating + + +# Reactions from non-mod roles will be removed +ALLOWED_ROLES: t.Set[int] = set(Guild.moderation_roles) + +# Message must have all of these emoji to pass the `has_signals` check +ALL_SIGNALS: t.Set[str] = {signal.value for signal in Signal} + +# An embed coupled with an optional file to be dispatched +# If the file is not None, the embed attempts to show it in its body +FileEmbed = t.Tuple[discord.Embed, t.Optional[discord.File]] + + +async def download_file(attachment: discord.Attachment) -> t.Optional[discord.File]: +    """ +    Download & return `attachment` file. + +    If the download fails, the reason is logged and None will be returned. +    404 and 403 errors are only logged at debug level. +    """ +    log.debug(f"Attempting to download attachment: {attachment.filename}") +    try: +        return await attachment.to_file() +    except (discord.NotFound, discord.Forbidden) as exc: +        log.debug(f"Failed to download attachment: {exc}") +    except Exception: +        log.exception("Failed to download attachment") + + +async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: discord.Member) -> FileEmbed: +    """ +    Create an embed representation of `incident` for the #incidents-archive channel. + +    The name & discriminator of `actioned_by` and `outcome` will be presented in the +    embed footer. Additionally, the embed is coloured based on `outcome`. + +    The author of `incident` is not shown in the embed. It is assumed that this piece +    of information will be relayed in other ways, e.g. webhook username. + +    As mentions in embeds do not ping, we do not need to use `incident.clean_content`. + +    If `incident` contains attachments, the first attachment will be downloaded and +    returned alongside the embed. The embed attempts to display the attachment. +    Should the download fail, we fallback on linking the `proxy_url`, which should +    remain functional for some time after the original message is deleted. +    """ +    log.trace(f"Creating embed for {incident.id=}") + +    if outcome is Signal.ACTIONED: +        colour = Colours.soft_green +        footer = f"Actioned by {actioned_by}" +    else: +        colour = Colours.soft_red +        footer = f"Rejected by {actioned_by}" + +    embed = discord.Embed( +        description=incident.content, +        timestamp=datetime.utcnow(), +        colour=colour, +    ) +    embed.set_footer(text=footer, icon_url=actioned_by.avatar_url) + +    if incident.attachments: +        attachment = incident.attachments[0]  # User-sent messages can only contain one attachment +        file = await download_file(attachment) + +        if file is not None: +            embed.set_image(url=f"attachment://{attachment.filename}")  # Embed displays the attached file +        else: +            embed.set_author(name="[Failed to relay attachment]", url=attachment.proxy_url)  # Embed links the file +    else: +        file = None + +    return embed, file + + +def is_incident(message: discord.Message) -> bool: +    """True if `message` qualifies as an incident, False otherwise.""" +    conditions = ( +        message.channel.id == Channels.incidents,  # Message sent in #incidents +        not message.author.bot,                    # Not by a bot +        not message.content.startswith("#"),       # Doesn't start with a hash +        not message.pinned,                        # And isn't header +    ) +    return all(conditions) + + +def own_reactions(message: discord.Message) -> t.Set[str]: +    """Get the set of reactions placed on `message` by the bot itself.""" +    return {str(reaction.emoji) for reaction in message.reactions if reaction.me} + + +def has_signals(message: discord.Message) -> bool: +    """True if `message` already has all `Signal` reactions, False otherwise.""" +    return ALL_SIGNALS.issubset(own_reactions(message)) + + +async def add_signals(incident: discord.Message) -> None: +    """ +    Add `Signal` member emoji to `incident` as reactions. + +    If the emoji has already been placed on `incident` by the bot, it will be skipped. +    """ +    existing_reacts = own_reactions(incident) + +    for signal_emoji in Signal: +        if signal_emoji.value in existing_reacts:  # This would not raise, but it is a superfluous API call +            log.trace(f"Skipping emoji as it's already been placed: {signal_emoji}") +        else: +            log.trace(f"Adding reaction: {signal_emoji}") +            await incident.add_reaction(signal_emoji.value) + + +class Incidents(Cog): +    """ +    Automation for the #incidents channel. + +    This cog does not provide a command API, it only reacts to the following events. + +    On start-up: +        * Crawl #incidents and add missing `Signal` emoji where appropriate +        * This is to retro-actively add the available options for messages which +          were sent while the bot wasn't listening +        * Pinned messages and message starting with # do not qualify as incidents +        * See: `crawl_incidents` + +    On message: +        * Add `Signal` member emoji if message qualifies as an incident +        * Ignore messages starting with # +            * Use this if verbal communication is necessary +            * Each such message must be deleted manually once appropriate +        * See: `on_message` + +    On reaction: +        * Remove reaction if not permitted +            * User does not have any of the roles in `ALLOWED_ROLES` +            * Used emoji is not a `Signal` member +        * If `Signal.ACTIONED` or `Signal.NOT_ACTIONED` were chosen, attempt to +          relay the incident message to #incidents-archive +        * If relay successful, delete original message +        * See: `on_raw_reaction_add` + +    Please refer to function docstrings for implementation details. +    """ + +    def __init__(self, bot: Bot) -> None: +        """Prepare `event_lock` and schedule `crawl_task` on start-up.""" +        self.bot = bot + +        self.event_lock = asyncio.Lock() +        self.crawl_task = self.bot.loop.create_task(self.crawl_incidents()) + +    async def crawl_incidents(self) -> None: +        """ +        Crawl #incidents and add missing emoji where necessary. + +        This is to catch-up should an incident be reported while the bot wasn't listening. +        After adding each reaction, we take a short break to avoid drowning in ratelimits. + +        Once this task is scheduled, listeners that change messages should await it. +        The crawl assumes that the channel history doesn't change as we go over it. + +        Behaviour is configured by: `CRAWL_LIMIT`, `CRAWL_SLEEP`. +        """ +        await self.bot.wait_until_guild_available() +        incidents: discord.TextChannel = self.bot.get_channel(Channels.incidents) + +        log.debug(f"Crawling messages in #incidents: {CRAWL_LIMIT=}, {CRAWL_SLEEP=}") +        async for message in incidents.history(limit=CRAWL_LIMIT): + +            if not is_incident(message): +                log.trace(f"Skipping message {message.id}: not an incident") +                continue + +            if has_signals(message): +                log.trace(f"Skipping message {message.id}: already has all signals") +                continue + +            await add_signals(message) +            await asyncio.sleep(CRAWL_SLEEP) + +        log.debug("Crawl task finished!") + +    async def archive(self, incident: discord.Message, outcome: Signal, actioned_by: discord.Member) -> bool: +        """ +        Relay an embed representation of `incident` to the #incidents-archive channel. + +        The following pieces of information are relayed: +            * Incident message content (as embed description) +            * Incident attachment (if image, shown in archive embed) +            * Incident author name (as webhook author) +            * Incident author avatar (as webhook avatar) +            * Resolution signal `outcome` (as embed colour & footer) +            * Moderator `actioned_by` (name & discriminator shown in footer) + +        If `incident` contains an attachment, we try to add it to the archive embed. There is +        no handing of extensions / file types - we simply dispatch the attachment file with the +        webhook, and try to display it in the embed. Testing indicates that if the attachment +        cannot be displayed (e.g. a text file), it's invisible in the embed, with no error. + +        Return True if the relay finishes successfully. If anything goes wrong, meaning +        not all information was relayed, return False. This signals that the original +        message is not safe to be deleted, as we will lose some information. +        """ +        log.debug(f"Archiving incident: {incident.id} (outcome: {outcome}, actioned by: {actioned_by})") +        embed, attachment_file = await make_embed(incident, outcome, actioned_by) + +        try: +            webhook = await self.bot.fetch_webhook(Webhooks.incidents_archive) +            await webhook.send( +                embed=embed, +                username=sub_clyde(incident.author.name), +                avatar_url=incident.author.avatar_url, +                file=attachment_file, +            ) +        except Exception: +            log.exception(f"Failed to archive incident {incident.id} to #incidents-archive") +            return False +        else: +            log.trace("Message archived successfully!") +            return True + +    def make_confirmation_task(self, incident: discord.Message, timeout: int = 5) -> asyncio.Task: +        """ +        Create a task to wait `timeout` seconds for `incident` to be deleted. + +        If `timeout` passes, this will raise `asyncio.TimeoutError`, signaling that we haven't +        been able to confirm that the message was deleted. +        """ +        log.trace(f"Confirmation task will wait {timeout=} seconds for {incident.id=} to be deleted") + +        def check(payload: discord.RawReactionActionEvent) -> bool: +            return payload.message_id == incident.id + +        coroutine = self.bot.wait_for(event="raw_message_delete", check=check, timeout=timeout) +        return self.bot.loop.create_task(coroutine) + +    async def process_event(self, reaction: str, incident: discord.Message, member: discord.Member) -> None: +        """ +        Process a `reaction_add` event in #incidents. + +        First, we check that the reaction is a recognized `Signal` member, and that it was sent by +        a permitted user (at least one role in `ALLOWED_ROLES`). If not, the reaction is removed. + +        If the reaction was either `Signal.ACTIONED` or `Signal.NOT_ACTIONED`, we attempt to relay +        the report to #incidents-archive. If successful, the original message is deleted. + +        We do not release `event_lock` until we receive the corresponding `message_delete` event. +        This ensures that if there is a racing event awaiting the lock, it will fail to find the +        message, and will abort. There is a `timeout` to ensure that this doesn't hold the lock +        forever should something go wrong. +        """ +        members_roles: t.Set[int] = {role.id for role in member.roles} +        if not members_roles & ALLOWED_ROLES:  # Intersection is truthy on at least 1 common element +            log.debug(f"Removing invalid reaction: user {member} is not permitted to send signals") +            await incident.remove_reaction(reaction, member) +            return + +        try: +            signal = Signal(reaction) +        except ValueError: +            log.debug(f"Removing invalid reaction: emoji {reaction} is not a valid signal") +            await incident.remove_reaction(reaction, member) +            return + +        log.trace(f"Received signal: {signal}") + +        if signal not in (Signal.ACTIONED, Signal.NOT_ACTIONED): +            log.debug("Reaction was valid, but no action is currently defined for it") +            return + +        relay_successful = await self.archive(incident, signal, actioned_by=member) +        if not relay_successful: +            log.trace("Original message will not be deleted as we failed to relay it to the archive") +            return + +        timeout = 5  # Seconds +        confirmation_task = self.make_confirmation_task(incident, timeout) + +        log.trace("Deleting original message") +        await incident.delete() + +        log.trace(f"Awaiting deletion confirmation: {timeout=} seconds") +        try: +            await confirmation_task +        except asyncio.TimeoutError: +            log.warning(f"Did not receive incident deletion confirmation within {timeout} seconds!") +        else: +            log.trace("Deletion was confirmed") + +    async def resolve_message(self, message_id: int) -> t.Optional[discord.Message]: +        """ +        Get `discord.Message` for `message_id` from cache, or API. + +        We first look into the local cache to see if the message is present. + +        If not, we try to fetch the message from the API. This is necessary for messages +        which were sent before the bot's current session. + +        In an edge-case, it is also possible that the message was already deleted, and +        the API will respond with a 404. In such a case, None will be returned. +        This signals that the event for `message_id` should be ignored. +        """ +        await self.bot.wait_until_guild_available()  # First make sure that the cache is ready +        log.trace(f"Resolving message for: {message_id=}") +        message: t.Optional[discord.Message] = self.bot._connection._get_message(message_id) + +        if message is not None: +            log.trace("Message was found in cache") +            return message + +        log.trace("Message not found, attempting to fetch") +        try: +            message = await self.bot.get_channel(Channels.incidents).fetch_message(message_id) +        except discord.NotFound: +            log.trace("Message doesn't exist, it was likely already relayed") +        except Exception: +            log.exception(f"Failed to fetch message {message_id}!") +        else: +            log.trace("Message fetched successfully!") +            return message + +    @Cog.listener() +    async def on_raw_reaction_add(self, payload: discord.RawReactionActionEvent) -> None: +        """ +        Pre-process `payload` and pass it to `process_event` if appropriate. + +        We abort instantly if `payload` doesn't relate to a message sent in #incidents, +        or if it was sent by a bot. + +        If `payload` relates to a message in #incidents, we first ensure that `crawl_task` has +        finished, to make sure we don't mutate channel state as we're crawling it. + +        Next, we acquire `event_lock` - to prevent racing, events are processed one at a time. + +        Once we have the lock, the `discord.Message` object for this event must be resolved. +        If the lock was previously held by an event which successfully relayed the incident, +        this will fail and we abort the current event. + +        Finally, with both the lock and the `discord.Message` instance in our hands, we delegate +        to `process_event` to handle the event. + +        The justification for using a raw listener is the need to receive events for messages +        which were not cached in the current session. As a result, a certain amount of +        complexity is introduced, but at the moment this doesn't appear to be avoidable. +        """ +        if payload.channel_id != Channels.incidents or payload.member.bot: +            return + +        log.trace(f"Received reaction add event in #incidents, waiting for crawler: {self.crawl_task.done()=}") +        await self.crawl_task + +        log.trace(f"Acquiring event lock: {self.event_lock.locked()=}") +        async with self.event_lock: +            message = await self.resolve_message(payload.message_id) + +            if message is None: +                log.debug("Listener will abort as related message does not exist!") +                return + +            if not is_incident(message): +                log.debug("Ignoring event for a non-incident message") +                return + +            await self.process_event(str(payload.emoji), message, payload.member) +            log.trace("Releasing event lock") + +    @Cog.listener() +    async def on_message(self, message: discord.Message) -> None: +        """Pass `message` to `add_signals` if and only if it satisfies `is_incident`.""" +        if is_incident(message): +            await add_signals(message) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 662f90869..52c8b6f88 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -202,7 +202,7 @@ class Snekbox(Cog):                  output, paste_link = await self.format_output(results["stdout"])              icon = self.get_status_emoji(results) -            msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" +            msg = f"{ctx.author.mention} {icon} {msg}.\n\n```\n{output}\n```"              if paste_link:                  msg = f"{msg}\nFull output: {paste_link}" diff --git a/bot/cogs/source.py b/bot/cogs/source.py new file mode 100644 index 000000000..f1db745cd --- /dev/null +++ b/bot/cogs/source.py @@ -0,0 +1,133 @@ +import inspect +from pathlib import Path +from typing import Optional, Tuple, Union + +from discord import Embed +from discord.ext import commands + +from bot.bot import Bot +from bot.constants import URLs + +SourceType = Union[commands.HelpCommand, commands.Command, commands.Cog, str, commands.ExtensionNotLoaded] + + +class SourceConverter(commands.Converter): +    """Convert an argument into a help command, tag, command, or cog.""" + +    async def convert(self, ctx: commands.Context, argument: str) -> SourceType: +        """Convert argument into source object.""" +        if argument.lower().startswith("help"): +            return ctx.bot.help_command + +        cog = ctx.bot.get_cog(argument) +        if cog: +            return cog + +        cmd = ctx.bot.get_command(argument) +        if cmd: +            return cmd + +        tags_cog = ctx.bot.get_cog("Tags") +        show_tag = True + +        if not tags_cog: +            show_tag = False +        elif argument.lower() in tags_cog._cache: +            return argument.lower() + +        raise commands.BadArgument( +            f"Unable to convert `{argument}` to valid command{', tag,' if show_tag else ''} or Cog." +        ) + + +class BotSource(commands.Cog): +    """Displays information about the bot's source code.""" + +    def __init__(self, bot: Bot): +        self.bot = bot + +    @commands.command(name="source", aliases=("src",)) +    async def source_command(self, ctx: commands.Context, *, source_item: SourceConverter = None) -> None: +        """Display information and a GitHub link to the source code of a command, tag, or cog.""" +        if not source_item: +            embed = Embed(title="Bot's GitHub Repository") +            embed.add_field(name="Repository", value=f"[Go to GitHub]({URLs.github_bot_repo})") +            embed.set_thumbnail(url="https://avatars1.githubusercontent.com/u/9919") +            await ctx.send(embed=embed) +            return + +        embed = await self.build_embed(source_item) +        await ctx.send(embed=embed) + +    def get_source_link(self, source_item: SourceType) -> Tuple[str, str, Optional[int]]: +        """Build GitHub link of source item, return this link, file location and first line number.""" +        if isinstance(source_item, commands.HelpCommand): +            src = type(source_item) +            filename = inspect.getsourcefile(src) +        elif isinstance(source_item, commands.Command): +            if source_item.cog_name == "Alias": +                cmd_name = source_item.callback.__name__.replace("_alias", "") +                cmd = self.bot.get_command(cmd_name.replace("_", " ")) +                src = cmd.callback.__code__ +                filename = src.co_filename +            else: +                src = source_item.callback.__code__ +                filename = src.co_filename +        elif isinstance(source_item, str): +            tags_cog = self.bot.get_cog("Tags") +            filename = tags_cog._cache[source_item]["location"] +        else: +            src = type(source_item) +            filename = inspect.getsourcefile(src) + +        if not isinstance(source_item, str): +            lines, first_line_no = inspect.getsourcelines(src) +            lines_extension = f"#L{first_line_no}-L{first_line_no+len(lines)-1}" +        else: +            first_line_no = None +            lines_extension = "" + +        # Handle tag file location differently than others to avoid errors in some cases +        if not first_line_no: +            file_location = Path(filename).relative_to("/bot/") +        else: +            file_location = Path(filename).relative_to(Path.cwd()).as_posix() + +        url = f"{URLs.github_bot_repo}/blob/master/{file_location}{lines_extension}" + +        return url, file_location, first_line_no or None + +    async def build_embed(self, source_object: SourceType) -> Optional[Embed]: +        """Build embed based on source object.""" +        url, location, first_line = self.get_source_link(source_object) + +        if isinstance(source_object, commands.HelpCommand): +            title = "Help Command" +            description = source_object.__doc__.splitlines()[1] +        elif isinstance(source_object, commands.Command): +            if source_object.cog_name == "Alias": +                cmd_name = source_object.callback.__name__.replace("_alias", "") +                cmd = self.bot.get_command(cmd_name.replace("_", " ")) +                description = cmd.short_doc +            else: +                description = source_object.short_doc + +            title = f"Command: {source_object.qualified_name}" +        elif isinstance(source_object, str): +            title = f"Tag: {source_object}" +            description = "" +        else: +            title = f"Cog: {source_object.qualified_name}" +            description = source_object.description.splitlines()[0] + +        embed = Embed(title=title, description=description) +        embed.add_field(name="Source Code", value=f"[Go to GitHub]({url})") +        line_text = f":{first_line}" if first_line else "" +        embed.set_footer(text=f"{location}{line_text}") + +        return embed + + +def setup(bot: Bot) -> None: +    """Load the BotSource cog.""" +    bot.add_cog(BotSource(bot)) diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index 6f03a3475..3d76c5c08 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -47,6 +47,7 @@ class Tags(Cog):                          "description": file.read_text(encoding="utf8"),                      },                      "restricted_to": "developers", +                    "location": f"/bot/{file}"                  }                  # Convert to a list to allow negative indexing. diff --git a/bot/constants.py b/bot/constants.py index 778bc093c..cf4f3f666 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -272,6 +272,10 @@ class Emojis(metaclass=YAMLGetter):      status_idle: str      status_dnd: str +    incident_actioned: str +    incident_unactioned: str +    incident_investigating: str +      failmail: str      trashcan: str @@ -400,6 +404,7 @@ class Channels(metaclass=YAMLGetter):      helpers: int      how_to_get_help: int      incidents: int +    incidents_archive: int      message_log: int      meta: int      mod_alerts: int @@ -423,12 +428,13 @@ class Webhooks(metaclass=YAMLGetter):      section = "guild"      subsection = "webhooks" -    talent_pool: int      big_brother: int -    reddit: int -    duck_pond: int      dev_log: int      dm_log: int +    duck_pond: int +    incidents_archive: int +    reddit: int +    talent_pool: int  class Roles(metaclass=YAMLGetter): diff --git a/bot/converters.py b/bot/converters.py index 898822165..4a0633951 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -330,6 +330,25 @@ def proxy_user(user_id: str) -> discord.Object:      return user +class UserMentionOrID(UserConverter): +    """ +    Converts to a `discord.User`, but only if a mention or userID is provided. + +    Unlike the default `UserConverter`, it does allow conversion from name, or name#descrim. + +    This is useful in cases where that lookup strategy would lead to ambiguity. +    """ + +    async def convert(self, ctx: Context, argument: str) -> discord.User: +        """Convert the `arg` to a `discord.User`.""" +        match = self._get_id_match(argument) or re.match(r'<@!?([0-9]+)>$', argument) + +        if match is not None: +            return await super().convert(ctx, argument) +        else: +            raise BadArgument(f"`{argument}` is not a User mention or a User ID.") + +  class FetchedUser(UserConverter):      """      Converts to a `discord.User` or, if it fails, a `discord.Object`. diff --git a/bot/resources/tags/or-gotcha.md b/bot/resources/tags/or-gotcha.md index 00c2db1f8..d75a73d78 100644 --- a/bot/resources/tags/or-gotcha.md +++ b/bot/resources/tags/or-gotcha.md @@ -3,7 +3,7 @@ When checking if something is equal to one thing or another, you might think tha  if favorite_fruit == 'grapefruit' or 'lemon':      print("That's a weird favorite fruit to have.")  ``` -After all, that's how you would normally phrase it in plain English. In Python, however, you have to have _complete instructions on both sides of the logical operator_. +While this makes sense in English, it may not behave the way you would expect. In Python, you should have _[complete instructions on both sides of the logical operator](https://docs.python.org/3/reference/expressions.html#boolean-operations)_.  So, if you want to check if something is equal to one thing or another, there are two common ways:  ```py diff --git a/config-default.yml b/config-default.yml index f2eb17b89..ad6149f6f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -38,6 +38,10 @@ style:          status_dnd:     "<:status_dnd:470326272082313216>"          status_offline: "<:status_offline:470326266537705472>" +        incident_actioned:      "<:incident_actioned:719645530128646266>" +        incident_unactioned:    "<:incident_unactioned:719645583245180960>" +        incident_investigating: "<:incident_investigating:719645658671480924>" +          failmail: "<:failmail:633660039931887616>"          trashcan: "<:trashcan:637136429717389331>" @@ -168,12 +172,13 @@ guild:          admin_spam:         &ADMIN_SPAM     563594791770914816          defcon:             &DEFCON         464469101889454091          helpers:            &HELPERS        385474242440986624 +        incidents:                          714214212200562749 +        incidents_archive:                  720668923636351037          mods:               &MODS           305126844661760000          mod_alerts:         &MOD_ALERTS     473092532147060736          mod_spam:           &MOD_SPAM       620607373828030464          organisation:       &ORGANISATION   551789653284356126          staff_lounge:       &STAFF_LOUNGE   464905259261755392 -        incidents:                          714214212200562749          # Voice          admins_voice:       &ADMINS_VOICE   500734494840717332 @@ -246,13 +251,14 @@ guild:          - *HELPERS_ROLE      webhooks: -        talent_pool:                    569145364800602132 -        big_brother:                    569133704568373283 -        reddit:                         635408384794951680 -        duck_pond:                      637821475327311927 -        dev_log:                        680501655111729222 -        python_news:    &PYNEWS_WEBHOOK 704381182279942324 -        dm_log:                         654567640664244225 +        big_brother:                        569133704568373283 +        dev_log:                            680501655111729222 +        dm_log:                             654567640664244225 +        duck_pond:                          637821475327311927 +        incidents_archive:                  720671599790915702 +        python_news:        &PYNEWS_WEBHOOK 704381182279942324 +        reddit:                             635408384794951680 +        talent_pool:                        569145364800602132  filter:      # What do we filter? diff --git a/tests/bot/cogs/moderation/test_incidents.py b/tests/bot/cogs/moderation/test_incidents.py new file mode 100644 index 000000000..435a1cd51 --- /dev/null +++ b/tests/bot/cogs/moderation/test_incidents.py @@ -0,0 +1,770 @@ +import asyncio +import enum +import logging +import typing as t +import unittest +from unittest.mock import AsyncMock, MagicMock, call, patch + +import aiohttp +import discord + +from bot.cogs.moderation import Incidents, incidents +from bot.constants import Colours +from tests.helpers import ( +    MockAsyncWebhook, +    MockAttachment, +    MockBot, +    MockMember, +    MockMessage, +    MockReaction, +    MockRole, +    MockTextChannel, +    MockUser, +) + + +class MockAsyncIterable: +    """ +    Helper for mocking asynchronous for loops. + +    It does not appear that the `unittest` library currently provides anything that would +    allow us to simply mock an async iterator, such as `discord.TextChannel.history`. + +    We therefore write our own helper to wrap a regular synchronous iterable, and feed +    its values via `__anext__` rather than `__next__`. + +    This class was written for the purposes of testing the `Incidents` cog - it may not +    be generic enough to be placed in the `tests.helpers` module. +    """ + +    def __init__(self, messages: t.Iterable): +        """Take a sync iterable to be wrapped.""" +        self.iter_messages = iter(messages) + +    def __aiter__(self): +        """Return `self` as we provide the `__anext__` method.""" +        return self + +    async def __anext__(self): +        """ +        Feed the next item, or raise `StopAsyncIteration`. + +        Since we're wrapping a sync iterator, it will communicate that it has been depleted +        by raising a `StopIteration`. The `async for` construct does not expect it, and we +        therefore need to substitute it for the appropriate exception type. +        """ +        try: +            return next(self.iter_messages) +        except StopIteration: +            raise StopAsyncIteration + + +class MockSignal(enum.Enum): +    A = "A" +    B = "B" + + +mock_404 = discord.NotFound( +    response=MagicMock(aiohttp.ClientResponse),  # Mock the erroneous response +    message="Not found", +) + + +class TestDownloadFile(unittest.IsolatedAsyncioTestCase): +    """Collection of tests for the `download_file` helper function.""" + +    async def test_download_file_success(self): +        """If `to_file` succeeds, function returns the acquired `discord.File`.""" +        file = MagicMock(discord.File, filename="bigbadlemon.jpg") +        attachment = MockAttachment(to_file=AsyncMock(return_value=file)) + +        acquired_file = await incidents.download_file(attachment) +        self.assertIs(file, acquired_file) + +    async def test_download_file_404(self): +        """If `to_file` encounters a 404, function handles the exception & returns None.""" +        attachment = MockAttachment(to_file=AsyncMock(side_effect=mock_404)) + +        acquired_file = await incidents.download_file(attachment) +        self.assertIsNone(acquired_file) + +    async def test_download_file_fail(self): +        """If `to_file` fails on a non-404 error, function logs the exception & returns None.""" +        arbitrary_error = discord.HTTPException(MagicMock(aiohttp.ClientResponse), "Arbitrary API error") +        attachment = MockAttachment(to_file=AsyncMock(side_effect=arbitrary_error)) + +        with self.assertLogs(logger=incidents.log, level=logging.ERROR): +            acquired_file = await incidents.download_file(attachment) + +        self.assertIsNone(acquired_file) + + +class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): +    """Collection of tests for the `make_embed` helper function.""" + +    async def test_make_embed_actioned(self): +        """Embed is coloured green and footer contains 'Actioned' when `outcome=Signal.ACTIONED`.""" +        embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.ACTIONED, MockMember()) + +        self.assertEqual(embed.colour.value, Colours.soft_green) +        self.assertIn("Actioned", embed.footer.text) + +    async def test_make_embed_not_actioned(self): +        """Embed is coloured red and footer contains 'Rejected' when `outcome=Signal.NOT_ACTIONED`.""" +        embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.NOT_ACTIONED, MockMember()) + +        self.assertEqual(embed.colour.value, Colours.soft_red) +        self.assertIn("Rejected", embed.footer.text) + +    async def test_make_embed_content(self): +        """Incident content appears as embed description.""" +        incident = MockMessage(content="this is an incident") +        embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) + +        self.assertEqual(incident.content, embed.description) + +    async def test_make_embed_with_attachment_succeeds(self): +        """Incident's attachment is downloaded and displayed in the embed's image field.""" +        file = MagicMock(discord.File, filename="bigbadjoe.jpg") +        attachment = MockAttachment(filename="bigbadjoe.jpg") +        incident = MockMessage(content="this is an incident", attachments=[attachment]) + +        # Patch `download_file` to return our `file` +        with patch("bot.cogs.moderation.incidents.download_file", AsyncMock(return_value=file)): +            embed, returned_file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) + +        self.assertIs(file, returned_file) +        self.assertEqual("attachment://bigbadjoe.jpg", embed.image.url) + +    async def test_make_embed_with_attachment_fails(self): +        """Incident's attachment fails to download, proxy url is linked instead.""" +        attachment = MockAttachment(proxy_url="discord.com/bigbadjoe.jpg") +        incident = MockMessage(content="this is an incident", attachments=[attachment]) + +        # Patch `download_file` to return None as if the download failed +        with patch("bot.cogs.moderation.incidents.download_file", AsyncMock(return_value=None)): +            embed, returned_file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) + +        self.assertIsNone(returned_file) + +        # The author name field is simply expected to have something in it, we do not assert the message +        self.assertGreater(len(embed.author.name), 0) +        self.assertEqual(embed.author.url, "discord.com/bigbadjoe.jpg")  # However, it should link the exact url + + +@patch("bot.constants.Channels.incidents", 123) +class TestIsIncident(unittest.TestCase): +    """ +    Collection of tests for the `is_incident` helper function. + +    In `setUp`, we will create a mock message which should qualify as an incident. Each +    test case will then mutate this instance to make it **not** qualify, in various ways. + +    Notice that we patch the #incidents channel id globally for this class. +    """ + +    def setUp(self) -> None: +        """Prepare a mock message which should qualify as an incident.""" +        self.incident = MockMessage( +            channel=MockTextChannel(id=123), +            content="this is an incident", +            author=MockUser(bot=False), +            pinned=False, +        ) + +    def test_is_incident_true(self): +        """Message qualifies as an incident if unchanged.""" +        self.assertTrue(incidents.is_incident(self.incident)) + +    def check_false(self): +        """Assert that `self.incident` does **not** qualify as an incident.""" +        self.assertFalse(incidents.is_incident(self.incident)) + +    def test_is_incident_false_channel(self): +        """Message doesn't qualify if sent outside of #incidents.""" +        self.incident.channel = MockTextChannel(id=456) +        self.check_false() + +    def test_is_incident_false_content(self): +        """Message doesn't qualify if content begins with hash symbol.""" +        self.incident.content = "# this is a comment message" +        self.check_false() + +    def test_is_incident_false_author(self): +        """Message doesn't qualify if author is a bot.""" +        self.incident.author = MockUser(bot=True) +        self.check_false() + +    def test_is_incident_false_pinned(self): +        """Message doesn't qualify if it is pinned.""" +        self.incident.pinned = True +        self.check_false() + + +class TestOwnReactions(unittest.TestCase): +    """Assertions for the `own_reactions` function.""" + +    def test_own_reactions(self): +        """Only bot's own emoji are extracted from the input incident.""" +        reactions = ( +            MockReaction(emoji="A", me=True), +            MockReaction(emoji="B", me=True), +            MockReaction(emoji="C", me=False), +        ) +        message = MockMessage(reactions=reactions) +        self.assertSetEqual(incidents.own_reactions(message), {"A", "B"}) + + +@patch("bot.cogs.moderation.incidents.ALL_SIGNALS", {"A", "B"}) +class TestHasSignals(unittest.TestCase): +    """ +    Assertions for the `has_signals` function. + +    We patch `ALL_SIGNALS` globally. Each test function then patches `own_reactions` +    as appropriate. +    """ + +    def test_has_signals_true(self): +        """True when `own_reactions` returns all emoji in `ALL_SIGNALS`.""" +        message = MockMessage() +        own_reactions = MagicMock(return_value={"A", "B"}) + +        with patch("bot.cogs.moderation.incidents.own_reactions", own_reactions): +            self.assertTrue(incidents.has_signals(message)) + +    def test_has_signals_false(self): +        """False when `own_reactions` does not return all emoji in `ALL_SIGNALS`.""" +        message = MockMessage() +        own_reactions = MagicMock(return_value={"A", "C"}) + +        with patch("bot.cogs.moderation.incidents.own_reactions", own_reactions): +            self.assertFalse(incidents.has_signals(message)) + + +@patch("bot.cogs.moderation.incidents.Signal", MockSignal) +class TestAddSignals(unittest.IsolatedAsyncioTestCase): +    """ +    Assertions for the `add_signals` coroutine. + +    These are all fairly similar and could go into a single test function, but I found the +    patching & sub-testing fairly awkward in that case and decided to split them up +    to avoid unnecessary syntax noise. +    """ + +    def setUp(self): +        """Prepare a mock incident message for tests to use.""" +        self.incident = MockMessage() + +    @patch("bot.cogs.moderation.incidents.own_reactions", MagicMock(return_value=set())) +    async def test_add_signals_missing(self): +        """All emoji are added when none are present.""" +        await incidents.add_signals(self.incident) +        self.incident.add_reaction.assert_has_calls([call("A"), call("B")]) + +    @patch("bot.cogs.moderation.incidents.own_reactions", MagicMock(return_value={"A"})) +    async def test_add_signals_partial(self): +        """Only missing emoji are added when some are present.""" +        await incidents.add_signals(self.incident) +        self.incident.add_reaction.assert_has_calls([call("B")]) + +    @patch("bot.cogs.moderation.incidents.own_reactions", MagicMock(return_value={"A", "B"})) +    async def test_add_signals_present(self): +        """No emoji are added when all are present.""" +        await incidents.add_signals(self.incident) +        self.incident.add_reaction.assert_not_called() + + +class TestIncidents(unittest.IsolatedAsyncioTestCase): +    """ +    Tests for bound methods of the `Incidents` cog. + +    Use this as a base class for `Incidents` tests - it will prepare a fresh instance +    for each test function, but not make any assertions on its own. Tests can mutate +    the instance as they wish. +    """ + +    def setUp(self): +        """ +        Prepare a fresh `Incidents` instance for each test. + +        Note that this will not schedule `crawl_incidents` in the background, as everything +        is being mocked. The `crawl_task` attribute will end up being None. +        """ +        self.cog_instance = Incidents(MockBot()) + + +@patch("asyncio.sleep", AsyncMock())  # Prevent the coro from sleeping to speed up the test +class TestCrawlIncidents(TestIncidents): +    """ +    Tests for the `Incidents.crawl_incidents` coroutine. + +    Apart from `test_crawl_incidents_waits_until_cache_ready`, all tests in this class +    will patch the return values of `is_incident` and `has_signal` and then observe +    whether the `AsyncMock` for `add_signals` was awaited or not. + +    The `add_signals` mock is added by each test separately to ensure it is clean (has not +    been awaited by another test yet). The mock can be reset, but this appears to be the +    cleaner way. + +    For each test, we inject a mock channel with a history of 1 message only (see: `setUp`). +    """ + +    def setUp(self): +        """For each test, ensure `bot.get_channel` returns a channel with 1 arbitrary message.""" +        super().setUp()  # First ensure we get `cog_instance` from parent + +        incidents_history = MagicMock(return_value=MockAsyncIterable([MockMessage()])) +        self.cog_instance.bot.get_channel = MagicMock(return_value=MockTextChannel(history=incidents_history)) + +    async def test_crawl_incidents_waits_until_cache_ready(self): +        """ +        The coroutine will await the `wait_until_guild_available` event. + +        Since this task is schedule in the `__init__`, it is critical that it waits for the +        cache to be ready, so that it can safely get the #incidents channel. +        """ +        await self.cog_instance.crawl_incidents() +        self.cog_instance.bot.wait_until_guild_available.assert_awaited() + +    @patch("bot.cogs.moderation.incidents.add_signals", AsyncMock()) +    @patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=False))  # Message doesn't qualify +    @patch("bot.cogs.moderation.incidents.has_signals", MagicMock(return_value=False)) +    async def test_crawl_incidents_noop_if_is_not_incident(self): +        """Signals are not added for a non-incident message.""" +        await self.cog_instance.crawl_incidents() +        incidents.add_signals.assert_not_awaited() + +    @patch("bot.cogs.moderation.incidents.add_signals", AsyncMock()) +    @patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=True))  # Message qualifies +    @patch("bot.cogs.moderation.incidents.has_signals", MagicMock(return_value=True))  # But already has signals +    async def test_crawl_incidents_noop_if_message_already_has_signals(self): +        """Signals are not added for messages which already have them.""" +        await self.cog_instance.crawl_incidents() +        incidents.add_signals.assert_not_awaited() + +    @patch("bot.cogs.moderation.incidents.add_signals", AsyncMock()) +    @patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=True))  # Message qualifies +    @patch("bot.cogs.moderation.incidents.has_signals", MagicMock(return_value=False))  # And doesn't have signals +    async def test_crawl_incidents_add_signals_called(self): +        """Message has signals added as it does not have them yet and qualifies as an incident.""" +        await self.cog_instance.crawl_incidents() +        incidents.add_signals.assert_awaited_once() + + +class TestArchive(TestIncidents): +    """Tests for the `Incidents.archive` coroutine.""" + +    async def test_archive_webhook_not_found(self): +        """ +        Method recovers and returns False when the webhook is not found. + +        Implicitly, this also tests that the error is handled internally and doesn't +        propagate out of the method, which is just as important. +        """ +        self.cog_instance.bot.fetch_webhook = AsyncMock(side_effect=mock_404) +        self.assertFalse( +            await self.cog_instance.archive(incident=MockMessage(), outcome=MagicMock(), actioned_by=MockMember()) +        ) + +    async def test_archive_relays_incident(self): +        """ +        If webhook is found, method relays `incident` properly. + +        This test will assert that the fetched webhook's `send` method is fed the correct arguments, +        and that the `archive` method returns True. +        """ +        webhook = MockAsyncWebhook() +        self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook)  # Patch in our webhook + +        # Define our own `incident` to be archived +        incident = MockMessage( +            content="this is an incident", +            author=MockUser(name="author_name", avatar_url="author_avatar"), +            id=123, +        ) +        built_embed = MagicMock(discord.Embed, id=123)  # We patch `make_embed` to return this + +        with patch("bot.cogs.moderation.incidents.make_embed", AsyncMock(return_value=(built_embed, None))): +            archive_return = await self.cog_instance.archive(incident, MagicMock(value="A"), MockMember()) + +        # Now we check that the webhook was given the correct args, and that `archive` returned True +        webhook.send.assert_called_once_with( +            embed=built_embed, +            username="author_name", +            avatar_url="author_avatar", +            file=None, +        ) +        self.assertTrue(archive_return) + +    async def test_archive_clyde_username(self): +        """ +        The archive webhook username is cleansed using `sub_clyde`. + +        Discord will reject any webhook with "clyde" in the username field, as it impersonates +        the official Clyde bot. Since we do not control what the username will be (the incident +        author name is used), we must ensure the name is cleansed, otherwise the relay may fail. + +        This test assumes the username is passed as a kwarg. If this test fails, please review +        whether the passed argument is being retrieved correctly. +        """ +        webhook = MockAsyncWebhook() +        self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) + +        message_from_clyde = MockMessage(author=MockUser(name="clyde the great")) +        await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal), MockMember()) + +        self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) + + +class TestMakeConfirmationTask(TestIncidents): +    """ +    Tests for the `Incidents.make_confirmation_task` method. + +    Writing tests for this method is difficult, as it mostly just delegates the provided +    information elsewhere. There is very little internal logic. Whether our approach +    works conceptually is difficult to prove using unit tests. +    """ + +    def test_make_confirmation_task_check(self): +        """ +        The internal check will recognize the passed incident. + +        This is a little tricky - we first pass a message with a specific `id` in, and then +        retrieve the built check from the `call_args` of the `wait_for` method. This relies +        on the check being passed as a kwarg. + +        Once the check is retrieved, we assert that it gives True for our incident's `id`, +        and False for any other. + +        If this function begins to fail, first check that `created_check` is being retrieved +        correctly. It should be the function that is built locally in the tested method. +        """ +        self.cog_instance.make_confirmation_task(MockMessage(id=123)) + +        self.cog_instance.bot.wait_for.assert_called_once() +        created_check = self.cog_instance.bot.wait_for.call_args.kwargs["check"] + +        # The `message_id` matches the `id` of our incident +        self.assertTrue(created_check(payload=MagicMock(message_id=123))) + +        # This `message_id` does not match +        self.assertFalse(created_check(payload=MagicMock(message_id=0))) + + +@patch("bot.cogs.moderation.incidents.ALLOWED_ROLES", {1, 2}) +@patch("bot.cogs.moderation.incidents.Incidents.make_confirmation_task", AsyncMock())  # Generic awaitable +class TestProcessEvent(TestIncidents): +    """Tests for the `Incidents.process_event` coroutine.""" + +    async def test_process_event_bad_role(self): +        """The reaction is removed when the author lacks all allowed roles.""" +        incident = MockMessage() +        member = MockMember(roles=[MockRole(id=0)])  # Must have role 1 or 2 + +        await self.cog_instance.process_event("reaction", incident, member) +        incident.remove_reaction.assert_called_once_with("reaction", member) + +    async def test_process_event_bad_emoji(self): +        """ +        The reaction is removed when an invalid emoji is used. + +        This requires that we pass in a `member` with valid roles, as we need the role check +        to succeed. +        """ +        incident = MockMessage() +        member = MockMember(roles=[MockRole(id=1)])  # Member has allowed role + +        await self.cog_instance.process_event("invalid_signal", incident, member) +        incident.remove_reaction.assert_called_once_with("invalid_signal", member) + +    async def test_process_event_no_archive_on_investigating(self): +        """Message is not archived on `Signal.INVESTIGATING`.""" +        with patch("bot.cogs.moderation.incidents.Incidents.archive", AsyncMock()) as mocked_archive: +            await self.cog_instance.process_event( +                reaction=incidents.Signal.INVESTIGATING.value, +                incident=MockMessage(), +                member=MockMember(roles=[MockRole(id=1)]), +            ) + +        mocked_archive.assert_not_called() + +    async def test_process_event_no_delete_if_archive_fails(self): +        """ +        Original message is not deleted when `Incidents.archive` returns False. + +        This is the way of signaling that the relay failed, and we should not remove the original, +        as that would result in losing the incident record. +        """ +        incident = MockMessage() + +        with patch("bot.cogs.moderation.incidents.Incidents.archive", AsyncMock(return_value=False)): +            await self.cog_instance.process_event( +                reaction=incidents.Signal.ACTIONED.value, +                incident=incident, +                member=MockMember(roles=[MockRole(id=1)]) +            ) + +        incident.delete.assert_not_called() + +    async def test_process_event_confirmation_task_is_awaited(self): +        """Task given by `Incidents.make_confirmation_task` is awaited before method exits.""" +        mock_task = AsyncMock() + +        with patch("bot.cogs.moderation.incidents.Incidents.make_confirmation_task", mock_task): +            await self.cog_instance.process_event( +                reaction=incidents.Signal.ACTIONED.value, +                incident=MockMessage(), +                member=MockMember(roles=[MockRole(id=1)]) +            ) + +        mock_task.assert_awaited() + +    async def test_process_event_confirmation_task_timeout_is_handled(self): +        """ +        Confirmation task `asyncio.TimeoutError` is handled gracefully. + +        We have `make_confirmation_task` return a mock with a side effect, and then catch the +        exception should it propagate out of `process_event`. This is so that we can then manually +        fail the test with a more informative message than just the plain traceback. +        """ +        mock_task = AsyncMock(side_effect=asyncio.TimeoutError()) + +        try: +            with patch("bot.cogs.moderation.incidents.Incidents.make_confirmation_task", mock_task): +                await self.cog_instance.process_event( +                    reaction=incidents.Signal.ACTIONED.value, +                    incident=MockMessage(), +                    member=MockMember(roles=[MockRole(id=1)]) +                ) +        except asyncio.TimeoutError: +            self.fail("TimeoutError was not handled gracefully, and propagated out of `process_event`!") + + +class TestResolveMessage(TestIncidents): +    """Tests for the `Incidents.resolve_message` coroutine.""" + +    async def test_resolve_message_pass_message_id(self): +        """Method will call `_get_message` with the passed `message_id`.""" +        await self.cog_instance.resolve_message(123) +        self.cog_instance.bot._connection._get_message.assert_called_once_with(123) + +    async def test_resolve_message_in_cache(self): +        """ +        No API call is made if the queried message exists in the cache. + +        We mock the `_get_message` return value regardless of input. Whether it finds the message +        internally is considered d.py's responsibility, not ours. +        """ +        cached_message = MockMessage(id=123) +        self.cog_instance.bot._connection._get_message = MagicMock(return_value=cached_message) + +        return_value = await self.cog_instance.resolve_message(123) + +        self.assertIs(return_value, cached_message) +        self.cog_instance.bot.get_channel.assert_not_called()  # The `fetch_message` line was never hit + +    async def test_resolve_message_not_in_cache(self): +        """ +        The message is retrieved from the API if it isn't cached. + +        This is desired behaviour for messages which exist, but were sent before the bot's +        current session. +        """ +        self.cog_instance.bot._connection._get_message = MagicMock(return_value=None)  # Cache returns None + +        # API returns our message +        uncached_message = MockMessage() +        fetch_message = AsyncMock(return_value=uncached_message) +        self.cog_instance.bot.get_channel = MagicMock(return_value=MockTextChannel(fetch_message=fetch_message)) + +        retrieved_message = await self.cog_instance.resolve_message(123) +        self.assertIs(retrieved_message, uncached_message) + +    async def test_resolve_message_doesnt_exist(self): +        """ +        If the API returns a 404, the function handles it gracefully and returns None. + +        This is an edge-case happening with racing events - event A will relay the message +        to the archive and delete the original. Once event B acquires the `event_lock`, +        it will not find the message in the cache, and will ask the API. +        """ +        self.cog_instance.bot._connection._get_message = MagicMock(return_value=None)  # Cache returns None + +        fetch_message = AsyncMock(side_effect=mock_404) +        self.cog_instance.bot.get_channel = MagicMock(return_value=MockTextChannel(fetch_message=fetch_message)) + +        self.assertIsNone(await self.cog_instance.resolve_message(123)) + +    async def test_resolve_message_fetch_fails(self): +        """ +        Non-404 errors are handled, logged & None is returned. + +        In contrast with a 404, this should make an error-level log. We assert that at least +        one such log was made - we do not make any assertions about the log's message. +        """ +        self.cog_instance.bot._connection._get_message = MagicMock(return_value=None)  # Cache returns None + +        arbitrary_error = discord.HTTPException( +            response=MagicMock(aiohttp.ClientResponse), +            message="Arbitrary error", +        ) +        fetch_message = AsyncMock(side_effect=arbitrary_error) +        self.cog_instance.bot.get_channel = MagicMock(return_value=MockTextChannel(fetch_message=fetch_message)) + +        with self.assertLogs(logger=incidents.log, level=logging.ERROR): +            self.assertIsNone(await self.cog_instance.resolve_message(123)) + + +@patch("bot.constants.Channels.incidents", 123) +class TestOnRawReactionAdd(TestIncidents): +    """ +    Tests for the `Incidents.on_raw_reaction_add` listener. + +    Writing tests for this listener comes with additional complexity due to the listener +    awaiting the `crawl_task` task. See `asyncSetUp` for further details, which attempts +    to make unit testing this function possible. +    """ + +    def setUp(self): +        """ +        Prepare & assign `payload` attribute. + +        This attribute represents an *ideal* payload which will not be rejected by the +        listener. As each test will receive a fresh instance, it can be mutated to +        observe how the listener's behaviour changes with different attributes on +        the passed payload. +        """ +        super().setUp()  # Ensure `cog_instance` is assigned + +        self.payload = MagicMock( +            discord.RawReactionActionEvent, +            channel_id=123,  # Patched at class level +            message_id=456, +            member=MockMember(bot=False), +            emoji="reaction", +        ) + +    async def asyncSetUp(self):  # noqa: N802 +        """ +        Prepare an empty task and assign it as `crawl_task`. + +        It appears that the `unittest` framework does not provide anything for mocking +        asyncio tasks. An `AsyncMock` instance can be called and then awaited, however, +        it does not provide the `done` method or any other parts of the `asyncio.Task` +        interface. + +        Although we do not need to make any assertions about the task itself while +        testing the listener, the code will still await it and call the `done` method, +        and so we must inject something that will not fail on either action. + +        Note that this is done in an `asyncSetUp`, which runs after `setUp`. +        The justification is that creating an actual task requires the event +        loop to be ready, which is not the case in the `setUp`. +        """ +        mock_task = asyncio.create_task(AsyncMock()())  # Mock async func, then a coro +        self.cog_instance.crawl_task = mock_task + +    async def test_on_raw_reaction_add_wrong_channel(self): +        """ +        Events outside of #incidents will be ignored. + +        We check this by asserting that `resolve_message` was never queried. +        """ +        self.payload.channel_id = 0 +        self.cog_instance.resolve_message = AsyncMock() + +        await self.cog_instance.on_raw_reaction_add(self.payload) +        self.cog_instance.resolve_message.assert_not_called() + +    async def test_on_raw_reaction_add_user_is_bot(self): +        """ +        Events dispatched by bot accounts will be ignored. + +        We check this by asserting that `resolve_message` was never queried. +        """ +        self.payload.member = MockMember(bot=True) +        self.cog_instance.resolve_message = AsyncMock() + +        await self.cog_instance.on_raw_reaction_add(self.payload) +        self.cog_instance.resolve_message.assert_not_called() + +    async def test_on_raw_reaction_add_message_doesnt_exist(self): +        """ +        Listener gracefully handles the case where `resolve_message` gives None. + +        We check this by asserting that `process_event` was never called. +        """ +        self.cog_instance.process_event = AsyncMock() +        self.cog_instance.resolve_message = AsyncMock(return_value=None) + +        await self.cog_instance.on_raw_reaction_add(self.payload) +        self.cog_instance.process_event.assert_not_called() + +    async def test_on_raw_reaction_add_message_is_not_an_incident(self): +        """ +        The event won't be processed if the related message is not an incident. + +        This is an edge-case that can happen if someone manually leaves a reaction +        on a pinned message, or a comment. + +        We check this by asserting that `process_event` was never called. +        """ +        self.cog_instance.process_event = AsyncMock() +        self.cog_instance.resolve_message = AsyncMock(return_value=MockMessage()) + +        with patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=False)): +            await self.cog_instance.on_raw_reaction_add(self.payload) + +        self.cog_instance.process_event.assert_not_called() + +    async def test_on_raw_reaction_add_valid_event_is_processed(self): +        """ +        If the reaction event is valid, it is passed to `process_event`. + +        This is the case when everything goes right: +            * The reaction was placed in #incidents, and not by a bot +            * The message was found successfully +            * The message qualifies as an incident + +        Additionally, we check that all arguments were passed as expected. +        """ +        incident = MockMessage(id=1) + +        self.cog_instance.process_event = AsyncMock() +        self.cog_instance.resolve_message = AsyncMock(return_value=incident) + +        with patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=True)): +            await self.cog_instance.on_raw_reaction_add(self.payload) + +        self.cog_instance.process_event.assert_called_with( +            "reaction",  # Defined in `self.payload` +            incident, +            self.payload.member, +        ) + + +class TestOnMessage(TestIncidents): +    """ +    Tests for the `Incidents.on_message` listener. + +    Notice the decorators mocking the `is_incident` return value. The `is_incidents` +    function is tested in `TestIsIncident` - here we do not worry about it. +    """ + +    @patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=True)) +    async def test_on_message_incident(self): +        """Messages qualifying as incidents are passed to `add_signals`.""" +        incident = MockMessage() + +        with patch("bot.cogs.moderation.incidents.add_signals", AsyncMock()) as mock_add_signals: +            await self.cog_instance.on_message(incident) + +        mock_add_signals.assert_called_once_with(incident) + +    @patch("bot.cogs.moderation.incidents.is_incident", MagicMock(return_value=False)) +    async def test_on_message_non_incident(self): +        """Messages not qualifying as incidents are ignored.""" +        with patch("bot.cogs.moderation.incidents.add_signals", AsyncMock()) as mock_add_signals: +            await self.cog_instance.on_message(MockMessage()) + +        mock_add_signals.assert_not_called() diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 98dee7a1b..343e37db9 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -239,7 +239,7 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase):          await self.cog.send_eval(ctx, 'MyAwesomeCode')          ctx.send.assert_called_once_with( -            '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```' +            '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```\n[No output]\n```'          )          self.cog.post_eval.assert_called_once_with('MyAwesomeCode')          self.cog.get_status_emoji.assert_called_once_with({'stdout': '', 'returncode': 0}) @@ -265,7 +265,7 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase):          await self.cog.send_eval(ctx, 'MyAwesomeCode')          ctx.send.assert_called_once_with(              '@LemonLemonishBeard#0042 :yay!: Return code 0.' -            '\n\n```py\nWay too long beard\n```\nFull output: lookatmybeard.com' +            '\n\n```\nWay too long beard\n```\nFull output: lookatmybeard.com'          )          self.cog.post_eval.assert_called_once_with('MyAwesomeCode')          self.cog.get_status_emoji.assert_called_once_with({'stdout': 'Way too long beard', 'returncode': 0}) @@ -289,7 +289,7 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase):          await self.cog.send_eval(ctx, 'MyAwesomeCode')          ctx.send.assert_called_once_with( -            '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```py\nBeard got stuck in the eval\n```' +            '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```\nBeard got stuck in the eval\n```'          )          self.cog.post_eval.assert_called_once_with('MyAwesomeCode')          self.cog.get_status_emoji.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) | 
