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 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 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 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 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 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 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