From 4efb97c5020f591d8cdd1e214e06df294e72d8f1 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 20 Oct 2019 18:32:25 +0200 Subject: add handling for duplicate symbols in docs inventories --- bot/cogs/doc.py | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index a13464bff..43315f477 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -23,7 +23,17 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) logging.getLogger('urllib3').setLevel(logging.WARNING) - +NO_OVERRIDE_GROUPS = ( + "2to3fixer", + "token", + "label", + "pdbcommand", + "term", + "function" +) +NO_OVERRIDE_PACKAGES = ( + "Python", +) UNWANTED_SIGNATURE_SYMBOLS = ('[source]', '¶') WHITESPACE_AFTER_NEWLINES_RE = re.compile(r"(?<=\n\n)(\s+)") @@ -125,6 +135,7 @@ class Doc(commands.Cog): self.base_urls = {} self.bot = bot self.inventories = {} + self.renamed_symbols = set() self.bot.loop.create_task(self.init_refresh_inventory()) @@ -151,12 +162,32 @@ class Doc(commands.Cog): self.base_urls[package_name] = base_url fetch_func = functools.partial(intersphinx.fetch_inventory, config, '', inventory_url) - for _, value in (await self.bot.loop.run_in_executor(None, fetch_func)).items(): + for group, value in (await self.bot.loop.run_in_executor(None, fetch_func)).items(): # Each value has a bunch of information in the form # `(package_name, version, relative_url, ???)`, and we only - # need the relative documentation URL. - for symbol, (_, _, relative_doc_url, _) in value.items(): + # need the package_name and the relative documentation URL. + for symbol, (package_name, _, relative_doc_url, _) in value.items(): absolute_doc_url = base_url + relative_doc_url + + if symbol in self.inventories: + # get `group_name` from _:group_name + group_name = group.split(":")[1] + if (group_name in NO_OVERRIDE_GROUPS + # check if any package from `NO_OVERRIDE_PACKAGES` + # is in base URL of the symbol that would be overridden + or any(package in self.inventories[symbol].split("/", 3)[2] + for package in NO_OVERRIDE_PACKAGES)): + + symbol = f"{group_name}.{symbol}" + # if renamed `symbol` was already exists, add library name in front + if symbol in self.renamed_symbols: + # split `package_name` because of packages like Pillow that have spaces in them + symbol = f"{package_name.split()[0]}.{symbol}" + + self.inventories[symbol] = absolute_doc_url + self.renamed_symbols.add(symbol) + continue + self.inventories[symbol] = absolute_doc_url log.trace(f"Fetched inventory for {package_name}.") -- cgit v1.2.3 From f1dbb63e6c4a7ed38f8bed994c109e638498d546 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 20 Oct 2019 18:39:08 +0200 Subject: show renamed duplicates in embed footer --- bot/cogs/doc.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 43315f477..ecff43864 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -281,18 +281,23 @@ class Doc(commands.Cog): if not signature: # It's some "meta-page", for example: # https://docs.djangoproject.com/en/dev/ref/views/#module-django.views - return discord.Embed( + embed = discord.Embed( title=f'`{symbol}`', url=permalink, description="This appears to be a generic page not tied to a specific symbol." ) - - signature = textwrap.shorten(signature, 500) - return discord.Embed( - title=f'`{symbol}`', - url=permalink, - description=f"```py\n{signature}```{description}" - ) + else: + signature = textwrap.shorten(signature, 500) + embed = discord.Embed( + title=f'`{symbol}`', + url=permalink, + description=f"```py\n{signature}```{description}" + ) + # show all symbols with the same name that were renamed in the footer + embed.set_footer(text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} + if renamed.endswith(f".{symbol}")) + ) + return embed @commands.group(name='docs', aliases=('doc', 'd'), invoke_without_command=True) async def docs_group(self, ctx: commands.Context, symbol: commands.clean_content = None) -> None: -- cgit v1.2.3 From a05f28c97d0f2ea9d3dafcdbd24444c59905af84 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 20 Oct 2019 18:42:59 +0200 Subject: Auto delete messages when docs are not found --- bot/cogs/doc.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index ecff43864..9bb21cce3 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -4,17 +4,19 @@ import logging import re import textwrap from collections import OrderedDict +from contextlib import suppress from typing import Any, Callable, Optional, Tuple import discord from bs4 import BeautifulSoup from bs4.element import PageElement +from discord.errors import NotFound from discord.ext import commands from markdownify import MarkdownConverter from requests import ConnectionError from sphinx.ext import intersphinx -from bot.constants import MODERATION_ROLES +from bot.constants import MODERATION_ROLES, RedirectOutput from bot.converters import ValidPythonIdentifier, ValidURL from bot.decorators import with_role from bot.pagination import LinePaginator @@ -23,6 +25,7 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) logging.getLogger('urllib3').setLevel(logging.WARNING) +NOT_FOUND_DELETE_DELAY = RedirectOutput.delete_delay NO_OVERRIDE_GROUPS = ( "2to3fixer", "token", @@ -343,7 +346,10 @@ class Doc(commands.Cog): description=f"Sorry, I could not find any documentation for `{symbol}`.", colour=discord.Colour.red() ) - await ctx.send(embed=error_embed) + error_message = await ctx.send(embed=error_embed) + with suppress(NotFound): + await error_message.delete(delay=NOT_FOUND_DELETE_DELAY) + await ctx.message.delete(delay=NOT_FOUND_DELETE_DELAY) else: await ctx.send(embed=doc_embed) -- cgit v1.2.3 From eda6cd7ff818454ad7bf448040a87ff0077025bc Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 20 Oct 2019 21:15:12 +0200 Subject: remove "function" from NO_OVERRIDE_GROUPS --- bot/cogs/doc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 9bb21cce3..f1213d170 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -32,7 +32,6 @@ NO_OVERRIDE_GROUPS = ( "label", "pdbcommand", "term", - "function" ) NO_OVERRIDE_PACKAGES = ( "Python", -- cgit v1.2.3 From d5dea25fef79e16d726f1f0ce8d2bb25291d6c49 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 21 Oct 2019 22:09:46 +0200 Subject: Don't include a signature and only get first paragraphs when scraping when symbol is a module --- bot/cogs/doc.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index f1213d170..a13552ac0 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -222,12 +222,11 @@ class Doc(commands.Cog): """ Given a Python symbol, return its signature and description. - Returns a tuple in the form (str, str), or `None`. - The first tuple element is the signature of the given symbol as a markup-free string, and the second tuple element is the description of the given symbol with HTML markup included. - If the given symbol could not be found, returns `None`. + If the given symbol is a module, returns a tuple `(None, str)` + else if the symbol could not be found, returns `None`. """ url = self.inventories.get(symbol) if url is None: @@ -245,14 +244,23 @@ class Doc(commands.Cog): if symbol_heading is None: return None - # Traverse the tags of the signature header and ignore any - # unwanted symbols from it. Add all of it to a temporary buffer. - for tag in symbol_heading.strings: - if tag not in UNWANTED_SIGNATURE_SYMBOLS: - signature_buffer.append(tag.replace('\\', '')) + if symbol_id == f"module-{symbol}": + # Get all paragraphs until the first div after the section div + # if searched symbol is a module. + trailing_div = symbol_heading.findNext("div") + info_paragraphs = trailing_div.find_previous_siblings("p")[::-1] + signature = None + description = ''.join(str(paragraph) for paragraph in info_paragraphs).replace('¶', '') - signature = ''.join(signature_buffer) - description = str(symbol_heading.next_sibling.next_sibling).replace('¶', '') + else: + # Traverse the tags of the signature header and ignore any + # unwanted symbols from it. Add all of it to a temporary buffer. + + for tag in symbol_heading.strings: + if tag not in UNWANTED_SIGNATURE_SYMBOLS: + signature_buffer.append(tag.replace('\\', '')) + signature = ''.join(signature_buffer) + description = str(symbol_heading.next_sibling.next_sibling).replace('¶', '') return signature, description -- cgit v1.2.3 From 55b276a1f7e56a950e215bd8289b7f946b2f180e Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 21 Oct 2019 22:10:45 +0200 Subject: Allow embeds to not include signatures in case the symbol is a module --- bot/cogs/doc.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index a13552ac0..0c370f665 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -288,21 +288,24 @@ class Doc(commands.Cog): description = WHITESPACE_AFTER_NEWLINES_RE.sub('', description) - if not signature: + if signature is None: + # If symbol is a module, don't show signature. + embed_description = description + + elif not signature: # It's some "meta-page", for example: # https://docs.djangoproject.com/en/dev/ref/views/#module-django.views - embed = discord.Embed( - title=f'`{symbol}`', - url=permalink, - description="This appears to be a generic page not tied to a specific symbol." - ) + embed_description = "This appears to be a generic page not tied to a specific symbol." + else: signature = textwrap.shorten(signature, 500) - embed = discord.Embed( - title=f'`{symbol}`', - url=permalink, - description=f"```py\n{signature}```{description}" - ) + embed_description = f"```py\n{signature}```{description}" + + embed = discord.Embed( + title=f'`{symbol}`', + url=permalink, + description=embed_description + ) # show all symbols with the same name that were renamed in the footer embed.set_footer(text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} if renamed.endswith(f".{symbol}")) -- cgit v1.2.3 From 09f5cd78142201ff0133a25ee1ea6cff1c739e1f Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 21 Oct 2019 22:11:20 +0200 Subject: Grammar check comment --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 0c370f665..8b81b3053 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -306,7 +306,7 @@ class Doc(commands.Cog): url=permalink, description=embed_description ) - # show all symbols with the same name that were renamed in the footer + # Show all symbols with the same name that were renamed in the footer. embed.set_footer(text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} if renamed.endswith(f".{symbol}")) ) -- cgit v1.2.3 From 1c0310c398664bc7565232494f5547b35d05c648 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 21 Oct 2019 13:16:53 -0700 Subject: Moderation: create a class to handle scheduling of infractions The class handles application, pardoning, and expiration of infractions. This will allow other cogs, such as superstarify, to later subclass it to receive the same functionality that the infractions cog had. --- bot/cogs/moderation/infractions.py | 357 +---------------------------------- bot/cogs/moderation/scheduler.py | 368 +++++++++++++++++++++++++++++++++++++ 2 files changed, 372 insertions(+), 353 deletions(-) create mode 100644 bot/cogs/moderation/scheduler.py diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 997ffe524..9d65dfa8a 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -1,8 +1,6 @@ import logging -import textwrap import typing as t from datetime import datetime -from gettext import ngettext import dateutil.parser import discord @@ -11,14 +9,11 @@ from discord.ext import commands from discord.ext.commands import Context, command from bot import constants -from bot.api import ResponseCodeError -from bot.constants import Colours, Event, STAFF_CHANNELS +from bot.constants import Event from bot.decorators import respect_role_hierarchy -from bot.utils import time from bot.utils.checks import with_role_check -from bot.utils.scheduling import Scheduler from . import utils -from .modlog import ModLog +from .scheduler import InfractionScheduler from .utils import MemberObject log = logging.getLogger(__name__) @@ -26,38 +21,18 @@ log = logging.getLogger(__name__) MemberConverter = t.Union[utils.UserTypes, utils.proxy_user] -class Infractions(Scheduler, commands.Cog): +class Infractions(InfractionScheduler, commands.Cog): """Apply and pardon infractions on users for moderation purposes.""" category = "Moderation" category_description = "Server moderation tools." def __init__(self, bot: commands.Bot): - super().__init__() + super().__init__(bot) - self.bot = bot self.category = "Moderation" self._muted_role = discord.Object(constants.Roles.muted) - self.bot.loop.create_task(self.reschedule_infractions()) - - @property - def mod_log(self) -> ModLog: - """Get currently loaded ModLog cog instance.""" - return self.bot.get_cog("ModLog") - - async def reschedule_infractions(self) -> None: - """Schedule expiration for previous infractions.""" - await self.bot.wait_until_ready() - - infractions = await self.bot.api_client.get( - 'bot/infractions', - params={'active': 'true'} - ) - for infraction in infractions: - if infraction["expires_at"] is not None: - self.schedule_task(self.bot.loop, infraction["id"], infraction) - @commands.Cog.listener() async def on_member_join(self, member: Member) -> None: """Reapply active mute infractions for returning members.""" @@ -277,330 +252,6 @@ class Infractions(Scheduler, commands.Cog): action = ctx.guild.ban(user, reason=reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) - # endregion - # region: Utility functions - - async def _scheduled_task(self, infraction: utils.Infraction) -> None: - """ - Marks an infraction expired after the delay from time of scheduling to time of expiration. - - At the time of expiration, the infraction is marked as inactive on the website and the - expiration task is cancelled. - """ - _id = infraction["id"] - - expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) - await time.wait_until(expiry) - - log.debug(f"Marking infraction {_id} as inactive (expired).") - await self.deactivate_infraction(infraction) - - async def deactivate_infraction( - self, - infraction: utils.Infraction, - send_log: bool = True - ) -> t.Dict[str, str]: - """ - Deactivate an active infraction and return a dictionary of lines to send in a mod log. - - The infraction is removed from Discord, marked as inactive in the database, and has its - expiration task cancelled. If `send_log` is True, a mod log is sent for the - deactivation of the infraction. - - Supported infraction types are mute and ban. Other types will raise a ValueError. - """ - guild = self.bot.get_guild(constants.Guild.id) - mod_role = guild.get_role(constants.Roles.moderator) - user_id = infraction["user"] - _type = infraction["type"] - _id = infraction["id"] - reason = f"Infraction #{_id} expired or was pardoned." - - log.debug(f"Marking infraction #{_id} as inactive (expired).") - - log_content = None - log_text = { - "Member": str(user_id), - "Actor": str(self.bot.user), - "Reason": infraction["reason"] - } - - try: - if _type == "mute": - user = guild.get_member(user_id) - if user: - # Remove the muted role. - self.mod_log.ignore(Event.member_update, user.id) - await user.remove_roles(self._muted_role, reason=reason) - - # DM the user about the expiration. - notified = await utils.notify_pardon( - user=user, - title="You have been unmuted.", - content="You may now send messages in the server.", - icon_url=utils.INFRACTION_ICONS["mute"][1] - ) - - log_text["Member"] = f"{user.mention}(`{user.id}`)" - log_text["DM"] = "Sent" if notified else "**Failed**" - else: - log.info(f"Failed to unmute user {user_id}: user not found") - log_text["Failure"] = "User was not found in the guild." - elif _type == "ban": - user = discord.Object(user_id) - self.mod_log.ignore(Event.member_unban, user_id) - try: - await guild.unban(user, reason=reason) - except discord.NotFound: - log.info(f"Failed to unban user {user_id}: no active ban found on Discord") - log_text["Note"] = "No active ban found on Discord." - else: - raise ValueError( - f"Attempted to deactivate an unsupported infraction #{_id} ({_type})!" - ) - except discord.Forbidden: - log.warning(f"Failed to deactivate infraction #{_id} ({_type}): bot lacks permissions") - log_text["Failure"] = f"The bot lacks permissions to do this (role hierarchy?)" - log_content = mod_role.mention - except discord.HTTPException as e: - log.exception(f"Failed to deactivate infraction #{_id} ({_type})") - log_text["Failure"] = f"HTTPException with code {e.code}." - log_content = mod_role.mention - - # Check if the user is currently being watched by Big Brother. - try: - active_watch = await self.bot.api_client.get( - "bot/infractions", - params={ - "active": "true", - "type": "watch", - "user__id": user_id - } - ) - - log_text["Watching"] = "Yes" if active_watch else "No" - except ResponseCodeError: - log.exception(f"Failed to fetch watch status for user {user_id}") - log_text["Watching"] = "Unknown - failed to fetch watch status." - - try: - # Mark infraction as inactive in the database. - await self.bot.api_client.patch( - f"bot/infractions/{_id}", - json={"active": False} - ) - except ResponseCodeError as e: - log.exception(f"Failed to deactivate infraction #{_id} ({_type})") - log_line = f"API request failed with code {e.status}." - log_content = mod_role.mention - - # Append to an existing failure message if possible - if "Failure" in log_text: - log_text["Failure"] += f" {log_line}" - else: - log_text["Failure"] = log_line - - # Cancel the expiration task. - if infraction["expires_at"] is not None: - self.cancel_task(infraction["id"]) - - # Send a log message to the mod log. - if send_log: - log_title = f"expiration failed" if "Failure" in log_text else "expired" - - await self.mod_log.send_log_message( - icon_url=utils.INFRACTION_ICONS[_type][1], - colour=Colours.soft_green, - title=f"Infraction {log_title}: {_type}", - text="\n".join(f"{k}: {v}" for k, v in log_text.items()), - footer=f"ID: {_id}", - content=log_content, - ) - - return log_text - - async def apply_infraction( - self, - ctx: Context, - infraction: utils.Infraction, - user: MemberObject, - action_coro: t.Optional[t.Awaitable] = None - ) -> None: - """Apply an infraction to the user, log the infraction, and optionally notify the user.""" - infr_type = infraction["type"] - icon = utils.INFRACTION_ICONS[infr_type][0] - reason = infraction["reason"] - expiry = infraction["expires_at"] - - if expiry: - expiry = time.format_infraction(expiry) - - # Default values for the confirmation message and mod log. - confirm_msg = f":ok_hand: applied" - - # Specifying an expiry for a note or warning makes no sense. - if infr_type in ("note", "warning"): - expiry_msg = "" - else: - expiry_msg = f" until {expiry}" if expiry else " permanently" - - dm_result = "" - dm_log_text = "" - expiry_log_text = f"Expires: {expiry}" if expiry else "" - log_title = "applied" - log_content = None - - # DM the user about the infraction if it's not a shadow/hidden infraction. - if not infraction["hidden"]: - # Sometimes user is a discord.Object; make it a proper user. - await self.bot.fetch_user(user.id) - - # Accordingly display whether the user was successfully notified via DM. - if await utils.notify_infraction(user, infr_type, expiry, reason, icon): - dm_result = ":incoming_envelope: " - dm_log_text = "\nDM: Sent" - else: - dm_log_text = "\nDM: **Failed**" - log_content = ctx.author.mention - - if infraction["actor"] == self.bot.user.id: - end_msg = f" (reason: {infraction['reason']})" - elif ctx.channel.id not in STAFF_CHANNELS: - end_msg = '' - else: - infractions = await self.bot.api_client.get( - "bot/infractions", - params={"user__id": str(user.id)} - ) - total = len(infractions) - end_msg = f" ({total} infraction{ngettext('', 's', total)} total)" - - # Execute the necessary actions to apply the infraction on Discord. - if action_coro: - try: - await action_coro - if expiry: - # Schedule the expiration of the infraction. - self.schedule_task(ctx.bot.loop, infraction["id"], infraction) - except discord.Forbidden: - # Accordingly display that applying the infraction failed. - confirm_msg = f":x: failed to apply" - expiry_msg = "" - log_content = ctx.author.mention - log_title = "failed to apply" - - # Send a confirmation message to the invoking context. - await ctx.send( - f"{dm_result}{confirm_msg} **{infr_type}** to {user.mention}{expiry_msg}{end_msg}." - ) - - # Send a log message to the mod log. - await self.mod_log.send_log_message( - icon_url=icon, - colour=Colours.soft_red, - title=f"Infraction {log_title}: {infr_type}", - thumbnail=user.avatar_url_as(static_format="png"), - text=textwrap.dedent(f""" - Member: {user.mention} (`{user.id}`) - Actor: {ctx.message.author}{dm_log_text} - Reason: {reason} - {expiry_log_text} - """), - content=log_content, - footer=f"ID {infraction['id']}" - ) - - async def pardon_infraction(self, ctx: Context, infr_type: str, user: MemberObject) -> None: - """Prematurely end an infraction for a user and log the action in the mod log.""" - # Check the current active infraction - response = await self.bot.api_client.get( - 'bot/infractions', - params={ - 'active': 'true', - 'type': infr_type, - 'user__id': user.id - } - ) - - if not response: - await ctx.send(f":x: There's no active {infr_type} infraction for user {user.mention}.") - return - - # Deactivate the infraction and cancel its scheduled expiration task. - log_text = await self.deactivate_infraction(response[0], send_log=False) - - log_text["Member"] = f"{user.mention}(`{user.id}`)" - log_text["Actor"] = str(ctx.message.author) - log_content = None - footer = f"ID: {response[0]['id']}" - - # If multiple active infractions were found, mark them as inactive in the database - # and cancel their expiration tasks. - if len(response) > 1: - log.warning(f"Found more than one active {infr_type} infraction for user {user.id}") - - footer = f"Infraction IDs: {', '.join(str(infr['id']) for infr in response)}" - - log_note = f"Found multiple **active** {infr_type} infractions in the database." - if "Note" in log_text: - log_text["Note"] = f" {log_note}" - else: - log_text["Note"] = log_note - - # deactivate_infraction() is not called again because: - # 1. Discord cannot store multiple active bans or assign multiples of the same role - # 2. It would send a pardon DM for each active infraction, which is redundant - for infraction in response[1:]: - _id = infraction['id'] - try: - # Mark infraction as inactive in the database. - await self.bot.api_client.patch( - f"bot/infractions/{_id}", - json={"active": False} - ) - except ResponseCodeError: - log.exception(f"Failed to deactivate infraction #{_id} ({infr_type})") - # This is simpler and cleaner than trying to concatenate all the errors. - log_text["Failure"] = "See bot's logs for details." - - # Cancel pending expiration task. - if infraction["expires_at"] is not None: - self.cancel_task(infraction["id"]) - - # Accordingly display whether the user was successfully notified via DM. - dm_emoji = "" - if log_text.get("DM") == "Sent": - dm_emoji = ":incoming_envelope: " - elif "DM" in log_text: - # Mention the actor because the DM failed to send. - log_content = ctx.author.mention - - # Accordingly display whether the pardon failed. - if "Failure" in log_text: - confirm_msg = ":x: failed to pardon" - log_title = "pardon failed" - log_content = ctx.author.mention - else: - confirm_msg = f":ok_hand: pardoned" - log_title = "pardoned" - - # Send a confirmation message to the invoking context. - await ctx.send( - f"{dm_emoji}{confirm_msg} infraction **{infr_type}** for {user.mention}. " - f"{log_text.get('Failure', '')}" - ) - - # Send a log message to the mod log. - await self.mod_log.send_log_message( - icon_url=utils.INFRACTION_ICONS[infr_type][1], - colour=Colours.soft_green, - title=f"Infraction {log_title}: {infr_type}", - thumbnail=user.avatar_url_as(static_format="png"), - text="\n".join(f"{k}: {v}" for k, v in log_text.items()), - footer=footer, - content=log_content, - ) - # endregion # This cannot be static (must have a __func__ attribute). diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py new file mode 100644 index 000000000..408e9943e --- /dev/null +++ b/bot/cogs/moderation/scheduler.py @@ -0,0 +1,368 @@ +import logging +import textwrap +import typing as t +from gettext import ngettext + +import dateutil.parser +import discord +from discord.ext import commands +from discord.ext.commands import Context + +from bot import constants +from bot.api import ResponseCodeError +from bot.constants import Colours, Event, STAFF_CHANNELS +from bot.utils import time +from bot.utils.scheduling import Scheduler +from . import utils +from .modlog import ModLog +from .utils import MemberObject + +log = logging.getLogger(__name__) + + +class InfractionScheduler(Scheduler): + """Handles the application, pardoning, and expiration of infractions.""" + + def __init__(self, bot: commands.Bot): + super().__init__() + + self.bot = bot + self.bot.loop.create_task(self.reschedule_infractions()) + + @property + def mod_log(self) -> ModLog: + """Get the currently loaded ModLog cog instance.""" + return self.bot.get_cog("ModLog") + + async def reschedule_infractions(self) -> None: + """Schedule expiration for previous infractions.""" + await self.bot.wait_until_ready() + + infractions = await self.bot.api_client.get( + 'bot/infractions', + params={'active': 'true'} + ) + for infraction in infractions: + if infraction["expires_at"] is not None: + self.schedule_task(self.bot.loop, infraction["id"], infraction) + + async def apply_infraction( + self, + ctx: Context, + infraction: utils.Infraction, + user: MemberObject, + action_coro: t.Optional[t.Awaitable] = None + ) -> None: + """Apply an infraction to the user, log the infraction, and optionally notify the user.""" + infr_type = infraction["type"] + icon = utils.INFRACTION_ICONS[infr_type][0] + reason = infraction["reason"] + expiry = infraction["expires_at"] + + if expiry: + expiry = time.format_infraction(expiry) + + # Default values for the confirmation message and mod log. + confirm_msg = f":ok_hand: applied" + + # Specifying an expiry for a note or warning makes no sense. + if infr_type in ("note", "warning"): + expiry_msg = "" + else: + expiry_msg = f" until {expiry}" if expiry else " permanently" + + dm_result = "" + dm_log_text = "" + expiry_log_text = f"Expires: {expiry}" if expiry else "" + log_title = "applied" + log_content = None + + # DM the user about the infraction if it's not a shadow/hidden infraction. + if not infraction["hidden"]: + # Sometimes user is a discord.Object; make it a proper user. + await self.bot.fetch_user(user.id) + + # Accordingly display whether the user was successfully notified via DM. + if await utils.notify_infraction(user, infr_type, expiry, reason, icon): + dm_result = ":incoming_envelope: " + dm_log_text = "\nDM: Sent" + else: + dm_log_text = "\nDM: **Failed**" + log_content = ctx.author.mention + + if infraction["actor"] == self.bot.user.id: + end_msg = f" (reason: {infraction['reason']})" + elif ctx.channel.id not in STAFF_CHANNELS: + end_msg = "" + else: + infractions = await self.bot.api_client.get( + "bot/infractions", + params={"user__id": str(user.id)} + ) + total = len(infractions) + end_msg = f" ({total} infraction{ngettext('', 's', total)} total)" + + # Execute the necessary actions to apply the infraction on Discord. + if action_coro: + try: + await action_coro + if expiry: + # Schedule the expiration of the infraction. + self.schedule_task(ctx.bot.loop, infraction["id"], infraction) + except discord.Forbidden: + # Accordingly display that applying the infraction failed. + confirm_msg = f":x: failed to apply" + expiry_msg = "" + log_content = ctx.author.mention + log_title = "failed to apply" + + # Send a confirmation message to the invoking context. + await ctx.send( + f"{dm_result}{confirm_msg} **{infr_type}** to {user.mention}{expiry_msg}{end_msg}." + ) + + # Send a log message to the mod log. + await self.mod_log.send_log_message( + icon_url=icon, + colour=Colours.soft_red, + title=f"Infraction {log_title}: {infr_type}", + thumbnail=user.avatar_url_as(static_format="png"), + text=textwrap.dedent(f""" + Member: {user.mention} (`{user.id}`) + Actor: {ctx.message.author}{dm_log_text} + Reason: {reason} + {expiry_log_text} + """), + content=log_content, + footer=f"ID {infraction['id']}" + ) + + async def pardon_infraction(self, ctx: Context, infr_type: str, user: MemberObject) -> None: + """Prematurely end an infraction for a user and log the action in the mod log.""" + # Check the current active infraction + response = await self.bot.api_client.get( + 'bot/infractions', + params={ + 'active': 'true', + 'type': infr_type, + 'user__id': user.id + } + ) + + if not response: + await ctx.send(f":x: There's no active {infr_type} infraction for user {user.mention}.") + return + + # Deactivate the infraction and cancel its scheduled expiration task. + log_text = await self.deactivate_infraction(response[0], send_log=False) + + log_text["Member"] = f"{user.mention}(`{user.id}`)" + log_text["Actor"] = str(ctx.message.author) + log_content = None + footer = f"ID: {response[0]['id']}" + + # If multiple active infractions were found, mark them as inactive in the database + # and cancel their expiration tasks. + if len(response) > 1: + log.warning(f"Found more than one active {infr_type} infraction for user {user.id}") + + footer = f"Infraction IDs: {', '.join(str(infr['id']) for infr in response)}" + + log_note = f"Found multiple **active** {infr_type} infractions in the database." + if "Note" in log_text: + log_text["Note"] = f" {log_note}" + else: + log_text["Note"] = log_note + + # deactivate_infraction() is not called again because: + # 1. Discord cannot store multiple active bans or assign multiples of the same role + # 2. It would send a pardon DM for each active infraction, which is redundant + for infraction in response[1:]: + _id = infraction['id'] + try: + # Mark infraction as inactive in the database. + await self.bot.api_client.patch( + f"bot/infractions/{_id}", + json={"active": False} + ) + except ResponseCodeError: + log.exception(f"Failed to deactivate infraction #{_id} ({infr_type})") + # This is simpler and cleaner than trying to concatenate all the errors. + log_text["Failure"] = "See bot's logs for details." + + # Cancel pending expiration task. + if infraction["expires_at"] is not None: + self.cancel_task(infraction["id"]) + + # Accordingly display whether the user was successfully notified via DM. + dm_emoji = "" + if log_text.get("DM") == "Sent": + dm_emoji = ":incoming_envelope: " + elif "DM" in log_text: + # Mention the actor because the DM failed to send. + log_content = ctx.author.mention + + # Accordingly display whether the pardon failed. + if "Failure" in log_text: + confirm_msg = ":x: failed to pardon" + log_title = "pardon failed" + log_content = ctx.author.mention + else: + confirm_msg = f":ok_hand: pardoned" + log_title = "pardoned" + + # Send a confirmation message to the invoking context. + await ctx.send( + f"{dm_emoji}{confirm_msg} infraction **{infr_type}** for {user.mention}. " + f"{log_text.get('Failure', '')}" + ) + + # Send a log message to the mod log. + await self.mod_log.send_log_message( + icon_url=utils.INFRACTION_ICONS[infr_type][1], + colour=Colours.soft_green, + title=f"Infraction {log_title}: {infr_type}", + thumbnail=user.avatar_url_as(static_format="png"), + text="\n".join(f"{k}: {v}" for k, v in log_text.items()), + footer=footer, + content=log_content, + ) + + async def deactivate_infraction( + self, + infraction: utils.Infraction, + send_log: bool = True + ) -> t.Dict[str, str]: + """ + Deactivate an active infraction and return a dictionary of lines to send in a mod log. + + The infraction is removed from Discord, marked as inactive in the database, and has its + expiration task cancelled. If `send_log` is True, a mod log is sent for the + deactivation of the infraction. + + Supported infraction types are mute and ban. Other types will raise a ValueError. + """ + guild = self.bot.get_guild(constants.Guild.id) + mod_role = guild.get_role(constants.Roles.moderator) + user_id = infraction["user"] + _type = infraction["type"] + _id = infraction["id"] + reason = f"Infraction #{_id} expired or was pardoned." + + log.debug(f"Marking infraction #{_id} as inactive (expired).") + + log_content = None + log_text = { + "Member": str(user_id), + "Actor": str(self.bot.user), + "Reason": infraction["reason"] + } + + try: + if _type == "mute": + user = guild.get_member(user_id) + if user: + # Remove the muted role. + self.mod_log.ignore(Event.member_update, user.id) + await user.remove_roles(self._muted_role, reason=reason) + + # DM the user about the expiration. + notified = await utils.notify_pardon( + user=user, + title="You have been unmuted.", + content="You may now send messages in the server.", + icon_url=utils.INFRACTION_ICONS["mute"][1] + ) + + log_text["Member"] = f"{user.mention}(`{user.id}`)" + log_text["DM"] = "Sent" if notified else "**Failed**" + else: + log.info(f"Failed to unmute user {user_id}: user not found") + log_text["Failure"] = "User was not found in the guild." + elif _type == "ban": + user = discord.Object(user_id) + self.mod_log.ignore(Event.member_unban, user_id) + try: + await guild.unban(user, reason=reason) + except discord.NotFound: + log.info(f"Failed to unban user {user_id}: no active ban found on Discord") + log_text["Note"] = "No active ban found on Discord." + else: + raise ValueError( + f"Attempted to deactivate an unsupported infraction #{_id} ({_type})!" + ) + except discord.Forbidden: + log.warning(f"Failed to deactivate infraction #{_id} ({_type}): bot lacks permissions") + log_text["Failure"] = f"The bot lacks permissions to do this (role hierarchy?)" + log_content = mod_role.mention + except discord.HTTPException as e: + log.exception(f"Failed to deactivate infraction #{_id} ({_type})") + log_text["Failure"] = f"HTTPException with code {e.code}." + log_content = mod_role.mention + + # Check if the user is currently being watched by Big Brother. + try: + active_watch = await self.bot.api_client.get( + "bot/infractions", + params={ + "active": "true", + "type": "watch", + "user__id": user_id + } + ) + + log_text["Watching"] = "Yes" if active_watch else "No" + except ResponseCodeError: + log.exception(f"Failed to fetch watch status for user {user_id}") + log_text["Watching"] = "Unknown - failed to fetch watch status." + + try: + # Mark infraction as inactive in the database. + await self.bot.api_client.patch( + f"bot/infractions/{_id}", + json={"active": False} + ) + except ResponseCodeError as e: + log.exception(f"Failed to deactivate infraction #{_id} ({_type})") + log_line = f"API request failed with code {e.status}." + log_content = mod_role.mention + + # Append to an existing failure message if possible + if "Failure" in log_text: + log_text["Failure"] += f" {log_line}" + else: + log_text["Failure"] = log_line + + # Cancel the expiration task. + if infraction["expires_at"] is not None: + self.cancel_task(infraction["id"]) + + # Send a log message to the mod log. + if send_log: + log_title = f"expiration failed" if "Failure" in log_text else "expired" + + await self.mod_log.send_log_message( + icon_url=utils.INFRACTION_ICONS[_type][1], + colour=Colours.soft_green, + title=f"Infraction {log_title}: {_type}", + text="\n".join(f"{k}: {v}" for k, v in log_text.items()), + footer=f"ID: {_id}", + content=log_content, + ) + + return log_text + + async def _scheduled_task(self, infraction: utils.Infraction) -> None: + """ + Marks an infraction expired after the delay from time of scheduling to time of expiration. + + At the time of expiration, the infraction is marked as inactive on the website and the + expiration task is cancelled. + """ + _id = infraction["id"] + + expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) + await time.wait_until(expiry) + + log.debug(f"Marking infraction {_id} as inactive (expired).") + await self.deactivate_infraction(infraction) -- cgit v1.2.3 From 832c7129d57a919f15919e883607845df6b6dacc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 21 Oct 2019 17:33:27 -0700 Subject: InfractionScheduler: delegate type-specific pardon code to subclasses * Create an abstract method for performing type-specific infraction deactivation * Move infraction-specific pardon code to separate functions --- bot/cogs/moderation/infractions.py | 61 +++++++++++++++++++++++++++++++++++++- bot/cogs/moderation/scheduler.py | 51 +++++++++++-------------------- 2 files changed, 77 insertions(+), 35 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 9d65dfa8a..fbfcb8ae3 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -209,7 +209,7 @@ class Infractions(InfractionScheduler, commands.Cog): await self.pardon_infraction(ctx, "ban", user) # endregion - # region: Base infraction functions + # region: Base apply functions async def apply_mute(self, ctx: Context, user: Member, reason: str, **kwargs) -> None: """Apply a mute infraction with kwargs passed to `post_infraction`.""" @@ -252,6 +252,65 @@ class Infractions(InfractionScheduler, commands.Cog): action = ctx.guild.ban(user, reason=reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) + # endregion + # region: Base pardon functions + + async def pardon_mute(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + """Remove a user's muted role, DM them a notification, and return a log dict.""" + user = guild.get_member(user_id) + log_text = {} + + if user: + # Remove the muted role. + self.mod_log.ignore(Event.member_update, user.id) + await user.remove_roles(self._muted_role, reason=reason) + + # DM the user about the expiration. + notified = await utils.notify_pardon( + user=user, + title="You have been unmuted.", + content="You may now send messages in the server.", + icon_url=utils.INFRACTION_ICONS["mute"][1] + ) + + log_text["Member"] = f"{user.mention}(`{user.id}`)" + log_text["DM"] = "Sent" if notified else "**Failed**" + else: + log.info(f"Failed to unmute user {user_id}: user not found") + log_text["Failure"] = "User was not found in the guild." + + return log_text + + async def pardon_ban(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + """Remove a user's ban on the Discord guild and return a log dict.""" + user = discord.Object(user_id) + log_text = {} + + self.mod_log.ignore(Event.member_unban, user_id) + + try: + await guild.unban(user, reason=reason) + except discord.NotFound: + log.info(f"Failed to unban user {user_id}: no active ban found on Discord") + log_text["Note"] = "No active ban found on Discord." + + return log_text + + async def _pardon_action(self, infraction: utils.Infraction) -> t.Optional[t.Dict[str, str]]: + """ + Execute deactivation steps specific to the infraction's type and return a log dict. + + If an infraction type is unsupported, return None instead. + """ + guild = self.bot.get_guild(constants.Guild.id) + user_id = infraction["user"] + reason = f"Infraction #{infraction['id']} expired or was pardoned." + + if infraction["type"] == "mute": + return await self.pardon_mute(user_id, guild, reason) + elif infraction["type"] == "ban": + return await self.pardon_ban(user_id, guild, reason) + # endregion # This cannot be static (must have a __func__ attribute). diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 408e9943e..045b0bd13 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -1,16 +1,16 @@ import logging import textwrap import typing as t +from abc import abstractmethod from gettext import ngettext import dateutil.parser import discord -from discord.ext import commands -from discord.ext.commands import Context +from discord.ext.commands import Bot, Context from bot import constants from bot.api import ResponseCodeError -from bot.constants import Colours, Event, STAFF_CHANNELS +from bot.constants import Colours, STAFF_CHANNELS from bot.utils import time from bot.utils.scheduling import Scheduler from . import utils @@ -23,7 +23,7 @@ log = logging.getLogger(__name__) class InfractionScheduler(Scheduler): """Handles the application, pardoning, and expiration of infractions.""" - def __init__(self, bot: commands.Bot): + def __init__(self, bot: Bot): super().__init__() self.bot = bot @@ -240,14 +240,13 @@ class InfractionScheduler(Scheduler): expiration task cancelled. If `send_log` is True, a mod log is sent for the deactivation of the infraction. - Supported infraction types are mute and ban. Other types will raise a ValueError. + Infractions of unsupported types will raise a ValueError. """ guild = self.bot.get_guild(constants.Guild.id) mod_role = guild.get_role(constants.Roles.moderator) user_id = infraction["user"] _type = infraction["type"] _id = infraction["id"] - reason = f"Infraction #{_id} expired or was pardoned." log.debug(f"Marking infraction #{_id} as inactive (expired).") @@ -259,34 +258,9 @@ class InfractionScheduler(Scheduler): } try: - if _type == "mute": - user = guild.get_member(user_id) - if user: - # Remove the muted role. - self.mod_log.ignore(Event.member_update, user.id) - await user.remove_roles(self._muted_role, reason=reason) - - # DM the user about the expiration. - notified = await utils.notify_pardon( - user=user, - title="You have been unmuted.", - content="You may now send messages in the server.", - icon_url=utils.INFRACTION_ICONS["mute"][1] - ) - - log_text["Member"] = f"{user.mention}(`{user.id}`)" - log_text["DM"] = "Sent" if notified else "**Failed**" - else: - log.info(f"Failed to unmute user {user_id}: user not found") - log_text["Failure"] = "User was not found in the guild." - elif _type == "ban": - user = discord.Object(user_id) - self.mod_log.ignore(Event.member_unban, user_id) - try: - await guild.unban(user, reason=reason) - except discord.NotFound: - log.info(f"Failed to unban user {user_id}: no active ban found on Discord") - log_text["Note"] = "No active ban found on Discord." + returned_log = await self._pardon_action(infraction) + if returned_log is not None: + log_text = {**log_text, **returned_log} # Merge the logs together else: raise ValueError( f"Attempted to deactivate an unsupported infraction #{_id} ({_type})!" @@ -352,6 +326,15 @@ class InfractionScheduler(Scheduler): return log_text + @abstractmethod + async def _pardon_action(self, infraction: utils.Infraction) -> t.Optional[t.Dict[str, str]]: + """ + Execute deactivation steps specific to the infraction's type and return a log dict. + + If an infraction type is unsupported, return None instead. + """ + raise NotImplementedError + async def _scheduled_task(self, infraction: utils.Infraction) -> None: """ Marks an infraction expired after the delay from time of scheduling to time of expiration. -- cgit v1.2.3 From c1ab8ecfb6992f175351b7c921fabada9ce520d2 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 23 Oct 2019 14:57:59 -0700 Subject: Superstarify: add icons to constants and config --- bot/cogs/moderation/utils.py | 7 ++++--- bot/constants.py | 3 +++ config-default.yml | 3 +++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 788a40d40..c05cb69a1 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -15,11 +15,12 @@ log = logging.getLogger(__name__) # apply icon, pardon icon INFRACTION_ICONS = { - "mute": (Icons.user_mute, Icons.user_unmute), - "kick": (Icons.sign_out, None), "ban": (Icons.user_ban, Icons.user_unban), - "warning": (Icons.user_warn, None), + "kick": (Icons.sign_out, None), + "mute": (Icons.user_mute, Icons.user_unmute), "note": (Icons.user_warn, None), + "superstar": (Icons.superstarify, Icons.unsuperstarify), + "warning": (Icons.user_warn, None), } RULES_URL = "https://pythondiscord.com/pages/rules" APPEALABLE_INFRACTIONS = ("ban", "mute") diff --git a/bot/constants.py b/bot/constants.py index 838fe7a79..ad68919ea 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -308,6 +308,9 @@ class Icons(metaclass=YAMLGetter): questionmark: str + superstarify: str + unsuperstarify: str + class CleanMessages(metaclass=YAMLGetter): section = "bot" diff --git a/config-default.yml b/config-default.yml index 4638a89ee..8d7481833 100644 --- a/config-default.yml +++ b/config-default.yml @@ -82,6 +82,9 @@ style: questionmark: "https://cdn.discordapp.com/emojis/512367613339369475.png" + superstarify: "https://cdn.discordapp.com/emojis/636288153044516874.png" + unsuperstarify: "https://cdn.discordapp.com/emojis/636288201258172446.png" + guild: id: 267624335836053506 -- cgit v1.2.3 From 6ba259ab5a39cf4257084f5458b93ab02fbfa8fa Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 23 Oct 2019 17:43:08 -0700 Subject: InfractionScheduler: use fetched user for notify_infraction --- bot/cogs/moderation/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 045b0bd13..cef96de99 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -80,7 +80,7 @@ class InfractionScheduler(Scheduler): # DM the user about the infraction if it's not a shadow/hidden infraction. if not infraction["hidden"]: # Sometimes user is a discord.Object; make it a proper user. - await self.bot.fetch_user(user.id) + user = await self.bot.fetch_user(user.id) # Accordingly display whether the user was successfully notified via DM. if await utils.notify_infraction(user, infr_type, expiry, reason, icon): -- cgit v1.2.3 From 3cf0dbf59cfb58fe010fd07b0d5a7e4acedcca0c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 24 Oct 2019 22:27:58 -0700 Subject: Superstarify: schedule infractions by subclassing InfractionScheduler --- bot/cogs/moderation/superstarify.py | 67 ++++++++++++++----------------------- 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 82f8621fc..ed2412c35 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -1,17 +1,18 @@ import json import logging import random +import typing as t from pathlib import Path from discord import Colour, Embed, Member from discord.errors import Forbidden -from discord.ext.commands import Bot, Cog, Context, command +from discord.ext.commands import Cog, Context, command from bot import constants from bot.utils.checks import with_role_check from bot.utils.time import format_infraction from . import utils -from .modlog import ModLog +from .scheduler import InfractionScheduler log = logging.getLogger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" @@ -20,17 +21,9 @@ with Path("bot/resources/stars.json").open(encoding="utf-8") as stars_file: STAR_NAMES = json.load(stars_file) -class Superstarify(Cog): +class Superstarify(InfractionScheduler, Cog): """A set of commands to moderate terrible nicknames.""" - def __init__(self, bot: Bot): - self.bot = bot - - @property - def modlog(self) -> ModLog: - """Get currently loaded ModLog cog instance.""" - return self.bot.get_cog("ModLog") - @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """ @@ -189,7 +182,7 @@ class Superstarify(Cog): f"New nickname: `{forced_nick}`\n" f"Superstardom ends: **{expiry_str}**" ) - await self.modlog.send_log_message( + await self.mod_log.send_log_message( icon_url=constants.Icons.user_update, colour=Colour.gold(), title="Member Achieved Superstardom", @@ -207,45 +200,35 @@ class Superstarify(Cog): # Change the nick and return the embed log.trace("Changing the users nickname and sending the embed.") await member.edit(nick=forced_nick) + self.schedule_task(ctx.bot.loop, infraction["id"], infraction) await ctx.send(embed=embed) @command(name='unsuperstarify', aliases=('release_nick', 'unstar')) async def unsuperstarify(self, ctx: Context, member: Member) -> None: - """Remove the superstarify entry from our database, allowing the user to change their nickname.""" - log.debug(f"Attempting to unsuperstarify the following user: {member.display_name}") - - embed = Embed() - embed.colour = Colour.blurple() - - active_superstarifies = await self.bot.api_client.get( - 'bot/infractions', - params={ - 'active': 'true', - 'type': 'superstar', - 'user__id': str(member.id) - } - ) - if not active_superstarifies: - await ctx.send(":x: There is no active superstarify infraction for this user.") - return + """Remove the superstarify infraction and allow the user to change their nickname.""" + await self.pardon_infraction(ctx, "superstar", member) - [infraction] = active_superstarifies - await self.bot.api_client.patch( - 'bot/infractions/' + str(infraction['id']), - json={'active': False} - ) + async def _pardon_action(self, infraction: utils.Infraction) -> t.Optional[t.Dict[str, str]]: + """Pardon a superstar infraction and return a log dict.""" + guild = self.bot.get_guild(constants.Guild.id) + user = guild.get_member(infraction["user"]) - embed = Embed() - embed.description = "User has been released from superstar-prison." - embed.title = random.choice(constants.POSITIVE_REPLIES) + # Don't bother sending a notification if the user left the guild. + if infraction["type"] != "mute" or not user: + return {} - await utils.notify_pardon( - user=member, + # DM the user about the expiration. + notified = await utils.notify_pardon( + user=user, title="You are no longer superstarified.", - content="You may now change your nickname on the server." + content="You may now change your nickname on the server.", + icon_url=utils.INFRACTION_ICONS["superstar"][1] ) - log.trace(f"{member.display_name} was successfully released from superstar-prison.") - await ctx.send(embed=embed) + + return { + "Member": f"{user.mention}(`{user.id}`)", + "DM": "Sent" if notified else "**Failed**" + } @staticmethod def get_nick(infraction_id: int, member_id: int) -> str: -- cgit v1.2.3 From d922892204f20fe4936684880cb744c0dcfdfa8f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 24 Oct 2019 22:30:39 -0700 Subject: Superstarify: refactor superstarify command * Edit the nickname and schedule the infraction before sending messages * Reformat mod log description to be consistent with other infractions * Use INFRACTION_ICONS constant --- bot/cogs/moderation/superstarify.py | 64 ++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index ed2412c35..051eaf6b5 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -1,6 +1,7 @@ import json import logging import random +import textwrap import typing as t from pathlib import Path @@ -158,51 +159,56 @@ class Superstarify(InfractionScheduler, Cog): if await utils.has_active_infraction(ctx, member, "superstar"): return - reason = reason or ('old nick: ' + member.display_name) - infraction = await utils.post_infraction(ctx, member, 'superstar', reason, expires_at=duration) - forced_nick = self.get_nick(infraction['id'], member.id) + # Post the infraction to the API + reason = reason or f"old nick: {member.display_name}" + infraction = await utils.post_infraction(ctx, member, "superstar", reason, expires_at=duration) + + forced_nick = self.get_nick(infraction["id"], member.id) expiry_str = format_infraction(infraction["expires_at"]) + # Apply the infraction and schedule the expiration task. + await member.edit(nick=forced_nick, reason=reason) + self.schedule_task(ctx.bot.loop, infraction["id"], infraction) + + # Send a DM to the user to notify them of their new infraction. + await utils.notify_infraction( + user=member, + infr_type="Superstarify", + expires_at=expiry_str, + reason=f"Your nickname didn't comply with our [nickname policy]({NICKNAME_POLICY_URL}).", + icon_url=utils.INFRACTION_ICONS["superstar"][0] + ) + + # Send an embed with the infraction information to the invoking context. embed = Embed() embed.title = "Congratulations!" embed.description = ( - f"Your previous nickname, **{member.display_name}**, was so bad that we have decided to change it. " + f"Your previous nickname, **{member.display_name}**, " + f"was so bad that we have decided to change it. " f"Your new nickname will be **{forced_nick}**.\n\n" f"You will be unable to change your nickname until \n**{expiry_str}**.\n\n" "If you're confused by this, please read our " f"[official nickname policy]({NICKNAME_POLICY_URL})." ) + await ctx.send(embed=embed) - # Log to the mod_log channel - log.trace("Logging to the #mod-log channel. This could fail because of channel permissions.") - mod_log_message = ( - f"**{member}** (`{member.id}`)\n\n" - f"Superstarified by **{ctx.author.name}**\n" - f"Old nickname: `{member.display_name}`\n" - f"New nickname: `{forced_nick}`\n" - f"Superstardom ends: **{expiry_str}**" - ) + # Log to the mod log channel. await self.mod_log.send_log_message( - icon_url=constants.Icons.user_update, + icon_url=utils.INFRACTION_ICONS["superstar"][0], colour=Colour.gold(), title="Member Achieved Superstardom", - text=mod_log_message, - thumbnail=member.avatar_url_as(static_format="png") - ) - - await utils.notify_infraction( - user=member, - infr_type="Superstarify", - expires_at=expiry_str, - reason=f"Your nickname didn't comply with our [nickname policy]({NICKNAME_POLICY_URL})." + thumbnail=member.avatar_url_as(static_format="png"), + text=textwrap.dedent(f""" + Member: {member.mentiom} (`{member.id}`) + Actor: {ctx.message.author} + Reason: {reason} + Expires: {expiry_str} + Old nickname: `{member.display_name}` + New nickname: `{forced_nick}` + """), + footer=f"ID {infraction['id']}" ) - # Change the nick and return the embed - log.trace("Changing the users nickname and sending the embed.") - await member.edit(nick=forced_nick) - self.schedule_task(ctx.bot.loop, infraction["id"], infraction) - await ctx.send(embed=embed) - @command(name='unsuperstarify', aliases=('release_nick', 'unstar')) async def unsuperstarify(self, ctx: Context, member: Member) -> None: """Remove the superstarify infraction and allow the user to change their nickname.""" -- cgit v1.2.3 From 91e03a8ae03e90a50afb7cfee3cfb2a963365b30 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 24 Oct 2019 23:30:35 -0700 Subject: InfractionScheduler: create generic function for reapplying infractions reapply_infraction() is a generic function which reapplies a given infraction if it is still active or deactivates it if less than 60 seconds remain. Most of the code from Infractions.on_member_joined now lives in this new function. This simplifies the "fancier" DM and mod log that were previously sent for superstar in on_member_join. --- bot/cogs/moderation/infractions.py | 30 ++++++------------- bot/cogs/moderation/scheduler.py | 20 +++++++++++++ bot/cogs/moderation/superstarify.py | 57 +++++++------------------------------ 3 files changed, 39 insertions(+), 68 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index fbfcb8ae3..7e8d58144 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -1,8 +1,6 @@ import logging import typing as t -from datetime import datetime -import dateutil.parser import discord from discord import Member from discord.ext import commands @@ -37,31 +35,19 @@ class Infractions(InfractionScheduler, commands.Cog): async def on_member_join(self, member: Member) -> None: """Reapply active mute infractions for returning members.""" active_mutes = await self.bot.api_client.get( - 'bot/infractions', + "bot/infractions", params={ - 'user__id': str(member.id), - 'type': 'mute', - 'active': 'true' + "active": "true", + "type": "mute", + "user__id": member.id } ) - if not active_mutes: - return - - # Assume a single mute because of restrictions elsewhere. - mute = active_mutes[0] - # Calculate the time remaining, in seconds, for the mute. - expiry = dateutil.parser.isoparse(mute["expires_at"]).replace(tzinfo=None) - delta = (expiry - datetime.utcnow()).total_seconds() - - # Mark as inactive if less than a minute remains. - if delta < 60: - await self.deactivate_infraction(mute) - return + if active_mutes: + reason = f"Re-applying active mute: {active_mutes[0]['id']}" + action = member.add_roles(self._muted_role, reason=reason) - # Allowing mod log since this is a passive action that should be logged. - await member.add_roles(self._muted_role, reason=f"Re-applying active mute: {mute['id']}") - log.debug(f"User {member.id} has been re-muted on rejoin.") + await self.reapply_infraction(active_mutes[0], action) # region: Permanent infractions diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index cef96de99..fe8e43fbe 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -2,6 +2,7 @@ import logging import textwrap import typing as t from abc import abstractmethod +from datetime import datetime from gettext import ngettext import dateutil.parser @@ -46,6 +47,25 @@ class InfractionScheduler(Scheduler): if infraction["expires_at"] is not None: self.schedule_task(self.bot.loop, infraction["id"], infraction) + async def reapply_infraction( + self, + infraction: utils.Infraction, + apply_coro: t.Optional[t.Awaitable] + ) -> None: + """Reapply an infraction if it's still active or deactivate it if less than 60 sec left.""" + # Calculate the time remaining, in seconds, for the mute. + expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) + delta = (expiry - datetime.utcnow()).total_seconds() + + # Mark as inactive if less than a minute remains. + if delta < 60: + await self.deactivate_infraction(infraction) + return + + # Allowing mod log since this is a passive action that should be logged. + await apply_coro + log.info(f"Re-applied {infraction['type']} to user {infraction['user']} upon rejoining.") + async def apply_infraction( self, ctx: Context, diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 051eaf6b5..3200087ae 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -82,60 +82,25 @@ class Superstarify(InfractionScheduler, Cog): @Cog.listener() async def on_member_join(self, member: Member) -> None: - """ - This event will trigger when someone (re)joins the server. - - At this point we will look up the user in our database and check whether they are in - superstar-prison. If so, we will change their name back to the forced nickname. - """ + """Reapply active superstar infractions for returning members.""" active_superstarifies = await self.bot.api_client.get( - 'bot/infractions', + "bot/infractions", params={ - 'active': 'true', - 'type': 'superstar', - 'user__id': member.id + "active": "true", + "type": "superstar", + "user__id": member.id } ) if active_superstarifies: - [infraction] = active_superstarifies - forced_nick = self.get_nick(infraction['id'], member.id) - await member.edit(nick=forced_nick) - end_timestamp_human = format_infraction(infraction['expires_at']) - - try: - await member.send( - "You have left and rejoined the **Python Discord** server, effectively resetting " - f"your nickname from **{forced_nick}** to **{member.name}**, " - "but as you are currently in superstar-prison, you do not have permission to do so. " - "Therefore your nickname was automatically changed back. You will be allowed to " - "change your nickname again at the following time:\n\n" - f"**{end_timestamp_human}**." - ) - except Forbidden: - log.warning( - "The user left and rejoined the server while in superstar-prison. " - "This led to the bot trying to DM the user to let them know their name was restored, " - "but the user had either blocked the bot or disabled DMs, so it was not possible " - "to DM them, and a discord.errors.Forbidden error was incurred." - ) - - # Log to the mod_log channel - log.trace("Logging to the #mod-log channel. This could fail because of channel permissions.") - mod_log_message = ( - f"**{member}** (`{member.id}`)\n\n" - f"Superstarified member potentially tried to escape the prison.\n" - f"Restored enforced nickname: `{forced_nick}`\n" - f"Superstardom ends: **{end_timestamp_human}**" - ) - await self.modlog.send_log_message( - icon_url=constants.Icons.user_update, - colour=Colour.gold(), - title="Superstar member rejoined server", - text=mod_log_message, - thumbnail=member.avatar_url_as(static_format="png") + infraction = active_superstarifies[0] + action = member.edit( + nick=self.get_nick(infraction["id"], member.id), + reason=f"Superstarified member tried to escape the prison: {infraction['id']}" ) + await self.reapply_infraction(infraction, action) + @command(name='superstarify', aliases=('force_nick', 'star')) async def superstarify(self, ctx: Context, member: Member, duration: utils.Expiry, reason: str = None) -> None: """ -- cgit v1.2.3 From df48de7f6330e43782a9af5ff2e4ebee5c7ed9b1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 24 Oct 2019 23:51:33 -0700 Subject: InfractionScheduler: prevent duplicate rescheduling of infractions --- bot/cogs/moderation/infractions.py | 2 +- bot/cogs/moderation/scheduler.py | 8 ++++---- bot/cogs/moderation/superstarify.py | 5 ++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 7e8d58144..0e29b02f7 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -26,7 +26,7 @@ class Infractions(InfractionScheduler, commands.Cog): category_description = "Server moderation tools." def __init__(self, bot: commands.Bot): - super().__init__(bot) + super().__init__(bot, supported_infractions={"ban", "kick", "mute", "note", "warning"}) self.category = "Moderation" self._muted_role = discord.Object(constants.Roles.muted) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index fe8e43fbe..74ff62981 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -24,18 +24,18 @@ log = logging.getLogger(__name__) class InfractionScheduler(Scheduler): """Handles the application, pardoning, and expiration of infractions.""" - def __init__(self, bot: Bot): + def __init__(self, bot: Bot, supported_infractions: t.Container[str]): super().__init__() self.bot = bot - self.bot.loop.create_task(self.reschedule_infractions()) + self.bot.loop.create_task(self.reschedule_infractions(supported_infractions)) @property def mod_log(self) -> ModLog: """Get the currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") - async def reschedule_infractions(self) -> None: + async def reschedule_infractions(self, supported_infractions: t.Container[str]) -> None: """Schedule expiration for previous infractions.""" await self.bot.wait_until_ready() @@ -44,7 +44,7 @@ class InfractionScheduler(Scheduler): params={'active': 'true'} ) for infraction in infractions: - if infraction["expires_at"] is not None: + if infraction["expires_at"] is not None and infraction["type"] in supported_infractions: self.schedule_task(self.bot.loop, infraction["id"], infraction) async def reapply_infraction( diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 3200087ae..a534ec762 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -7,7 +7,7 @@ from pathlib import Path from discord import Colour, Embed, Member from discord.errors import Forbidden -from discord.ext.commands import Cog, Context, command +from discord.ext.commands import Bot, Cog, Context, command from bot import constants from bot.utils.checks import with_role_check @@ -25,6 +25,9 @@ with Path("bot/resources/stars.json").open(encoding="utf-8") as stars_file: class Superstarify(InfractionScheduler, Cog): """A set of commands to moderate terrible nicknames.""" + def __init__(self, bot: Bot): + super().__init__(bot, supported_infractions={"superstar"}) + @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """ -- cgit v1.2.3 From a8e85349f22435c5bdb12287f6f632f95ee9389b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 00:00:10 -0700 Subject: Superstarify: ignore member update event when applying --- bot/cogs/moderation/superstarify.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index a534ec762..06a68d89e 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -135,6 +135,7 @@ class Superstarify(InfractionScheduler, Cog): expiry_str = format_infraction(infraction["expires_at"]) # Apply the infraction and schedule the expiration task. + self.mod_log.ignore(constants.Event.member_update, member.id) await member.edit(nick=forced_nick, reason=reason) self.schedule_task(ctx.bot.loop, infraction["id"], infraction) -- cgit v1.2.3 From 58de283e68253ce75c33d25433d2c5825e2d732b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 14:58:48 -0700 Subject: Superstarify: refactor on_member_update event * Use notify_infraction to be more consistent * Add an audit log reason to the member edit * Simplify docstring * Simplify log message --- bot/cogs/moderation/superstarify.py | 74 ++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 06a68d89e..45db6676a 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -6,7 +6,6 @@ import typing as t from pathlib import Path from discord import Colour, Embed, Member -from discord.errors import Forbidden from discord.ext.commands import Bot, Cog, Context, command from bot import constants @@ -30,13 +29,7 @@ class Superstarify(InfractionScheduler, Cog): @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: - """ - This event will trigger when someone changes their name. - - At this point we will look up the user in our database and check whether they are allowed to - change their names, or if they are in superstar-prison. If they are not allowed, we will - change it back. - """ + """Revert nickname edits if the user has an active superstarify infraction.""" if before.display_name == after.display_name: return # User didn't change their nickname. Abort! @@ -46,42 +39,45 @@ class Superstarify(InfractionScheduler, Cog): ) active_superstarifies = await self.bot.api_client.get( - 'bot/infractions', + "bot/infractions", params={ - 'active': 'true', - 'type': 'superstar', - 'user__id': str(before.id) + "active": "true", + "type": "superstar", + "user__id": str(before.id) } ) if active_superstarifies: - [infraction] = active_superstarifies - forced_nick = self.get_nick(infraction['id'], before.id) - if after.display_name == forced_nick: - return # Nick change was triggered by this event. Ignore. - - log.info( - f"{after.display_name} is currently in superstar-prison. " - f"Changing the nick back to {before.display_name}." - ) - await after.edit(nick=forced_nick) - end_timestamp_human = format_infraction(infraction['expires_at']) - - try: - await after.send( - "You have tried to change your nickname on the **Python Discord** server " - f"from **{before.display_name}** to **{after.display_name}**, but as you " - "are currently in superstar-prison, you do not have permission to do so. " - "You will be allowed to change your nickname again at the following time:\n\n" - f"**{end_timestamp_human}**." - ) - except Forbidden: - log.warning( - "The user tried to change their nickname while in superstar-prison. " - "This led to the bot trying to DM the user to let them know they cannot do that, " - "but the user had either blocked the bot or disabled DMs, so it was not possible " - "to DM them, and a discord.errors.Forbidden error was incurred." - ) + return + + infraction = active_superstarifies[0] + forced_nick = self.get_nick(infraction["id"], before.id) + if after.display_name == forced_nick: + return # Nick change was triggered by this event. Ignore. + + log.info( + f"{after.display_name} is currently in superstar-prison. " + f"Changing the nick back to {before.display_name}." + ) + await after.edit( + nick=forced_nick, + reason=f"Superstarified member tried to escape the prison: {infraction['id']}" + ) + + notified = await utils.notify_infraction( + user=after, + infr_type="Superstarify", + expires_at=format_infraction(infraction["expires_at"]), + reason=( + "You have tried to change your nickname on the **Python Discord** server " + f"from **{before.display_name}** to **{after.display_name}**, but as you " + "are currently in superstar-prison, you do not have permission to do so." + ), + icon_url=utils.INFRACTION_ICONS["superstar"][0] + ) + + if not notified: + log.warning("Failed to DM user about why they cannot change their nickname.") @Cog.listener() async def on_member_join(self, member: Member) -> None: -- cgit v1.2.3 From ad64553291b1962f8a5f7cf2d5fdd22f6c6a24f5 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:06:22 -0700 Subject: Superstarify: fit within 100 columns & use double quotes --- bot/cogs/moderation/superstarify.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 45db6676a..fb89a4982 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -100,10 +100,16 @@ class Superstarify(InfractionScheduler, Cog): await self.reapply_infraction(infraction, action) - @command(name='superstarify', aliases=('force_nick', 'star')) - async def superstarify(self, ctx: Context, member: Member, duration: utils.Expiry, reason: str = None) -> None: + @command(name="superstarify", aliases=("force_nick", "star")) + async def superstarify( + self, + ctx: Context, + member: Member, + duration: utils.Expiry, + reason: str = None + ) -> None: """ - Force a random superstar name (like Taylor Swift) to be the user's nickname for a specified duration. + Temporarily force a random superstar name (like Taylor Swift) to be the user's nickname. A unit of time should be appended to the duration. Units (∗case-sensitive): @@ -125,7 +131,7 @@ class Superstarify(InfractionScheduler, Cog): # Post the infraction to the API reason = reason or f"old nick: {member.display_name}" - infraction = await utils.post_infraction(ctx, member, "superstar", reason, expires_at=duration) + infraction = await utils.post_infraction(ctx, member, "superstar", reason, duration) forced_nick = self.get_nick(infraction["id"], member.id) expiry_str = format_infraction(infraction["expires_at"]) @@ -140,8 +146,8 @@ class Superstarify(InfractionScheduler, Cog): user=member, infr_type="Superstarify", expires_at=expiry_str, - reason=f"Your nickname didn't comply with our [nickname policy]({NICKNAME_POLICY_URL}).", - icon_url=utils.INFRACTION_ICONS["superstar"][0] + icon_url=utils.INFRACTION_ICONS["superstar"][0], + reason=f"Your nickname didn't comply with our [nickname policy]({NICKNAME_POLICY_URL})." ) # Send an embed with the infraction information to the invoking context. @@ -174,7 +180,7 @@ class Superstarify(InfractionScheduler, Cog): footer=f"ID {infraction['id']}" ) - @command(name='unsuperstarify', aliases=('release_nick', 'unstar')) + @command(name="unsuperstarify", aliases=("release_nick", "unstar")) async def unsuperstarify(self, ctx: Context, member: Member) -> None: """Remove the superstarify infraction and allow the user to change their nickname.""" await self.pardon_infraction(ctx, "superstar", member) -- cgit v1.2.3 From 8ec8446c5e6d62758ac17cd40c8fa32ff50c92db Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:16:55 -0700 Subject: Superstarify: fix mod log mention --- bot/cogs/moderation/superstarify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index fb89a4982..55e1fe04d 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -170,7 +170,7 @@ class Superstarify(InfractionScheduler, Cog): title="Member Achieved Superstardom", thumbnail=member.avatar_url_as(static_format="png"), text=textwrap.dedent(f""" - Member: {member.mentiom} (`{member.id}`) + Member: {member.mention} (`{member.id}`) Actor: {ctx.message.author} Reason: {reason} Expires: {expiry_str} -- cgit v1.2.3 From 4f2a50a15c9dac107f0bd2c8be6d81e9a80df2f0 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:17:32 -0700 Subject: Superstarify: negate active infractions check in on_member_update --- bot/cogs/moderation/superstarify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 55e1fe04d..95962b750 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -47,7 +47,7 @@ class Superstarify(InfractionScheduler, Cog): } ) - if active_superstarifies: + if not active_superstarifies: return infraction = active_superstarifies[0] -- cgit v1.2.3 From 1acdf4c40e14bc15a842d84385bafd312088a03d Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:20:02 -0700 Subject: Superstarify: save the old nickname before editing it --- bot/cogs/moderation/superstarify.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 95962b750..803ecee1c 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -133,6 +133,7 @@ class Superstarify(InfractionScheduler, Cog): reason = reason or f"old nick: {member.display_name}" infraction = await utils.post_infraction(ctx, member, "superstar", reason, duration) + old_nick = member.display_name forced_nick = self.get_nick(infraction["id"], member.id) expiry_str = format_infraction(infraction["expires_at"]) @@ -154,7 +155,7 @@ class Superstarify(InfractionScheduler, Cog): embed = Embed() embed.title = "Congratulations!" embed.description = ( - f"Your previous nickname, **{member.display_name}**, " + f"Your previous nickname, **{old_nick}**, " f"was so bad that we have decided to change it. " f"Your new nickname will be **{forced_nick}**.\n\n" f"You will be unable to change your nickname until \n**{expiry_str}**.\n\n" @@ -174,7 +175,7 @@ class Superstarify(InfractionScheduler, Cog): Actor: {ctx.message.author} Reason: {reason} Expires: {expiry_str} - Old nickname: `{member.display_name}` + Old nickname: `{old_nick}` New nickname: `{forced_nick}` """), footer=f"ID {infraction['id']}" -- cgit v1.2.3 From 757adde72470b1e54e96990894563ee3e52f3538 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:22:36 -0700 Subject: InfractionScheduler: remove redundant logging --- bot/cogs/moderation/scheduler.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 74ff62981..7990df226 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -362,10 +362,7 @@ class InfractionScheduler(Scheduler): At the time of expiration, the infraction is marked as inactive on the website and the expiration task is cancelled. """ - _id = infraction["id"] - expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) await time.wait_until(expiry) - log.debug(f"Marking infraction {_id} as inactive (expired).") await self.deactivate_infraction(infraction) -- cgit v1.2.3 From e17fc1e7583d70545c067410cb7431afb8956af4 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:48:25 -0700 Subject: Superstarify: fix incorrect infraction type check in _pardon_action --- bot/cogs/moderation/superstarify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 803ecee1c..598b5a40e 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -192,7 +192,7 @@ class Superstarify(InfractionScheduler, Cog): user = guild.get_member(infraction["user"]) # Don't bother sending a notification if the user left the guild. - if infraction["type"] != "mute" or not user: + if infraction["type"] != "superstar" or not user: return {} # DM the user about the expiration. -- cgit v1.2.3 From 0621715392103e395c1f6c9c8c2231148f7b1b29 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:49:25 -0700 Subject: Superstarify: make the apply embed colour gold --- bot/cogs/moderation/superstarify.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 598b5a40e..352fccb10 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -152,15 +152,17 @@ class Superstarify(InfractionScheduler, Cog): ) # Send an embed with the infraction information to the invoking context. - embed = Embed() - embed.title = "Congratulations!" - embed.description = ( - f"Your previous nickname, **{old_nick}**, " - f"was so bad that we have decided to change it. " - f"Your new nickname will be **{forced_nick}**.\n\n" - f"You will be unable to change your nickname until \n**{expiry_str}**.\n\n" - "If you're confused by this, please read our " - f"[official nickname policy]({NICKNAME_POLICY_URL})." + embed = Embed( + title="Congratulations!", + colour=constants.Colours.soft_orange, + description=( + f"Your previous nickname, **{old_nick}**, " + f"was so bad that we have decided to change it. " + f"Your new nickname will be **{forced_nick}**.\n\n" + f"You will be unable to change your nickname until **{expiry_str}**.\n\n" + "If you're confused by this, please read our " + f"[official nickname policy]({NICKNAME_POLICY_URL})." + ) ) await ctx.send(embed=embed) -- cgit v1.2.3 From 4ceb7354523836b455670bacbf6c52488e21964b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 15:58:41 -0700 Subject: Superstarify: make mod log title lowercase for consistency --- bot/cogs/moderation/superstarify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 352fccb10..518bb3bae 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -170,7 +170,7 @@ class Superstarify(InfractionScheduler, Cog): await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS["superstar"][0], colour=Colour.gold(), - title="Member Achieved Superstardom", + title="Member achieved superstardom", thumbnail=member.avatar_url_as(static_format="png"), text=textwrap.dedent(f""" Member: {member.mention} (`{member.id}`) -- cgit v1.2.3 From 6077e8a18fb5598d5dfc979e0bae1fe199c1945f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 16:01:12 -0700 Subject: Moderation: remove full stops from pardon embed titles --- bot/cogs/moderation/infractions.py | 2 +- bot/cogs/moderation/superstarify.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 0e29b02f7..2713a1b68 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -254,7 +254,7 @@ class Infractions(InfractionScheduler, commands.Cog): # DM the user about the expiration. notified = await utils.notify_pardon( user=user, - title="You have been unmuted.", + title="You have been unmuted", content="You may now send messages in the server.", icon_url=utils.INFRACTION_ICONS["mute"][1] ) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 518bb3bae..f43f3d027 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -200,7 +200,7 @@ class Superstarify(InfractionScheduler, Cog): # DM the user about the expiration. notified = await utils.notify_pardon( user=user, - title="You are no longer superstarified.", + title="You are no longer superstarified", content="You may now change your nickname on the server.", icon_url=utils.INFRACTION_ICONS["superstar"][1] ) -- cgit v1.2.3 From dd537c9802e8f39849592a99cb33af3cd764a9a2 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 16:03:17 -0700 Subject: Moderation: make infraction notification embed title lowercase --- bot/cogs/moderation/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index c05cb69a1..9179c0afb 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -127,7 +127,7 @@ async def notify_infraction( colour=Colours.soft_red ) - embed.set_author(name="Infraction Information", icon_url=icon_url, url=RULES_URL) + embed.set_author(name="Infraction information", icon_url=icon_url, url=RULES_URL) embed.title = f"Please review our rules over at {RULES_URL}" embed.url = RULES_URL -- cgit v1.2.3 From 47f07ff3a7be12096df4a2f1463c7bf2357761e9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 17:02:04 -0700 Subject: Superstarify: return None in _pardon_action for supported types --- bot/cogs/moderation/superstarify.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index f43f3d027..c66222e5a 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -190,11 +190,14 @@ class Superstarify(InfractionScheduler, Cog): async def _pardon_action(self, infraction: utils.Infraction) -> t.Optional[t.Dict[str, str]]: """Pardon a superstar infraction and return a log dict.""" + if infraction["type"] != "superstar": + return + guild = self.bot.get_guild(constants.Guild.id) user = guild.get_member(infraction["user"]) # Don't bother sending a notification if the user left the guild. - if infraction["type"] != "superstar" or not user: + if not user: return {} # DM the user about the expiration. -- cgit v1.2.3 From 538048189c8babfa72c70563a81a4b725fcb90e4 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Mon, 28 Oct 2019 13:44:46 +0800 Subject: Chain words before length check --- bot/cogs/off_topic_names.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/bot/cogs/off_topic_names.py b/bot/cogs/off_topic_names.py index 1f9fb0b4f..78792240f 100644 --- a/bot/cogs/off_topic_names.py +++ b/bot/cogs/off_topic_names.py @@ -24,6 +24,9 @@ class OffTopicName(Converter): """Attempt to replace any invalid characters with their approximate Unicode equivalent.""" allowed_characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ!?'`-" + # Chain multiple words to a single one + argument = "-".join(argument.split()) + if not (2 <= len(argument) <= 96): raise BadArgument("Channel name must be between 2 and 96 chars long") @@ -97,15 +100,12 @@ class OffTopicNames(Cog): @otname_group.command(name='add', aliases=('a',)) @with_role(*MODERATION_ROLES) - async def add_command(self, ctx: Context, *names: OffTopicName) -> None: + async def add_command(self, ctx: Context, *, name: OffTopicName) -> None: """ Adds a new off-topic name to the rotation. The name is not added if it is too similar to an existing name. """ - # Chain multiple words to a single one - name = "-".join(names) - existing_names = await self.bot.api_client.get('bot/off-topic-channel-names') close_match = difflib.get_close_matches(name, existing_names, n=1, cutoff=0.8) @@ -123,10 +123,8 @@ class OffTopicNames(Cog): @otname_group.command(name='forceadd', aliases=('fa',)) @with_role(*MODERATION_ROLES) - async def force_add_command(self, ctx: Context, *names: OffTopicName) -> None: + async def force_add_command(self, ctx: Context, *, name: OffTopicName) -> None: """Forcefully adds a new off-topic name to the rotation.""" - # Chain multiple words to a single one - name = "-".join(names) await self._add_name(ctx, name) async def _add_name(self, ctx: Context, name: str) -> None: @@ -138,10 +136,8 @@ class OffTopicNames(Cog): @otname_group.command(name='delete', aliases=('remove', 'rm', 'del', 'd')) @with_role(*MODERATION_ROLES) - async def delete_command(self, ctx: Context, *names: OffTopicName) -> None: + async def delete_command(self, ctx: Context, *, name: OffTopicName) -> None: """Removes a off-topic name from the rotation.""" - # Chain multiple words to a single one - name = "-".join(names) await self.bot.api_client.delete(f'bot/off-topic-channel-names/{name}') log.info(f"{ctx.author} deleted the off-topic channel name '{name}'") -- cgit v1.2.3 From c5d0eb473a9a1dc486dd2dd60603463435e49da4 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 28 Oct 2019 15:56:34 +0100 Subject: Change generation of child mocks - https://docs.python.org/3/library/unittest.mock.html We previously used an override of the `__new__` method to prevent our custom mock types from instantiating their children with their own type instead of a general mock type like `MagicMock` or `Mock`. As it turns out, the Python documentation suggests another method of doing this that does not involve overriding `__new__`. This commit implements this new method to make sure we're using the idiomatic way of handling this. The suggested method is overriding the `_get_child_mock` method in the subclass. To make our code DRY, I've created a mixin that should come BEFORE the mock type we're subclassing in the MRO. --- In addition, I have also added this new mixin to our `AsyncMock` class to make sure that its `__call__` method returns a proper mock object after it has been awaited. This makes sure that subsequent attribute access on the returned object is mocked as expected. --- tests/helpers.py | 85 +++++++++++++++++++++++++++++++-------------------- tests/test_helpers.py | 18 +++++++++-- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 892d42e6c..9375d0986 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -24,19 +24,6 @@ def async_test(wrapped): return wrapper -# TODO: Remove me in Python 3.8 -class AsyncMock(unittest.mock.MagicMock): - """ - A MagicMock subclass to mock async callables. - - Python 3.8 will introduce an AsyncMock class in the standard library that will have some more - features; this stand-in only overwrites the `__call__` method to an async version. - """ - - async def __call__(self, *args, **kwargs): - return super(AsyncMock, self).__call__(*args, **kwargs) - - class HashableMixin(discord.mixins.EqualityComparable): """ Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. @@ -61,15 +48,43 @@ class ColourMixin: self.colour = color -class AttributeMock: +class GetChildMockMixin: """Ensures attributes of our mock types will be instantiated with the correct mock type.""" - def __new__(cls, *args, **kwargs): - """Stops the regular parent class from propagating to newly mocked attributes.""" - if 'parent' in kwargs: - return cls.attribute_mocktype(*args, **kwargs) + def _get_child_mock(self, **kw): + """ + Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. + + Mock objects automatically create children when you access an attribute or call a method on them. By default, + the class of these children is the type of the parent itself. However, this would mean that the children created + for our custom mock types would also be instances of that custom mock type. This is not desirable, as attributes + of, e.g., a `Bot` object are not `Bot` objects themselves. The Python docs for `unittest.mock` hint that + overwriting this method is the best way to deal with that. + + This override will look for an attribute called `child_mock_type` and use that as the type of the child mock. + """ + klass = self.child_mock_type - return super().__new__(cls) + if self._mock_sealed: + attribute = "." + kw["name"] if "name" in kw else "()" + mock_name = self._extract_mock_name() + attribute + raise AttributeError(mock_name) + + return klass(**kw) + + +# TODO: Remove me in Python 3.8 +class AsyncMock(GetChildMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock async callables. + + Python 3.8 will introduce an AsyncMock class in the standard library that will have some more + features; this stand-in only overwrites the `__call__` method to an async version. + """ + child_mock_type = unittest.mock.MagicMock + + async def __call__(self, *args, **kwargs): + return super(AsyncMock, self).__call__(*args, **kwargs) # Create a guild instance to get a realistic Mock of `discord.Guild` @@ -95,7 +110,7 @@ guild_data = { guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) -class MockGuild(AttributeMock, unittest.mock.Mock, HashableMixin): +class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): """ A `Mock` subclass to mock `discord.Guild` objects. @@ -122,7 +137,7 @@ class MockGuild(AttributeMock, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__( self, @@ -175,7 +190,7 @@ role_data = {'name': 'role', 'id': 1} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) -class MockRole(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock `discord.Role` objects. @@ -183,7 +198,7 @@ class MockRole(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: super().__init__(spec=role_instance, **kwargs) @@ -208,7 +223,7 @@ state_mock = unittest.mock.MagicMock() member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) -class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. @@ -216,7 +231,7 @@ class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__( self, @@ -254,7 +269,7 @@ class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) -class MockBot(AttributeMock, unittest.mock.MagicMock): +class MockBot(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Bot objects. @@ -262,11 +277,15 @@ class MockBot(AttributeMock, unittest.mock.MagicMock): For more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs) -> None: super().__init__(spec=bot_instance, **kwargs) + # Our custom attributes and methods + self.http_session = unittest.mock.MagicMock() + self.api_client = unittest.mock.MagicMock() + # `discord.ext.commands.Bot` coroutines self._before_invoke = AsyncMock() self._after_invoke = AsyncMock() @@ -303,7 +322,7 @@ class MockBot(AttributeMock, unittest.mock.MagicMock): context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) -class MockContext(AttributeMock, unittest.mock.MagicMock): +class MockContext(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Context objects. @@ -311,7 +330,7 @@ class MockContext(AttributeMock, unittest.mock.MagicMock): instances. For more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs) -> None: super().__init__(spec=context_instance, **kwargs) @@ -346,7 +365,7 @@ guild = unittest.mock.MagicMock() channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) -class MockTextChannel(AttributeMock, unittest.mock.Mock, HashableMixin): +class MockTextChannel(GetChildMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. @@ -354,7 +373,7 @@ class MockTextChannel(AttributeMock, unittest.mock.Mock, HashableMixin): more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: super().__init__(spec=channel_instance, **kwargs) @@ -402,7 +421,7 @@ channel = unittest.mock.MagicMock() message_instance = discord.Message(state=state, channel=channel, data=message_data) -class MockMessage(AttributeMock, unittest.mock.MagicMock): +class MockMessage(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. @@ -410,7 +429,7 @@ class MockMessage(AttributeMock, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs) -> None: super().__init__(spec=message_instance, **kwargs) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index f08239981..62007ff4e 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -221,10 +221,10 @@ class DiscordMocksTests(unittest.TestCase): @unittest.mock.patch(f'{__name__}.DiscordMocksTests.subTest') def test_the_custom_mock_methods_test(self, subtest_mock): """The custom method test should raise AssertionError for invalid methods.""" - class FakeMockBot(helpers.AttributeMock, unittest.mock.MagicMock): + class FakeMockBot(helpers.GetChildMockMixin, unittest.mock.MagicMock): """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs): super().__init__(spec=helpers.bot_instance, **kwargs) @@ -331,6 +331,18 @@ class MockObjectTests(unittest.TestCase): self.assertFalse(instance_one != instance_two) self.assertTrue(instance_one != instance_three) + def test_get_child_mock_mixin_accepts_mock_seal(self): + """The `GetChildMockMixin` should support `unittest.mock.seal`.""" + class MyMock(helpers.GetChildMockMixin, unittest.mock.MagicMock): + + child_mock_type = unittest.mock.MagicMock + pass + + mock = MyMock() + unittest.mock.seal(mock) + with self.assertRaises(AttributeError, msg="MyMock.shirayuki"): + mock.shirayuki = "hello!" + def test_spec_propagation_of_mock_subclasses(self): """Test if the `spec` does not propagate to attributes of the mock object.""" test_values = ( @@ -346,7 +358,7 @@ class MockObjectTests(unittest.TestCase): mock = mock_type() self.assertTrue(isinstance(mock, mock_type)) attribute = getattr(mock, valid_attribute) - self.assertTrue(isinstance(attribute, mock_type.attribute_mocktype)) + self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) def test_async_mock_provides_coroutine_for_dunder_call(self): """Test if AsyncMock objects have a coroutine for their __call__ method.""" -- cgit v1.2.3 From 4a5165a9f46046a9e9db6562fb421e36749fd32a Mon Sep 17 00:00:00 2001 From: Manuel Ignacio Pérez Alcolea Date: Mon, 28 Oct 2019 19:40:14 -0300 Subject: Remove redirection on rules command Previously restricted for the staff. This change was suggested due its possible usefulness for regular users. --- bot/cogs/site.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/cogs/site.py b/bot/cogs/site.py index d95359159..683613788 100644 --- a/bot/cogs/site.py +++ b/bot/cogs/site.py @@ -3,8 +3,7 @@ import logging from discord import Colour, Embed from discord.ext.commands import Bot, Cog, Context, group -from bot.constants import Channels, STAFF_ROLES, URLs -from bot.decorators import redirect_output +from bot.constants import URLs from bot.pagination import LinePaginator log = logging.getLogger(__name__) @@ -105,7 +104,6 @@ class Site(Cog): await ctx.send(embed=embed) @site_group.command(aliases=['r', 'rule'], name='rules') - @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES) async def site_rules(self, ctx: Context, *rules: int) -> None: """Provides a link to all rules or, if specified, displays specific rule(s).""" rules_embed = Embed(title='Rules', color=Colour.blurple()) -- cgit v1.2.3 From 1a1ef440aa8c1c63e9ed38742641d18da68c30c3 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 13:44:13 +0100 Subject: Enhance Reddit webhook embeds https://github.com/python-discord/bot/issues/634 I have changes the appearance of the embed generated for the reddit webhook. The changes: - Bold markdown around the links to prevent it breaking on android. - Stylized the meta-data line with newly created emoji-based icons. - Removed redundant mentions of the subreddit from the embed itself. The emojis were uploaded to the `Emojis II` guild and the IDs have been added to the constants files. In addition, I've set the reddit logo as the default avatar for the webhook in the guild. This commit closes #634 --- bot/cogs/reddit.py | 6 +++--- bot/constants.py | 4 ++++ config-default.yml | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 7749d237f..f947a7d78 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -9,7 +9,7 @@ from discord import Colour, Embed, TextChannel from discord.ext.commands import Bot, Cog, Context, group from discord.ext.tasks import loop -from bot.constants import Channels, ERROR_REPLIES, Reddit as RedditConfig, STAFF_ROLES, Webhooks +from bot.constants import Channels, ERROR_REPLIES, Emojis, Reddit as RedditConfig, STAFF_ROLES, Webhooks from bot.converters import Subreddit from bot.decorators import with_role from bot.pagination import LinePaginator @@ -117,9 +117,9 @@ class Reddit(Cog): link = self.URL + data["permalink"] embed.description += ( - f"[**{title}**]({link})\n" + f"**[{title}]({link})**\n" f"{text}" - f"| {ups} upvotes | {comments} comments | u/{author} | {subreddit} |\n\n" + f"{Emojis.upvotes} {ups} {Emojis.comments} {comments} {Emojis.user} {author}\n\n" ) embed.colour = Colour.blurple() diff --git a/bot/constants.py b/bot/constants.py index 838fe7a79..d3e79b4c2 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -259,6 +259,10 @@ class Emojis(metaclass=YAMLGetter): pencil: str cross_mark: str + upvotes: str + comments: str + user: str + class Icons(metaclass=YAMLGetter): section = "style" diff --git a/config-default.yml b/config-default.yml index 4638a89ee..9c7830077 100644 --- a/config-default.yml +++ b/config-default.yml @@ -37,6 +37,10 @@ style: new: "\U0001F195" cross_mark: "\u274C" + upvotes: "<:upvotes:638706000714792962>" + comments: "<:comments:638706001159258132>" + user: "<:user:638706001217978368>" + icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" crown_green: "https://cdn.discordapp.com/emojis/469964154719961088.png" -- cgit v1.2.3 From 55b83342f389aeacdbd5d2558dea4981c90aa2c4 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 14:27:56 +0100 Subject: Use 'local' emojis for reddit webhook embed Apparently, webhooks cannot use external emojis. For now, I have added the emojis to the guild directly and updated the IDs in this commit. I don't really like having "utility" emojis on the main guild, but the alternative would be to not use webhooks, which is a change we will need to discuss before we apply it. (I don't see anything against it now we just send a daily digest, although, in principle, this is precisely what a webhook is meant for.) --- config-default.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config-default.yml b/config-default.yml index 9c7830077..bce6ea266 100644 --- a/config-default.yml +++ b/config-default.yml @@ -37,9 +37,9 @@ style: new: "\U0001F195" cross_mark: "\u274C" - upvotes: "<:upvotes:638706000714792962>" - comments: "<:comments:638706001159258132>" - user: "<:user:638706001217978368>" + upvotes: "<:upvotes:638729835245731840>" + comments: "<:comments:638729835073765387>" + user: "<:user:638729835442602003>" icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" -- cgit v1.2.3 From 618ba6a523dababde230382e1965ecc89f23aaf5 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 10:04:37 +0100 Subject: Enhance custom mock helpers I have enhanced the custom mocks defined in `tests/helpers.py` in a couple of important ways. 1. Automatically create AsyncMock attributes using `inspect` Our previous approach, hard-coding AsynckMock attributes for all the coroutine function methods defined for the class we are trying to mock is prone to human error and not resilient against changes introduced in updates of the library we are using. Instead, I have now created a helper method in our `CustomMockMixin` (formerly `GetChildMockMixin`) that automatically inspects the spec instance we've passed for `coroutine functions` using the `inspect` module. It then sets the according attributes with instances of the AsyncMock class. There is one caveat: `discord.py` very rarely defines regular methods that return a coroutine object. Since the returned coroutine should still be awaited, these regular methods should also be mocked with an AsyncMock. However, since they are regular methods, `inspect` does not detect them and they have to be added manually. (The only case of this I've found so far is `Client.wait_for`.) 2. Properly set special attributes using `kwargs.get` As we want attributes that point to other discord.py objects to use our custom mocks (.e.g, `Message.author` should use `MockMember`), the `__init__` method of our custom mocks make sure to correctly instantiate these attributes. However, the way we previously did that means we can't instantiate the custom mock with a mock instance we provide, since this special instantiation would overwrite the custom object we'd passed. I've solved this by using `kwargs.get`, with a new mock as the default value. This makes sure we only create a new mock if we didn't pass a custom one: ```py class MockMesseage: def __init__(self, **kwargs): self.author = kwargs.get('author', MockMember()) ``` As you can see, we will only create a new MockMember if we did not pass an `author` argument. 3. Factoring out duplicate lines Since our `CustomMockMixin` is a parent to all of our custom mock types, it makes sense to use it to factor out common code of all of our custom mocks. I've made the following changes: - Set a default child mock type in the mixin. - Create an `__init__` that takes care of the `inspect` of point 1 This means we won't have to repeat this in all of the child classes. 4. Three new Mock types: Emoji, PartialEmoji, and Reaction I have added three more custom mocks: - MockEmoji - MockPartialEmoji - MockReaction --- tests/helpers.py | 253 +++++++++++++++++++------------------------------- tests/test_helpers.py | 58 +++++++++++- 2 files changed, 150 insertions(+), 161 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 9375d0986..673beae3f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -2,8 +2,9 @@ from __future__ import annotations import asyncio import functools +import inspect import unittest.mock -from typing import Iterable, Optional +from typing import Any, Iterable, Optional import discord from discord.ext.commands import Bot, Context @@ -48,9 +49,16 @@ class ColourMixin: self.colour = color -class GetChildMockMixin: +class CustomMockMixin: """Ensures attributes of our mock types will be instantiated with the correct mock type.""" + child_mock_type = unittest.mock.MagicMock + + def __init__(self, spec: Any = None, **kwargs): + super().__init__(spec=spec, **kwargs) + if spec: + self._extract_coroutine_methods_from_spec_instance(spec) + def _get_child_mock(self, **kw): """ Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. @@ -72,17 +80,20 @@ class GetChildMockMixin: return klass(**kw) + def _extract_coroutine_methods_from_spec_instance(self, source: Any) -> None: + """Automatically detect coroutine functions in `source` and set them as AsyncMock attributes.""" + for name, _method in inspect.getmembers(source, inspect.iscoroutinefunction): + setattr(self, name, AsyncMock()) + # TODO: Remove me in Python 3.8 -class AsyncMock(GetChildMockMixin, unittest.mock.MagicMock): +class AsyncMock(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock async callables. Python 3.8 will introduce an AsyncMock class in the standard library that will have some more features; this stand-in only overwrites the `__call__` method to an async version. """ - child_mock_type = unittest.mock.MagicMock - async def __call__(self, *args, **kwargs): return super(AsyncMock, self).__call__(*args, **kwargs) @@ -110,7 +121,7 @@ guild_data = { guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) -class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): +class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A `Mock` subclass to mock `discord.Guild` objects. @@ -136,9 +147,6 @@ class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - - child_mock_type = unittest.mock.MagicMock - def __init__( self, guild_id: int = 1, @@ -158,39 +166,13 @@ class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): if members: self.members.extend(members) - # `discord.Guild` coroutines - self.create_category_channel = AsyncMock() - self.ban = AsyncMock() - self.bans = AsyncMock() - self.create_category = AsyncMock() - self.create_custom_emoji = AsyncMock() - self.create_role = AsyncMock() - self.create_text_channel = AsyncMock() - self.create_voice_channel = AsyncMock() - self.delete = AsyncMock() - self.edit = AsyncMock() - self.estimate_pruned_members = AsyncMock() - self.fetch_ban = AsyncMock() - self.fetch_channels = AsyncMock() - self.fetch_emoji = AsyncMock() - self.fetch_emojis = AsyncMock() - self.fetch_member = AsyncMock() - self.invites = AsyncMock() - self.kick = AsyncMock() - self.leave = AsyncMock() - self.prune_members = AsyncMock() - self.unban = AsyncMock() - self.vanity_invite = AsyncMock() - self.webhooks = AsyncMock() - self.widget = AsyncMock() - # Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) -class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock `discord.Role` objects. @@ -208,10 +190,6 @@ class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin self.position = position self.mention = f'&{self.name}' - # 'discord.Role' coroutines - self.delete = AsyncMock() - self.edit = AsyncMock() - def __lt__(self, other): """Simplified position-based comparisons similar to those of `discord.Role`.""" return self.position < other.position @@ -223,16 +201,13 @@ state_mock = unittest.mock.MagicMock() member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) -class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__( self, name: str = "member", @@ -251,34 +226,18 @@ class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMix self.mention = f"@{self.name}" - # `discord.Member` coroutines - self.add_roles = AsyncMock() - self.ban = AsyncMock() - self.edit = AsyncMock() - self.fetch_message = AsyncMock() - self.kick = AsyncMock() - self.move_to = AsyncMock() - self.pins = AsyncMock() - self.remove_roles = AsyncMock() - self.send = AsyncMock() - self.trigger_typing = AsyncMock() - self.unban = AsyncMock() - # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) -class MockBot(GetChildMockMixin, unittest.mock.MagicMock): +class MockBot(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Bot objects. Instances of this class will follow the specifications of `discord.ext.commands.Bot` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__(self, **kwargs) -> None: super().__init__(spec=bot_instance, **kwargs) @@ -286,69 +245,12 @@ class MockBot(GetChildMockMixin, unittest.mock.MagicMock): self.http_session = unittest.mock.MagicMock() self.api_client = unittest.mock.MagicMock() - # `discord.ext.commands.Bot` coroutines - self._before_invoke = AsyncMock() - self._after_invoke = AsyncMock() - self.application_info = AsyncMock() - self.change_presence = AsyncMock() - self.connect = AsyncMock() - self.close = AsyncMock() - self.create_guild = AsyncMock() - self.delete_invite = AsyncMock() - self.fetch_channel = AsyncMock() - self.fetch_guild = AsyncMock() - self.fetch_guilds = AsyncMock() - self.fetch_invite = AsyncMock() - self.fetch_user = AsyncMock() - self.fetch_user_profile = AsyncMock() - self.fetch_webhook = AsyncMock() - self.fetch_widget = AsyncMock() - self.get_context = AsyncMock() - self.get_prefix = AsyncMock() - self.invoke = AsyncMock() - self.is_owner = AsyncMock() - self.login = AsyncMock() - self.logout = AsyncMock() - self.on_command_error = AsyncMock() - self.on_error = AsyncMock() - self.process_commands = AsyncMock() - self.request_offline_members = AsyncMock() - self.start = AsyncMock() - self.wait_until_ready = AsyncMock() + # self.wait_for is *not* a coroutine function, but returns a coroutine nonetheless and + # and should therefore be awaited. (The documentation calls it a coroutine as well, which + # is technically incorrect, since it's a regular def.) self.wait_for = AsyncMock() -# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` -context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) - - -class MockContext(GetChildMockMixin, unittest.mock.MagicMock): - """ - A MagicMock subclass to mock Context objects. - - Instances of this class will follow the specifications of `discord.ext.commands.Context` - instances. For more information, see the `MockGuild` docstring. - """ - - child_mock_type = unittest.mock.MagicMock - - def __init__(self, **kwargs) -> None: - super().__init__(spec=context_instance, **kwargs) - self.bot = MockBot() - self.guild = MockGuild() - self.author = MockMember() - self.command = unittest.mock.MagicMock() - - # `discord.ext.commands.Context` coroutines - self.fetch_message = AsyncMock() - self.invoke = AsyncMock() - self.pins = AsyncMock() - self.reinvoke = AsyncMock() - self.send = AsyncMock() - self.send_help = AsyncMock() - self.trigger_typing = AsyncMock() - - # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { 'id': 1, @@ -365,39 +267,20 @@ guild = unittest.mock.MagicMock() channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) -class MockTextChannel(GetChildMockMixin, unittest.mock.Mock, HashableMixin): +class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: super().__init__(spec=channel_instance, **kwargs) self.id = channel_id self.name = name - self.guild = MockGuild() + self.guild = kwargs.get('guild', MockGuild()) self.mention = f"#{self.name}" - # `discord.TextChannel` coroutines - self.clone = AsyncMock() - self.create_invite = AsyncMock() - self.create_webhook = AsyncMock() - self.delete = AsyncMock() - self.delete_messages = AsyncMock() - self.edit = AsyncMock() - self.fetch_message = AsyncMock() - self.invites = AsyncMock() - self.pins = AsyncMock() - self.purge = AsyncMock() - self.send = AsyncMock() - self.set_permissions = AsyncMock() - self.trigger_typing = AsyncMock() - self.webhooks = AsyncMock() - # Create a Message instance to get a realistic MagicMock of `discord.Message` message_data = { @@ -421,27 +304,83 @@ channel = unittest.mock.MagicMock() message_instance = discord.Message(state=state, channel=channel, data=message_data) -class MockMessage(GetChildMockMixin, unittest.mock.MagicMock): +# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` +context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) + + +class MockContext(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Context objects. + + Instances of this class will follow the specifications of `discord.ext.commands.Context` + instances. For more information, see the `MockGuild` docstring. + """ + def __init__(self, **kwargs) -> None: + super().__init__(spec=context_instance, **kwargs) + self.bot = kwargs.get('bot', MockBot()) + self.guild = kwargs.get('guild', MockGuild()) + self.author = kwargs.get('author', MockMember()) + self.channel = kwargs.get('channel', MockTextChannel()) + self.command = kwargs.get('command', unittest.mock.MagicMock()) + + +class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. Instances of this class will follow the specifications of `discord.Message` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: + super().__init__(spec=message_instance, **kwargs) + self.author = kwargs.get('author', MockMember()) + self.channel = kwargs.get('channel', MockTextChannel()) - child_mock_type = unittest.mock.MagicMock +emoji_data = {'require_colons': True, 'managed': True, 'id': 1, 'name': 'hyperlemon'} +emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) + + +class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Emoji objects. + + Instances of this class will follow the specifications of `discord.Emoji` instances. For more + information, see the `MockGuild` docstring. + """ def __init__(self, **kwargs) -> None: - super().__init__(spec=message_instance, **kwargs) - self.author = MockMember() - self.channel = MockTextChannel() - - # `discord.Message` coroutines - self.ack = AsyncMock() - self.add_reaction = AsyncMock() - self.clear_reactions = AsyncMock() - self.delete = AsyncMock() - self.edit = AsyncMock() - self.pin = AsyncMock() - self.remove_reaction = AsyncMock() - self.unpin = AsyncMock() + super().__init__(spec=emoji_instance, **kwargs) + self.guild = kwargs.get('guild', MockGuild()) + + # Get all coroutine functions and set them as AsyncMock attributes + self._extract_coroutine_methods_from_spec_instance(emoji_instance) + + +partial_emoji_instance = discord.PartialEmoji(animated=False, name='guido') + + +class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock PartialEmoji objects. + + Instances of this class will follow the specifications of `discord.PartialEmoji` instances. For + more information, see the `MockGuild` docstring. + """ + def __init__(self, **kwargs) -> None: + super().__init__(spec=partial_emoji_instance, **kwargs) + + +reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) + + +class MockReaction(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Reaction objects. + + Instances of this class will follow the specifications of `discord.Reaction` instances. For + more information, see the `MockGuild` docstring. + """ + def __init__(self, **kwargs) -> None: + super().__init__(spec=reaction_instance, **kwargs) + self.emoji = kwargs.get('emoji', MockEmoji()) + self.message = kwargs.get('message', MockMessage()) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 62007ff4e..2b58634dd 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -221,7 +221,7 @@ class DiscordMocksTests(unittest.TestCase): @unittest.mock.patch(f'{__name__}.DiscordMocksTests.subTest') def test_the_custom_mock_methods_test(self, subtest_mock): """The custom method test should raise AssertionError for invalid methods.""" - class FakeMockBot(helpers.GetChildMockMixin, unittest.mock.MagicMock): + class FakeMockBot(helpers.CustomMockMixin, unittest.mock.MagicMock): """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" child_mock_type = unittest.mock.MagicMock @@ -331,9 +331,9 @@ class MockObjectTests(unittest.TestCase): self.assertFalse(instance_one != instance_two) self.assertTrue(instance_one != instance_three) - def test_get_child_mock_mixin_accepts_mock_seal(self): - """The `GetChildMockMixin` should support `unittest.mock.seal`.""" - class MyMock(helpers.GetChildMockMixin, unittest.mock.MagicMock): + def test_custom_mock_mixin_accepts_mock_seal(self): + """The `CustomMockMixin` should support `unittest.mock.seal`.""" + class MyMock(helpers.CustomMockMixin, unittest.mock.MagicMock): child_mock_type = unittest.mock.MagicMock pass @@ -351,6 +351,10 @@ class MockObjectTests(unittest.TestCase): (helpers.MockMember, "display_name"), (helpers.MockBot, "owner_id"), (helpers.MockContext, "command_failed"), + (helpers.MockMessage, "mention_everyone"), + (helpers.MockEmoji, 'managed'), + (helpers.MockPartialEmoji, 'url'), + (helpers.MockReaction, 'me'), ) for mock_type, valid_attribute in test_values: @@ -360,6 +364,52 @@ class MockObjectTests(unittest.TestCase): attribute = getattr(mock, valid_attribute) self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) + def test_extract_coroutine_methods_from_spec_instance_should_extract_all_and_only_coroutines(self): + """Test if all coroutine functions are extracted, but not regular methods or attributes.""" + class CoroutineDonor: + def __init__(self): + self.some_attribute = 'alpha' + + async def first_coroutine(): + """This coroutine function should be extracted.""" + + async def second_coroutine(): + """This coroutine function should be extracted.""" + + def regular_method(): + """This regular function should not be extracted.""" + + class Receiver: + pass + + donor = CoroutineDonor() + receiver = Receiver() + + helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance(receiver, donor) + + self.assertIsInstance(receiver.first_coroutine, helpers.AsyncMock) + self.assertIsInstance(receiver.second_coroutine, helpers.AsyncMock) + self.assertFalse(hasattr(receiver, 'regular_method')) + self.assertFalse(hasattr(receiver, 'some_attribute')) + + @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) + @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") + def test_custom_mock_mixin_init_with_spec(self, extract_method_mock): + """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" + spec = "pydis" + + helpers.CustomMockMixin(spec=spec) + + extract_method_mock.assert_called_once_with(spec) + + @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) + @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") + def test_custom_mock_mixin_init_without_spec(self, extract_method_mock): + """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" + helpers.CustomMockMixin() + + extract_method_mock.assert_not_called() + def test_async_mock_provides_coroutine_for_dunder_call(self): """Test if AsyncMock objects have a coroutine for their __call__ method.""" async_mock = helpers.AsyncMock() -- cgit v1.2.3 From 586ae18842de0dd92e93945c63ed5e6cd158c1f7 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 30 Oct 2019 22:54:27 +0100 Subject: Update docstring and remove redundant attribute I accidentally forgot to update the docstring of `CustomMockMixin`, which changed quite dramatically in scope with the last commit. This commit remedies that. In addition, I inadvertently forgot to remove the `child_mock_type` class attribute from `MockRole`. Since it uses the default value, it is no longer necessary to specify it in the child class as well. --- tests/helpers.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 673beae3f..8496ba031 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -50,7 +50,15 @@ class ColourMixin: class CustomMockMixin: - """Ensures attributes of our mock types will be instantiated with the correct mock type.""" + """ + Provides common functionality for our custom Mock types. + + The cooperative `__init__` automatically creates `AsyncMock` attributes for every coroutine + function `inspect` detects in the `spec` instance we provide. In addition, this mixin takes care + of making sure child mocks are instantiated with the correct class. By default, the mock of the + children will be `unittest.mock.MagicMock`, but this can be overwritten by setting the attribute + `child_mock_type` on the custom mock inheriting from this mixin. + """ child_mock_type = unittest.mock.MagicMock @@ -179,9 +187,6 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): Instances of this class will follow the specifications of `discord.Role` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: super().__init__(spec=role_instance, **kwargs) -- cgit v1.2.3 From 6e57f41d727c9cde51d36adc04393c3723d62472 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 28 Oct 2019 15:54:31 +0100 Subject: Enhance the output of the user command https://github.com/python-discord/bot/issues/628 https://github.com/python-discord/bot/issues/339 This commit introduces several changes to the output of the `!user` command for moderation staff. The output for regular users has not changed. Changes: - When issued in a moderation channel, the infraction count of the user will now be broken down by type as described in #339. This allows moderators to get a quicker overview of someone's history by providing more information. The command will display the total number of infractions per type, with the number of active infractions in parentheses behind it if there are any. This change also means that there no longer a need for the `hidden` parameter: When issued in a moderation channel, hidden infractions are included by default; when issued outside of a mod channel, the command will be equal to what a regular user would get. In addition to broken-down infraction info, the command now also shows information about the nominations of a user when it's issued inside of a moderation channel. - The code has been refactored to smaller units that take care of a single action to make unit testing easier. I have included tests that cover the command and all of the new helper methods. Tests for the other methods/commands in the cog will be added in the specific issue calling for tests for this cog (#581) This commit closes #628 and closes #339 --- bot/cogs/information.py | 170 ++++++++++---- tests/bot/cogs/test_information.py | 444 +++++++++++++++++++++++++++++++++++-- 2 files changed, 552 insertions(+), 62 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 3a7ba0444..4a3af7edd 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -3,6 +3,7 @@ import logging import pprint import textwrap import typing +from collections import defaultdict from typing import Any, Mapping, Optional import discord @@ -10,7 +11,7 @@ from discord import CategoryChannel, Colour, Embed, Member, Role, TextChannel, V from discord.ext import commands from discord.ext.commands import Bot, BucketType, Cog, Context, command, group -from bot.constants import Channels, Emojis, MODERATION_ROLES, STAFF_ROLES +from bot import constants from bot.decorators import InChannelCheckFailure, in_channel, with_role from bot.utils.checks import cooldown_with_role_bypass, with_role_check from bot.utils.time import time_since @@ -24,7 +25,7 @@ class Information(Cog): def __init__(self, bot: Bot): self.bot = bot - @with_role(*MODERATION_ROLES) + @with_role(*constants.MODERATION_ROLES) @command(name="roles") async def roles_info(self, ctx: Context) -> None: """Returns a list of all roles and their corresponding IDs.""" @@ -48,7 +49,7 @@ class Information(Cog): await ctx.send(embed=embed) - @with_role(*MODERATION_ROLES) + @with_role(*constants.MODERATION_ROLES) @command(name="role") async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None: """ @@ -148,10 +149,10 @@ class Information(Cog): Channel categories: {category_channels} **Members** - {Emojis.status_online} {online} - {Emojis.status_idle} {idle} - {Emojis.status_dnd} {dnd} - {Emojis.status_offline} {offline} + {constants.Emojis.status_online} {online} + {constants.Emojis.status_idle} {idle} + {constants.Emojis.status_dnd} {dnd} + {constants.Emojis.status_offline} {offline} """) ) @@ -160,59 +161,38 @@ class Information(Cog): await ctx.send(embed=embed) @command(name="user", aliases=["user_info", "member", "member_info"]) - async def user_info(self, ctx: Context, user: Member = None, hidden: bool = False) -> None: + async def user_info(self, ctx: Context, user: Member = None) -> None: """Returns info about a user.""" if user is None: user = ctx.author # Do a role check if this is being executed on someone other than the caller - if user != ctx.author and not with_role_check(ctx, *MODERATION_ROLES): + if 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 - # Non-moderators may only do this in #bot-commands and can't see hidden infractions. - if not with_role_check(ctx, *STAFF_ROLES): - if not ctx.channel.id == Channels.bot: - raise InChannelCheckFailure(Channels.bot) - # Hide hidden infractions for users without a moderation role - hidden = False + # Non-staff may only do this in #bot-commands + if not with_role_check(ctx, *constants.STAFF_ROLES): + if not ctx.channel.id == constants.Channels.bot: + raise InChannelCheckFailure(constants.Channels.bot) - # User information + embed = await self.create_user_embed(ctx, user) + + await ctx.send(embed=embed) + + async def create_user_embed(self, ctx: Context, user: Member) -> Embed: + """Creates an embed containing information on the `user`.""" created = time_since(user.created_at, max_units=3) name = str(user) if user.nick: name = f"{user.nick} ({name})" - # Member information joined = time_since(user.joined_at, precision="days") - - # You're welcome, Volcyyyyyyyyyyyyyyyy roles = ", ".join(role.mention for role in user.roles if role.name != "@everyone") - # Infractions - infractions = await self.bot.api_client.get( - 'bot/infractions', - params={ - 'hidden': str(hidden), - 'user__id': str(user.id) - } - ) - - infr_total = 0 - infr_active = 0 - - # At least it's readable. - for infr in infractions: - if infr["active"]: - infr_active += 1 - - infr_total += 1 - - # Let's build the embed now - embed = Embed( - title=name, - description=textwrap.dedent(f""" + description = [ + textwrap.dedent(f""" **User Information** Created: {created} Profile: {user.mention} @@ -221,17 +201,109 @@ class Information(Cog): **Member Information** Joined: {joined} Roles: {roles or None} + """).strip() + ] - **Infractions** - Total: {infr_total} - Active: {infr_active} - """) + # Show more verbose output in moderation channels for infractions and nominations + if ctx.channel.id in constants.MODERATION_CHANNELS: + description.append(await self.expanded_user_infraction_counts(user)) + description.append(await self.user_nomination_counts(user)) + else: + description.append(await self.basic_user_infraction_counts(user)) + + # Let's build the embed now + embed = Embed( + title=name, + description="\n\n".join(description) ) embed.set_thumbnail(url=user.avatar_url_as(format="png")) embed.colour = user.top_role.colour if roles else Colour.blurple() - await ctx.send(embed=embed) + return embed + + async def basic_user_infraction_counts(self, member: Member) -> str: + """Gets the total and active infraction counts for the given `member`.""" + infractions = await self.bot.api_client.get( + 'bot/infractions', + params={ + 'hidden': 'False', + 'user__id': str(member.id) + } + ) + + total_infractions = len(infractions) + active_infractions = sum(infraction['active'] for infraction in infractions) + + infraction_output = f"**Infractions**\nTotal: {total_infractions}\nActive: {active_infractions}" + + return infraction_output + + async def expanded_user_infraction_counts(self, member: Member) -> str: + """ + Gets expanded infraction counts for the given `member`. + + The counts will be split by infraction type and the number of active infractions for each type will indicated + in the output as well. + """ + infractions = await self.bot.api_client.get( + 'bot/infractions', + params={ + 'user__id': str(member.id) + } + ) + + infraction_output = ["**Infractions**"] + if not infractions: + infraction_output.append("This user has never received an infraction.") + else: + # Count infractions split by `type` and `active` status for this user + infraction_types = set() + infraction_counter = defaultdict(int) + for infraction in infractions: + infraction_type = infraction["type"] + infraction_active = 'active' if infraction["active"] else 'inactive' + + infraction_types.add(infraction_type) + infraction_counter[f"{infraction_active} {infraction_type}"] += 1 + + # Format the output of the infraction counts + for infraction_type in sorted(infraction_types): + active_count = infraction_counter[f"active {infraction_type}"] + total_count = active_count + infraction_counter[f"inactive {infraction_type}"] + + line = f"{infraction_type.capitalize()}s: {total_count}" + if active_count: + line += f" ({active_count} active)" + + infraction_output.append(line) + + return "\n".join(infraction_output) + + async def user_nomination_counts(self, member: Member) -> str: + """Gets the active and historical nomination counts for the given `member`.""" + nominations = await self.bot.api_client.get( + 'bot/nominations', + params={ + 'user__id': str(member.id) + } + ) + + output = ["**Nominations**"] + + if not nominations: + output.append("This user has never been nominated.") + else: + count = len(nominations) + is_currently_nominated = any(nomination["active"] for nomination in nominations) + nomination_noun = "nomination" if count == 1 else "nominations" + + if is_currently_nominated: + output.append(f"This user is **currently** nominated ({count} {nomination_noun} in total).") + else: + output.append(f"This user has {count} historical {nomination_noun}, but is currently not nominated.") + + return "\n".join(output) def format_fields(self, mapping: Mapping[str, Any], field_width: Optional[int] = None) -> str: """Format a mapping to be readable to a human.""" @@ -268,9 +340,9 @@ class Information(Cog): # remove trailing whitespace return out.rstrip() - @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=STAFF_ROLES) + @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_channel(Channels.bot, bypass_roles=STAFF_ROLES) + @in_channel(constants.Channels.bot, bypass_roles=constants.STAFF_ROLES) async def raw(self, ctx: Context, *, message: discord.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 diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 9bbd35a91..5c34541d8 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -7,7 +7,11 @@ import discord from bot import constants from bot.cogs import information -from tests.helpers import AsyncMock, MockBot, MockContext, MockGuild, MockMember, MockRole +from bot.decorators import InChannelCheckFailure +from tests import helpers + + +COG_PATH = "bot.cogs.information.Information" class InformationCogTests(unittest.TestCase): @@ -15,22 +19,22 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = MockRole(name="Moderator", role_id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", role_id=constants.Roles.moderator) def setUp(self): """Sets up fresh objects for each test.""" - self.bot = MockBot() + self.bot = helpers.MockBot() self.cog = information.Information(self.bot) - self.ctx = MockContext() + self.ctx = helpers.MockContext() self.ctx.author.roles.append(self.moderator_role) def test_roles_command_command(self): """Test if the `role_info` command correctly returns the `moderator_role`.""" self.ctx.guild.roles.append(self.moderator_role) - self.cog.roles_info.can_run = AsyncMock() + self.cog.roles_info.can_run = helpers.AsyncMock() self.cog.roles_info.can_run.return_value = True coroutine = self.cog.roles_info.callback(self.cog, self.ctx) @@ -48,7 +52,7 @@ class InformationCogTests(unittest.TestCase): def test_role_info_command(self): """Tests the `role info` command.""" - dummy_role = MockRole( + dummy_role = helpers.MockRole( name="Dummy", role_id=112233445566778899, colour=discord.Colour.blurple(), @@ -57,7 +61,7 @@ class InformationCogTests(unittest.TestCase): permissions=discord.Permissions(0) ) - admin_role = MockRole( + admin_role = helpers.MockRole( name="Admins", role_id=998877665544332211, colour=discord.Colour.red(), @@ -68,7 +72,7 @@ class InformationCogTests(unittest.TestCase): self.ctx.guild.roles.append([dummy_role, admin_role]) - self.cog.role_info.can_run = AsyncMock() + self.cog.role_info.can_run = helpers.AsyncMock() self.cog.role_info.can_run.return_value = True coroutine = self.cog.role_info.callback(self.cog, self.ctx, dummy_role, admin_role) @@ -99,7 +103,7 @@ class InformationCogTests(unittest.TestCase): def test_server_info_command(self, time_since_patch): time_since_patch.return_value = '2 days ago' - self.ctx.guild = MockGuild( + self.ctx.guild = helpers.MockGuild( features=('lemons', 'apples'), region="The Moon", roles=[self.moderator_role], @@ -121,10 +125,10 @@ class InformationCogTests(unittest.TestCase): ) ], members=[ - *(MockMember(status='online') for _ in range(2)), - *(MockMember(status='idle') for _ in range(1)), - *(MockMember(status='dnd') for _ in range(4)), - *(MockMember(status='offline') for _ in range(3)), + *(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)), ], member_count=1_234, icon_url='a-lemon.jpg', @@ -162,3 +166,417 @@ class InformationCogTests(unittest.TestCase): ) ) self.assertEqual(embed.thumbnail.url, 'a-lemon.jpg') + + +class UserInfractionHelperMethodTests(unittest.TestCase): + """Tests for the helper methods of the `!user` command.""" + + def setUp(self): + """Common set-up steps done before for each test.""" + self.bot = helpers.MockBot() + self.bot.api_client.get = helpers.AsyncMock() + self.cog = information.Information(self.bot) + self.member = helpers.MockMember(user_id=1234) + + def test_user_command_helper_method_get_requests(self): + """The helper methods should form the correct get requests.""" + test_values = ( + { + "helper_method": self.cog.basic_user_infraction_counts, + "expected_args": ("bot/infractions", {'hidden': 'False', 'user__id': str(self.member.id)}), + }, + { + "helper_method": self.cog.expanded_user_infraction_counts, + "expected_args": ("bot/infractions", {'user__id': str(self.member.id)}), + }, + { + "helper_method": self.cog.user_nomination_counts, + "expected_args": ("bot/nominations", {'user__id': str(self.member.id)}), + }, + ) + + for test_value in test_values: + helper_method = test_value["helper_method"] + endpoint, params = test_value["expected_args"] + + with self.subTest(method=helper_method, endpoint=endpoint, params=params): + asyncio.run(helper_method(self.member)) + self.bot.api_client.get.assert_called_once_with(endpoint, params=params) + self.bot.api_client.get.reset_mock() + + def _method_subtests(self, method, test_values, default_header): + """Helper method that runs the subtests for the different helper methods.""" + for test_value in test_values: + api_response = test_value["api response"] + expected_lines = test_value["expected_lines"] + + with self.subTest(method=method, api_response=api_response, expected_lines=expected_lines): + self.bot.api_client.get.return_value = api_response + + expected_output = "\n".join(default_header + expected_lines) + actual_output = asyncio.run(method(self.member)) + + self.assertEqual(expected_output, actual_output) + + def test_basic_user_infraction_counts_returns_correct_strings(self): + """The method should correctly list both the total and active number of non-hidden infractions.""" + test_values = ( + # No infractions means zero counts + { + "api response": [], + "expected_lines": ["Total: 0", "Active: 0"], + }, + # Simple, single-infraction dictionaries + { + "api response": [{"type": "ban", "active": True}], + "expected_lines": ["Total: 1", "Active: 1"], + }, + { + "api response": [{"type": "ban", "active": False}], + "expected_lines": ["Total: 1", "Active: 0"], + }, + # Multiple infractions with various `active` status + { + "api response": [ + {"type": "ban", "active": True}, + {"type": "kick", "active": False}, + {"type": "ban", "active": True}, + {"type": "ban", "active": False}, + ], + "expected_lines": ["Total: 4", "Active: 2"], + }, + ) + + header = ["**Infractions**"] + + self._method_subtests(self.cog.basic_user_infraction_counts, test_values, header) + + def test_expanded_user_infraction_counts_returns_correct_strings(self): + """The method should correctly list the total and active number of all infractions split by infraction type.""" + test_values = ( + { + "api response": [], + "expected_lines": ["This user has never received an infraction."], + }, + # Shows non-hidden inactive infraction as expected + { + "api response": [{"type": "kick", "active": False, "hidden": False}], + "expected_lines": ["Kicks: 1"], + }, + # Shows non-hidden active infraction as expected + { + "api response": [{"type": "mute", "active": True, "hidden": False}], + "expected_lines": ["Mutes: 1 (1 active)"], + }, + # Shows hidden inactive infraction as expected + { + "api response": [{"type": "superstar", "active": False, "hidden": True}], + "expected_lines": ["Superstars: 1"], + }, + # Shows hidden active infraction as expected + { + "api response": [{"type": "ban", "active": True, "hidden": True}], + "expected_lines": ["Bans: 1 (1 active)"], + }, + # Correctly displays tally of multiple infractions of mixed properties in alphabetical order + { + "api response": [ + {"type": "kick", "active": False, "hidden": True}, + {"type": "ban", "active": True, "hidden": True}, + {"type": "superstar", "active": True, "hidden": True}, + {"type": "mute", "active": True, "hidden": True}, + {"type": "ban", "active": False, "hidden": False}, + {"type": "note", "active": False, "hidden": True}, + {"type": "note", "active": False, "hidden": True}, + {"type": "warn", "active": False, "hidden": False}, + {"type": "note", "active": False, "hidden": True}, + ], + "expected_lines": [ + "Bans: 2 (1 active)", + "Kicks: 1", + "Mutes: 1 (1 active)", + "Notes: 3", + "Superstars: 1 (1 active)", + "Warns: 1", + ], + }, + ) + + header = ["**Infractions**"] + + self._method_subtests(self.cog.expanded_user_infraction_counts, test_values, header) + + def test_user_nomination_counts_returns_correct_strings(self): + """The method should list the number of active and historical nominations for the user.""" + test_values = ( + { + "api response": [], + "expected_lines": ["This user has never been nominated."], + }, + { + "api response": [{'active': True}], + "expected_lines": ["This user is **currently** nominated (1 nomination in total)."], + }, + { + "api response": [{'active': True}, {'active': False}], + "expected_lines": ["This user is **currently** nominated (2 nominations in total)."], + }, + { + "api response": [{'active': False}], + "expected_lines": ["This user has 1 historical nomination, but is currently not nominated."], + }, + { + "api response": [{'active': False}, {'active': False}], + "expected_lines": ["This user has 2 historical nominations, but is currently not nominated."], + }, + + ) + + header = ["**Nominations**"] + + self._method_subtests(self.cog.user_nomination_counts, test_values, header) + + +@unittest.mock.patch("bot.cogs.information.time_since", new=unittest.mock.MagicMock(return_value="1 year ago")) +@unittest.mock.patch("bot.cogs.information.constants.MODERATION_CHANNELS", new=[50]) +class UserEmbedTests(unittest.TestCase): + """Tests for the creation of the `!user` embed.""" + + def setUp(self): + """Common set-up steps done before for each test.""" + self.bot = helpers.MockBot() + self.bot.api_client.get = helpers.AsyncMock() + self.cog = information.Information(self.bot) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_string_representation_of_user_in_title_if_nick_is_not_available(self): + """The embed should use the string representation of the user if they don't have a nick.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + user = helpers.MockMember() + user.nick = None + user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.title, "Mr. Hemlock") + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_nick_in_title_if_available(self): + """The embed should use the nick if it's available.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + user = helpers.MockMember() + user.nick = "Cat lover" + user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.title, "Cat lover (Mr. Hemlock)") + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_ignores_everyone_role(self): + """Created `!user` embeds should not contain mention of the @everyone-role.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + admins_role = helpers.MockRole('Admins') + admins_role.colour = 100 + + # A `MockMember` has the @Everyone role by default; we add the Admins to that. + user = helpers.MockMember(roles=[admins_role], top_role=admins_role) + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertIn("&Admins", embed.description) + self.assertNotIn("&Everyone", embed.description) + + @unittest.mock.patch(f"{COG_PATH}.expanded_user_infraction_counts", new_callable=helpers.AsyncMock) + @unittest.mock.patch(f"{COG_PATH}.user_nomination_counts", new_callable=helpers.AsyncMock) + def test_create_user_embed_expanded_information_in_moderation_channels(self, nomination_counts, infraction_counts): + """The embed should contain expanded infractions and nomination info in mod channels.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=50)) + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + infraction_counts.return_value = "expanded infractions info" + nomination_counts.return_value = "nomination info" + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + infraction_counts.assert_called_once_with(user) + nomination_counts.assert_called_once_with(user) + + self.assertEqual( + textwrap.dedent(f""" + **User Information** + Created: {"1 year ago"} + Profile: {user.mention} + ID: {user.id} + + **Member Information** + Joined: {"1 year ago"} + Roles: &Moderators + + expanded infractions info + + nomination info + """).strip(), + embed.description + ) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new_callable=helpers.AsyncMock) + def test_create_user_embed_basic_information_outside_of_moderation_channels(self, infraction_counts): + """The embed should contain only basic infraction data outside of mod channels.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=100)) + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + infraction_counts.return_value = "basic infractions info" + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + infraction_counts.assert_called_once_with(user) + + self.assertEqual( + textwrap.dedent(f""" + **User Information** + Created: {"1 year ago"} + Profile: {user.mention} + ID: {user.id} + + **Member Information** + Joined: {"1 year ago"} + Roles: &Moderators + + basic infractions info + """).strip(), + embed.description + ) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_top_role_colour_when_user_has_roles(self): + """The embed should be created with the colour of the top role, if a top role is available.""" + ctx = helpers.MockContext() + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.colour, discord.Colour(moderators_role.colour)) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_blurple_colour_when_user_has_no_roles(self): + """The embed should be created with a blurple colour if the user has no assigned roles.""" + ctx = helpers.MockContext() + + user = helpers.MockMember(user_id=217) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.colour, discord.Colour.blurple()) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_png_format_of_user_avatar_as_thumbnail(self): + """The embed thumbnail should be set to the user's avatar in `png` format.""" + ctx = helpers.MockContext() + + user = helpers.MockMember(user_id=217) + user.avatar_url_as.return_value = "avatar url" + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + user.avatar_url_as.assert_called_once_with(format="png") + self.assertEqual(embed.thumbnail.url, "avatar url") + + +@unittest.mock.patch("bot.cogs.information.constants") +class UserCommandTests(unittest.TestCase): + """Tests for the `!user` command.""" + + def setUp(self): + """Set up steps executed before each test is run.""" + self.bot = helpers.MockBot() + self.cog = information.Information(self.bot) + + self.moderator_role = helpers.MockRole("Moderators", role_id=2, position=10) + self.flautist_role = helpers.MockRole("Flautists", role_id=3, position=2) + self.bassist_role = helpers.MockRole("Bassists", role_id=4, position=3) + + self.author = helpers.MockMember(user_id=1, name="syntaxaire") + self.moderator = helpers.MockMember(user_id=2, name="riffautae", roles=[self.moderator_role]) + self.target = helpers.MockMember(user_id=3, name="__fluzz__") + + def test_regular_member_cannot_target_another_member(self, constants): + """A regular user should not be able to use `!user` targeting another user.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + + ctx = helpers.MockContext(author=self.author) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) + + ctx.send.assert_called_once_with("You may not use this command on users other than yourself.") + + def test_regular_member_cannot_use_command_outside_of_bot_commands(self, constants): + """A regular user should not be able to use this command outside of bot-commands.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=100)) + + msg = "Sorry, but you may only use this command within <#50>." + with self.assertRaises(InChannelCheckFailure, msg=msg): + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_regular_user_may_use_command_in_bot_commands_channel(self, create_embed, constants): + """A regular user should be allowed to use `!user` targeting themselves in bot-commands.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + create_embed.assert_called_once_with(ctx, self.author) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_regular_user_can_explicitly_target_themselves(self, create_embed, constants): + """A user should target itself with `!user` when a `user` argument was not provided.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author)) + + create_embed.assert_called_once_with(ctx, self.author) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_staff_members_can_bypass_channel_restriction(self, create_embed, constants): + """Staff members should be able to bypass the bot-commands channel restriction.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=200)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + create_embed.assert_called_once_with(ctx, self.moderator) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_moderators_can_target_another_member(self, create_embed, constants): + """A moderator should be able to use `!user` targeting another user.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + constants.STAFF_ROLES = [self.moderator_role.id] + + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) + + create_embed.assert_called_once_with(ctx, self.target) + ctx.send.assert_called_once() -- cgit v1.2.3 From eee3d54318b3edd5eeb7229f4175323b139e754a Mon Sep 17 00:00:00 2001 From: Dan Herrera Date: Thu, 31 Oct 2019 21:59:50 -0400 Subject: Use timedelta to correctly calculate next midnight --- bot/cogs/reddit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index f947a7d78..0d06e9c26 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -2,7 +2,7 @@ import asyncio import logging import random import textwrap -from datetime import datetime +from datetime import datetime, timedelta from typing import List from discord import Colour, Embed, TextChannel @@ -130,7 +130,8 @@ class Reddit(Cog): """Post the top 5 posts daily, and the top 5 posts weekly.""" # once we upgrade to d.py 1.3 this can be removed and the loop can use the `time=datetime.time.min` parameter now = datetime.utcnow() - midnight_tomorrow = now.replace(day=now.day + 1, hour=0, minute=0, second=0) + tomorrow = now + timedelta(days=1) + midnight_tomorrow = tomorrow.replace(hour=0, minute=0, second=0) seconds_until = (midnight_tomorrow - now).total_seconds() await asyncio.sleep(seconds_until) -- cgit v1.2.3 From d1b35becef79540954adec4078ce4a2a47b34cfa Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Fri, 1 Nov 2019 20:49:10 +0100 Subject: Write tests for `bot.utils`. Closes #604. --- tests/bot/test_utils.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tests/bot/test_utils.py diff --git a/tests/bot/test_utils.py b/tests/bot/test_utils.py new file mode 100644 index 000000000..0a32b8411 --- /dev/null +++ b/tests/bot/test_utils.py @@ -0,0 +1,49 @@ +import unittest + +from bot import utils + + +class CaseInsensitiveDictTests(unittest.TestCase): + """Tests for the `CaseInsensitiveDict` container.""" + + def test_case_insensitive_key_access(self): + """Tests case insensitive key access and storage.""" + instance = utils.CaseInsensitiveDict() + + key = 'LEMON' + value = 'trees' + + instance[key] = value + self.assertIn(key, instance) + self.assertEqual(instance.get(key), value) + self.assertEqual(instance.pop(key), value) + + instance.setdefault(key, value) + del instance[key] + self.assertNotIn(key, instance) + + def test_initialization_from_kwargs(self): + """Tests creating the dictionary from keyword arguments.""" + instance = utils.CaseInsensitiveDict({'FOO': 'bar'}) + self.assertEqual(instance['foo'], 'bar') + + def test_update_from_other_mapping(self): + """Tests updating the dictionary from another mapping.""" + instance = utils.CaseInsensitiveDict() + instance.update({'FOO': 'bar'}) + self.assertEqual(instance['foo'], 'bar') + + +class ChunkTests(unittest.TestCase): + """Tests the `chunk` method.""" + + def test_empty_chunking(self): + """Tests chunking on an empty iterable.""" + generator = utils.chunks(iterable=[], size=5) + self.assertEqual(list(generator), []) + + def test_list_chunking(self): + """Tests chunking a non-empty list.""" + iterable = [1, 2, 3, 4, 5] + generator = utils.chunks(iterable=iterable, size=2) + self.assertEqual(list(generator), [[1, 2], [3, 4], [5]]) -- cgit v1.2.3 From 38d9e705c77003008f1562188cba079a4245061b Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:01:45 +0100 Subject: Add unit test for links antispam rule --- tests/bot/rules/test_links.py | 93 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 tests/bot/rules/test_links.py diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py new file mode 100644 index 000000000..f71a8e6bb --- /dev/null +++ b/tests/bot/rules/test_links.py @@ -0,0 +1,93 @@ +import unittest +from typing import List, NamedTuple, Tuple + +from bot.rules import links +from tests.helpers import async_test + + +class FakeMessage(NamedTuple): + author: str + content: str + + +class Case(NamedTuple): + recent_messages: List[FakeMessage] + relevant_messages: Tuple[FakeMessage] + culprit: Tuple[str] + total_links: int + + +def msg(author: str, total_links: int) -> FakeMessage: + """Makes a message with `total_links` links.""" + content = " ".join(["https://pydis.com"] * total_links) + return FakeMessage(author=author, content=content) + + +class LinksTests(unittest.TestCase): + """Tests applying the `links` rule.""" + + def setUp(self): + self.config = { + "max": 2, + "interval": 10 + } + + @async_test + async def test_links_within_limit(self): + """Messages with an allowed amount of links.""" + cases = ( + [msg("bob", 0)], + [msg("bob", 2)], + [msg("bob", 3)], + [msg("bob", 3), msg("alice", 3)] + ) + + for recent_messages in cases: + last_message = recent_messages[0] + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config + ): + self.assertIsNone( + await links.apply(last_message, recent_messages, self.config) + ) + + @async_test + async def test_links_exceeding_limit(self): + """Messages with a a higher than allowed amount of links.""" + cases = ( + Case( + [msg("bob", 1), msg("bob", 2)], + (msg("bob", 1), msg("bob", 2)), + ("bob",), + 3 + ), + Case( + [msg("alice", 2), msg("bob", 3), msg("alice", 1)], + (msg("alice", 2), msg("alice", 1)), + ("alice",), + 3 + ) + ) + + for recent_messages, relevant_messages, culprit, total_links in cases: + last_message = recent_messages[0] + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + relevant_messages=relevant_messages, + culprit=culprit, + total_links=total_links, + config=self.config + ): + desired_output = ( + f"sent {total_links} links in {self.config['interval']}s", + culprit, + relevant_messages + ) + self.assertTupleEqual( + await links.apply(last_message, recent_messages, self.config), + desired_output + ) -- cgit v1.2.3 From a21670459953599c8f13286595520e033da4199a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:24:20 +0100 Subject: Add two more test cases for links rule unit test --- tests/bot/rules/test_links.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f71a8e6bb..f043495cf 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -39,6 +39,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 0)], [msg("bob", 2)], [msg("bob", 3)], + [msg("bob", 1), msg("bob", 1)], [msg("bob", 3), msg("alice", 3)] ) @@ -63,6 +64,12 @@ class LinksTests(unittest.TestCase): ("bob",), 3 ), + Case( + [msg("alice", 1), msg("alice", 1), msg("alice", 1)], + (msg("alice", 1), msg("alice", 1), msg("alice", 1)), + ("alice",), + 3 + ), Case( [msg("alice", 2), msg("bob", 3), msg("alice", 1)], (msg("alice", 2), msg("alice", 1)), -- cgit v1.2.3 From 9e16b0cf17e93295167956d31ad9f92396ca3402 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:19:07 +0100 Subject: Annotate unclear test cases with inline comments --- tests/bot/rules/test_links.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f043495cf..a04b90c13 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -38,9 +38,9 @@ class LinksTests(unittest.TestCase): cases = ( [msg("bob", 0)], [msg("bob", 2)], - [msg("bob", 3)], + [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] + [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From f6ed29c8692759dc0c8b003ebf2d4ce3d5ca6ed9 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:34:46 +0100 Subject: Adjust case to only test a single aspect --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index a04b90c13..95f107f91 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -40,7 +40,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 2)], [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count + [msg("bob", 2), msg("alice", 2)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From 9e825ed657cebc9f47208af7dc5fa46f31d9ef41 Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Sat, 2 Nov 2019 11:04:57 +0100 Subject: Use `casefold` in some cases. --- tests/bot/test_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/bot/test_utils.py b/tests/bot/test_utils.py index 0a32b8411..58ae2a81a 100644 --- a/tests/bot/test_utils.py +++ b/tests/bot/test_utils.py @@ -16,7 +16,10 @@ class CaseInsensitiveDictTests(unittest.TestCase): instance[key] = value self.assertIn(key, instance) self.assertEqual(instance.get(key), value) - self.assertEqual(instance.pop(key), value) + self.assertEqual(instance.get(key.casefold()), value) + self.assertEqual(instance.pop(key.casefold()), value) + self.assertNotIn(key, instance) + self.assertNotIn(key.casefold(), instance) instance.setdefault(key, value) del instance[key] -- cgit v1.2.3 From efe592cc0420f325ab266afc822b8d4b8135d467 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sat, 2 Nov 2019 17:26:50 +0100 Subject: Do not cut off description in code blocks --- bot/cogs/doc.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 8b81b3053..4a095fa51 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -284,7 +284,13 @@ class Doc(commands.Cog): if len(description) > 1000: shortened = description[:1000] last_paragraph_end = shortened.rfind('\n\n') - description = description[:last_paragraph_end] + f"... [read more]({permalink})" + description = description[:last_paragraph_end] + + # If there is an incomplete code block, cut it out + if description.count("```") % 2: + codeblock_start = description.rfind('```py') + description = description[:codeblock_start].rstrip() + description += f"... [read more]({permalink})" description = WHITESPACE_AFTER_NEWLINES_RE.sub('', description) -- cgit v1.2.3 From 82e1f3764ba0d102ede007ba6352406cfe3fb82a Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sat, 2 Nov 2019 17:37:42 +0100 Subject: Get symbol description by searching for a dd tag instead of traversing the siblings --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 4a095fa51..96f737c03 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -260,7 +260,7 @@ class Doc(commands.Cog): if tag not in UNWANTED_SIGNATURE_SYMBOLS: signature_buffer.append(tag.replace('\\', '')) signature = ''.join(signature_buffer) - description = str(symbol_heading.next_sibling.next_sibling).replace('¶', '') + description = str(symbol_heading.find_next_sibling("dd")).replace('¶', '') return signature, description -- cgit v1.2.3 From ae8c862a353ddc10593d36d557fc7215232baf5b Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sat, 2 Nov 2019 17:44:02 +0100 Subject: Get up to 3 signatures of a symbol --- bot/cogs/doc.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 96f737c03..2987f7245 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -36,7 +36,7 @@ NO_OVERRIDE_GROUPS = ( NO_OVERRIDE_PACKAGES = ( "Python", ) -UNWANTED_SIGNATURE_SYMBOLS = ('[source]', '¶') +UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") WHITESPACE_AFTER_NEWLINES_RE = re.compile(r"(?<=\n\n)(\s+)") @@ -218,7 +218,7 @@ class Doc(commands.Cog): ] await asyncio.gather(*coros) - async def get_symbol_html(self, symbol: str) -> Optional[Tuple[str, str]]: + async def get_symbol_html(self, symbol: str) -> Optional[Tuple[list, str]]: """ Given a Python symbol, return its signature and description. @@ -239,7 +239,7 @@ class Doc(commands.Cog): symbol_id = url.split('#')[-1] soup = BeautifulSoup(html, 'lxml') symbol_heading = soup.find(id=symbol_id) - signature_buffer = [] + signatures = [] if symbol_heading is None: return None @@ -253,16 +253,14 @@ class Doc(commands.Cog): description = ''.join(str(paragraph) for paragraph in info_paragraphs).replace('¶', '') else: - # Traverse the tags of the signature header and ignore any - # unwanted symbols from it. Add all of it to a temporary buffer. - - for tag in symbol_heading.strings: - if tag not in UNWANTED_SIGNATURE_SYMBOLS: - signature_buffer.append(tag.replace('\\', '')) - signature = ''.join(signature_buffer) + # Get text of up to 3 signatures, remove unwanted symbols + for element in [symbol_heading] + symbol_heading.find_next_siblings("dt", limit=2): + signature = UNWANTED_SIGNATURE_SYMBOLS_RE.sub("", element.text) + if signature: + signatures.append(signature) description = str(symbol_heading.find_next_sibling("dd")).replace('¶', '') - return signature, description + return signatures, description @async_cache(arg_offset=1) async def get_symbol_embed(self, symbol: str) -> Optional[discord.Embed]: @@ -275,7 +273,7 @@ class Doc(commands.Cog): if scraped_html is None: return None - signature = scraped_html[0] + signatures = scraped_html[0] permalink = self.inventories[symbol] description = markdownify(scraped_html[1]) @@ -294,18 +292,18 @@ class Doc(commands.Cog): description = WHITESPACE_AFTER_NEWLINES_RE.sub('', description) - if signature is None: + if signatures is None: # If symbol is a module, don't show signature. embed_description = description - elif not signature: + elif not signatures: # It's some "meta-page", for example: # https://docs.djangoproject.com/en/dev/ref/views/#module-django.views embed_description = "This appears to be a generic page not tied to a specific symbol." else: - signature = textwrap.shorten(signature, 500) - embed_description = f"```py\n{signature}```{description}" + embed_description = "".join(f"```py\n{textwrap.shorten(signature, 500)}```" for signature in signatures) + embed_description += description embed = discord.Embed( title=f'`{symbol}`', -- cgit v1.2.3 From 1aed2e4f4996f5546652bbb26e8fbf403e28aac4 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sat, 2 Nov 2019 18:28:04 +0100 Subject: Improve module description searching --- bot/cogs/doc.py | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 2987f7245..30a14f26c 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -9,7 +9,7 @@ from typing import Any, Callable, Optional, Tuple import discord from bs4 import BeautifulSoup -from bs4.element import PageElement +from bs4.element import PageElement, Tag from discord.errors import NotFound from discord.ext import commands from markdownify import MarkdownConverter @@ -37,6 +37,16 @@ NO_OVERRIDE_PACKAGES = ( "Python", ) UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") +SEARCH_END_TAG_ATTRS = ( + "data", + "function", + "class", + "exception", + "seealso", + "section", + "rubric", + "sphinxsidebar", +) WHITESPACE_AFTER_NEWLINES_RE = re.compile(r"(?<=\n\n)(\s+)") @@ -245,12 +255,21 @@ class Doc(commands.Cog): return None if symbol_id == f"module-{symbol}": - # Get all paragraphs until the first div after the section div - # if searched symbol is a module. - trailing_div = symbol_heading.findNext("div") - info_paragraphs = trailing_div.find_previous_siblings("p")[::-1] - signature = None - description = ''.join(str(paragraph) for paragraph in info_paragraphs).replace('¶', '') + search_html = str(soup) + # Get page content from the module headerlink to the + # first tag that has its class in `SEARCH_END_TAG_ATTRS` + start_tag = symbol_heading.find("a", attrs={"class": "headerlink"}) + if start_tag is None: + return [], "" + + end_tag = start_tag.find_next(self._match_end_tag) + if end_tag is None: + return [], "" + + description_start_index = search_html.find(str(start_tag.parent)) + len(str(start_tag.parent)) + description_end_index = search_html.find(str(end_tag)) + description = search_html[description_start_index:description_end_index].replace('¶', '') + signatures = None else: # Get text of up to 3 signatures, remove unwanted symbols @@ -422,6 +441,15 @@ class Doc(commands.Cog): await self.refresh_inventory() await ctx.send(f"Successfully deleted `{package_name}` and refreshed inventory.") + @staticmethod + def _match_end_tag(tag: Tag) -> bool: + """Matches `tag` if its class value is in `SEARCH_END_TAG_ATTRS` or the tag is table.""" + for attr in SEARCH_END_TAG_ATTRS: + if attr in tag.get("class", ()): + return True + + return tag.name == "table" + def setup(bot: commands.Bot) -> None: """Doc cog load.""" -- cgit v1.2.3 From 3140b01bff9c4912b9f89589e3b3f200dbad99ee Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 3 Nov 2019 18:36:16 +0100 Subject: Handle exceptions when fetching inventories --- bot/cogs/doc.py | 88 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 31 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 30a14f26c..55b69e9a4 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -13,8 +13,9 @@ from bs4.element import PageElement, Tag from discord.errors import NotFound from discord.ext import commands from markdownify import MarkdownConverter -from requests import ConnectionError +from requests import ConnectTimeout, ConnectionError, HTTPError from sphinx.ext import intersphinx +from urllib3.exceptions import ProtocolError from bot.constants import MODERATION_ROLES, RedirectOutput from bot.converters import ValidPythonIdentifier, ValidURL @@ -36,6 +37,7 @@ NO_OVERRIDE_GROUPS = ( NO_OVERRIDE_PACKAGES = ( "Python", ) +FAILED_REQUEST_RETRY_AMOUNT = 3 UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") SEARCH_END_TAG_ATTRS = ( "data", @@ -173,36 +175,37 @@ class Doc(commands.Cog): """ self.base_urls[package_name] = base_url - fetch_func = functools.partial(intersphinx.fetch_inventory, config, '', inventory_url) - for group, value in (await self.bot.loop.run_in_executor(None, fetch_func)).items(): - # Each value has a bunch of information in the form - # `(package_name, version, relative_url, ???)`, and we only - # need the package_name and the relative documentation URL. - for symbol, (package_name, _, relative_doc_url, _) in value.items(): - absolute_doc_url = base_url + relative_doc_url - - if symbol in self.inventories: - # get `group_name` from _:group_name - group_name = group.split(":")[1] - if (group_name in NO_OVERRIDE_GROUPS - # check if any package from `NO_OVERRIDE_PACKAGES` - # is in base URL of the symbol that would be overridden - or any(package in self.inventories[symbol].split("/", 3)[2] - for package in NO_OVERRIDE_PACKAGES)): - - symbol = f"{group_name}.{symbol}" - # if renamed `symbol` was already exists, add library name in front - if symbol in self.renamed_symbols: - # split `package_name` because of packages like Pillow that have spaces in them - symbol = f"{package_name.split()[0]}.{symbol}" - - self.inventories[symbol] = absolute_doc_url - self.renamed_symbols.add(symbol) - continue - - self.inventories[symbol] = absolute_doc_url - - log.trace(f"Fetched inventory for {package_name}.") + package = await self._fetch_inventory(inventory_url, config) + if package: + for group, value in package.items(): + # Each value has a bunch of information in the form + # `(package_name, version, relative_url, ???)`, and we only + # need the package_name and the relative documentation URL. + for symbol, (package_name, _, relative_doc_url, _) in value.items(): + absolute_doc_url = base_url + relative_doc_url + + if symbol in self.inventories: + # get `group_name` from _:group_name + group_name = group.split(":")[1] + if (group_name in NO_OVERRIDE_GROUPS + # check if any package from `NO_OVERRIDE_PACKAGES` + # is in base URL of the symbol that would be overridden + or any(package in self.inventories[symbol].split("/", 3)[2] + for package in NO_OVERRIDE_PACKAGES)): + + symbol = f"{group_name}.{symbol}" + # if renamed `symbol` was already exists, add library name in front + if symbol in self.renamed_symbols: + # split `package_name` because of packages like Pillow that have spaces in them + symbol = f"{package_name.split()[0]}.{symbol}" + + self.inventories[symbol] = absolute_doc_url + self.renamed_symbols.add(symbol) + continue + + self.inventories[symbol] = absolute_doc_url + + log.trace(f"Fetched inventory for {package_name}.") async def refresh_inventory(self) -> None: """Refresh internal documentation inventory.""" @@ -441,6 +444,29 @@ class Doc(commands.Cog): await self.refresh_inventory() await ctx.send(f"Successfully deleted `{package_name}` and refreshed inventory.") + async def _fetch_inventory(self, inventory_url: str, config: SphinxConfiguration) -> Optional[dict]: + """Get and return inventory from `inventory_url`. If fetching fails, return None.""" + fetch_func = functools.partial(intersphinx.fetch_inventory, config, '', inventory_url) + for retry in range(1, FAILED_REQUEST_RETRY_AMOUNT+1): + try: + package = await self.bot.loop.run_in_executor(None, fetch_func) + except ConnectTimeout: + log.error(f"Fetching of inventory {inventory_url} timed out," + f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})") + except ProtocolError: + log.error(f"Connection lost while fetching inventory {inventory_url}," + f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})") + except HTTPError as e: + log.error(f"Fetching of inventory {inventory_url} failed with status code {e.response.status_code}.") + return None + except ConnectionError: + log.error(f"Couldn't establish connection to inventory {inventory_url}.") + return None + else: + return package + log.error(f"Fetching of inventory {inventory_url} failed.") + return None + @staticmethod def _match_end_tag(tag: Tag) -> bool: """Matches `tag` if its class value is in `SEARCH_END_TAG_ATTRS` or the tag is table.""" -- cgit v1.2.3 From 0572b18e1986eee91c014fb38626b2fbfd571a47 Mon Sep 17 00:00:00 2001 From: Jeremiah Boby Date: Sun, 3 Nov 2019 17:49:17 +0000 Subject: Show custom statuses in !user output Resolves #647 --- bot/cogs/information.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 3a7ba0444..01d1a0994 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -9,6 +9,7 @@ import discord from discord import CategoryChannel, Colour, Embed, Member, Role, TextChannel, VoiceChannel, utils from discord.ext import commands from discord.ext.commands import Bot, BucketType, Cog, Context, command, group +from discord.utils import escape_markdown from bot.constants import Channels, Emojis, MODERATION_ROLES, STAFF_ROLES from bot.decorators import InChannelCheckFailure, in_channel, with_role @@ -180,6 +181,13 @@ class Information(Cog): # User information created = time_since(user.created_at, max_units=3) + # Custom status + custom_status = '' + for activity in user.activities: + if activity.name == 'Custom Status': + state = escape_markdown(activity.state) + custom_status = f'Status: {state}\n' + name = str(user) if user.nick: name = f"{user.nick} ({name})" @@ -217,7 +225,7 @@ class Information(Cog): Created: {created} Profile: {user.mention} ID: {user.id} - + {custom_status} **Member Information** Joined: {joined} Roles: {roles or None} -- cgit v1.2.3 From a8475f5fedb91c9e0f1c5c28c7d64aebbbef64f4 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 3 Nov 2019 20:06:15 +0100 Subject: Fix case for the python package name in `NO_OVERRIDE_PACKAGES` --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 55b69e9a4..563f83040 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -35,7 +35,7 @@ NO_OVERRIDE_GROUPS = ( "term", ) NO_OVERRIDE_PACKAGES = ( - "Python", + "python", ) FAILED_REQUEST_RETRY_AMOUNT = 3 UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") -- cgit v1.2.3 From 1b0a8c8109240615e5d9309937a434e1d29bcf24 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 3 Nov 2019 20:06:41 +0100 Subject: Comment grammar --- bot/cogs/doc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 563f83040..934cb2a6d 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -194,9 +194,9 @@ class Doc(commands.Cog): for package in NO_OVERRIDE_PACKAGES)): symbol = f"{group_name}.{symbol}" - # if renamed `symbol` was already exists, add library name in front + # If renamed `symbol` already exists, add library name in front. if symbol in self.renamed_symbols: - # split `package_name` because of packages like Pillow that have spaces in them + # Split `package_name` because of packages like Pillow that have spaces in them. symbol = f"{package_name.split()[0]}.{symbol}" self.inventories[symbol] = absolute_doc_url -- cgit v1.2.3 From 254dfbb616651f875936598c9884761921de7b76 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 3 Nov 2019 20:28:07 +0100 Subject: Make sure only signatures belonging to the symbol are fetched --- bot/cogs/doc.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 934cb2a6d..dcbcfe3ad 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -253,12 +253,12 @@ class Doc(commands.Cog): soup = BeautifulSoup(html, 'lxml') symbol_heading = soup.find(id=symbol_id) signatures = [] + search_html = str(soup) if symbol_heading is None: return None if symbol_id == f"module-{symbol}": - search_html = str(soup) # Get page content from the module headerlink to the # first tag that has its class in `SEARCH_END_TAG_ATTRS` start_tag = symbol_heading.find("a", attrs={"class": "headerlink"}) @@ -275,12 +275,13 @@ class Doc(commands.Cog): signatures = None else: + description = str(symbol_heading.find_next_sibling("dd")).replace('¶', '') + description_pos = search_html.find(description) # Get text of up to 3 signatures, remove unwanted symbols for element in [symbol_heading] + symbol_heading.find_next_siblings("dt", limit=2): signature = UNWANTED_SIGNATURE_SYMBOLS_RE.sub("", element.text) - if signature: + if signature and search_html.find(signature) < description_pos: signatures.append(signature) - description = str(symbol_heading.find_next_sibling("dd")).replace('¶', '') return signatures, description -- cgit v1.2.3 From 7fdb51b8cef9a8d7d2edf48bb2724cf9348a0f93 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Mon, 4 Nov 2019 12:15:52 +0800 Subject: Change invite filter message to accurately reflect the new rules --- bot/cogs/filtering.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index be9b95bc7..4195783f1 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -63,7 +63,7 @@ class Filtering(Cog): "content_only": True, "user_notification": Filter.notify_user_invites, "notification_msg": ( - f"Per Rule 10, your invite link has been removed. {_staff_mistake_str}\n\n" + f"Per Rule 6, your invite link has been removed. {_staff_mistake_str}\n\n" r"Our server rules can be found here: " ) }, -- cgit v1.2.3 From 4dccacaa5794751546336de1aa42cd5a9095b706 Mon Sep 17 00:00:00 2001 From: Derek Date: Mon, 4 Nov 2019 17:48:48 -0500 Subject: Reword periodic #checkpoint message --- bot/cogs/verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 5b115deaa..3d85edae6 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -31,7 +31,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! PERIODIC_PING = ( f"@everyone To verify that you have read our rules, please type `{BotConfig.prefix}accept`." - f" Ping <@&{Roles.admin}> if you encounter any problems during the verification process." + f" If you encounter any problems during the verification process, ping the <@&{Roles.admin}> role in this channel." ) -- cgit v1.2.3 From 071bc9d775a48750cc8d44236523c7ec5a30f7f9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 25 Oct 2019 17:29:28 -0700 Subject: Add logging for moderation functions --- bot/cogs/moderation/scheduler.py | 51 ++++++++++++++++++++++++++++++++++--- bot/cogs/moderation/superstarify.py | 21 +++++++++++---- bot/cogs/moderation/utils.py | 12 +++++++++ bot/utils/scheduling.py | 6 ++++- 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 7990df226..7a08fc236 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -39,6 +39,8 @@ class InfractionScheduler(Scheduler): """Schedule expiration for previous infractions.""" await self.bot.wait_until_ready() + log.trace(f"Rescheduling infractions for {self.__class__.__name__}.") + infractions = await self.bot.api_client.get( 'bot/infractions', params={'active': 'true'} @@ -59,6 +61,10 @@ class InfractionScheduler(Scheduler): # Mark as inactive if less than a minute remains. if delta < 60: + log.info( + "Infraction will be deactivated instead of re-applied " + "because less than 1 minute remains." + ) await self.deactivate_infraction(infraction) return @@ -78,6 +84,9 @@ class InfractionScheduler(Scheduler): icon = utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] expiry = infraction["expires_at"] + _id = infraction['id'] + + log.trace(f"Applying {infr_type} infraction #{_id} to {user}.") if expiry: expiry = time.format_infraction(expiry) @@ -111,10 +120,20 @@ class InfractionScheduler(Scheduler): log_content = ctx.author.mention if infraction["actor"] == self.bot.user.id: + log.trace( + f"Infraction #{_id} actor is bot; including the reason in the confirmation message." + ) + end_msg = f" (reason: {infraction['reason']})" elif ctx.channel.id not in STAFF_CHANNELS: + log.trace( + f"Infraction #{_id} context is not in a staff channel; omitting infraction count." + ) + end_msg = "" else: + log.trace(f"Fetching total infraction count for {user}.") + infractions = await self.bot.api_client.get( "bot/infractions", params={"user__id": str(user.id)} @@ -124,6 +143,7 @@ class InfractionScheduler(Scheduler): # Execute the necessary actions to apply the infraction on Discord. if action_coro: + log.trace(f"Awaiting the infraction #{_id} application action coroutine.") try: await action_coro if expiry: @@ -136,12 +156,16 @@ class InfractionScheduler(Scheduler): log_content = ctx.author.mention log_title = "failed to apply" + log.warning(f"Failed to apply {infr_type} infraction #{_id} to {user}.") + # Send a confirmation message to the invoking context. + log.trace(f"Sending infraction #{_id} confirmation message.") await ctx.send( f"{dm_result}{confirm_msg} **{infr_type}** to {user.mention}{expiry_msg}{end_msg}." ) # Send a log message to the mod log. + log.trace(f"Sending apply mod log for infraction #{_id}.") await self.mod_log.send_log_message( icon_url=icon, colour=Colours.soft_red, @@ -157,9 +181,14 @@ class InfractionScheduler(Scheduler): footer=f"ID {infraction['id']}" ) + log.info(f"Applied {infr_type} infraction #{_id} to {user}.") + async def pardon_infraction(self, ctx: Context, infr_type: str, user: MemberObject) -> None: """Prematurely end an infraction for a user and log the action in the mod log.""" + log.trace(f"Pardoning {infr_type} infraction for {user}.") + # Check the current active infraction + log.trace(f"Fetching active {infr_type} infractions for {user}.") response = await self.bot.api_client.get( 'bot/infractions', params={ @@ -170,6 +199,7 @@ class InfractionScheduler(Scheduler): ) if not response: + log.debug(f"No active {infr_type} infraction found for {user}.") await ctx.send(f":x: There's no active {infr_type} infraction for user {user.mention}.") return @@ -179,12 +209,16 @@ class InfractionScheduler(Scheduler): log_text["Member"] = f"{user.mention}(`{user.id}`)" log_text["Actor"] = str(ctx.message.author) log_content = None - footer = f"ID: {response[0]['id']}" + _id = response[0]['id'] + footer = f"ID: {_id}" # If multiple active infractions were found, mark them as inactive in the database # and cancel their expiration tasks. if len(response) > 1: - log.warning(f"Found more than one active {infr_type} infraction for user {user.id}") + log.warning( + f"Found more than one active {infr_type} infraction for user {user.id}; " + "deactivating the extra active infractions too." + ) footer = f"Infraction IDs: {', '.join(str(infr['id']) for infr in response)}" @@ -227,11 +261,16 @@ class InfractionScheduler(Scheduler): confirm_msg = ":x: failed to pardon" log_title = "pardon failed" log_content = ctx.author.mention + + log.warning(f"Failed to pardon {infr_type} infraction #{_id} for {user}.") else: confirm_msg = f":ok_hand: pardoned" log_title = "pardoned" + log.info(f"Pardoned {infr_type} infraction #{_id} for {user}.") + # Send a confirmation message to the invoking context. + log.trace(f"Sending infraction #{_id} pardon confirmation message.") await ctx.send( f"{dm_emoji}{confirm_msg} infraction **{infr_type}** for {user.mention}. " f"{log_text.get('Failure', '')}" @@ -268,7 +307,7 @@ class InfractionScheduler(Scheduler): _type = infraction["type"] _id = infraction["id"] - log.debug(f"Marking infraction #{_id} as inactive (expired).") + log.info(f"Marking infraction #{_id} as inactive (expired).") log_content = None log_text = { @@ -278,7 +317,9 @@ class InfractionScheduler(Scheduler): } try: + log.trace("Awaiting the pardon action coroutine.") returned_log = await self._pardon_action(infraction) + if returned_log is not None: log_text = {**log_text, **returned_log} # Merge the logs together else: @@ -296,6 +337,8 @@ class InfractionScheduler(Scheduler): # Check if the user is currently being watched by Big Brother. try: + log.trace(f"Determining if user {user_id} is currently being watched by Big Brother.") + active_watch = await self.bot.api_client.get( "bot/infractions", params={ @@ -312,6 +355,7 @@ class InfractionScheduler(Scheduler): try: # Mark infraction as inactive in the database. + log.trace(f"Marking infraction #{_id} as inactive in the database.") await self.bot.api_client.patch( f"bot/infractions/{_id}", json={"active": False} @@ -335,6 +379,7 @@ class InfractionScheduler(Scheduler): if send_log: log_title = f"expiration failed" if "Failure" in log_text else "expired" + log.trace(f"Sending deactivation mod log for infraction #{_id}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS[_type][1], colour=Colours.soft_green, diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index c66222e5a..9ab870823 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -34,8 +34,8 @@ class Superstarify(InfractionScheduler, Cog): return # User didn't change their nickname. Abort! log.trace( - f"{before.display_name} is trying to change their nickname to {after.display_name}. " - "Checking if the user is in superstar-prison..." + f"{before} ({before.display_name}) is trying to change their nickname to " + f"{after.display_name}. Checking if the user is in superstar-prison..." ) active_superstarifies = await self.bot.api_client.get( @@ -48,6 +48,7 @@ class Superstarify(InfractionScheduler, Cog): ) if not active_superstarifies: + log.trace(f"{before} has no active superstar infractions.") return infraction = active_superstarifies[0] @@ -132,15 +133,17 @@ class Superstarify(InfractionScheduler, Cog): # Post the infraction to the API reason = reason or f"old nick: {member.display_name}" infraction = await utils.post_infraction(ctx, member, "superstar", reason, duration) + _id = infraction["id"] old_nick = member.display_name - forced_nick = self.get_nick(infraction["id"], member.id) + forced_nick = self.get_nick(_id, member.id) expiry_str = format_infraction(infraction["expires_at"]) # Apply the infraction and schedule the expiration task. + log.debug(f"Changing nickname of {member} to {forced_nick}.") self.mod_log.ignore(constants.Event.member_update, member.id) await member.edit(nick=forced_nick, reason=reason) - self.schedule_task(ctx.bot.loop, infraction["id"], infraction) + self.schedule_task(ctx.bot.loop, _id, infraction) # Send a DM to the user to notify them of their new infraction. await utils.notify_infraction( @@ -152,6 +155,7 @@ class Superstarify(InfractionScheduler, Cog): ) # Send an embed with the infraction information to the invoking context. + log.trace(f"Sending superstar #{_id} embed.") embed = Embed( title="Congratulations!", colour=constants.Colours.soft_orange, @@ -167,6 +171,7 @@ class Superstarify(InfractionScheduler, Cog): await ctx.send(embed=embed) # Log to the mod log channel. + log.trace(f"Sending apply mod log for superstar #{_id}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS["superstar"][0], colour=Colour.gold(), @@ -180,7 +185,7 @@ class Superstarify(InfractionScheduler, Cog): Old nickname: `{old_nick}` New nickname: `{forced_nick}` """), - footer=f"ID {infraction['id']}" + footer=f"ID {_id}" ) @command(name="unsuperstarify", aliases=("release_nick", "unstar")) @@ -198,6 +203,10 @@ class Superstarify(InfractionScheduler, Cog): # Don't bother sending a notification if the user left the guild. if not user: + log.debug( + "User left the guild and therefore won't be notified about superstar " + f"{infraction['id']} pardon." + ) return {} # DM the user about the expiration. @@ -216,6 +225,8 @@ class Superstarify(InfractionScheduler, Cog): @staticmethod def get_nick(infraction_id: int, member_id: int) -> str: """Randomly select a nickname from the Superstarify nickname list.""" + log.trace(f"Choosing a random nickname for superstar #{infraction_id}.") + rng = random.Random(str(infraction_id) + str(member_id)) return rng.choice(STAR_NAMES) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 9179c0afb..325b9567a 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -37,6 +37,8 @@ def proxy_user(user_id: str) -> discord.Object: Used when a Member or User object cannot be resolved. """ + log.trace(f"Attempting to create a proxy user for the user id {user_id}.") + try: user_id = int(user_id) except ValueError: @@ -59,6 +61,8 @@ async def post_infraction( active: bool = True, ) -> t.Optional[dict]: """Posts an infraction to the API.""" + log.trace(f"Posting {infr_type} infraction for {user} to the API.") + payload = { "actor": ctx.message.author.id, "hidden": hidden, @@ -92,6 +96,8 @@ async def post_infraction( async def has_active_infraction(ctx: Context, user: MemberObject, infr_type: str) -> bool: """Checks if a user already has an active infraction of the given type.""" + log.trace(f"Checking if {user} has active infractions of type {infr_type}.") + active_infractions = await ctx.bot.api_client.get( 'bot/infractions', params={ @@ -101,12 +107,14 @@ async def has_active_infraction(ctx: Context, user: MemberObject, infr_type: str } ) if active_infractions: + log.trace(f"{user} has active infractions of type {infr_type}.") await ctx.send( f":x: According to my records, this user already has a {infr_type} infraction. " f"See infraction **#{active_infractions[0]['id']}**." ) return True else: + log.trace(f"{user} does not have active infractions of type {infr_type}.") return False @@ -118,6 +126,8 @@ async def notify_infraction( icon_url: str = Icons.token_removed ) -> bool: """DM a user about their new infraction and return True if the DM is successful.""" + log.trace(f"Sending {user} a DM about their {infr_type} infraction.") + embed = discord.Embed( description=textwrap.dedent(f""" **Type:** {infr_type.capitalize()} @@ -146,6 +156,8 @@ async def notify_pardon( icon_url: str = Icons.user_verified ) -> bool: """DM a user about their pardoned infraction and return True if the DM is successful.""" + log.trace(f"Sending {user} a DM about their pardoned infraction.") + embed = discord.Embed( description=content, colour=Colours.soft_green diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 08abd91d7..ee6c0a8e6 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -36,11 +36,15 @@ class Scheduler(metaclass=CogABCMeta): `task_data` is passed to `Scheduler._scheduled_expiration` """ if task_id in self.scheduled_tasks: + log.debug( + f"{self.cog_name}: did not schedule task #{task_id}; task was already scheduled." + ) return task: asyncio.Task = create_task(loop, self._scheduled_task(task_data)) self.scheduled_tasks[task_id] = task + log.debug(f"{self.cog_name}: scheduled task #{task_id}.") def cancel_task(self, task_id: str) -> None: """Un-schedules a task.""" @@ -51,7 +55,7 @@ class Scheduler(metaclass=CogABCMeta): return task.cancel() - log.debug(f"{self.cog_name}: Unscheduled {task_id}.") + log.debug(f"{self.cog_name}: unscheduled task #{task_id}.") del self.scheduled_tasks[task_id] -- cgit v1.2.3 From 818e48baec3564e73cc223eb91fbbfe768d73c67 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 31 Oct 2019 14:26:25 -0700 Subject: Moderation: use trailing _ instead of leading for variable names PEP 8 states the convention is a trailing underscore when used to prevent name conflicts. --- bot/cogs/moderation/scheduler.py | 54 ++++++++++++++++++------------------- bot/cogs/moderation/superstarify.py | 12 ++++----- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 7a08fc236..462c7fc7f 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -84,9 +84,9 @@ class InfractionScheduler(Scheduler): icon = utils.INFRACTION_ICONS[infr_type][0] reason = infraction["reason"] expiry = infraction["expires_at"] - _id = infraction['id'] + id_ = infraction['id'] - log.trace(f"Applying {infr_type} infraction #{_id} to {user}.") + log.trace(f"Applying {infr_type} infraction #{id_} to {user}.") if expiry: expiry = time.format_infraction(expiry) @@ -121,13 +121,13 @@ class InfractionScheduler(Scheduler): if infraction["actor"] == self.bot.user.id: log.trace( - f"Infraction #{_id} actor is bot; including the reason in the confirmation message." + f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) end_msg = f" (reason: {infraction['reason']})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( - f"Infraction #{_id} context is not in a staff channel; omitting infraction count." + f"Infraction #{id_} context is not in a staff channel; omitting infraction count." ) end_msg = "" @@ -143,7 +143,7 @@ class InfractionScheduler(Scheduler): # Execute the necessary actions to apply the infraction on Discord. if action_coro: - log.trace(f"Awaiting the infraction #{_id} application action coroutine.") + log.trace(f"Awaiting the infraction #{id_} application action coroutine.") try: await action_coro if expiry: @@ -156,16 +156,16 @@ class InfractionScheduler(Scheduler): log_content = ctx.author.mention log_title = "failed to apply" - log.warning(f"Failed to apply {infr_type} infraction #{_id} to {user}.") + log.warning(f"Failed to apply {infr_type} infraction #{id_} to {user}.") # Send a confirmation message to the invoking context. - log.trace(f"Sending infraction #{_id} confirmation message.") + log.trace(f"Sending infraction #{id_} confirmation message.") await ctx.send( f"{dm_result}{confirm_msg} **{infr_type}** to {user.mention}{expiry_msg}{end_msg}." ) # Send a log message to the mod log. - log.trace(f"Sending apply mod log for infraction #{_id}.") + log.trace(f"Sending apply mod log for infraction #{id_}.") await self.mod_log.send_log_message( icon_url=icon, colour=Colours.soft_red, @@ -181,7 +181,7 @@ class InfractionScheduler(Scheduler): footer=f"ID {infraction['id']}" ) - log.info(f"Applied {infr_type} infraction #{_id} to {user}.") + log.info(f"Applied {infr_type} infraction #{id_} to {user}.") async def pardon_infraction(self, ctx: Context, infr_type: str, user: MemberObject) -> None: """Prematurely end an infraction for a user and log the action in the mod log.""" @@ -209,8 +209,8 @@ class InfractionScheduler(Scheduler): log_text["Member"] = f"{user.mention}(`{user.id}`)" log_text["Actor"] = str(ctx.message.author) log_content = None - _id = response[0]['id'] - footer = f"ID: {_id}" + id_ = response[0]['id'] + footer = f"ID: {id_}" # If multiple active infractions were found, mark them as inactive in the database # and cancel their expiration tasks. @@ -232,15 +232,15 @@ class InfractionScheduler(Scheduler): # 1. Discord cannot store multiple active bans or assign multiples of the same role # 2. It would send a pardon DM for each active infraction, which is redundant for infraction in response[1:]: - _id = infraction['id'] + id_ = infraction['id'] try: # Mark infraction as inactive in the database. await self.bot.api_client.patch( - f"bot/infractions/{_id}", + f"bot/infractions/{id_}", json={"active": False} ) except ResponseCodeError: - log.exception(f"Failed to deactivate infraction #{_id} ({infr_type})") + log.exception(f"Failed to deactivate infraction #{id_} ({infr_type})") # This is simpler and cleaner than trying to concatenate all the errors. log_text["Failure"] = "See bot's logs for details." @@ -262,15 +262,15 @@ class InfractionScheduler(Scheduler): log_title = "pardon failed" log_content = ctx.author.mention - log.warning(f"Failed to pardon {infr_type} infraction #{_id} for {user}.") + log.warning(f"Failed to pardon {infr_type} infraction #{id_} for {user}.") else: confirm_msg = f":ok_hand: pardoned" log_title = "pardoned" - log.info(f"Pardoned {infr_type} infraction #{_id} for {user}.") + log.info(f"Pardoned {infr_type} infraction #{id_} for {user}.") # Send a confirmation message to the invoking context. - log.trace(f"Sending infraction #{_id} pardon confirmation message.") + log.trace(f"Sending infraction #{id_} pardon confirmation message.") await ctx.send( f"{dm_emoji}{confirm_msg} infraction **{infr_type}** for {user.mention}. " f"{log_text.get('Failure', '')}" @@ -305,9 +305,9 @@ class InfractionScheduler(Scheduler): mod_role = guild.get_role(constants.Roles.moderator) user_id = infraction["user"] _type = infraction["type"] - _id = infraction["id"] + id_ = infraction["id"] - log.info(f"Marking infraction #{_id} as inactive (expired).") + log.info(f"Marking infraction #{id_} as inactive (expired).") log_content = None log_text = { @@ -324,14 +324,14 @@ class InfractionScheduler(Scheduler): log_text = {**log_text, **returned_log} # Merge the logs together else: raise ValueError( - f"Attempted to deactivate an unsupported infraction #{_id} ({_type})!" + f"Attempted to deactivate an unsupported infraction #{id_} ({_type})!" ) except discord.Forbidden: - log.warning(f"Failed to deactivate infraction #{_id} ({_type}): bot lacks permissions") + log.warning(f"Failed to deactivate infraction #{id_} ({_type}): bot lacks permissions") log_text["Failure"] = f"The bot lacks permissions to do this (role hierarchy?)" log_content = mod_role.mention except discord.HTTPException as e: - log.exception(f"Failed to deactivate infraction #{_id} ({_type})") + log.exception(f"Failed to deactivate infraction #{id_} ({_type})") log_text["Failure"] = f"HTTPException with code {e.code}." log_content = mod_role.mention @@ -355,13 +355,13 @@ class InfractionScheduler(Scheduler): try: # Mark infraction as inactive in the database. - log.trace(f"Marking infraction #{_id} as inactive in the database.") + log.trace(f"Marking infraction #{id_} as inactive in the database.") await self.bot.api_client.patch( - f"bot/infractions/{_id}", + f"bot/infractions/{id_}", json={"active": False} ) except ResponseCodeError as e: - log.exception(f"Failed to deactivate infraction #{_id} ({_type})") + log.exception(f"Failed to deactivate infraction #{id_} ({_type})") log_line = f"API request failed with code {e.status}." log_content = mod_role.mention @@ -379,13 +379,13 @@ class InfractionScheduler(Scheduler): if send_log: log_title = f"expiration failed" if "Failure" in log_text else "expired" - log.trace(f"Sending deactivation mod log for infraction #{_id}.") + log.trace(f"Sending deactivation mod log for infraction #{id_}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS[_type][1], colour=Colours.soft_green, title=f"Infraction {log_title}: {_type}", text="\n".join(f"{k}: {v}" for k, v in log_text.items()), - footer=f"ID: {_id}", + footer=f"ID: {id_}", content=log_content, ) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 9ab870823..9b3c62403 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -133,17 +133,17 @@ class Superstarify(InfractionScheduler, Cog): # Post the infraction to the API reason = reason or f"old nick: {member.display_name}" infraction = await utils.post_infraction(ctx, member, "superstar", reason, duration) - _id = infraction["id"] + id_ = infraction["id"] old_nick = member.display_name - forced_nick = self.get_nick(_id, member.id) + forced_nick = self.get_nick(id_, member.id) expiry_str = format_infraction(infraction["expires_at"]) # Apply the infraction and schedule the expiration task. log.debug(f"Changing nickname of {member} to {forced_nick}.") self.mod_log.ignore(constants.Event.member_update, member.id) await member.edit(nick=forced_nick, reason=reason) - self.schedule_task(ctx.bot.loop, _id, infraction) + self.schedule_task(ctx.bot.loop, id_, infraction) # Send a DM to the user to notify them of their new infraction. await utils.notify_infraction( @@ -155,7 +155,7 @@ class Superstarify(InfractionScheduler, Cog): ) # Send an embed with the infraction information to the invoking context. - log.trace(f"Sending superstar #{_id} embed.") + log.trace(f"Sending superstar #{id_} embed.") embed = Embed( title="Congratulations!", colour=constants.Colours.soft_orange, @@ -171,7 +171,7 @@ class Superstarify(InfractionScheduler, Cog): await ctx.send(embed=embed) # Log to the mod log channel. - log.trace(f"Sending apply mod log for superstar #{_id}.") + log.trace(f"Sending apply mod log for superstar #{id_}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS["superstar"][0], colour=Colour.gold(), @@ -185,7 +185,7 @@ class Superstarify(InfractionScheduler, Cog): Old nickname: `{old_nick}` New nickname: `{forced_nick}` """), - footer=f"ID {_id}" + footer=f"ID {id_}" ) @command(name="unsuperstarify", aliases=("release_nick", "unstar")) -- cgit v1.2.3 From ad79540f058e5f04bde72dc9dba86533b1e296a2 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 6 Nov 2019 17:36:03 -0800 Subject: Use trailing _ instead of leading for some variable names PEP 8 states the convention is a trailing underscore when used to prevent name conflicts. --- bot/cogs/eval.py | 4 ++-- bot/cogs/filtering.py | 8 ++++---- bot/cogs/moderation/scheduler.py | 14 +++++++------- bot/interpreter.py | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/bot/cogs/eval.py b/bot/cogs/eval.py index 9ce854f2c..00b988dde 100644 --- a/bot/cogs/eval.py +++ b/bot/cogs/eval.py @@ -148,7 +148,7 @@ class CodeEval(Cog): self.env.update(env) # Ignore this code, it works - _code = """ + code_ = """ async def func(): # (None,) -> Any try: with contextlib.redirect_stdout(self.stdout): @@ -162,7 +162,7 @@ async def func(): # (None,) -> Any """.format(textwrap.indent(code, ' ')) try: - exec(_code, self.env) # noqa: B102,S102 + exec(code_, self.env) # noqa: B102,S102 func = self.env['func'] res = await func() diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 4195783f1..1e7521054 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -43,7 +43,7 @@ class Filtering(Cog): def __init__(self, bot: Bot): self.bot = bot - _staff_mistake_str = "If you believe this was a mistake, please let staff know!" + staff_mistake_str = "If you believe this was a mistake, please let staff know!" self.filters = { "filter_zalgo": { "enabled": Filter.filter_zalgo, @@ -53,7 +53,7 @@ class Filtering(Cog): "user_notification": Filter.notify_user_zalgo, "notification_msg": ( "Your post has been removed for abusing Unicode character rendering (aka Zalgo text). " - f"{_staff_mistake_str}" + f"{staff_mistake_str}" ) }, "filter_invites": { @@ -63,7 +63,7 @@ class Filtering(Cog): "content_only": True, "user_notification": Filter.notify_user_invites, "notification_msg": ( - f"Per Rule 6, your invite link has been removed. {_staff_mistake_str}\n\n" + f"Per Rule 6, your invite link has been removed. {staff_mistake_str}\n\n" r"Our server rules can be found here: " ) }, @@ -74,7 +74,7 @@ class Filtering(Cog): "content_only": True, "user_notification": Filter.notify_user_domains, "notification_msg": ( - f"Your URL has been removed because it matched a blacklisted domain. {_staff_mistake_str}" + f"Your URL has been removed because it matched a blacklisted domain. {staff_mistake_str}" ) }, "watch_rich_embeds": { diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 462c7fc7f..49b61f35e 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -304,7 +304,7 @@ class InfractionScheduler(Scheduler): guild = self.bot.get_guild(constants.Guild.id) mod_role = guild.get_role(constants.Roles.moderator) user_id = infraction["user"] - _type = infraction["type"] + type_ = infraction["type"] id_ = infraction["id"] log.info(f"Marking infraction #{id_} as inactive (expired).") @@ -324,14 +324,14 @@ class InfractionScheduler(Scheduler): log_text = {**log_text, **returned_log} # Merge the logs together else: raise ValueError( - f"Attempted to deactivate an unsupported infraction #{id_} ({_type})!" + f"Attempted to deactivate an unsupported infraction #{id_} ({type_})!" ) except discord.Forbidden: - log.warning(f"Failed to deactivate infraction #{id_} ({_type}): bot lacks permissions") + log.warning(f"Failed to deactivate infraction #{id_} ({type_}): bot lacks permissions") log_text["Failure"] = f"The bot lacks permissions to do this (role hierarchy?)" log_content = mod_role.mention except discord.HTTPException as e: - log.exception(f"Failed to deactivate infraction #{id_} ({_type})") + log.exception(f"Failed to deactivate infraction #{id_} ({type_})") log_text["Failure"] = f"HTTPException with code {e.code}." log_content = mod_role.mention @@ -361,7 +361,7 @@ class InfractionScheduler(Scheduler): json={"active": False} ) except ResponseCodeError as e: - log.exception(f"Failed to deactivate infraction #{id_} ({_type})") + log.exception(f"Failed to deactivate infraction #{id_} ({type_})") log_line = f"API request failed with code {e.status}." log_content = mod_role.mention @@ -381,9 +381,9 @@ class InfractionScheduler(Scheduler): log.trace(f"Sending deactivation mod log for infraction #{id_}.") await self.mod_log.send_log_message( - icon_url=utils.INFRACTION_ICONS[_type][1], + icon_url=utils.INFRACTION_ICONS[type_][1], colour=Colours.soft_green, - title=f"Infraction {log_title}: {_type}", + title=f"Infraction {log_title}: {type_}", text="\n".join(f"{k}: {v}" for k, v in log_text.items()), footer=f"ID: {id_}", content=log_content, diff --git a/bot/interpreter.py b/bot/interpreter.py index a42b45a2d..76a3fc293 100644 --- a/bot/interpreter.py +++ b/bot/interpreter.py @@ -20,8 +20,8 @@ class Interpreter(InteractiveInterpreter): write_callable = None def __init__(self, bot: Bot): - _locals = {"bot": bot} - super().__init__(_locals) + locals_ = {"bot": bot} + super().__init__(locals_) async def run(self, code: str, ctx: Context, io: StringIO, *args, **kwargs) -> Any: """Execute the provided source code as the bot & return the output.""" -- cgit v1.2.3 From a916ec15867187907abe9214eb4bfa12a9e982f4 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Fri, 8 Nov 2019 11:58:21 +0800 Subject: Allow helpers to nominate people --- bot/cogs/watchchannels/talentpool.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bot/cogs/watchchannels/talentpool.py b/bot/cogs/watchchannels/talentpool.py index 176c6f760..4ec42dcc1 100644 --- a/bot/cogs/watchchannels/talentpool.py +++ b/bot/cogs/watchchannels/talentpool.py @@ -7,14 +7,13 @@ from discord import Color, Embed, Member, User from discord.ext.commands import Bot, Cog, Context, group from bot.api import ResponseCodeError -from bot.constants import Channels, Guild, Roles, Webhooks +from bot.constants import Channels, Guild, MODERATION_ROLES, STAFF_ROLES, Webhooks from bot.decorators import with_role from bot.pagination import LinePaginator from bot.utils import time from .watchchannel import WatchChannel, proxy_user log = logging.getLogger(__name__) -STAFF_ROLES = Roles.owner, Roles.admin, Roles.moderator, Roles.helpers # <- In constants after the merge? class TalentPool(WatchChannel, Cog, name="Talentpool"): @@ -31,13 +30,13 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): ) @group(name='talentpool', aliases=('tp', 'talent', 'nomination', 'n'), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def nomination_group(self, ctx: Context) -> None: """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" await ctx.invoke(self.bot.get_command("help"), "talentpool") @nomination_group.command(name='watched', aliases=('all', 'list')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watched_command(self, ctx: Context, update_cache: bool = True) -> None: """ Shows the users that are currently being monitored in the talent pool. @@ -48,7 +47,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): await self.list_watched_users(ctx, update_cache) @nomination_group.command(name='watch', aliases=('w', 'add', 'a')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*STAFF_ROLES) async def watch_command(self, ctx: Context, user: Union[Member, User, proxy_user], *, reason: str) -> None: """ Relay messages sent by the given `user` to the `#talent-pool` channel. @@ -113,7 +112,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): await ctx.send(msg) @nomination_group.command(name='history', aliases=('info', 'search')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def history_command(self, ctx: Context, user: Union[User, proxy_user]) -> None: """Shows the specified user's nomination history.""" result = await self.bot.api_client.get( @@ -142,7 +141,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): ) @nomination_group.command(name='unwatch', aliases=('end', )) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def unwatch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """ Ends the active nomination of the specified user with the given reason. @@ -170,13 +169,13 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): self._remove_user(user.id) @nomination_group.group(name='edit', aliases=('e',), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def nomination_edit_group(self, ctx: Context) -> None: """Commands to edit nominations.""" await ctx.invoke(self.bot.get_command("help"), "talentpool", "edit") @nomination_edit_group.command(name='reason') - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def edit_reason_command(self, ctx: Context, nomination_id: int, *, reason: str) -> None: """ Edits the reason/unnominate reason for the nomination with the given `id` depending on the status. -- cgit v1.2.3 From 25f7f6019772e22072a2e1d16d6c4ff57862022f Mon Sep 17 00:00:00 2001 From: kosayoda Date: Fri, 8 Nov 2019 12:11:47 +0800 Subject: Refactor bigbrother to use constants --- bot/cogs/watchchannels/bigbrother.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/cogs/watchchannels/bigbrother.py b/bot/cogs/watchchannels/bigbrother.py index c516508ca..49783bb09 100644 --- a/bot/cogs/watchchannels/bigbrother.py +++ b/bot/cogs/watchchannels/bigbrother.py @@ -6,7 +6,7 @@ from discord import User from discord.ext.commands import Bot, Cog, Context, group from bot.cogs.moderation.utils import post_infraction -from bot.constants import Channels, Roles, Webhooks +from bot.constants import Channels, MODERATION_ROLES, Webhooks from bot.decorators import with_role from .watchchannel import WatchChannel, proxy_user @@ -27,13 +27,13 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): ) @group(name='bigbrother', aliases=('bb',), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def bigbrother_group(self, ctx: Context) -> None: """Monitors users by relaying their messages to the Big Brother watch channel.""" await ctx.invoke(self.bot.get_command("help"), "bigbrother") @bigbrother_group.command(name='watched', aliases=('all', 'list')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watched_command(self, ctx: Context, update_cache: bool = True) -> None: """ Shows the users that are currently being monitored by Big Brother. @@ -44,7 +44,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): await self.list_watched_users(ctx, update_cache) @bigbrother_group.command(name='watch', aliases=('w',)) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """ Relay messages sent by the given `user` to the `#big-brother` channel. @@ -91,7 +91,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): await ctx.send(msg) @bigbrother_group.command(name='unwatch', aliases=('uw',)) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def unwatch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """Stop relaying messages by the given `user`.""" active_watches = await self.bot.api_client.get( -- cgit v1.2.3 From 9000d92ab8ceaf855d23c0e6dfdb09d05a40e59a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 8 Nov 2019 22:07:00 +0100 Subject: Add whitespace for readability, consistency & allure --- tests/bot/rules/test_links.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 95f107f91..40336beb0 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -45,6 +45,7 @@ class LinksTests(unittest.TestCase): for recent_messages in cases: last_message = recent_messages[0] + with self.subTest( last_message=last_message, recent_messages=recent_messages, -- cgit v1.2.3 From 760f265339b1462b60ea7d5af1fd8d4476c134f0 Mon Sep 17 00:00:00 2001 From: kwzrd <44734341+kwzrd@users.noreply.github.com> Date: Fri, 8 Nov 2019 22:52:47 +0100 Subject: Update docstring to use asterisks when referring to argument names Co-Authored-By: Kyle Stanley --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 40336beb0..be832843b 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -18,7 +18,7 @@ class Case(NamedTuple): def msg(author: str, total_links: int) -> FakeMessage: - """Makes a message with `total_links` links.""" + """Makes a message with *total_links* links.""" content = " ".join(["https://pydis.com"] * total_links) return FakeMessage(author=author, content=content) -- cgit v1.2.3 From 01ab4ad37dc68f38dd0fd15487dbff9bbd58c24e Mon Sep 17 00:00:00 2001 From: Derek Date: Sat, 9 Nov 2019 20:08:08 -0500 Subject: Forward user/role pings in checkpoint to mod-alerts [kaizen] limit on_message listener to verification channel --- bot/cogs/verification.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 3d85edae6..cd4311916 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,12 +1,16 @@ import logging from datetime import datetime -from discord import Message, NotFound, Object +from discord import Colour, Message, NotFound, Object from discord.ext import tasks from discord.ext.commands import Bot, Cog, Context, command from bot.cogs.moderation import ModLog -from bot.constants import Bot as BotConfig, Channels, Event, Roles +from bot.constants import ( + Bot as BotConfig, + Channels, Colours, Event, + Filter, Icons, Roles +) from bot.decorators import InChannelCheckFailure, in_channel, without_role log = logging.getLogger(__name__) @@ -53,6 +57,34 @@ class Verification(Cog): if message.author.bot: return # They're a bot, ignore + if message.channel.id != Channels.verification: + return # Only listen for #checkpoint messages + + # if a user mentions a role or guild member + # alert the mods in mod-alerts channel + if message.mentions or message.role_mentions: + log.debug( + f"{message.author} mentioned one or more users " + f"and/or roles in {message.channel.name}" + ) + + embed_text = ( + f"{message.author.mention} sent a message in " + f"{message.channel.mention} that contained user and/or role mentions." + f"\n\n**Original message:**\n>>> {message.content}" + ) + + # Send pretty mod log embed to mod-alerts + await self.mod_log.send_log_message( + icon_url=Icons.filtering, + colour=Colour(Colours.soft_red), + title=f"User/Role mentioned in {message.channel.name}", + text=embed_text, + thumbnail=message.author.avatar_url_as(static_format="png"), + channel_id=Channels.mod_alerts, + ping_everyone=Filter.ping_everyone, + ) + ctx = await self.bot.get_context(message) # type: Context if ctx.command is not None and ctx.command.name == "accept": -- cgit v1.2.3 From 0425e6c49ecf091871a06762a064755ea3dda04c Mon Sep 17 00:00:00 2001 From: Derek Date: Sat, 9 Nov 2019 21:07:11 -0500 Subject: [kaizen] Remove now duplicate channel check Unindent subsequent lines after check --- bot/cogs/verification.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index cd4311916..b5e8d4357 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -90,27 +90,26 @@ class Verification(Cog): if ctx.command is not None and ctx.command.name == "accept": return # They used the accept command - if ctx.channel.id == Channels.verification: # We're in the verification channel - 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 - - 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.", - delete_after=20 - ) + 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 + + 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.", + delete_after=20 + ) - log.trace(f"Deleting the message posted by {ctx.author}") + log.trace(f"Deleting the message posted by {ctx.author}") - try: - await ctx.message.delete() - except NotFound: - log.trace("No message found, it must have been deleted by another bot.") + try: + 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) -- cgit v1.2.3 From 4d5d307f9a499cd874d90e6500f877ce560c012f Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 19:34:28 +0100 Subject: fix signatures and descriptions not being found when present --- bot/cogs/doc.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index dcbcfe3ad..6e50cd27d 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -271,19 +271,19 @@ class Doc(commands.Cog): description_start_index = search_html.find(str(start_tag.parent)) + len(str(start_tag.parent)) description_end_index = search_html.find(str(end_tag)) - description = search_html[description_start_index:description_end_index].replace('¶', '') + description = search_html[description_start_index:description_end_index] signatures = None else: - description = str(symbol_heading.find_next_sibling("dd")).replace('¶', '') + description = str(symbol_heading.find_next_sibling("dd")) description_pos = search_html.find(description) # Get text of up to 3 signatures, remove unwanted symbols for element in [symbol_heading] + symbol_heading.find_next_siblings("dt", limit=2): signature = UNWANTED_SIGNATURE_SYMBOLS_RE.sub("", element.text) - if signature and search_html.find(signature) < description_pos: + if signature and search_html.find(str(element)) < description_pos: signatures.append(signature) - return signatures, description + return signatures, description.replace('¶', '') @async_cache(arg_offset=1) async def get_symbol_embed(self, symbol: str) -> Optional[discord.Embed]: -- cgit v1.2.3 From 7de5156a7719f0639021e8186f7ea17f5b853af7 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 19:39:14 +0100 Subject: Add a newline after signatures for readability --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 6e50cd27d..653d48528 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -326,7 +326,7 @@ class Doc(commands.Cog): else: embed_description = "".join(f"```py\n{textwrap.shorten(signature, 500)}```" for signature in signatures) - embed_description += description + embed_description += f"\n{description}" embed = discord.Embed( title=f'`{symbol}`', -- cgit v1.2.3 From 4795da86d0fef72ac677ae0a8f9e988da1923e17 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 19:43:56 +0100 Subject: Cut off description at 1000 chars if paragraph is not found --- bot/cogs/doc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 653d48528..b04355e28 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -305,6 +305,8 @@ class Doc(commands.Cog): if len(description) > 1000: shortened = description[:1000] last_paragraph_end = shortened.rfind('\n\n') + if last_paragraph_end == -1: + last_paragraph_end = 1000 description = description[:last_paragraph_end] # If there is an incomplete code block, cut it out -- cgit v1.2.3 From 34510f52c6bbe5e2a8bbfc34f8e5d648d0d39a96 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 20:03:48 +0100 Subject: Move paragraph search to not cut off long starting paragraphs Co-authored-by: scargly <29337040+scragly@users.noreply.github.com> --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index b04355e28..73895e3eb 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -304,7 +304,7 @@ class Doc(commands.Cog): # of a double newline (interpreted as a paragraph) before index 1000. if len(description) > 1000: shortened = description[:1000] - last_paragraph_end = shortened.rfind('\n\n') + last_paragraph_end = shortened.rfind('\n\n', 100) if last_paragraph_end == -1: last_paragraph_end = 1000 description = description[:last_paragraph_end] -- cgit v1.2.3 From 219cde70f03476ac6ae4a7f84322757bebeec51e Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 21:30:26 +0100 Subject: Add a command for refreshing inventories --- bot/cogs/doc.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 73895e3eb..8cf32fc7f 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -447,6 +447,28 @@ class Doc(commands.Cog): await self.refresh_inventory() await ctx.send(f"Successfully deleted `{package_name}` and refreshed inventory.") + @docs_group.command(name="refresh", aliases=("rfsh", "r")) + @with_role(*MODERATION_ROLES) + async def refresh_command(self, ctx: commands.Context) -> None: + """Refresh inventories and send differences to channel.""" + old_inventories = set(self.base_urls) + with ctx.typing(): + await self.refresh_inventory() + # Get differences of added and removed inventories + added = ', '.join(inv for inv in self.base_urls if inv not in old_inventories) + if added: + added = f"`+ {added}`" + + removed = ', '.join(inv for inv in old_inventories if inv not in self.base_urls) + if removed: + removed = f"`- {removed}`" + + embed = discord.Embed( + title="Inventories refreshed", + description=f"{added}\n{removed}" if added or removed else "" + ) + await ctx.send(embed=embed) + async def _fetch_inventory(self, inventory_url: str, config: SphinxConfiguration) -> Optional[dict]: """Get and return inventory from `inventory_url`. If fetching fails, return None.""" fetch_func = functools.partial(intersphinx.fetch_inventory, config, '', inventory_url) -- cgit v1.2.3 From 4f393d7b95101cc31269eb30742195e771deb705 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Sun, 10 Nov 2019 21:31:47 +0100 Subject: Move signatures definition --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 8cf32fc7f..f7e8ae9d6 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -252,7 +252,6 @@ class Doc(commands.Cog): symbol_id = url.split('#')[-1] soup = BeautifulSoup(html, 'lxml') symbol_heading = soup.find(id=symbol_id) - signatures = [] search_html = str(soup) if symbol_heading is None: @@ -275,6 +274,7 @@ class Doc(commands.Cog): signatures = None else: + signatures = [] description = str(symbol_heading.find_next_sibling("dd")) description_pos = search_html.find(description) # Get text of up to 3 signatures, remove unwanted symbols -- cgit v1.2.3 From 6944175cea2c6595ec29b9ef67ff2ad9a8efb8ae Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 11 Nov 2019 00:58:32 +0100 Subject: clear renamed symbols on inventory refresh --- bot/cogs/doc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index f7e8ae9d6..90f496ceb 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -216,6 +216,7 @@ class Doc(commands.Cog): # Also, reset the cache used for fetching documentation. self.base_urls.clear() self.inventories.clear() + self.renamed_symbols.clear() async_cache.cache = OrderedDict() # Since Intersphinx is intended to be used with Sphinx, -- cgit v1.2.3 From 4a7de0bd155a4717f6cbc593a60dbec130e7ca40 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 11 Nov 2019 01:12:21 +0100 Subject: Do not cut off text arbitrarily but at last sentence to make sure no unfinished markdown is left in --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 90f496ceb..bf6cee101 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -307,7 +307,7 @@ class Doc(commands.Cog): shortened = description[:1000] last_paragraph_end = shortened.rfind('\n\n', 100) if last_paragraph_end == -1: - last_paragraph_end = 1000 + last_paragraph_end = shortened.rfind('. ') description = description[:last_paragraph_end] # If there is an incomplete code block, cut it out -- cgit v1.2.3 From fb338545c4c2a133e23a664c77813d2ce9aba41c Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 11 Nov 2019 01:15:01 +0100 Subject: syntax highlight diff of reloaded inventories --- bot/cogs/doc.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index bf6cee101..0d4884e8b 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -458,15 +458,15 @@ class Doc(commands.Cog): # Get differences of added and removed inventories added = ', '.join(inv for inv in self.base_urls if inv not in old_inventories) if added: - added = f"`+ {added}`" + added = f"+ {added}" removed = ', '.join(inv for inv in old_inventories if inv not in self.base_urls) if removed: - removed = f"`- {removed}`" + removed = f"- {removed}" embed = discord.Embed( title="Inventories refreshed", - description=f"{added}\n{removed}" if added or removed else "" + description=f"```diff\n{added}\n{removed}```" if added or removed else "" ) await ctx.send(embed=embed) -- cgit v1.2.3 From 160962a56110ed970c7419ed650d9d8a84dbaa9a Mon Sep 17 00:00:00 2001 From: Numerlor Date: Tue, 12 Nov 2019 16:46:51 +0100 Subject: Adjust code style and comments --- bot/cogs/doc.py | 77 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 0d4884e8b..b82eac5fe 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -176,36 +176,34 @@ class Doc(commands.Cog): self.base_urls[package_name] = base_url package = await self._fetch_inventory(inventory_url, config) - if package: - for group, value in package.items(): - # Each value has a bunch of information in the form - # `(package_name, version, relative_url, ???)`, and we only - # need the package_name and the relative documentation URL. - for symbol, (package_name, _, relative_doc_url, _) in value.items(): - absolute_doc_url = base_url + relative_doc_url - - if symbol in self.inventories: - # get `group_name` from _:group_name - group_name = group.split(":")[1] - if (group_name in NO_OVERRIDE_GROUPS - # check if any package from `NO_OVERRIDE_PACKAGES` - # is in base URL of the symbol that would be overridden - or any(package in self.inventories[symbol].split("/", 3)[2] - for package in NO_OVERRIDE_PACKAGES)): - - symbol = f"{group_name}.{symbol}" - # If renamed `symbol` already exists, add library name in front. - if symbol in self.renamed_symbols: - # Split `package_name` because of packages like Pillow that have spaces in them. - symbol = f"{package_name.split()[0]}.{symbol}" - - self.inventories[symbol] = absolute_doc_url - self.renamed_symbols.add(symbol) - continue - - self.inventories[symbol] = absolute_doc_url - - log.trace(f"Fetched inventory for {package_name}.") + if not package: + return None + + for group, value in package.items(): + for symbol, (package_name, _, relative_doc_url, _) in value.items(): + absolute_doc_url = base_url + relative_doc_url + + if symbol in self.inventories: + group_name = group.split(":")[1] + symbol_base_url = self.inventories[symbol].split("/", 3)[2] + if ( + group_name in NO_OVERRIDE_GROUPS + or any(package in symbol_base_url for package in NO_OVERRIDE_PACKAGES) + ): + + symbol = f"{group_name}.{symbol}" + # If renamed `symbol` already exists, add library name in front to differentiate between them. + if symbol in self.renamed_symbols: + # Split `package_name` because of packages like Pillow that have spaces in them. + symbol = f"{package_name.split()[0]}.{symbol}" + + self.inventories[symbol] = absolute_doc_url + self.renamed_symbols.add(symbol) + continue + + self.inventories[symbol] = absolute_doc_url + + log.trace(f"Fetched inventory for {package_name}.") async def refresh_inventory(self) -> None: """Refresh internal documentation inventory.""" @@ -337,9 +335,10 @@ class Doc(commands.Cog): description=embed_description ) # Show all symbols with the same name that were renamed in the footer. - embed.set_footer(text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} - if renamed.endswith(f".{symbol}")) - ) + embed.set_footer( + text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} + if renamed.endswith(f".{symbol}")) + ) return embed @commands.group(name='docs', aliases=('doc', 'd'), invoke_without_command=True) @@ -477,11 +476,15 @@ class Doc(commands.Cog): try: package = await self.bot.loop.run_in_executor(None, fetch_func) except ConnectTimeout: - log.error(f"Fetching of inventory {inventory_url} timed out," - f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})") + log.error( + f"Fetching of inventory {inventory_url} timed out," + f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})" + ) except ProtocolError: - log.error(f"Connection lost while fetching inventory {inventory_url}," - f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})") + log.error( + f"Connection lost while fetching inventory {inventory_url}," + f" trying again. ({retry}/{FAILED_REQUEST_RETRY_AMOUNT})" + ) except HTTPError as e: log.error(f"Fetching of inventory {inventory_url} failed with status code {e.response.status_code}.") return None -- cgit v1.2.3 From dceafb83e829548638e8589c88f80364e8009821 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 15:32:13 +0100 Subject: Prevent unwanted logging while running tests Previously, logging messages would output to std.out. when running individual test files (instead of running the entire suite). To prevent this, I've added a `for`-loop to `tests.helpers` that sets the level of all registered loggers to `CRITICAL`. The reason for adding this to `tests.helpers` is simple: It's the most common file to be imported in individual tests, increasing the chance of the code being run for individual test files. A small downside of this way of handling logging is that when we are trying to assert logging messages are being emitted, we need to set the logger explicitly in the `self.assertLogs` context manager. This is a small downside, though, and probably good practice anyway. There was one test in `tests.bot.test_api` that did not do this, so I have changed this to make the test compatible with the new set-up. --- tests/bot/test_api.py | 4 +++- tests/helpers.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py index e0ede0eb1..5a88adc5c 100644 --- a/tests/bot/test_api.py +++ b/tests/bot/test_api.py @@ -121,7 +121,9 @@ class LoggingHandlerTests(LoggingTestCase): def test_schedule_queued_tasks_for_nonempty_queue(self): """`APILoggingHandler` should schedule logs when the queue is not empty.""" - with self.assertLogs(level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: + log = logging.getLogger("bot.api") + + with self.assertLogs(logger=log, level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: self.log_handler.queue = [555] self.log_handler.schedule_queued_tasks() self.assertListEqual(self.log_handler.queue, []) diff --git a/tests/helpers.py b/tests/helpers.py index 8496ba031..8d661513d 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio import functools import inspect +import logging import unittest.mock from typing import Any, Iterable, Optional @@ -10,6 +11,16 @@ import discord from discord.ext.commands import Bot, Context +for logger in logging.Logger.manager.loggerDict.values(): + # Set all loggers to CRITICAL by default to prevent screen clutter during testing + + if not isinstance(logger, logging.Logger): + # There might be some logging.PlaceHolder objects in there + continue + + logger.setLevel(logging.CRITICAL) + + def async_test(wrapped): """ Run a test case via asyncio. -- cgit v1.2.3 From bf7720f16fa69716f15b16e3dcd0f20c186958b8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 16:06:51 +0100 Subject: Allow `name` attribute to be set during Mock init The `name` keyword argument has a special meaning for the default mockobjects provided by `unittest.mock`. This means that by default, the common d.py `name` attribute can't be set during initalization of one of our custom Mock-objects by passing it to the constructor. Since it's unlikely for us to make use of the special `name` feature of mocks and more likely to want to set the d.py `name` attribute, I added special handling of the `name` kwarg. --- tests/helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 8d661513d..5dc7a0d2f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -74,7 +74,10 @@ class CustomMockMixin: child_mock_type = unittest.mock.MagicMock def __init__(self, spec: Any = None, **kwargs): + name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. super().__init__(spec=spec, **kwargs) + if name: + self.name = name if spec: self._extract_coroutine_methods_from_spec_instance(spec) -- cgit v1.2.3 From 36e9de480dcabb3c844090a4fd87561534536c04 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 11:24:58 +0100 Subject: Prevent setting unknown attributes on d.py mocks Our custom `discord.py` now follow the specifications of the object they are mocking more strictly by using the `spec_set` instead of the `spec` kwarg to initialize the specifications. This means that trying to set an attribute that does not follow the specifications will now also result in an `AttributeError`. To make sure we are not trying to set illegal attributes during the default initialization of the mock objects, I've changed the way we handle default values of parameters. This does introduce a breaking change: Instead of passing a `suffix_id`, the `id` attribute should now be passed using the exact name. `id`. This commit also makes sure existing tests follow this change. --- tests/bot/cogs/test_information.py | 58 ++++++++++----------- tests/bot/cogs/test_token_remover.py | 2 +- tests/bot/utils/test_checks.py | 6 +-- tests/helpers.py | 94 +++++++++++++--------------------- tests/test_helpers.py | 99 ++++++++++++------------------------ 5 files changed, 101 insertions(+), 158 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 5c34541d8..4496a2ae0 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -19,7 +19,7 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = helpers.MockRole(name="Moderator", role_id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderator) def setUp(self): """Sets up fresh objects for each test.""" @@ -54,7 +54,7 @@ class InformationCogTests(unittest.TestCase): """Tests the `role info` command.""" dummy_role = helpers.MockRole( name="Dummy", - role_id=112233445566778899, + id=112233445566778899, colour=discord.Colour.blurple(), position=10, members=[self.ctx.author], @@ -63,7 +63,7 @@ class InformationCogTests(unittest.TestCase): admin_role = helpers.MockRole( name="Admins", - role_id=998877665544332211, + id=998877665544332211, colour=discord.Colour.red(), position=3, members=[self.ctx.author], @@ -176,7 +176,7 @@ class UserInfractionHelperMethodTests(unittest.TestCase): self.bot = helpers.MockBot() self.bot.api_client.get = helpers.AsyncMock() self.cog = information.Information(self.bot) - self.member = helpers.MockMember(user_id=1234) + self.member = helpers.MockMember(id=1234) def test_user_command_helper_method_get_requests(self): """The helper methods should form the correct get requests.""" @@ -351,7 +351,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_string_representation_of_user_in_title_if_nick_is_not_available(self): """The embed should use the string representation of the user if they don't have a nick.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = None user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -363,7 +363,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_nick_in_title_if_available(self): """The embed should use the nick if it's available.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = "Cat lover" user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -375,8 +375,8 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_ignores_everyone_role(self): """Created `!user` embeds should not contain mention of the @everyone-role.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) - admins_role = helpers.MockRole('Admins') + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) + admins_role = helpers.MockRole(name='Admins') admins_role.colour = 100 # A `MockMember` has the @Everyone role by default; we add the Admins to that. @@ -391,15 +391,15 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.user_nomination_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_expanded_information_in_moderation_channels(self, nomination_counts, infraction_counts): """The embed should contain expanded infractions and nomination info in mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=50)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "expanded infractions info" nomination_counts.return_value = "nomination info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -426,14 +426,14 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_basic_information_outside_of_moderation_channels(self, infraction_counts): """The embed should contain only basic infraction data outside of mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=100)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=100)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "basic infractions info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -459,10 +459,10 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with the colour of the top role, if a top role is available.""" ctx = helpers.MockContext() - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour(moderators_role.colour)) @@ -472,7 +472,7 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with a blurple colour if the user has no assigned roles.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour.blurple()) @@ -482,7 +482,7 @@ class UserEmbedTests(unittest.TestCase): """The embed thumbnail should be set to the user's avatar in `png` format.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) user.avatar_url_as.return_value = "avatar url" embed = asyncio.run(self.cog.create_user_embed(ctx, user)) @@ -499,13 +499,13 @@ class UserCommandTests(unittest.TestCase): self.bot = helpers.MockBot() self.cog = information.Information(self.bot) - self.moderator_role = helpers.MockRole("Moderators", role_id=2, position=10) - self.flautist_role = helpers.MockRole("Flautists", role_id=3, position=2) - self.bassist_role = helpers.MockRole("Bassists", role_id=4, position=3) + self.moderator_role = helpers.MockRole(name="Moderators", id=2, position=10) + self.flautist_role = helpers.MockRole(name="Flautists", id=3, position=2) + self.bassist_role = helpers.MockRole(name="Bassists", id=4, position=3) - self.author = helpers.MockMember(user_id=1, name="syntaxaire") - self.moderator = helpers.MockMember(user_id=2, name="riffautae", roles=[self.moderator_role]) - self.target = helpers.MockMember(user_id=3, name="__fluzz__") + self.author = helpers.MockMember(id=1, name="syntaxaire") + self.moderator = helpers.MockMember(id=2, name="riffautae", roles=[self.moderator_role]) + self.target = helpers.MockMember(id=3, name="__fluzz__") def test_regular_member_cannot_target_another_member(self, constants): """A regular user should not be able to use `!user` targeting another user.""" @@ -523,7 +523,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=100)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) msg = "Sorry, but you may only use this command within <#50>." with self.assertRaises(InChannelCheckFailure, msg=msg): @@ -535,7 +535,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -548,7 +548,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author)) @@ -561,7 +561,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=200)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -574,7 +574,7 @@ class UserCommandTests(unittest.TestCase): constants.MODERATION_ROLES = [self.moderator_role.id] constants.STAFF_ROLES = [self.moderator_role.id] - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index dfb1bafc9..3276cf5a5 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -24,7 +24,7 @@ class TokenRemoverTests(unittest.TestCase): self.bot.get_cog.return_value.send_log_message = AsyncMock() self.cog = TokenRemover(bot=self.bot) - self.msg = MockMessage(message_id=555, content='') + self.msg = MockMessage(id=555, content='') self.msg.author.__str__ = MagicMock() self.msg.author.__str__.return_value = 'lemon' self.msg.author.bot = False diff --git a/tests/bot/utils/test_checks.py b/tests/bot/utils/test_checks.py index 19b758336..9610771e5 100644 --- a/tests/bot/utils/test_checks.py +++ b/tests/bot/utils/test_checks.py @@ -22,7 +22,7 @@ class ChecksTests(unittest.TestCase): def test_with_role_check_with_guild_and_required_role(self): """`with_role_check` returns `True` if `Context.author` has the required role.""" - self.ctx.author.roles.append(MockRole(role_id=10)) + self.ctx.author.roles.append(MockRole(id=10)) self.assertTrue(checks.with_role_check(self.ctx, 10)) def test_without_role_check_without_guild(self): @@ -33,13 +33,13 @@ class ChecksTests(unittest.TestCase): def test_without_role_check_returns_false_with_unwanted_role(self): """`without_role_check` returns `False` if `Context.author` has unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertFalse(checks.without_role_check(self.ctx, role_id)) def test_without_role_check_returns_true_without_unwanted_role(self): """`without_role_check` returns `True` if `Context.author` does not have unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertTrue(checks.without_role_check(self.ctx, role_id + 10)) def test_in_channel_check_for_correct_channel(self): diff --git a/tests/helpers.py b/tests/helpers.py index 5dc7a0d2f..35f2c288c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,10 @@ from __future__ import annotations import asyncio +import collections import functools import inspect +import itertools import logging import unittest.mock from typing import Any, Iterable, Optional @@ -72,14 +74,16 @@ class CustomMockMixin: """ child_mock_type = unittest.mock.MagicMock + discord_id = itertools.count(0) - def __init__(self, spec: Any = None, **kwargs): + def __init__(self, spec_set: Any = None, **kwargs): name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. - super().__init__(spec=spec, **kwargs) + super().__init__(spec_set=spec_set, **kwargs) + if name: self.name = name - if spec: - self._extract_coroutine_methods_from_spec_instance(spec) + if spec_set: + self._extract_coroutine_methods_from_spec_instance(spec_set) def _get_child_mock(self, **kw): """ @@ -169,25 +173,14 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - def __init__( - self, - guild_id: int = 1, - roles: Optional[Iterable[MockRole]] = None, - members: Optional[Iterable[MockMember]] = None, - **kwargs, - ) -> None: - super().__init__(spec=guild_instance, **kwargs) - - self.id = guild_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'members': []} + super().__init__(spec_set=guild_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) - self.members = [] - if members: - self.members.extend(members) - # Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} @@ -201,13 +194,12 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): Instances of this class will follow the specifications of `discord.Role` instances. For more information, see the `MockGuild` docstring. """ - def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: - super().__init__(spec=role_instance, **kwargs) + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'name': 'role', 'position': 1} + super().__init__(spec_set=role_instance, **collections.ChainMap(kwargs, default_kwargs)) - self.name = name - self.id = role_id - self.position = position - self.mention = f'&{self.name}' + if 'mention' not in kwargs: + self.mention = f'&{self.name}' def __lt__(self, other): """Simplified position-based comparisons similar to those of `discord.Role`.""" @@ -227,19 +219,11 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ - def __init__( - self, - name: str = "member", - user_id: int = 1, - roles: Optional[Iterable[MockRole]] = None, - **kwargs, - ) -> None: - super().__init__(spec=member_instance, **kwargs) - - self.name = name - self.id = user_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'name': 'member', 'id': next(self.discord_id)} + super().__init__(spec_set=member_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) @@ -248,6 +232,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) +bot_instance.http_session = None +bot_instance.api_client = None class MockBot(CustomMockMixin, unittest.mock.MagicMock): @@ -258,11 +244,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=bot_instance, **kwargs) - - # Our custom attributes and methods - self.http_session = unittest.mock.MagicMock() - self.api_client = unittest.mock.MagicMock() + super().__init__(spec_set=bot_instance, **kwargs) # self.wait_for is *not* a coroutine function, but returns a coroutine nonetheless and # and should therefore be awaited. (The documentation calls it a coroutine as well, which @@ -294,11 +276,11 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): more information, see the `MockGuild` docstring. """ def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: - super().__init__(spec=channel_instance, **kwargs) - self.id = channel_id - self.name = name - self.guild = kwargs.get('guild', MockGuild()) - self.mention = f"#{self.name}" + default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} + super().__init__(spec_set=channel_instance, **collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"#{self.name}" # Create a Message instance to get a realistic MagicMock of `discord.Message` @@ -335,12 +317,11 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): instances. For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=context_instance, **kwargs) + super().__init__(spec_set=context_instance, **kwargs) self.bot = kwargs.get('bot', MockBot()) self.guild = kwargs.get('guild', MockGuild()) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) - self.command = kwargs.get('command', unittest.mock.MagicMock()) class MockMessage(CustomMockMixin, unittest.mock.MagicMock): @@ -351,7 +332,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=message_instance, **kwargs) + super().__init__(spec_set=message_instance, **kwargs) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) @@ -368,12 +349,9 @@ class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=emoji_instance, **kwargs) + super().__init__(spec_set=emoji_instance, **kwargs) self.guild = kwargs.get('guild', MockGuild()) - # Get all coroutine functions and set them as AsyncMock attributes - self._extract_coroutine_methods_from_spec_instance(emoji_instance) - partial_emoji_instance = discord.PartialEmoji(animated=False, name='guido') @@ -386,7 +364,7 @@ class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=partial_emoji_instance, **kwargs) + super().__init__(spec_set=partial_emoji_instance, **kwargs) reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) @@ -400,6 +378,6 @@ class MockReaction(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=reaction_instance, **kwargs) + super().__init__(spec_set=reaction_instance, **kwargs) self.emoji = kwargs.get('emoji', MockEmoji()) self.message = kwargs.get('message', MockMessage()) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2b58634dd..e879ef97a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -19,7 +19,6 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(role, discord.Role) self.assertEqual(role.name, "role") - self.assertEqual(role.id, 1) self.assertEqual(role.position, 1) self.assertEqual(role.mention, "&role") @@ -27,7 +26,7 @@ class DiscordMocksTests(unittest.TestCase): """Test if MockRole initializes with the arguments provided.""" role = helpers.MockRole( name="Admins", - role_id=90210, + id=90210, position=10, ) @@ -67,22 +66,21 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(member, discord.Member) self.assertEqual(member.name, "member") - self.assertEqual(member.id, 1) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertEqual(member.mention, "@member") def test_mock_member_alternative_arguments(self): """Test if MockMember initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) member = helpers.MockMember( name="Mark", - user_id=12345, + id=12345, roles=[core_developer] ) self.assertEqual(member.name, "Mark") self.assertEqual(member.id, 12345) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1), core_developer]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) self.assertEqual(member.mention, "@Mark") def test_mock_member_accepts_dynamic_arguments(self): @@ -102,19 +100,19 @@ class DiscordMocksTests(unittest.TestCase): # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass self.assertIsInstance(guild, discord.Guild) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertListEqual(guild.members, []) def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) guild = helpers.MockGuild( roles=[core_developer], - members=[helpers.MockMember(user_id=54321)], + members=[helpers.MockMember(id=54321)], ) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1), core_developer]) - self.assertListEqual(guild.members, [helpers.MockMember(user_id=54321)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) + self.assertListEqual(guild.members, [helpers.MockMember(id=54321)]) def test_mock_guild_accepts_dynamic_arguments(self): """Test if MockGuild accepts and sets abitrary keyword arguments.""" @@ -191,51 +189,18 @@ class DiscordMocksTests(unittest.TestCase): with self.assertRaises(AttributeError): mock.the_cake_is_a_lie - def test_custom_mock_methods_are_valid_discord_object_methods(self): - """The `AsyncMock` attributes of the mocks should be valid for the class they're mocking.""" - mocks = ( - (helpers.MockGuild, helpers.guild_instance), - (helpers.MockRole, helpers.role_instance), - (helpers.MockMember, helpers.member_instance), - (helpers.MockBot, helpers.bot_instance), - (helpers.MockContext, helpers.context_instance), - (helpers.MockTextChannel, helpers.channel_instance), - (helpers.MockMessage, helpers.message_instance), + def test_mocks_use_mention_when_provided_as_kwarg(self): + """The mock should use the passed `mention` instead of the default one if present.""" + test_cases = ( + (helpers.MockRole, "role mention"), + (helpers.MockMember, "member mention"), + (helpers.MockTextChannel, "channel mention"), ) - for mock_class, instance in mocks: - mock = mock_class() - async_methods = ( - attr for attr in dir(mock) if isinstance(getattr(mock, attr), helpers.AsyncMock) - ) - - # spec_mock = unittest.mock.MagicMock(spec=instance) - for method in async_methods: - with self.subTest(mock_class=mock_class, method=method): - try: - getattr(instance, method) - except AttributeError: - msg = f"method {method} is not a method attribute of {instance.__class__}" - self.fail(msg) - - @unittest.mock.patch(f'{__name__}.DiscordMocksTests.subTest') - def test_the_custom_mock_methods_test(self, subtest_mock): - """The custom method test should raise AssertionError for invalid methods.""" - class FakeMockBot(helpers.CustomMockMixin, unittest.mock.MagicMock): - """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" - - child_mock_type = unittest.mock.MagicMock - - def __init__(self, **kwargs): - super().__init__(spec=helpers.bot_instance, **kwargs) - - # Fake attribute - self.release_the_walrus = helpers.AsyncMock() - - with unittest.mock.patch("tests.helpers.MockBot", new=FakeMockBot): - msg = "method release_the_walrus is not a valid method of " - with self.assertRaises(AssertionError, msg=msg): - self.test_custom_mock_methods_are_valid_discord_object_methods() + for mock_type, mention in test_cases: + with self.subTest(mock_type=mock_type, mention=mention): + mock = mock_type(mention=mention) + self.assertEqual(mock.mention, mention) class MockObjectTests(unittest.TestCase): @@ -266,14 +231,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_equality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly == eevee) @@ -281,14 +246,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_nonequality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly != python) @@ -298,7 +263,7 @@ class MockObjectTests(unittest.TestCase): """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" for mock in self.hashable_mocks: with self.subTest(mock_class=mock): - instance = helpers.MockRole(role_id=100) + instance = helpers.MockRole(id=100) self.assertEqual(hash(instance), instance.id) def test_mock_class_with_hashable_mixin_uses_id_for_equality(self): @@ -396,11 +361,11 @@ class MockObjectTests(unittest.TestCase): @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") def test_custom_mock_mixin_init_with_spec(self, extract_method_mock): """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" - spec = "pydis" + spec_set = "pydis" - helpers.CustomMockMixin(spec=spec) + helpers.CustomMockMixin(spec_set=spec_set) - extract_method_mock.assert_called_once_with(spec) + extract_method_mock.assert_called_once_with(spec_set) @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") -- cgit v1.2.3 From 7f4829e9fab007690d48188f499bfcc1a7baa437 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 17:29:09 +0100 Subject: Prevent await warnings for MockBot's create_task Previously, the coroutine object passed to `MockBot.loop.create_task` would trigger a `RuntimeWarning` for not being awaited as we do not actually create a task for it. To prevent these warnings, coroutine objects passed will now automatically be closed. --- tests/helpers.py | 8 +++++++- tests/test_helpers.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index 35f2c288c..8a14aeef4 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -227,7 +227,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin if roles: self.roles.extend(roles) - self.mention = f"@{self.name}" + if 'mention' not in kwargs: + self.mention = f"@{self.name}" # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` @@ -251,6 +252,11 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): # is technically incorrect, since it's a regular def.) self.wait_for = AsyncMock() + # Since calling `create_task` on our MockBot does not actually schedule the coroutine object + # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object + # to prevent "has not been awaited"-warnings. + self.loop.create_task.side_effect = lambda coroutine: coroutine.close() + # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { diff --git a/tests/test_helpers.py b/tests/test_helpers.py index e879ef97a..7894e104a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -202,6 +202,18 @@ class DiscordMocksTests(unittest.TestCase): mock = mock_type(mention=mention) self.assertEqual(mock.mention, mention) + def test_create_test_on_mock_bot_closes_passed_coroutine(self): + """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" + async def dementati(): + """Dummy coroutine for testing purposes.""" + + coroutine_object = dementati() + + bot = helpers.MockBot() + bot.loop.create_task(coroutine_object) + with self.assertRaises(RuntimeError, msg="cannot reuse already awaited coroutine"): + asyncio.run(coroutine_object) + class MockObjectTests(unittest.TestCase): """Tests the mock objects and mixins we've defined.""" -- cgit v1.2.3 From f212ddeea4de54d6eb75081c13162c2ad64bfeff Mon Sep 17 00:00:00 2001 From: Numerlor Date: Fri, 15 Nov 2019 13:10:19 +0100 Subject: join extra newline --- bot/cogs/doc.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index b82eac5fe..20bc010d9 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -336,8 +336,7 @@ class Doc(commands.Cog): ) # Show all symbols with the same name that were renamed in the footer. embed.set_footer( - text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} - if renamed.endswith(f".{symbol}")) + text=", ".join(renamed for renamed in self.renamed_symbols - {symbol} if renamed.endswith(f".{symbol}")) ) return embed -- cgit v1.2.3 From a0ed0c1d6c6d3ba32df4d9bb355ffe1a59e8f76b Mon Sep 17 00:00:00 2001 From: Numerlor Date: Fri, 15 Nov 2019 13:13:18 +0100 Subject: Add variable info after comment was deleted Co-authored-by: scargly <29337040+scragly@users.noreply.github.com> --- bot/cogs/doc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 20bc010d9..76fdcd831 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -180,7 +180,7 @@ class Doc(commands.Cog): return None for group, value in package.items(): - for symbol, (package_name, _, relative_doc_url, _) in value.items(): + for symbol, (package_name, _version, relative_doc_url, _) in value.items(): absolute_doc_url = base_url + relative_doc_url if symbol in self.inventories: -- cgit v1.2.3 From f1180d9cd05329f61439c8a45dedb47e841e7216 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Fri, 15 Nov 2019 13:35:44 +0100 Subject: group and order constants --- bot/cogs/doc.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bot/cogs/doc.py b/bot/cogs/doc.py index 76fdcd831..dc53937ee 100644 --- a/bot/cogs/doc.py +++ b/bot/cogs/doc.py @@ -26,7 +26,6 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) logging.getLogger('urllib3').setLevel(logging.WARNING) -NOT_FOUND_DELETE_DELAY = RedirectOutput.delete_delay NO_OVERRIDE_GROUPS = ( "2to3fixer", "token", @@ -37,8 +36,7 @@ NO_OVERRIDE_GROUPS = ( NO_OVERRIDE_PACKAGES = ( "python", ) -FAILED_REQUEST_RETRY_AMOUNT = 3 -UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") + SEARCH_END_TAG_ATTRS = ( "data", "function", @@ -49,8 +47,12 @@ SEARCH_END_TAG_ATTRS = ( "rubric", "sphinxsidebar", ) +UNWANTED_SIGNATURE_SYMBOLS_RE = re.compile(r"\[source]|\\\\|¶") WHITESPACE_AFTER_NEWLINES_RE = re.compile(r"(?<=\n\n)(\s+)") +FAILED_REQUEST_RETRY_AMOUNT = 3 +NOT_FOUND_DELETE_DELAY = RedirectOutput.delete_delay + def async_cache(max_size: int = 128, arg_offset: int = 0) -> Callable: """ -- cgit v1.2.3