From 9b8d688657f7044ad27ff81f7eb7d50f7f593ed6 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Fri, 25 Oct 2019 12:10:26 +0200 Subject: Autodelete offensive messages after one week. If the filter cog filter a message that's considered as offensive (filter["offensive_msg"] is True), the cog create a new offensive message object in the bot db with a delete_date of one week after it was sent. A background task run every day, pull up a list of message to delete, find them back, and delete them. --- bot/cogs/filtering.py | 89 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 1d1d74e74..d1d28ac10 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -1,12 +1,15 @@ +import asyncio +import datetime import logging import re from typing import Optional, Union import discord.errors from dateutil.relativedelta import relativedelta -from discord import Colour, DMChannel, Member, Message, TextChannel +from discord import Colour, DMChannel, Member, Message, NotFound, TextChannel from discord.ext.commands import Bot, Cog +from bot.api import ResponseCodeError from bot.cogs.moderation import ModLog from bot.constants import ( Channels, Colours, DEBUG_MODE, @@ -16,13 +19,13 @@ from bot.constants import ( log = logging.getLogger(__name__) INVITE_RE = re.compile( - r"(?:discord(?:[\.,]|dot)gg|" # Could be discord.gg/ - r"discord(?:[\.,]|dot)com(?:\/|slash)invite|" # or discord.com/invite/ + r"(?:discord(?:[\.,]|dot)gg|" # Could be discord.gg/ + r"discord(?:[\.,]|dot)com(?:\/|slash)invite|" # or discord.com/invite/ r"discordapp(?:[\.,]|dot)com(?:\/|slash)invite|" # or discordapp.com/invite/ - r"discord(?:[\.,]|dot)me|" # or discord.me - r"discord(?:[\.,]|dot)io" # or discord.io. - r")(?:[\/]|slash)" # / or 'slash' - r"([a-zA-Z0-9]+)", # the invite code itself + r"discord(?:[\.,]|dot)me|" # or discord.me + r"discord(?:[\.,]|dot)io" # or discord.io. + r")(?:[\/]|slash)" # / or 'slash' + r"([a-zA-Z0-9]+)", # the invite code itself flags=re.IGNORECASE ) @@ -36,6 +39,8 @@ TOKEN_WATCHLIST_PATTERNS = [ re.compile(fr'{expression}', flags=re.IGNORECASE) for expression in Filter.token_watchlist ] +OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) # Time before an offensive msg is deleted. + class Filtering(Cog): """Filtering out invites, blacklisting domains, and warning us of certain regular expressions.""" @@ -54,7 +59,8 @@ class Filtering(Cog): "notification_msg": ( "Your post has been removed for abusing Unicode character rendering (aka Zalgo text). " f"{_staff_mistake_str}" - ) + ), + "offensive_msg": False }, "filter_invites": { "enabled": Filter.filter_invites, @@ -65,7 +71,8 @@ class Filtering(Cog): "notification_msg": ( f"Per Rule 10, your invite link has been removed. {_staff_mistake_str}\n\n" r"Our server rules can be found here: " - ) + ), + "offensive_msg": False }, "filter_domains": { "enabled": Filter.filter_domains, @@ -75,28 +82,47 @@ class Filtering(Cog): "user_notification": Filter.notify_user_domains, "notification_msg": ( f"Your URL has been removed because it matched a blacklisted domain. {_staff_mistake_str}" - ) + ), + "offensive_msg": False }, "watch_rich_embeds": { "enabled": Filter.watch_rich_embeds, "function": self._has_rich_embed, "type": "watchlist", "content_only": False, + "offensive_msg": False }, "watch_words": { "enabled": Filter.watch_words, "function": self._has_watchlist_words, "type": "watchlist", "content_only": True, + "offensive_msg": True }, "watch_tokens": { "enabled": Filter.watch_tokens, "function": self._has_watchlist_tokens, "type": "watchlist", "content_only": True, + "offensive_msg": True }, } + self.deletion_task = None + self.bot.loop.create_task(self.init_deletion_task()) + + def cog_unload(self) -> None: + """Cancel any running updater tasks on cog unload.""" + if self.deletion_task is not None: + self.deletion_task.cancel() + + async def init_deletion_task(self) -> None: + """Start offensive messages deletion event loop if it hasn't already started.""" + await self.bot.wait_until_ready() + if self.deletion_task is None: + coro = delete_offensive_msg(self.bot) + self.deletion_task = self.bot.loop.create_task(coro) + @property def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" @@ -159,6 +185,21 @@ class Filtering(Cog): triggered = await _filter["function"](msg) if triggered: + # If the message is classed as offensive, we store it in the site db and + # it will be deleted it after one week. + if _filter["offensive_msg"]: + delete_date = msg.created_at.date() + OFFENSIVE_MSG_DELETE_TIME + await self.bot.api_client.post( + 'bot/offensive-message', + json={ + 'id': msg.id, + 'channel_id': msg.channel.id, + 'delete_date': delete_date.isoformat() + } + ) + log.trace(f"Offensive message will be deleted on " + f"{delete_date.isoformat()}") + # If this is a filter (not a watchlist), we should delete the message. if _filter["type"] == "filter": try: @@ -360,6 +401,34 @@ class Filtering(Cog): await channel.send(f"{filtered_member.mention} {reason}") +async def delete_offensive_msg(bot: Bot) -> None: + """Background task that pull up a list of offensive messages every day and delete them.""" + while True: + tomorrow = datetime.date.today() + datetime.timedelta(days=1) + time_until_next = datetime.datetime(tomorrow.year, tomorrow.month, tomorrow.day) - datetime.datetime.now() + try: + msg_list = await bot.api_client.get( + 'bot/offensive-message', + params={'delete_date': datetime.date.today().isoformat()} + ) + except ResponseCodeError as e: + log.error(f"Failed to get offending messages to delete (got code {e.response.status}), " + f"retrying in 30 minutes.") + time_until_next = datetime.timedelta(minutes=30) + msg_list = [] + for msg in msg_list: + try: + channel = bot.get_channel(msg['channel_id']) + if channel: + msg_obj = await channel.fetch_message(msg['id']) + await msg_obj.delete() + except NotFound: + log.info(f"Tried to delete message {msg['id']}, but the message can't be found " + f"(it has been probably already deleted).") + log.info(f"Deleted {len(msg_list)} offensive message(s).") + await asyncio.sleep(time_until_next.seconds) + + def setup(bot: Bot) -> None: """Filtering cog load.""" bot.add_cog(Filtering(bot)) -- cgit v1.2.3 From 59914bf3e741d654d999ff5eb7f9c12621628c6b Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Fri, 25 Oct 2019 15:00:20 +0200 Subject: Revert whitespace changes --- bot/cogs/filtering.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index d1d28ac10..4c99be0af 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -19,13 +19,13 @@ from bot.constants import ( log = logging.getLogger(__name__) INVITE_RE = re.compile( - r"(?:discord(?:[\.,]|dot)gg|" # Could be discord.gg/ - r"discord(?:[\.,]|dot)com(?:\/|slash)invite|" # or discord.com/invite/ + r"(?:discord(?:[\.,]|dot)gg|" # Could be discord.gg/ + r"discord(?:[\.,]|dot)com(?:\/|slash)invite|" # or discord.com/invite/ r"discordapp(?:[\.,]|dot)com(?:\/|slash)invite|" # or discordapp.com/invite/ - r"discord(?:[\.,]|dot)me|" # or discord.me - r"discord(?:[\.,]|dot)io" # or discord.io. - r")(?:[\/]|slash)" # / or 'slash' - r"([a-zA-Z0-9]+)", # the invite code itself + r"discord(?:[\.,]|dot)me|" # or discord.me + r"discord(?:[\.,]|dot)io" # or discord.io. + r")(?:[\/]|slash)" # / or 'slash' + r"([a-zA-Z0-9]+)", # the invite code itself flags=re.IGNORECASE ) -- cgit v1.2.3 From 9c78146c79dd7b4c3a633f3eabeef2036eb8ab7f Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Fri, 25 Oct 2019 18:39:58 +0200 Subject: Move offensive message delete time to config file. --- bot/cogs/filtering.py | 2 +- bot/constants.py | 1 + config-default.yml | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 4c99be0af..f9aee5a9a 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -39,7 +39,7 @@ TOKEN_WATCHLIST_PATTERNS = [ re.compile(fr'{expression}', flags=re.IGNORECASE) for expression in Filter.token_watchlist ] -OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=7) # Time before an offensive msg is deleted. +OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_time) class Filtering(Cog): diff --git a/bot/constants.py b/bot/constants.py index 4beae84e9..6106d911c 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -211,6 +211,7 @@ class Filter(metaclass=YAMLGetter): notify_user_domains: bool ping_everyone: bool + offensive_msg_delete_time: int guild_invite_whitelist: List[int] domain_blacklist: List[str] word_watchlist: List[str] diff --git a/config-default.yml b/config-default.yml index 197743296..fc702e991 100644 --- a/config-default.yml +++ b/config-default.yml @@ -161,7 +161,8 @@ filter: notify_user_domains: false # Filter configuration - ping_everyone: true # Ping @everyone when we send a mod-alert? + ping_everyone: true # Ping @everyone when we send a mod-alert? + offensive_msg_delete_time: 7 # How many days before deleting an offensive message? guild_invite_whitelist: - 280033776820813825 # Functional Programming -- cgit v1.2.3 From f67378c77a45c581c57d1dfdd5a704319e83ba3a Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Fri, 25 Oct 2019 18:43:27 +0200 Subject: Remove the possibility that we send a message to the API that the filter has already deleted. --- bot/cogs/filtering.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index f9aee5a9a..ea6919707 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -185,21 +185,6 @@ class Filtering(Cog): triggered = await _filter["function"](msg) if triggered: - # If the message is classed as offensive, we store it in the site db and - # it will be deleted it after one week. - if _filter["offensive_msg"]: - delete_date = msg.created_at.date() + OFFENSIVE_MSG_DELETE_TIME - await self.bot.api_client.post( - 'bot/offensive-message', - json={ - 'id': msg.id, - 'channel_id': msg.channel.id, - 'delete_date': delete_date.isoformat() - } - ) - log.trace(f"Offensive message will be deleted on " - f"{delete_date.isoformat()}") - # If this is a filter (not a watchlist), we should delete the message. if _filter["type"] == "filter": try: @@ -216,6 +201,21 @@ class Filtering(Cog): except discord.errors.NotFound: return + # If the message is classed as offensive, we store it in the site db and + # it will be deleted it after one week. + if _filter["offensive_msg"]: + delete_date = msg.created_at.date() + OFFENSIVE_MSG_DELETE_TIME + await self.bot.api_client.post( + 'bot/offensive-message', + json={ + 'id': msg.id, + 'channel_id': msg.channel.id, + 'delete_date': delete_date.isoformat() + } + ) + log.trace(f"Offensive message will be deleted on " + f"{delete_date.isoformat()}") + # Notify the user if the filter specifies if _filter["user_notification"]: await self.notify_member(msg.author, _filter["notification_msg"], msg.channel) -- cgit v1.2.3 From 1eb057b229c9dcae15834c55faf7188360b675a2 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Fri, 25 Oct 2019 18:44:50 +0200 Subject: Rename offensive_msg flag to schedule_deletion. --- bot/cogs/filtering.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index ea6919707..1342dade8 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -60,7 +60,7 @@ class Filtering(Cog): "Your post has been removed for abusing Unicode character rendering (aka Zalgo text). " f"{_staff_mistake_str}" ), - "offensive_msg": False + "schedule_deletion": False }, "filter_invites": { "enabled": Filter.filter_invites, @@ -72,7 +72,7 @@ class Filtering(Cog): f"Per Rule 10, your invite link has been removed. {_staff_mistake_str}\n\n" r"Our server rules can be found here: " ), - "offensive_msg": False + "schedule_deletion": False }, "filter_domains": { "enabled": Filter.filter_domains, @@ -83,28 +83,28 @@ class Filtering(Cog): "notification_msg": ( f"Your URL has been removed because it matched a blacklisted domain. {_staff_mistake_str}" ), - "offensive_msg": False + "schedule_deletion": False }, "watch_rich_embeds": { "enabled": Filter.watch_rich_embeds, "function": self._has_rich_embed, "type": "watchlist", "content_only": False, - "offensive_msg": False + "schedule_deletion": False }, "watch_words": { "enabled": Filter.watch_words, "function": self._has_watchlist_words, "type": "watchlist", "content_only": True, - "offensive_msg": True + "schedule_deletion": True }, "watch_tokens": { "enabled": Filter.watch_tokens, "function": self._has_watchlist_tokens, "type": "watchlist", "content_only": True, - "offensive_msg": True + "schedule_deletion": True }, } @@ -203,7 +203,7 @@ class Filtering(Cog): # If the message is classed as offensive, we store it in the site db and # it will be deleted it after one week. - if _filter["offensive_msg"]: + if _filter["schedule_deletion"]: delete_date = msg.created_at.date() + OFFENSIVE_MSG_DELETE_TIME await self.bot.api_client.post( 'bot/offensive-message', -- cgit v1.2.3 From f306c4153a5d4fb969856a2282e7ff3f7a111885 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Fri, 25 Oct 2019 21:16:04 +0200 Subject: Use Scheduler instead of a custom async loop --- bot/cogs/filtering.py | 84 ++++++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 1342dade8..35c14f101 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -2,19 +2,20 @@ import asyncio import datetime import logging import re -from typing import Optional, Union +from typing import Mapping, Optional, Union import discord.errors from dateutil.relativedelta import relativedelta from discord import Colour, DMChannel, Member, Message, NotFound, TextChannel from discord.ext.commands import Bot, Cog -from bot.api import ResponseCodeError from bot.cogs.moderation import ModLog from bot.constants import ( Channels, Colours, DEBUG_MODE, Filter, Icons, URLs ) +from bot.utils.scheduling import Scheduler +from bot.utils.time import wait_until log = logging.getLogger(__name__) @@ -42,11 +43,12 @@ TOKEN_WATCHLIST_PATTERNS = [ OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_time) -class Filtering(Cog): +class Filtering(Cog, Scheduler): """Filtering out invites, blacklisting domains, and warning us of certain regular expressions.""" def __init__(self, bot: Bot): self.bot = bot + super().__init__() _staff_mistake_str = "If you believe this was a mistake, please let staff know!" self.filters = { @@ -109,19 +111,7 @@ class Filtering(Cog): } self.deletion_task = None - self.bot.loop.create_task(self.init_deletion_task()) - - def cog_unload(self) -> None: - """Cancel any running updater tasks on cog unload.""" - if self.deletion_task is not None: - self.deletion_task.cancel() - - async def init_deletion_task(self) -> None: - """Start offensive messages deletion event loop if it hasn't already started.""" - await self.bot.wait_until_ready() - if self.deletion_task is None: - coro = delete_offensive_msg(self.bot) - self.deletion_task = self.bot.loop.create_task(coro) + self.bot.loop.create_task(self.reschedule_offensive_msg_deletion()) @property def mod_log(self) -> ModLog: @@ -400,33 +390,45 @@ class Filtering(Cog): except discord.errors.Forbidden: await channel.send(f"{filtered_member.mention} {reason}") + async def _scheduled_task(self, msg: dict) -> None: + """A coroutine which delete the offensive message once the delete date is reached.""" + delete_at = datetime.datetime.fromisoformat(msg['delete_date'][:-1]) + + await wait_until(delete_at) + await self.delete_offensive_msg(msg) + + self.cancel_task(msg['id']) + + async def reschedule_offensive_msg_deletion(self) -> None: + """Get all the pending message deletion from the API and reschedule them.""" + await self.bot.wait_until_ready() + response = await self.bot.api_client.get( + 'bot/offensive-message', + ) + + now = datetime.datetime.utcnow() + loop = asyncio.get_event_loop() -async def delete_offensive_msg(bot: Bot) -> None: - """Background task that pull up a list of offensive messages every day and delete them.""" - while True: - tomorrow = datetime.date.today() + datetime.timedelta(days=1) - time_until_next = datetime.datetime(tomorrow.year, tomorrow.month, tomorrow.day) - datetime.datetime.now() + for msg in response: + delete_at = datetime.datetime.fromisoformat(msg['delete_date'][:-1]) + + if delete_at < now: + await self.delete_offensive_msg(msg) + else: + self.schedule_task(loop, msg['id'], msg) + + async def delete_offensive_msg(self, msg: Mapping[str, str]) -> None: + """Delete an offensive message, and then delete it from the db.""" try: - msg_list = await bot.api_client.get( - 'bot/offensive-message', - params={'delete_date': datetime.date.today().isoformat()} - ) - except ResponseCodeError as e: - log.error(f"Failed to get offending messages to delete (got code {e.response.status}), " - f"retrying in 30 minutes.") - time_until_next = datetime.timedelta(minutes=30) - msg_list = [] - for msg in msg_list: - try: - channel = bot.get_channel(msg['channel_id']) - if channel: - msg_obj = await channel.fetch_message(msg['id']) - await msg_obj.delete() - except NotFound: - log.info(f"Tried to delete message {msg['id']}, but the message can't be found " - f"(it has been probably already deleted).") - log.info(f"Deleted {len(msg_list)} offensive message(s).") - await asyncio.sleep(time_until_next.seconds) + channel = self.bot.get_channel(msg['channel_id']) + if channel: + msg_obj = await channel.fetch_message(msg['id']) + await msg_obj.delete() + except NotFound: + log.info(f"Tried to delete message {msg['id']}, but the message can't be found " + f"(it has been probably already deleted).") + await self.bot.api_client.delete(f'bot/offensive-message/{msg["id"]}') + log.info(f"Deleted the offensive message with id {msg['id']}.") def setup(bot: Bot) -> None: -- cgit v1.2.3 From 95c6e56891a21ebb2d1555cf850daad375d57afe Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 26 Oct 2019 11:11:15 +0200 Subject: Switch to datetime.datetime --- bot/cogs/filtering.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 5bd72a584..8962a85c1 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -187,25 +187,25 @@ class Filtering(Cog, Scheduler): except discord.errors.NotFound: return + # Notify the user if the filter specifies + if _filter["user_notification"]: + await self.notify_member(msg.author, _filter["notification_msg"], msg.channel) + # If the message is classed as offensive, we store it in the site db and # it will be deleted it after one week. if _filter["schedule_deletion"]: - delete_date = msg.created_at.date() + OFFENSIVE_MSG_DELETE_TIME + delete_date = msg.created_at + OFFENSIVE_MSG_DELETE_TIME await self.bot.api_client.post( 'bot/offensive-message', json={ 'id': msg.id, 'channel_id': msg.channel.id, - 'delete_date': delete_date.isoformat() + 'delete_date': delete_date.isoformat()[:-1] } ) log.trace(f"Offensive message will be deleted on " f"{delete_date.isoformat()}") - # Notify the user if the filter specifies - if _filter["user_notification"]: - await self.notify_member(msg.author, _filter["notification_msg"], msg.channel) - if isinstance(msg.channel, DMChannel): channel_str = "via DM" else: -- cgit v1.2.3 From b57811ea12aa41285e4e4585b951dd105be4b275 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Thu, 12 Dec 2019 09:25:53 +0100 Subject: Add space for readability Co-Authored-By: Mark --- bot/cogs/filtering.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 61c8f389b..39ba22354 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -436,6 +436,7 @@ class Filtering(Cog, Scheduler): except NotFound: log.info(f"Tried to delete message {msg['id']}, but the message can't be found " f"(it has been probably already deleted).") + await self.bot.api_client.delete(f'bot/offensive-message/{msg["id"]}') log.info(f"Deleted the offensive message with id {msg['id']}.") -- cgit v1.2.3 From bc9b335454d0183013b4f89f2923b638dcc127ba Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Thu, 12 Dec 2019 09:38:26 +0100 Subject: Make use of the Bot subclass --- 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 ca84e6240..9d88d9153 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -7,7 +7,7 @@ from typing import Mapping, Optional, Union import discord.errors from dateutil.relativedelta import relativedelta from discord import Colour, DMChannel, Member, Message, NotFound, TextChannel -from discord.ext.commands import Bot, Cog +from discord.ext.commands import Cog from bot.bot import Bot from bot.cogs.moderation import ModLog -- cgit v1.2.3 From 7eea7ad353239a0be1dfd744486e3f46a99cd661 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:21:44 +0100 Subject: Filtering cog clean up --- bot/cogs/filtering.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 9d88d9153..172c5fa7e 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -111,7 +111,6 @@ class Filtering(Cog, Scheduler): }, } - self.deletion_task = None self.bot.loop.create_task(self.reschedule_offensive_msg_deletion()) @property @@ -201,11 +200,13 @@ class Filtering(Cog, Scheduler): json={ 'id': msg.id, 'channel_id': msg.channel.id, - 'delete_date': delete_date.isoformat()[:-1] + 'delete_date': delete_date.isoformat() } ) - log.trace(f"Offensive message will be deleted on " - f"{delete_date.isoformat()}") + log.trace( + f"Offensive message will be deleted on " + f"{delete_date.isoformat()}" + ) if isinstance(msg.channel, DMChannel): channel_str = "via DM" @@ -412,9 +413,7 @@ class Filtering(Cog, Scheduler): async def reschedule_offensive_msg_deletion(self) -> None: """Get all the pending message deletion from the API and reschedule them.""" await self.bot.wait_until_ready() - response = await self.bot.api_client.get( - 'bot/offensive-message', - ) + response = await self.bot.api_client.get('bot/offensive-message',) now = datetime.datetime.utcnow() loop = asyncio.get_event_loop() @@ -435,8 +434,10 @@ class Filtering(Cog, Scheduler): msg_obj = await channel.fetch_message(msg['id']) await msg_obj.delete() except NotFound: - log.info(f"Tried to delete message {msg['id']}, but the message can't be found " - f"(it has been probably already deleted).") + log.info( + f"Tried to delete message {msg['id']}, but the message can't be found " + f"(it has been probably already deleted)." + ) await self.bot.api_client.delete(f'bot/offensive-message/{msg["id"]}') log.info(f"Deleted the offensive message with id {msg['id']}.") -- cgit v1.2.3 From ba5af375a3acfc160de1ffefa063a915318e6bdd Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:25:59 +0100 Subject: Make use of dateutil.parser.isoparse --- bot/cogs/filtering.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 172c5fa7e..4388b29ad 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -4,6 +4,7 @@ import logging import re from typing import Mapping, Optional, Union +import dateutil import discord.errors from dateutil.relativedelta import relativedelta from discord import Colour, DMChannel, Member, Message, NotFound, TextChannel @@ -403,7 +404,7 @@ class Filtering(Cog, Scheduler): async def _scheduled_task(self, msg: dict) -> None: """A coroutine which delete the offensive message once the delete date is reached.""" - delete_at = datetime.datetime.fromisoformat(msg['delete_date'][:-1]) + delete_at = dateutil.parser.isoparse(msg['delete_date']) await wait_until(delete_at) await self.delete_offensive_msg(msg) @@ -419,7 +420,7 @@ class Filtering(Cog, Scheduler): loop = asyncio.get_event_loop() for msg in response: - delete_at = datetime.datetime.fromisoformat(msg['delete_date'][:-1]) + delete_at = dateutil.parser.isoparse(msg['delete_date']) if delete_at < now: await self.delete_offensive_msg(msg) -- cgit v1.2.3 From e6940b938882aaeda18baa9fcc23cc297c6cfcd2 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:31:32 +0100 Subject: Rename config entry to offensive_msg_delete_days --- bot/cogs/filtering.py | 2 +- bot/constants.py | 2 +- config-default.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 4388b29ad..c0e115a8f 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -42,7 +42,7 @@ TOKEN_WATCHLIST_PATTERNS = [ re.compile(fr'{expression}', flags=re.IGNORECASE) for expression in Filter.token_watchlist ] -OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_time) +OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_day) class Filtering(Cog, Scheduler): diff --git a/bot/constants.py b/bot/constants.py index 075722a01..e6f23ff61 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -211,7 +211,7 @@ class Filter(metaclass=YAMLGetter): notify_user_domains: bool ping_everyone: bool - offensive_msg_delete_time: int + offensive_msg_delete_day: int guild_invite_whitelist: List[int] domain_blacklist: List[str] word_watchlist: List[str] diff --git a/config-default.yml b/config-default.yml index 0765407af..33072790b 100644 --- a/config-default.yml +++ b/config-default.yml @@ -180,7 +180,7 @@ filter: # Filter configuration ping_everyone: true # Ping @everyone when we send a mod-alert? - offensive_msg_delete_time: 7 # How many days before deleting an offensive message? + offensive_msg_delete_days: 7 # How many days before deleting an offensive message? guild_invite_whitelist: - 280033776820813825 # Functional Programming -- cgit v1.2.3 From b2a5ab90622e986ace37be6204d6a59823390cce Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:36:37 +0100 Subject: Catch all HTTPExecption --- bot/cogs/filtering.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index c0e115a8f..c0a2c7d3b 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -7,7 +7,7 @@ from typing import Mapping, Optional, Union import dateutil import discord.errors from dateutil.relativedelta import relativedelta -from discord import Colour, DMChannel, Member, Message, NotFound, TextChannel +from discord import Colour, DMChannel, HTTPException, Member, Message, NotFound, TextChannel from discord.ext.commands import Cog from bot.bot import Bot @@ -439,6 +439,10 @@ class Filtering(Cog, Scheduler): f"Tried to delete message {msg['id']}, but the message can't be found " f"(it has been probably already deleted)." ) + except HTTPException: + log.warning( + f"Failed to delete message {msg['id']}." + ) await self.bot.api_client.delete(f'bot/offensive-message/{msg["id"]}') log.info(f"Deleted the offensive message with id {msg['id']}.") -- cgit v1.2.3 From ab6b0032ceb6ad638cee6f174778bc38254ab038 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:41:30 +0100 Subject: Actually schedule message for deletion --- bot/cogs/filtering.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index c0a2c7d3b..0879bfee6 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -204,6 +204,8 @@ class Filtering(Cog, Scheduler): 'delete_date': delete_date.isoformat() } ) + loop = asyncio.get_event_loop() + self.schedule_task(loop, msg.id, {'id': msg.id, 'channel_id': msg.channel.id}) log.trace( f"Offensive message will be deleted on " f"{delete_date.isoformat()}" -- cgit v1.2.3 From a81ad8f1f7d548f20d1f2428a808e14dbbfe22bc Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:42:10 +0100 Subject: Fix docstring typo --- 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 0879bfee6..2c3f41c05 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -405,7 +405,7 @@ class Filtering(Cog, Scheduler): await channel.send(f"{filtered_member.mention} {reason}") async def _scheduled_task(self, msg: dict) -> None: - """A coroutine which delete the offensive message once the delete date is reached.""" + """A coroutine that delete the offensive message once the delete date is reached.""" delete_at = dateutil.parser.isoparse(msg['delete_date']) await wait_until(delete_at) -- cgit v1.2.3 From 15958986ae6bcb508c4e2dd23c51624d4ee26cb5 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:47:08 +0100 Subject: Rename route /bot/offensive-message to /bot/offensive-messages --- bot/cogs/filtering.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 2c3f41c05..63f8685c9 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -197,7 +197,7 @@ class Filtering(Cog, Scheduler): if _filter["schedule_deletion"]: delete_date = msg.created_at + OFFENSIVE_MSG_DELETE_TIME await self.bot.api_client.post( - 'bot/offensive-message', + 'bot/offensive-messages', json={ 'id': msg.id, 'channel_id': msg.channel.id, @@ -416,7 +416,7 @@ class Filtering(Cog, Scheduler): async def reschedule_offensive_msg_deletion(self) -> None: """Get all the pending message deletion from the API and reschedule them.""" await self.bot.wait_until_ready() - response = await self.bot.api_client.get('bot/offensive-message',) + response = await self.bot.api_client.get('bot/offensive-messages',) now = datetime.datetime.utcnow() loop = asyncio.get_event_loop() @@ -446,7 +446,7 @@ class Filtering(Cog, Scheduler): f"Failed to delete message {msg['id']}." ) - await self.bot.api_client.delete(f'bot/offensive-message/{msg["id"]}') + await self.bot.api_client.delete(f'bot/offensive-messages/{msg["id"]}') log.info(f"Deleted the offensive message with id {msg['id']}.") -- cgit v1.2.3 From 2f5b9fbd7bc2a11452a982f61c7603d589ded95a Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sat, 14 Dec 2019 11:52:54 +0100 Subject: Make setting filter.offensive_msg_delete_days plural --- bot/cogs/filtering.py | 2 +- bot/constants.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 63f8685c9..a709fe7cd 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -42,7 +42,7 @@ TOKEN_WATCHLIST_PATTERNS = [ re.compile(fr'{expression}', flags=re.IGNORECASE) for expression in Filter.token_watchlist ] -OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_day) +OFFENSIVE_MSG_DELETE_TIME = datetime.timedelta(days=Filter.offensive_msg_delete_days) class Filtering(Cog, Scheduler): diff --git a/bot/constants.py b/bot/constants.py index e6f23ff61..f47688185 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -211,7 +211,7 @@ class Filter(metaclass=YAMLGetter): notify_user_domains: bool ping_everyone: bool - offensive_msg_delete_day: int + offensive_msg_delete_days: int guild_invite_whitelist: List[int] domain_blacklist: List[str] word_watchlist: List[str] -- cgit v1.2.3 From 4cf2166d6dee6a04a055db7e14cf4ece0656213a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 15 Dec 2019 09:39:58 -0800 Subject: Filtering: log the status code of caught HTTPException --- bot/cogs/filtering.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index a709fe7cd..4d91432e7 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -441,10 +441,8 @@ class Filtering(Cog, Scheduler): f"Tried to delete message {msg['id']}, but the message can't be found " f"(it has been probably already deleted)." ) - except HTTPException: - log.warning( - f"Failed to delete message {msg['id']}." - ) + except HTTPException as e: + log.warning(f"Failed to delete message {msg['id']}: status {e.status}") await self.bot.api_client.delete(f'bot/offensive-messages/{msg["id"]}') log.info(f"Deleted the offensive message with id {msg['id']}.") -- cgit v1.2.3 From 7e4a43546490d90c619d08837da69466768fae80 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 15 Dec 2019 09:39:00 -0800 Subject: Filtering: refactor scheduling of deletion task --- bot/cogs/filtering.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 4d91432e7..4c94b73a5 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -1,4 +1,3 @@ -import asyncio import datetime import logging import re @@ -195,21 +194,19 @@ class Filtering(Cog, Scheduler): # If the message is classed as offensive, we store it in the site db and # it will be deleted it after one week. if _filter["schedule_deletion"]: - delete_date = msg.created_at + OFFENSIVE_MSG_DELETE_TIME + delete_date = (msg.created_at + OFFENSIVE_MSG_DELETE_TIME).isoformat() await self.bot.api_client.post( 'bot/offensive-messages', json={ 'id': msg.id, 'channel_id': msg.channel.id, - 'delete_date': delete_date.isoformat() + 'delete_date': delete_date } ) - loop = asyncio.get_event_loop() - self.schedule_task(loop, msg.id, {'id': msg.id, 'channel_id': msg.channel.id}) - log.trace( - f"Offensive message will be deleted on " - f"{delete_date.isoformat()}" - ) + + task_data = {'id': msg.id, 'channel_id': msg.channel.id} + self.schedule_task(self.bot.loop, msg.id, task_data) + log.trace(f"Offensive message {msg.id} will be deleted on {delete_date}") if isinstance(msg.channel, DMChannel): channel_str = "via DM" @@ -335,7 +332,7 @@ class Filtering(Cog, Scheduler): Attempts to catch some of common ways to try to cheat the system. """ - # Remove backslashes to prevent escape character aroundfuckery like + # Remove backslashes to prevent escape character around fuckery like # discord\.gg/gdudes-pony-farm text = text.replace("\\", "") @@ -405,7 +402,7 @@ class Filtering(Cog, Scheduler): await channel.send(f"{filtered_member.mention} {reason}") async def _scheduled_task(self, msg: dict) -> None: - """A coroutine that delete the offensive message once the delete date is reached.""" + """Delete an offensive message once its deletion date is reached.""" delete_at = dateutil.parser.isoparse(msg['delete_date']) await wait_until(delete_at) @@ -419,7 +416,6 @@ class Filtering(Cog, Scheduler): response = await self.bot.api_client.get('bot/offensive-messages',) now = datetime.datetime.utcnow() - loop = asyncio.get_event_loop() for msg in response: delete_at = dateutil.parser.isoparse(msg['delete_date']) @@ -427,7 +423,7 @@ class Filtering(Cog, Scheduler): if delete_at < now: await self.delete_offensive_msg(msg) else: - self.schedule_task(loop, msg['id'], msg) + self.schedule_task(self.bot.loop, msg['id'], msg) async def delete_offensive_msg(self, msg: Mapping[str, str]) -> None: """Delete an offensive message, and then delete it from the db.""" -- cgit v1.2.3 From 832762add664186665904817e9ffc25f79ffb20a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 15 Dec 2019 10:03:19 -0800 Subject: Filtering: fix comparison between tz naïve and aware datetimes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 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 4c94b73a5..02e3011ab 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -418,7 +418,7 @@ class Filtering(Cog, Scheduler): now = datetime.datetime.utcnow() for msg in response: - delete_at = dateutil.parser.isoparse(msg['delete_date']) + delete_at = dateutil.parser.isoparse(msg['delete_date']).replace(tzinfo=None) if delete_at < now: await self.delete_offensive_msg(msg) -- cgit v1.2.3 From b36206e8c62459f22685c285fb1a7e299f08b1bd Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 15 Dec 2019 11:12:56 -0800 Subject: Filtering: fix missing deletion date in scheduled task data --- bot/cogs/filtering.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 02e3011ab..83e706a26 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -195,17 +195,14 @@ class Filtering(Cog, Scheduler): # it will be deleted it after one week. if _filter["schedule_deletion"]: delete_date = (msg.created_at + OFFENSIVE_MSG_DELETE_TIME).isoformat() - await self.bot.api_client.post( - 'bot/offensive-messages', - json={ - 'id': msg.id, - 'channel_id': msg.channel.id, - 'delete_date': delete_date - } - ) - - task_data = {'id': msg.id, 'channel_id': msg.channel.id} - self.schedule_task(self.bot.loop, msg.id, task_data) + data = { + 'id': msg.id, + 'channel_id': msg.channel.id, + 'delete_date': delete_date + } + + await self.bot.api_client.post('bot/offensive-messages', json=data) + self.schedule_task(self.bot.loop, msg.id, data) log.trace(f"Offensive message {msg.id} will be deleted on {delete_date}") if isinstance(msg.channel, DMChannel): -- cgit v1.2.3 From 0323919b08342fd650ff32e1f3a2fc2d9eee9c59 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Sat, 29 Feb 2020 08:30:14 +0100 Subject: Make sure that the offensive message deletion date returned by the API is naive It could have caused an issue later with a mix of naive and aware datetime Co-Authored-By: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> --- 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 83e706a26..2d91695f3 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -400,7 +400,7 @@ class Filtering(Cog, Scheduler): async def _scheduled_task(self, msg: dict) -> None: """Delete an offensive message once its deletion date is reached.""" - delete_at = dateutil.parser.isoparse(msg['delete_date']) + delete_at = dateutil.parser.isoparse(msg['delete_date']).replace(tzinfo=None) await wait_until(delete_at) await self.delete_offensive_msg(msg) -- cgit v1.2.3 From ae14de8745c40ebc9e09d785c55f0fd472dc50ae Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Mon, 2 Mar 2020 17:42:34 +0100 Subject: Remove task self cancel. Recent changes to the scheduler requires this line to be removed. --- bot/cogs/filtering.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 2d91695f3..6c9e9a4b7 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -405,8 +405,6 @@ class Filtering(Cog, Scheduler): await wait_until(delete_at) await self.delete_offensive_msg(msg) - self.cancel_task(msg['id']) - async def reschedule_offensive_msg_deletion(self) -> None: """Get all the pending message deletion from the API and reschedule them.""" await self.bot.wait_until_ready() -- cgit v1.2.3 From 0fc2ebbfeb6f6296f8eb74a7191785bfaa551f90 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Mon, 2 Mar 2020 17:48:40 +0100 Subject: Delete the loop argument from schedule_task calls The function doesn't take the loop as an argument anymore --- bot/cogs/filtering.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 6c9e9a4b7..347554ae2 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -202,7 +202,7 @@ class Filtering(Cog, Scheduler): } await self.bot.api_client.post('bot/offensive-messages', json=data) - self.schedule_task(self.bot.loop, msg.id, data) + self.schedule_task(msg.id, data) log.trace(f"Offensive message {msg.id} will be deleted on {delete_date}") if isinstance(msg.channel, DMChannel): @@ -418,7 +418,7 @@ class Filtering(Cog, Scheduler): if delete_at < now: await self.delete_offensive_msg(msg) else: - self.schedule_task(self.bot.loop, msg['id'], msg) + self.schedule_task(msg['id'], msg) async def delete_offensive_msg(self, msg: Mapping[str, str]) -> None: """Delete an offensive message, and then delete it from the db.""" -- cgit v1.2.3 From 4890cc5ba43ad73229ce4d2fe240acaf39194edb Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Tue, 14 Apr 2020 08:30:18 +0300 Subject: Created tests for `bot.cogs.logging` connected message. --- tests/bot/cogs/test_logging.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/bot/cogs/test_logging.py diff --git a/tests/bot/cogs/test_logging.py b/tests/bot/cogs/test_logging.py new file mode 100644 index 000000000..ba98a5a56 --- /dev/null +++ b/tests/bot/cogs/test_logging.py @@ -0,0 +1,42 @@ +import unittest +from unittest.mock import patch + +from bot import constants +from bot.cogs.logging import Logging +from tests.helpers import MockBot, MockTextChannel + + +class LoggingTests(unittest.IsolatedAsyncioTestCase): + """Test cases for connected login.""" + + def setUp(self): + self.bot = MockBot() + self.cog = Logging(self.bot) + self.dev_log = MockTextChannel(id=1234, name="dev-log") + + @patch("bot.cogs.logging.DEBUG_MODE", False) + async def test_debug_mode_false(self): + """Should send connected message to dev-log.""" + self.bot.get_channel.return_value = self.dev_log + + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_called_once_with(constants.Channels.dev_log) + + embed = self.dev_log.send.call_args[1]['embed'] + self.dev_log.send.assert_awaited_once_with(embed=embed) + + self.assertEqual(embed.description, "Connected!") + self.assertEqual(embed.author.name, "Python Bot") + self.assertEqual(embed.author.url, "https://github.com/python-discord/bot") + self.assertEqual( + embed.author.icon_url, + "https://raw.githubusercontent.com/python-discord/branding/master/logos/logo_circle/logo_circle_large.png" + ) + + @patch("bot.cogs.logging.DEBUG_MODE", True) + async def test_debug_mode_true(self): + """Should not send anything to dev-log.""" + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_not_called() -- cgit v1.2.3 From ededd1879cfb914445342b202d4c66aed23ee94b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 22 May 2020 08:43:10 +0300 Subject: Logging Tests: Simplify `DEBUG_MODE` `False` test - Remove embed attributes checks - Replace `self.dev_log.assert_awaited_once_with` with `self.dev_log.assert_awaited_once`. --- tests/bot/cogs/test_logging.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/bot/cogs/test_logging.py b/tests/bot/cogs/test_logging.py index ba98a5a56..8a18fdcd6 100644 --- a/tests/bot/cogs/test_logging.py +++ b/tests/bot/cogs/test_logging.py @@ -22,17 +22,7 @@ class LoggingTests(unittest.IsolatedAsyncioTestCase): await self.cog.startup_greeting() self.bot.wait_until_guild_available.assert_awaited_once_with() self.bot.get_channel.assert_called_once_with(constants.Channels.dev_log) - - embed = self.dev_log.send.call_args[1]['embed'] - self.dev_log.send.assert_awaited_once_with(embed=embed) - - self.assertEqual(embed.description, "Connected!") - self.assertEqual(embed.author.name, "Python Bot") - self.assertEqual(embed.author.url, "https://github.com/python-discord/bot") - self.assertEqual( - embed.author.icon_url, - "https://raw.githubusercontent.com/python-discord/branding/master/logos/logo_circle/logo_circle_large.png" - ) + self.dev_log.send.assert_awaited_once() @patch("bot.cogs.logging.DEBUG_MODE", True) async def test_debug_mode_true(self): -- cgit v1.2.3 From 876b4846f612fe0011cc2e0b498b4df9e54d74cb Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 19:17:07 +0200 Subject: Add support for bool values in RedisCache We're gonna need this for the help channel handling, and it seems like a reasonable type to support anyway. It requires a tiny bit of special handling, but nothing outrageous. --- bot/utils/redis_cache.py | 14 ++++++++++++-- tests/bot/utils/test_redis_cache.py | 4 +++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index de80cee84..2926e7a89 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import logging +from distutils.util import strtobool from functools import partialmethod from typing import Any, Dict, ItemsView, Optional, Tuple, Union @@ -11,7 +12,7 @@ log = logging.getLogger(__name__) # Type aliases RedisKeyType = Union[str, int] -RedisValueType = Union[str, int, float] +RedisValueType = Union[str, int, float, bool] RedisKeyOrValue = Union[RedisKeyType, RedisValueType] # Prefix tuples @@ -20,6 +21,7 @@ _VALUE_PREFIXES = ( ("f|", float), ("i|", int), ("s|", str), + ("b|", bool), ) _KEY_PREFIXES = ( ("i|", int), @@ -117,7 +119,8 @@ class RedisCache: def _to_typestring(key_or_value: RedisKeyOrValue, prefixes: _PrefixTuple) -> str: """Turn a valid Redis type into a typestring.""" for prefix, _type in prefixes: - if isinstance(key_or_value, _type): + # isinstance is a bad idea here, because isintance(False, int) == True. + if type(key_or_value) is _type: return f"{prefix}{key_or_value}" raise TypeError(f"RedisCache._to_typestring only supports the following: {prefixes}.") @@ -131,6 +134,13 @@ class RedisCache: # Now we convert our unicode string back into the type it originally was. for prefix, _type in prefixes: if key_or_value.startswith(prefix): + + # For booleans, we need special handling because bool("False") is True. + if prefix == "b|": + value = key_or_value[len(prefix):] + return bool(strtobool(value)) + + # Otherwise we can just convert normally. return _type(key_or_value[len(prefix):]) raise TypeError(f"RedisCache._from_typestring only supports the following: {prefixes}.") diff --git a/tests/bot/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py index 8c1a40640..62c411681 100644 --- a/tests/bot/utils/test_redis_cache.py +++ b/tests/bot/utils/test_redis_cache.py @@ -59,7 +59,9 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase): test_cases = ( ('favorite_fruit', 'melon'), ('favorite_number', 86), - ('favorite_fraction', 86.54) + ('favorite_fraction', 86.54), + ('favorite_boolean', False), + ('other_boolean', True), ) # Test that we can get and set different types. -- cgit v1.2.3 From 699760a3d803c379dad1236f36c919eb0775e490 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 6 Jun 2020 00:49:33 +0200 Subject: Refactor help_channels.py to use RedisCache. More specifically, we're turning three dicts into RedisCaches: - help_channel_claimants - unanswered - claim_times These will still work the same way, but will now persist their contents across restarts. --- bot/cogs/help_channels.py | 60 +++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 70cef339a..8c01e5dc4 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -9,12 +9,14 @@ from contextlib import suppress from datetime import datetime from pathlib import Path +import dateutil import discord import discord.abc from discord.ext import commands from bot import constants from bot.bot import Bot +from bot.utils import RedisCache from bot.utils.checks import with_role_check from bot.utils.scheduling import Scheduler @@ -99,13 +101,24 @@ class HelpChannels(Scheduler, commands.Cog): Help channels are named after the chemical elements in `bot/resources/elements.json`. """ + # This cache tracks which channels are claimed by which members. + # RedisCache[discord.TextChannel.id, t.Union[discord.User.id, discord.Member.id]] + help_channel_claimants = RedisCache() + + # This cache maps a help channel to whether it has had any + # activity other than the original claimant. True being no other + # activity and False being other activity. + # RedisCache[discord.TextChannel.id, bool] + unanswered = RedisCache() + + # This dictionary maps a help channel to the time it was claimed + # RedisCache[discord.TextChannel.id, datetime.datetime] + claim_times = RedisCache() + def __init__(self, bot: Bot): super().__init__() self.bot = bot - self.help_channel_claimants: ( - t.Dict[discord.TextChannel, t.Union[discord.Member, discord.User]] - ) = {} # Categories self.available_category: discord.CategoryChannel = None @@ -125,16 +138,6 @@ class HelpChannels(Scheduler, commands.Cog): self.on_message_lock = asyncio.Lock() self.init_task = self.bot.loop.create_task(self.init_cog()) - # Stats - - # This dictionary maps a help channel to the time it was claimed - self.claim_times: t.Dict[int, datetime] = {} - - # This dictionary maps a help channel to whether it has had any - # activity other than the original claimant. True being no other - # activity and False being other activity. - self.unanswered: t.Dict[int, bool] = {} - def cog_unload(self) -> None: """Cancel the init task and scheduled tasks when the cog unloads.""" log.trace("Cog unload: cancelling the init_cog task") @@ -197,7 +200,7 @@ class HelpChannels(Scheduler, commands.Cog): async def dormant_check(self, ctx: commands.Context) -> bool: """Return True if the user is the help channel claimant or passes the role check.""" - if self.help_channel_claimants.get(ctx.channel) == ctx.author: + if await self.help_channel_claimants.get(ctx.channel.id) == ctx.author.id: log.trace(f"{ctx.author} is the help channel claimant, passing the check for dormant.") self.bot.stats.incr("help.dormant_invoke.claimant") return True @@ -223,7 +226,7 @@ class HelpChannels(Scheduler, commands.Cog): if ctx.channel.category == self.in_use_category: if await self.dormant_check(ctx): with suppress(KeyError): - del self.help_channel_claimants[ctx.channel] + await self.help_channel_claimants.delete(ctx.channel.id) await self.remove_cooldown_role(ctx.author) # Ignore missing task when cooldown has passed but the channel still isn't dormant. @@ -546,13 +549,14 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr(f"help.dormant_calls.{caller}") - if channel.id in self.claim_times: - claimed = self.claim_times[channel.id] + if await self.claim_times.contains(channel.id): + claimed_datestring = await self.claim_times.get(channel.id) + claimed = dateutil.parser.parse(claimed_datestring) in_use_time = datetime.now() - claimed self.bot.stats.timing("help.in_use_time", in_use_time) - if channel.id in self.unanswered: - if self.unanswered[channel.id]: + if await self.unanswered.contains(channel.id): + if await self.unanswered.get(channel.id): self.bot.stats.incr("help.sessions.unanswered") else: self.bot.stats.incr("help.sessions.answered") @@ -638,16 +642,16 @@ class HelpChannels(Scheduler, commands.Cog): log.trace(f"Checking if #{channel} ({channel.id}) has been answered.") # Check if there is an entry in unanswered (does not persist across restarts) - if channel.id in self.unanswered: - claimant = self.help_channel_claimants.get(channel) - if not claimant: - # The mapping for this channel was lost, we can't do anything. + if await self.unanswered.contains(channel.id): + claimant_id = await self.help_channel_claimants.get(channel.id) + if not claimant_id: + # The mapping for this channel doesn't exist, we can't do anything. return # Check the message did not come from the claimant - if claimant.id != message.author.id: + if claimant_id != message.author.id: # Mark the channel as answered - self.unanswered[channel.id] = False + await self.unanswered.set(channel.id, False) @commands.Cog.listener() async def on_message(self, message: discord.Message) -> None: @@ -680,12 +684,12 @@ class HelpChannels(Scheduler, commands.Cog): await self.move_to_in_use(channel) await self.revoke_send_permissions(message.author) # Add user with channel for dormant check. - self.help_channel_claimants[channel] = message.author + await self.help_channel_claimants.set(channel.id, message.author.id) self.bot.stats.incr("help.claimed") - self.claim_times[channel.id] = datetime.now() - self.unanswered[channel.id] = True + await self.claim_times.set(channel.id, str(datetime.now())) + await self.unanswered.set(channel.id, True) log.trace(f"Releasing on_message lock for {message.id}.") -- cgit v1.2.3 From fc4eddc7eee3670fdbe5726b13d28ddba57a156b Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 6 Jun 2020 11:57:18 +0200 Subject: Store booleans as integers instead of strings. This means we don't need to rely on strtobool, and is a cleaner implementation overall. Thanks @MarkKoz. --- bot/utils/redis_cache.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index 2926e7a89..347a0e54a 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -2,7 +2,6 @@ from __future__ import annotations import asyncio import logging -from distutils.util import strtobool from functools import partialmethod from typing import Any, Dict, ItemsView, Optional, Tuple, Union @@ -119,9 +118,15 @@ class RedisCache: def _to_typestring(key_or_value: RedisKeyOrValue, prefixes: _PrefixTuple) -> str: """Turn a valid Redis type into a typestring.""" for prefix, _type in prefixes: + # Convert bools into integers before storing them. + if type(key_or_value) is bool: + bool_int = int(key_or_value) + return f"{prefix}{bool_int}" + # isinstance is a bad idea here, because isintance(False, int) == True. if type(key_or_value) is _type: return f"{prefix}{key_or_value}" + raise TypeError(f"RedisCache._to_typestring only supports the following: {prefixes}.") @staticmethod @@ -138,7 +143,7 @@ class RedisCache: # For booleans, we need special handling because bool("False") is True. if prefix == "b|": value = key_or_value[len(prefix):] - return bool(strtobool(value)) + return bool(int(value)) # Otherwise we can just convert normally. return _type(key_or_value[len(prefix):]) -- cgit v1.2.3 From 94f096fab3bde10ba0da767c568c7a8c3ff3259f Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 6 Jun 2020 12:33:06 +0200 Subject: Store epoch timestamps instead of strings. We're also switching from datetime.now() to datetime.utcnow(). --- bot/cogs/help_channels.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 8c01e5dc4..dd3e3cb8b 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -9,7 +9,6 @@ from contextlib import suppress from datetime import datetime from pathlib import Path -import dateutil import discord import discord.abc from discord.ext import commands @@ -550,9 +549,9 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr(f"help.dormant_calls.{caller}") if await self.claim_times.contains(channel.id): - claimed_datestring = await self.claim_times.get(channel.id) - claimed = dateutil.parser.parse(claimed_datestring) - in_use_time = datetime.now() - claimed + claimed_timestamp = await self.claim_times.get(channel.id) + claimed = datetime.fromtimestamp(claimed_timestamp) + in_use_time = datetime.utcnow() - claimed self.bot.stats.timing("help.in_use_time", in_use_time) if await self.unanswered.contains(channel.id): @@ -688,7 +687,7 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr("help.claimed") - await self.claim_times.set(channel.id, str(datetime.now())) + await self.claim_times.set(channel.id, datetime.utcnow().timestamp()) await self.unanswered.set(channel.id, True) log.trace(f"Releasing on_message lock for {message.id}.") -- cgit v1.2.3 From 40a774e0bb6ed8947a17fe0116e2f1dc0cf89156 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sat, 6 Jun 2020 12:37:01 +0200 Subject: Fix potential race condition. Instead of first checking if the channel.id exists and then checking what it is, we just do a single API call, to prevent cases where something fucky might happen inbetween the first and the second call. --- bot/cogs/help_channels.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index dd3e3cb8b..01c38b408 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -548,20 +548,20 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr(f"help.dormant_calls.{caller}") - if await self.claim_times.contains(channel.id): - claimed_timestamp = await self.claim_times.get(channel.id) + claimed_timestamp = await self.claim_times.get(channel.id) + if claimed_timestamp: claimed = datetime.fromtimestamp(claimed_timestamp) in_use_time = datetime.utcnow() - claimed self.bot.stats.timing("help.in_use_time", in_use_time) - if await self.unanswered.contains(channel.id): - if await self.unanswered.get(channel.id): + unanswered = await self.unanswered.get(channel.id) + if unanswered is not None: + if unanswered: self.bot.stats.incr("help.sessions.unanswered") else: self.bot.stats.incr("help.sessions.answered") log.trace(f"Position of #{channel} ({channel.id}) is actually {channel.position}.") - log.trace(f"Sending dormant message for #{channel} ({channel.id}).") embed = discord.Embed(description=DORMANT_MSG) await channel.send(embed=embed) -- cgit v1.2.3 From 8570bd2c9d644c82e69e4c3bbae3af24f95180e2 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Sun, 7 Jun 2020 03:15:51 -0500 Subject: Create cooldown.md --- bot/resources/tags/cooldown.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 bot/resources/tags/cooldown.md diff --git a/bot/resources/tags/cooldown.md b/bot/resources/tags/cooldown.md new file mode 100644 index 000000000..a4e237872 --- /dev/null +++ b/bot/resources/tags/cooldown.md @@ -0,0 +1,22 @@ +**Cooldowns** + +Cooldowns are used in discord.py to rate-limit. + +```python +from discord.ext import commands + +class SomeCog(commands.Cog): + def __init__(self): + self._cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) + + async def cog_check(self, ctx): + bucket = self._cd.get_bucket(ctx.message) + retry_after = bucket.update_rate_limit() + if retry_after: + # you're rate limited + # helpful message here + pass + # you're not rate limited +``` + +`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From 97710d5bb8145d10983187bebe554b845a9c0ef1 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Sun, 7 Jun 2020 03:40:59 -0500 Subject: Update cooldown.md --- bot/resources/tags/cooldown.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/resources/tags/cooldown.md b/bot/resources/tags/cooldown.md index a4e237872..3d34c078b 100644 --- a/bot/resources/tags/cooldown.md +++ b/bot/resources/tags/cooldown.md @@ -1,7 +1,7 @@ **Cooldowns** Cooldowns are used in discord.py to rate-limit. - + ```python from discord.ext import commands -- cgit v1.2.3 From 15dbbcf865dd24f5f8697fd85bd60d53d9450fcf Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 7 Jun 2020 12:37:31 +0200 Subject: Remove pointless suppress. Since help_channel_claimants.delete will never raise a KeyError, it's not necessary to suppress one. --- bot/cogs/help_channels.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 01c38b408..e521e3301 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -5,7 +5,6 @@ import logging import random import typing as t from collections import deque -from contextlib import suppress from datetime import datetime from pathlib import Path @@ -224,10 +223,11 @@ class HelpChannels(Scheduler, commands.Cog): log.trace("close command invoked; checking if the channel is in-use.") if ctx.channel.category == self.in_use_category: if await self.dormant_check(ctx): - with suppress(KeyError): - await self.help_channel_claimants.delete(ctx.channel.id) + # Remove the claimant and the cooldown role + await self.help_channel_claimants.delete(ctx.channel.id) await self.remove_cooldown_role(ctx.author) + # Ignore missing task when cooldown has passed but the channel still isn't dormant. self.cancel_task(ctx.author.id, ignore_missing=True) -- cgit v1.2.3 From 378ef81383050cf4c477afc2c23abb51b700ea68 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 8 Jun 2020 18:41:25 -0700 Subject: Help channels: fix claim timestamp being local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The datetime module returns a local timestamp for naïve datetimes. It has to be timezone-aware to ensure it will always be in UTC. --- bot/cogs/help_channels.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index e521e3301..40e625338 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -5,7 +5,7 @@ import logging import random import typing as t from collections import deque -from datetime import datetime +from datetime import datetime, timezone from pathlib import Path import discord @@ -110,7 +110,7 @@ class HelpChannels(Scheduler, commands.Cog): unanswered = RedisCache() # This dictionary maps a help channel to the time it was claimed - # RedisCache[discord.TextChannel.id, datetime.datetime] + # RedisCache[discord.TextChannel.id, UtcPosixTimestamp] claim_times = RedisCache() def __init__(self, bot: Bot): @@ -550,7 +550,7 @@ class HelpChannels(Scheduler, commands.Cog): claimed_timestamp = await self.claim_times.get(channel.id) if claimed_timestamp: - claimed = datetime.fromtimestamp(claimed_timestamp) + claimed = datetime.utcfromtimestamp(claimed_timestamp) in_use_time = datetime.utcnow() - claimed self.bot.stats.timing("help.in_use_time", in_use_time) @@ -687,7 +687,10 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr("help.claimed") - await self.claim_times.set(channel.id, datetime.utcnow().timestamp()) + # Must use a timezone-aware datetime to ensure a correct POSIX timestamp. + timestamp = datetime.now(timezone.utc).timestamp() + await self.claim_times.set(channel.id, timestamp) + await self.unanswered.set(channel.id, True) log.trace(f"Releasing on_message lock for {message.id}.") -- cgit v1.2.3 From 62cb3f7d9ecd861a13b594f3c63aed83dead2e0e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 8 Jun 2020 19:01:58 -0700 Subject: Help channels: add a function to get in use time Future code will also need to get this time, so moving it out to a separate function reduces redundancy. --- bot/cogs/help_channels.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 40e625338..13dee8e80 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -5,7 +5,7 @@ import logging import random import typing as t from collections import deque -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from pathlib import Path import discord @@ -286,6 +286,15 @@ class HelpChannels(Scheduler, commands.Cog): if channel.category_id == category.id and not self.is_excluded_channel(channel): yield channel + async def get_in_use_time(self, channel_id: int) -> t.Optional[timedelta]: + """Return the duration `channel_id` has been in use. Return None if it's not in use.""" + log.trace(f"Calculating in use time for channel {channel_id}.") + + claimed_timestamp = await self.claim_times.get(channel_id) + if claimed_timestamp: + claimed = datetime.utcfromtimestamp(claimed_timestamp) + return datetime.utcnow() - claimed + @staticmethod def get_names() -> t.List[str]: """ @@ -548,10 +557,8 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr(f"help.dormant_calls.{caller}") - claimed_timestamp = await self.claim_times.get(channel.id) - if claimed_timestamp: - claimed = datetime.utcfromtimestamp(claimed_timestamp) - in_use_time = datetime.utcnow() - claimed + in_use_time = await self.get_in_use_time(channel.id) + if in_use_time: self.bot.stats.timing("help.in_use_time", in_use_time) unanswered = await self.unanswered.get(channel.id) -- cgit v1.2.3 From 04b37cd054565368e5af42deec5d5ca14fc94199 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 8 Jun 2020 19:37:37 -0700 Subject: Help channels: add a function to schedule cooldown expiration Moving this code into a separate function reduces redundancy down the line. This will also get used to re-scheduled cooldowns after a restart. --- bot/cogs/help_channels.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 13dee8e80..f2785c932 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -794,11 +794,14 @@ class HelpChannels(Scheduler, commands.Cog): # Would mean the user somehow bypassed the lack of permissions (e.g. user is guild owner). self.cancel_task(member.id, ignore_missing=True) - timeout = constants.HelpChannels.claim_minutes * 60 - callback = self.remove_cooldown_role(member) + await self.schedule_cooldown_expiration(member, constants.HelpChannels.claim_minutes * 60) + + async def schedule_cooldown_expiration(self, member: discord.Member, seconds: int) -> None: + """Schedule the cooldown role for `member` to be removed after a duration of `seconds`.""" + log.trace(f"Scheduling removal of {member}'s ({member.id}) cooldown.") - log.trace(f"Scheduling {member}'s ({member.id}) send message permissions to be reinstated.") - self.schedule_task(member.id, TaskData(timeout, callback)) + callback = self.remove_cooldown_role(member) + self.schedule_task(member.id, TaskData(seconds, callback)) async def send_available_message(self, channel: discord.TextChannel) -> None: """Send the available message by editing a dormant message or sending a new message.""" -- cgit v1.2.3 From b49f3e5f4e707dece2de38882be44405563d82e4 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 8 Jun 2020 19:52:46 -0700 Subject: Help channels: use cache to remove cooldowns or re-schedule them Using the cache is more efficient since it can check only the users it expects to have a cooldown rather than searching all guild members. Furthermore, re-scheduling the cooldowns ensures members experience the full duration of the cooldown. Previously, all cooldowns were removed, regardless of whether they were expired. --- bot/cogs/help_channels.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index f2785c932..098634e96 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -397,7 +397,7 @@ class HelpChannels(Scheduler, commands.Cog): log.trace("Initialising the cog.") await self.init_categories() - await self.reset_send_permissions() + await self.check_cooldowns() self.channel_queue = self.create_channel_queue() self.name_queue = self.create_name_queue() @@ -733,15 +733,28 @@ class HelpChannels(Scheduler, commands.Cog): msg = await self.get_last_message(channel) return self.match_bot_embed(msg, AVAILABLE_MSG) - async def reset_send_permissions(self) -> None: - """Reset send permissions in the Available category for claimants.""" - log.trace("Resetting send permissions in the Available category.") + async def check_cooldowns(self) -> None: + """Remove expired cooldowns and re-schedule active ones.""" + log.trace("Checking all cooldowns to remove or re-schedule them.") guild = self.bot.get_guild(constants.Guild.id) + cooldown = constants.HelpChannels.claim_minutes * 60 - # TODO: replace with a persistent cache cause checking every member is quite slow - for member in guild.members: - if self.is_claimant(member): + for channel_id, member_id in await self.help_channel_claimants.items(): + member = guild.get_member(member_id) + if not member: + continue # Member probably left the guild. + + in_use_time = await self.get_in_use_time(channel_id) + + if not in_use_time or in_use_time.seconds > cooldown: + # Remove the role if no claim time could be retrieved or if the cooldown expired. + # Since the channel is in the claimants cache, it is definitely strange for a time + # to not exist. However, it isn't a reason to keep the user stuck with a cooldown. await self.remove_cooldown_role(member) + else: + # The member is still on a cooldown; re-schedule it for the remaining time. + remaining = cooldown - in_use_time.seconds + await self.schedule_cooldown_expiration(member, remaining) async def add_cooldown_role(self, member: discord.Member) -> None: """Add the help cooldown role to `member`.""" -- cgit v1.2.3 From be78a86abea68e05ac80c8a07085cb7f0fa6d3c1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 8 Jun 2020 20:11:38 -0700 Subject: Help channels: revise inaccurate comment --- bot/cogs/help_channels.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 098634e96..86579e940 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -647,7 +647,7 @@ class HelpChannels(Scheduler, commands.Cog): if self.is_in_category(channel, constants.Categories.help_in_use): log.trace(f"Checking if #{channel} ({channel.id}) has been answered.") - # Check if there is an entry in unanswered (does not persist across restarts) + # Check if there is an entry in unanswered if await self.unanswered.contains(channel.id): claimant_id = await self.help_channel_claimants.get(channel.id) if not claimant_id: -- cgit v1.2.3 From 06d8ab2f7203d4ee92a040444bbb1999a36accb3 Mon Sep 17 00:00:00 2001 From: Daniel Nash Date: Wed, 10 Jun 2020 15:49:14 -0500 Subject: Rename to customcooldown.md --- bot/resources/tags/cooldown.md | 22 ---------------------- bot/resources/tags/customcooldown.md | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 22 deletions(-) delete mode 100644 bot/resources/tags/cooldown.md create mode 100644 bot/resources/tags/customcooldown.md diff --git a/bot/resources/tags/cooldown.md b/bot/resources/tags/cooldown.md deleted file mode 100644 index 3d34c078b..000000000 --- a/bot/resources/tags/cooldown.md +++ /dev/null @@ -1,22 +0,0 @@ -**Cooldowns** - -Cooldowns are used in discord.py to rate-limit. - -```python -from discord.ext import commands - -class SomeCog(commands.Cog): - def __init__(self): - self._cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) - - async def cog_check(self, ctx): - bucket = self._cd.get_bucket(ctx.message) - retry_after = bucket.update_rate_limit() - if retry_after: - # you're rate limited - # helpful message here - pass - # you're not rate limited -``` - -`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md new file mode 100644 index 000000000..3d34c078b --- /dev/null +++ b/bot/resources/tags/customcooldown.md @@ -0,0 +1,22 @@ +**Cooldowns** + +Cooldowns are used in discord.py to rate-limit. + +```python +from discord.ext import commands + +class SomeCog(commands.Cog): + def __init__(self): + self._cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) + + async def cog_check(self, ctx): + bucket = self._cd.get_bucket(ctx.message) + retry_after = bucket.update_rate_limit() + if retry_after: + # you're rate limited + # helpful message here + pass + # you're not rate limited +``` + +`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From b71ff0f2bbd7be64d1a0009b9e6530ba3c179926 Mon Sep 17 00:00:00 2001 From: Daniel Nash Date: Wed, 10 Jun 2020 15:53:33 -0500 Subject: Update example to not be in a cog --- bot/resources/tags/customcooldown.md | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index 3d34c078b..35f28a1e5 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -1,22 +1,20 @@ **Cooldowns** -Cooldowns are used in discord.py to rate-limit. +Cooldowns can be used in discord.py to rate-limit. In this example, we're using it in an on_message. ```python from discord.ext import commands -class SomeCog(commands.Cog): - def __init__(self): - self._cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) +_cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) - async def cog_check(self, ctx): - bucket = self._cd.get_bucket(ctx.message) - retry_after = bucket.update_rate_limit() - if retry_after: - # you're rate limited - # helpful message here - pass - # you're not rate limited +@bot.event +async def on_message(message): + bucket = _cd.get_bucket(message) + retry_after = bucket.update_rate_limit() + if retry_after: + await message.channel.send("Slow down! You're sending messages too fast") + pass + # you're not rate limited ``` `from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From b2195f1990c6720dad6819cc118920b89e24beba Mon Sep 17 00:00:00 2001 From: Daniel Nash Date: Wed, 10 Jun 2020 15:56:00 -0500 Subject: Move the not rate-limited message into else --- bot/resources/tags/customcooldown.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index 35f28a1e5..b44d6b12f 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -13,8 +13,10 @@ async def on_message(message): retry_after = bucket.update_rate_limit() if retry_after: await message.channel.send("Slow down! You're sending messages too fast") + else: + # you're not rate limited pass - # you're not rate limited + # more code here ``` `from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From 76da8f09c7c8d62f8451bc561f020f489b3a0970 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Wed, 10 Jun 2020 17:09:17 -0500 Subject: change _cd to message_cooldown Apply suggestions from code review Co-authored-by: Joseph Banks --- bot/resources/tags/customcooldown.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index b44d6b12f..f304de246 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -5,11 +5,11 @@ Cooldowns can be used in discord.py to rate-limit. In this example, we're using ```python from discord.ext import commands -_cd = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) +message_cooldown = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) @bot.event async def on_message(message): - bucket = _cd.get_bucket(message) + bucket = message_cooldown.get_bucket(message) retry_after = bucket.update_rate_limit() if retry_after: await message.channel.send("Slow down! You're sending messages too fast") -- cgit v1.2.3 From b1b1765596469b76d6321e81732a54e46d3b5865 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Wed, 10 Jun 2020 17:35:44 -0500 Subject: Update bot/resources/tags/customcooldown.md Co-authored-by: Joseph Banks --- bot/resources/tags/customcooldown.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index f304de246..f7987556d 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -14,9 +14,7 @@ async def on_message(message): if retry_after: await message.channel.send("Slow down! You're sending messages too fast") else: - # you're not rate limited - pass - # more code here + await message.channel.send("Not ratelimited!") ``` `from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From ccd3a17d6a8723cabb21a4b2c7e0d8a835dc9e99 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Fri, 12 Jun 2020 08:42:30 -0500 Subject: Make title more specific Co-authored-by: Mark --- bot/resources/tags/customcooldown.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index f7987556d..4060a9827 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -1,4 +1,4 @@ -**Cooldowns** +**Cooldowns in discord.py** Cooldowns can be used in discord.py to rate-limit. In this example, we're using it in an on_message. -- cgit v1.2.3 From 844726f7bedc8c1d772c5f866a29a52cda6f3a9f Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Fri, 12 Jun 2020 08:48:16 -0500 Subject: Update customcooldown.md --- bot/resources/tags/customcooldown.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index 4060a9827..3f6db0ec6 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -10,6 +10,8 @@ message_cooldown = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.Bu @bot.event async def on_message(message): bucket = message_cooldown.get_bucket(message) + # update_rate_limit returns a time you need to wait before + # trying again retry_after = bucket.update_rate_limit() if retry_after: await message.channel.send("Slow down! You're sending messages too fast") -- cgit v1.2.3 From 08db3df18711ff57a56fef99d0ad725669448f3b Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Fri, 12 Jun 2020 08:50:30 -0500 Subject: Add scheme to URL --- bot/resources/tags/customcooldown.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index 3f6db0ec6..e877e4dae 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -19,4 +19,4 @@ async def on_message(message): await message.channel.send("Not ratelimited!") ``` -`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). +`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](https://discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). -- cgit v1.2.3 From 1d0cbbeb46a811b5a049d712aae1a90a1f3a7359 Mon Sep 17 00:00:00 2001 From: Dennis Pham Date: Mon, 15 Jun 2020 00:36:53 -0400 Subject: Add the C# guild to the whitelist --- config-default.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config-default.yml b/config-default.yml index 3388e5f78..aff5fb2e1 100644 --- a/config-default.yml +++ b/config-default.yml @@ -299,6 +299,7 @@ filter: - 185590609631903755 # Blender Hub - 420324994703163402 # /r/FlutterDev - 488751051629920277 # Python Atlanta + - 143867839282020352 # C# domain_blacklist: - pornhub.com -- cgit v1.2.3 From 09f53ca77ae79ceccad91da5e0d44d7013757f0e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 22:36:28 -0700 Subject: Check infraction reason isn't None before shortening it --- bot/cogs/moderation/infractions.py | 10 +++++++--- bot/cogs/moderation/scheduler.py | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 5bfaad796..f685f6991 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -226,7 +226,10 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - action = user.kick(reason=textwrap.shorten(reason, width=512, placeholder="...")) + if reason: + reason = textwrap.shorten(reason, width=512, placeholder="...") + + action = user.kick(reason=reason) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() @@ -259,9 +262,10 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - truncated_reason = textwrap.shorten(reason, width=512, placeholder="...") + if reason: + reason = textwrap.shorten(reason, width=512, placeholder="...") - action = ctx.guild.ban(user, reason=truncated_reason, delete_message_days=0) + action = ctx.guild.ban(user, reason=reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) if infraction.get('expires_at') is not None: diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index b03d89537..beb201b8c 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -127,11 +127,10 @@ class InfractionScheduler(Scheduler): dm_result = ":incoming_envelope: " dm_log_text = "\nDM: Sent" - if infraction["actor"] == self.bot.user.id: + if reason and infraction["actor"] == self.bot.user.id: log.trace( f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) - end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( -- cgit v1.2.3 From 08c96f9eb07a2a86e68fb0e0837b9d07c40dab5e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 22:41:09 -0700 Subject: Fix check for bot actor in infractions The reason None check should be nested to avoid affecting the else/elif statements that follow. --- bot/cogs/moderation/scheduler.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index beb201b8c..d75a72ddb 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -127,17 +127,17 @@ class InfractionScheduler(Scheduler): dm_result = ":incoming_envelope: " dm_log_text = "\nDM: Sent" - if reason and infraction["actor"] == self.bot.user.id: + end_msg = "" + if infraction["actor"] == self.bot.user.id: log.trace( f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) - end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" + if reason: + end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( f"Infraction #{id_} context is not in a staff channel; omitting infraction count." ) - - end_msg = "" else: log.trace(f"Fetching total infraction count for {user}.") -- cgit v1.2.3 From 89d8798e93d0b04f7964eca05f5d66c89c2a2f86 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 23:33:33 -0700 Subject: Sync: ignore events from other guilds --- bot/cogs/sync/cog.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 7cc3726b2..97ea31ba5 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -46,6 +46,9 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: """Adds newly create role to the database table over the API.""" + if role.guild != constants.Guild.id: + return + await self.bot.api_client.post( 'bot/roles', json={ @@ -60,11 +63,17 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_delete(self, role: Role) -> None: """Deletes role from the database when it's deleted from the guild.""" + if role.guild != constants.Guild.id: + return + await self.bot.api_client.delete(f'bot/roles/{role.id}') @Cog.listener() async def on_guild_role_update(self, before: Role, after: Role) -> None: """Syncs role with the database if any of the stored attributes were updated.""" + if after.guild != constants.Guild.id: + return + was_updated = ( before.name != after.name or before.colour != after.colour @@ -93,6 +102,9 @@ class Sync(Cog): previously left), it will update the user's information. If the user is not yet known by the database, the user is added. """ + if member.guild != constants.Guild.id: + return + packed = { 'discriminator': int(member.discriminator), 'id': member.id, @@ -122,11 +134,17 @@ class Sync(Cog): @Cog.listener() async def on_member_remove(self, member: Member) -> None: """Set the in_guild field to False when a member leaves the guild.""" + if member.guild != constants.Guild.id: + return + await self.patch_user(member.id, updated_information={"in_guild": False}) @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """Update the roles of the member in the database if a change is detected.""" + if after.guild != constants.Guild.id: + return + if before.roles != after.roles: updated_information = {"roles": sorted(role.id for role in after.roles)} await self.patch_user(after.id, updated_information=updated_information) -- cgit v1.2.3 From 81e50cb2c970fc5c203e135434f897b6a3f7e52a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 23:34:28 -0700 Subject: Sync tests: test listeners ignore events from other guilds --- tests/bot/cogs/sync/test_cog.py | 64 ++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index 14fd909c4..d7d60e961 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -131,6 +131,12 @@ class SyncCogListenerTests(SyncCogTestCase): super().setUp() self.cog.patch_user = mock.AsyncMock(spec_set=self.cog.patch_user) + self.guild_id_patcher = mock.patch("bot.cogs.sync.cog.constants.Guild.id", 5) + self.guild_id = self.guild_id_patcher.start() + + def tearDown(self): + self.guild_id_patcher.stop() + async def test_sync_cog_on_guild_role_create(self): """A POST request should be sent with the new role's data.""" self.assertTrue(self.cog.on_guild_role_create.__cog_listener__) @@ -142,20 +148,32 @@ class SyncCogListenerTests(SyncCogTestCase): "permissions": 8, "position": 23, } - role = helpers.MockRole(**role_data) + role = helpers.MockRole(**role_data, guild=self.guild_id) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_called_once_with("bot/roles", json=role_data) + async def test_sync_cog_on_guild_role_create_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_create(role) + self.bot.api_client.post.assert_not_awaited() + async def test_sync_cog_on_guild_role_delete(self): """A DELETE request should be sent.""" self.assertTrue(self.cog.on_guild_role_delete.__cog_listener__) - role = helpers.MockRole(id=99) + role = helpers.MockRole(id=99, guild=self.guild_id) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_called_once_with("bot/roles/99") + async def test_sync_cog_on_guild_role_delete_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_delete(role) + self.bot.api_client.delete.assert_not_awaited() + async def test_sync_cog_on_guild_role_update(self): """A PUT request should be sent if the colour, name, permissions, or position changes.""" self.assertTrue(self.cog.on_guild_role_update.__cog_listener__) @@ -180,8 +198,8 @@ class SyncCogListenerTests(SyncCogTestCase): after_role_data = role_data.copy() after_role_data[attribute] = 876 - before_role = helpers.MockRole(**role_data) - after_role = helpers.MockRole(**after_role_data) + before_role = helpers.MockRole(**role_data, guild=self.guild_id) + after_role = helpers.MockRole(**after_role_data, guild=self.guild_id) await self.cog.on_guild_role_update(before_role, after_role) @@ -193,11 +211,17 @@ class SyncCogListenerTests(SyncCogTestCase): else: self.bot.api_client.put.assert_not_called() + async def test_sync_cog_on_guild_role_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_update(role, role) + self.bot.api_client.put.assert_not_awaited() + async def test_sync_cog_on_member_remove(self): - """Member should patched to set in_guild as False.""" + """Member should be patched to set in_guild as False.""" self.assertTrue(self.cog.on_member_remove.__cog_listener__) - member = helpers.MockMember() + member = helpers.MockMember(guild=self.guild_id) await self.cog.on_member_remove(member) self.cog.patch_user.assert_called_once_with( @@ -205,14 +229,20 @@ class SyncCogListenerTests(SyncCogTestCase): updated_information={"in_guild": False} ) + async def test_sync_cog_on_member_remove_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_remove(member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_member_update_roles(self): """Members should be patched if their roles have changed.""" self.assertTrue(self.cog.on_member_update.__cog_listener__) # Roles are intentionally unsorted. before_roles = [helpers.MockRole(id=12), helpers.MockRole(id=30), helpers.MockRole(id=20)] - before_member = helpers.MockMember(roles=before_roles) - after_member = helpers.MockMember(roles=before_roles[1:]) + before_member = helpers.MockMember(roles=before_roles, guild=self.guild_id) + after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild_id) await self.cog.on_member_update(before_member, after_member) @@ -233,13 +263,19 @@ class SyncCogListenerTests(SyncCogTestCase): with self.subTest(attribute=attribute): self.cog.patch_user.reset_mock() - before_member = helpers.MockMember(**{attribute: old_value}) - after_member = helpers.MockMember(**{attribute: new_value}) + before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild_id) + after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild_id) await self.cog.on_member_update(before_member, after_member) self.cog.patch_user.assert_not_called() + async def test_sync_cog_on_member_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_update(member, member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_user_update(self): """A user should be patched only if the name, discriminator, or avatar changes.""" self.assertTrue(self.cog.on_user_update.__cog_listener__) @@ -290,6 +326,7 @@ class SyncCogListenerTests(SyncCogTestCase): member = helpers.MockMember( discriminator="1234", roles=[helpers.MockRole(id=22), helpers.MockRole(id=12)], + guild=self.guild_id, ) data = { @@ -334,6 +371,13 @@ class SyncCogListenerTests(SyncCogTestCase): self.bot.api_client.post.assert_not_called() + async def test_sync_cog_on_member_join_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_join(member) + self.bot.api_client.post.assert_not_awaited() + self.bot.api_client.put.assert_not_awaited() + class SyncCogCommandTests(SyncCogTestCase, CommandTestCase): """Tests for the commands in the Sync cog.""" -- cgit v1.2.3 From 4d6acdf32a323de8b88fed464358d70faf35c9d1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 23:47:40 -0700 Subject: Sync: ignore 404s in on_user_update 404s probably mean the user is from another guild. --- bot/cogs/sync/cog.py | 14 ++++++++------ tests/bot/cogs/sync/test_cog.py | 17 ++++++++++------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 97ea31ba5..578cccfc9 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -34,14 +34,15 @@ class Sync(Cog): for syncer in (self.role_syncer, self.user_syncer): await syncer.sync(guild) - async def patch_user(self, user_id: int, updated_information: Dict[str, Any]) -> None: + async def patch_user(self, user_id: int, json: Dict[str, Any], ignore_404: bool = False) -> None: """Send a PATCH request to partially update a user in the database.""" try: - await self.bot.api_client.patch(f"bot/users/{user_id}", json=updated_information) + await self.bot.api_client.patch(f"bot/users/{user_id}", json=json) except ResponseCodeError as e: if e.response.status != 404: raise - log.warning("Unable to update user, got 404. Assuming race condition from join event.") + if not ignore_404: + log.warning("Unable to update user, got 404. Assuming race condition from join event.") @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: @@ -137,7 +138,7 @@ class Sync(Cog): if member.guild != constants.Guild.id: return - await self.patch_user(member.id, updated_information={"in_guild": False}) + await self.patch_user(member.id, json={"in_guild": False}) @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: @@ -147,7 +148,7 @@ class Sync(Cog): if before.roles != after.roles: updated_information = {"roles": sorted(role.id for role in after.roles)} - await self.patch_user(after.id, updated_information=updated_information) + await self.patch_user(after.id, json=updated_information) @Cog.listener() async def on_user_update(self, before: User, after: User) -> None: @@ -158,7 +159,8 @@ class Sync(Cog): "name": after.name, "discriminator": int(after.discriminator), } - await self.patch_user(after.id, updated_information=updated_information) + # A 404 likely means the user is in another guild. + await self.patch_user(after.id, json=updated_information, ignore_404=True) @commands.group(name='sync') @commands.has_permissions(administrator=True) diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index d7d60e961..e5be14391 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -226,7 +226,7 @@ class SyncCogListenerTests(SyncCogTestCase): self.cog.patch_user.assert_called_once_with( member.id, - updated_information={"in_guild": False} + json={"in_guild": False} ) async def test_sync_cog_on_member_remove_ignores_guilds(self): @@ -247,7 +247,7 @@ class SyncCogListenerTests(SyncCogTestCase): await self.cog.on_member_update(before_member, after_member) data = {"roles": sorted(role.id for role in after_member.roles)} - self.cog.patch_user.assert_called_once_with(after_member.id, updated_information=data) + self.cog.patch_user.assert_called_once_with(after_member.id, json=data) async def test_sync_cog_on_member_update_other(self): """Members should not be patched if other attributes have changed.""" @@ -308,12 +308,15 @@ class SyncCogListenerTests(SyncCogTestCase): # Don't care if *all* keys are present; only the changed one is required call_args = self.cog.patch_user.call_args - self.assertEqual(call_args[0][0], after_user.id) - self.assertIn("updated_information", call_args[1]) + self.assertEqual(call_args.args[0], after_user.id) + self.assertIn("json", call_args.kwargs) - updated_information = call_args[1]["updated_information"] - self.assertIn(api_field, updated_information) - self.assertEqual(updated_information[api_field], api_value) + self.assertIn("ignore_404", call_args.kwargs) + self.assertTrue(call_args.kwargs["ignore_404"]) + + json = call_args.kwargs["json"] + self.assertIn(api_field, json) + self.assertEqual(json[api_field], api_value) else: self.cog.patch_user.assert_not_called() -- cgit v1.2.3 From efd27cf29f726627b9ba630e257a2e3e89d3a286 Mon Sep 17 00:00:00 2001 From: Daniel Nash <22755628+crazygmr101@users.noreply.github.com> Date: Mon, 15 Jun 2020 13:18:36 -0400 Subject: Update bot/resources/tags/customcooldown.md Co-authored-by: Mark --- bot/resources/tags/customcooldown.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md index e877e4dae..ac7e70aee 100644 --- a/bot/resources/tags/customcooldown.md +++ b/bot/resources/tags/customcooldown.md @@ -10,11 +10,9 @@ message_cooldown = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.Bu @bot.event async def on_message(message): bucket = message_cooldown.get_bucket(message) - # update_rate_limit returns a time you need to wait before - # trying again retry_after = bucket.update_rate_limit() if retry_after: - await message.channel.send("Slow down! You're sending messages too fast") + await message.channel.send(f"Slow down! Try again in {retry_after} seconds.") else: await message.channel.send("Not ratelimited!") ``` -- cgit v1.2.3 From c7373fa1143a2d2f2d784a59d40bcb40ee765bfb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:26:23 -0700 Subject: Token remover: ignore DMs It's a private channel so there's no risk of a token "leaking". Furthermore, messages cannot be deleted in DMs. --- bot/cogs/token_remover.py | 3 +++ tests/bot/cogs/test_token_remover.py | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index d55e079e9..493479df9 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -63,6 +63,9 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ + if not msg.guild: + return # Ignore DMs; can't delete messages in there anyway. + found_token = self.find_token_in_message(msg) if found_token: await self.take_action(msg, found_token) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index a10124d2d..22c31d7b1 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -121,6 +121,16 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): find_token_in_message.assert_called_once_with(self.msg) take_action.assert_not_awaited() + @autospec(TokenRemover, "find_token_in_message") + async def test_on_message_ignores_dms(self, find_token_in_message): + """Shouldn't parse a message if it is a DM.""" + cog = TokenRemover(self.bot) + self.msg.guild = None + + await cog.on_message(self.msg) + + find_token_in_message.assert_not_called() + @autospec("bot.cogs.token_remover", "TOKEN_RE") def test_find_token_ignores_bot_messages(self, token_re): """The token finder should ignore messages authored by bots.""" -- cgit v1.2.3 From 2fa7429327e787a65803c16609da21463723bfeb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:38:46 -0700 Subject: Token remover: move bot check to on_message It just makes more sense to me to filter out messages at an earlier stage. --- bot/cogs/token_remover.py | 8 +++----- tests/bot/cogs/test_token_remover.py | 23 +++++++---------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index 493479df9..1f7517501 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -63,8 +63,9 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ - if not msg.guild: - return # Ignore DMs; can't delete messages in there anyway. + # Ignore DMs; can't delete messages in there anyway. + if not msg.guild or msg.author.bot: + return found_token = self.find_token_in_message(msg) if found_token: @@ -115,9 +116,6 @@ class TokenRemover(Cog): @classmethod def find_token_in_message(cls, msg: Message) -> t.Optional[Token]: """Return a seemingly valid token found in `msg` or `None` if no token is found.""" - if msg.author.bot: - return - # Use finditer rather than search to guard against method calls prematurely returning the # token check (e.g. `message.channel.send` also matches our token pattern) for match in TOKEN_RE.finditer(msg.content): diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 22c31d7b1..98ea9f823 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -122,24 +122,15 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): take_action.assert_not_awaited() @autospec(TokenRemover, "find_token_in_message") - async def test_on_message_ignores_dms(self, find_token_in_message): - """Shouldn't parse a message if it is a DM.""" + async def test_on_message_ignores_dms_bots(self, find_token_in_message): + """Shouldn't parse a message if it is a DM or authored by a bot.""" cog = TokenRemover(self.bot) - self.msg.guild = None + dm_msg = MockMessage(guild=None) + bot_msg = MockMessage(author=MagicMock(bot=True)) - await cog.on_message(self.msg) - - find_token_in_message.assert_not_called() - - @autospec("bot.cogs.token_remover", "TOKEN_RE") - def test_find_token_ignores_bot_messages(self, token_re): - """The token finder should ignore messages authored by bots.""" - self.msg.author.bot = True - - return_value = TokenRemover.find_token_in_message(self.msg) - - self.assertIsNone(return_value) - token_re.finditer.assert_not_called() + for msg in (dm_msg, bot_msg): + await cog.on_message(msg) + find_token_in_message.assert_not_called() @autospec("bot.cogs.token_remover", "TOKEN_RE") def test_find_token_no_matches(self, token_re): -- cgit v1.2.3 From 3aecf14419c87e533d47fe082abeb54ca9edb73c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:49:18 -0700 Subject: Token remover: exit early if message already deleted --- bot/cogs/token_remover.py | 10 ++++++++-- tests/bot/cogs/test_token_remover.py | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index 1f7517501..ef979f222 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -4,7 +4,7 @@ import logging import re import typing as t -from discord import Colour, Message +from discord import Colour, Message, NotFound from discord.ext.commands import Cog from bot import utils @@ -83,7 +83,13 @@ class TokenRemover(Cog): async def take_action(self, msg: Message, found_token: Token) -> None: """Remove the `msg` containing the `found_token` and send a mod log message.""" self.mod_log.ignore(Event.message_delete, msg.id) - await msg.delete() + + try: + await msg.delete() + except NotFound: + log.debug(f"Failed to remove token in message {msg.id}: message already deleted.") + return + await msg.channel.send(DELETION_MESSAGE_TEMPLATE.format(mention=msg.author.mention)) log_message = self.format_log_message(msg, found_token) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 98ea9f823..3349caa73 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -3,7 +3,7 @@ from re import Match from unittest import mock from unittest.mock import MagicMock -from discord import Colour +from discord import Colour, NotFound from bot import constants from bot.cogs import token_remover @@ -282,6 +282,19 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): channel_id=constants.Channels.mod_alerts ) + @mock.patch.object(TokenRemover, "mod_log", new_callable=mock.PropertyMock) + async def test_take_action_delete_failure(self, mod_log_property): + """Shouldn't send any messages if the token message can't be deleted.""" + cog = TokenRemover(self.bot) + mod_log_property.return_value = mock.create_autospec(ModLog, spec_set=True, instance=True) + self.msg.delete.side_effect = NotFound(MagicMock(), MagicMock()) + + token = mock.create_autospec(Token, spec_set=True, instance=True) + await cog.take_action(self.msg, token) + + self.msg.delete.assert_called_once_with() + self.msg.channel.send.assert_not_awaited() + class TokenRemoverExtensionTests(unittest.TestCase): """Tests for the token_remover extension.""" -- cgit v1.2.3 From 0ad19a48680fe6bc729d0e893d32a517a21df7dc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:51:47 -0700 Subject: Webhook remover: ignore DMs and bot messages Can't remove messages in DMs, so don't bother trying. --- bot/cogs/webhook_remover.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bot/cogs/webhook_remover.py b/bot/cogs/webhook_remover.py index 1b5c3f821..74a353e98 100644 --- a/bot/cogs/webhook_remover.py +++ b/bot/cogs/webhook_remover.py @@ -59,6 +59,10 @@ class WebhookRemover(Cog): @Cog.listener() async def on_message(self, msg: Message) -> None: """Check if a Discord webhook URL is in `message`.""" + # Ignore DMs; can't delete messages in there anyway. + if not msg.guild or msg.author.bot: + return + matches = WEBHOOK_URL_RE.search(msg.content) if matches: await self.delete_and_respond(msg, matches[1] + "xxx") -- cgit v1.2.3 From 94a4f8e52f52e98ea50fb0233fedcbbe9ebe6266 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:52:01 -0700 Subject: Webhook remover: exit early if message already deleted --- bot/cogs/webhook_remover.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bot/cogs/webhook_remover.py b/bot/cogs/webhook_remover.py index 74a353e98..543869215 100644 --- a/bot/cogs/webhook_remover.py +++ b/bot/cogs/webhook_remover.py @@ -1,7 +1,7 @@ import logging import re -from discord import Colour, Message +from discord import Colour, Message, NotFound from discord.ext.commands import Cog from bot.bot import Bot @@ -35,7 +35,13 @@ class WebhookRemover(Cog): """Delete `msg` and send a warning that it contained the Discord webhook `redacted_url`.""" # Don't log this, due internal delete, not by user. Will make different entry. self.mod_log.ignore(Event.message_delete, msg.id) - await msg.delete() + + try: + await msg.delete() + except NotFound: + log.debug(f"Failed to remove webhook in message {msg.id}: message already deleted.") + return + await msg.channel.send(ALERT_MESSAGE_TEMPLATE.format(user=msg.author.mention)) message = ( -- cgit v1.2.3 From ae44563fe132436d98f50e074e5eb4421eda5538 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 17:41:34 -0700 Subject: Log exception info for failed attachment uploads --- bot/utils/messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index de8e186f3..23519a514 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -97,7 +97,7 @@ async def send_attachments( if link_large and e.status == 413: large.append(attachment) else: - log.warning(f"{failure_msg} with status {e.status}.") + log.warning(f"{failure_msg} with status {e.status}.", exc_info=e) if link_large and large: desc = "\n".join(f"[{attachment.filename}]({attachment.url})" for attachment in large) -- cgit v1.2.3 From bcf6993de7de726683e6ca9b0f102b6ad1a732fa Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 18:10:52 -0700 Subject: Fix 400 when "clyde" is in webhook username Discord just disallows this name. --- bot/cogs/duck_pond.py | 4 ++-- bot/cogs/python_news.py | 3 ++- bot/cogs/reddit.py | 8 +++++--- bot/cogs/watchchannels/watchchannel.py | 1 + bot/utils/messages.py | 16 ++++++++++++++-- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index 37d1786a2..5b6a7fd62 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -7,7 +7,7 @@ from discord.ext.commands import Cog from bot import constants from bot.bot import Bot -from bot.utils.messages import send_attachments +from bot.utils.messages import send_attachments, sub_clyde log = logging.getLogger(__name__) @@ -58,7 +58,7 @@ class DuckPond(Cog): try: await self.webhook.send( content=content, - username=username, + username=sub_clyde(username), avatar_url=avatar_url, embed=embed ) diff --git a/bot/cogs/python_news.py b/bot/cogs/python_news.py index d15d0371e..adefd5c7c 100644 --- a/bot/cogs/python_news.py +++ b/bot/cogs/python_news.py @@ -10,6 +10,7 @@ from discord.ext.tasks import loop from bot import constants from bot.bot import Bot +from bot.utils.messages import sub_clyde PEPS_RSS_URL = "https://www.python.org/dev/peps/peps.rss/" @@ -208,7 +209,7 @@ class PythonNews(Cog): return await self.webhook.send( embed=embed, - username=webhook_profile_name, + username=sub_clyde(webhook_profile_name), avatar_url=AVATAR_URL, wait=True ) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 3b77538a0..d853ab2ea 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -16,6 +16,7 @@ from bot.constants import Channels, ERROR_REPLIES, Emojis, Reddit as RedditConfi from bot.converters import Subreddit from bot.decorators import with_role from bot.pagination import LinePaginator +from bot.utils.messages import sub_clyde log = logging.getLogger(__name__) @@ -218,7 +219,8 @@ class Reddit(Cog): for subreddit in RedditConfig.subreddits: top_posts = await self.get_top_posts(subreddit=subreddit, time="day") - message = await self.webhook.send(username=f"{subreddit} Top Daily Posts", embed=top_posts, wait=True) + username = sub_clyde(f"{subreddit} Top Daily Posts") + message = await self.webhook.send(username=username, embed=top_posts, wait=True) if message.channel.is_news(): await message.publish() @@ -228,8 +230,8 @@ class Reddit(Cog): for subreddit in RedditConfig.subreddits: # Send and pin the new weekly posts. top_posts = await self.get_top_posts(subreddit=subreddit, time="week") - - message = await self.webhook.send(wait=True, username=f"{subreddit} Top Weekly Posts", embed=top_posts) + username = sub_clyde(f"{subreddit} Top Weekly Posts") + message = await self.webhook.send(wait=True, username=username, embed=top_posts) if subreddit.lower() == "r/python": if not self.channel: diff --git a/bot/cogs/watchchannels/watchchannel.py b/bot/cogs/watchchannels/watchchannel.py index 436778c46..7c58a0fb5 100644 --- a/bot/cogs/watchchannels/watchchannel.py +++ b/bot/cogs/watchchannels/watchchannel.py @@ -204,6 +204,7 @@ class WatchChannel(metaclass=CogABCMeta): embed: Optional[Embed] = None, ) -> None: """Sends a message to the webhook with the specified kwargs.""" + username = messages.sub_clyde(username) try: await self.webhook.send(content=content, username=username, avatar_url=avatar_url, embed=embed) except discord.HTTPException as exc: diff --git a/bot/utils/messages.py b/bot/utils/messages.py index 23519a514..6ad9351cc 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -1,6 +1,7 @@ import asyncio import contextlib import logging +import re from io import BytesIO from typing import List, Optional, Sequence, Union @@ -86,7 +87,7 @@ async def send_attachments( else: await destination.send( file=attachment_file, - username=message.author.display_name, + username=sub_clyde(message.author.display_name), avatar_url=message.author.avatar_url ) elif link_large: @@ -109,8 +110,19 @@ async def send_attachments( else: await destination.send( embed=embed, - username=message.author.display_name, + username=sub_clyde(message.author.display_name), avatar_url=message.author.avatar_url ) return urls + + +def sub_clyde(username: Optional[str]) -> Optional[str]: + """ + Replace "e" in any "clyde" in `username` with a similar Unicode char and return the new string. + + Discord disallows "clyde" anywhere in the username for webhooks. It will return a 400. + Return None only if `username` is None. + """ + if username: + return re.sub(r"(clyd)e", r"\1𝖾", username, flags=re.I) -- cgit v1.2.3 From 2020df342d3aa43acdf7ead026d593f779264002 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Tue, 16 Jun 2020 20:03:15 +0800 Subject: Refactor nested if-statement --- bot/cogs/help_channels.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 86579e940..4c464a7d2 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -562,11 +562,10 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.timing("help.in_use_time", in_use_time) unanswered = await self.unanswered.get(channel.id) - if unanswered is not None: - if unanswered: - self.bot.stats.incr("help.sessions.unanswered") - else: - self.bot.stats.incr("help.sessions.answered") + if unanswered: + self.bot.stats.incr("help.sessions.unanswered") + elif unanswered is not None: + self.bot.stats.incr("help.sessions.answered") log.trace(f"Position of #{channel} ({channel.id}) is actually {channel.position}.") log.trace(f"Sending dormant message for #{channel} ({channel.id}).") -- cgit v1.2.3 From 433f6b6843006aff57cf1e125d340e703c85669f Mon Sep 17 00:00:00 2001 From: kosayoda Date: Tue, 16 Jun 2020 20:10:23 +0800 Subject: Revise inaccurate docstring in RedisCache --- bot/utils/redis_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index 347a0e54a..f342bbb62 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -48,8 +48,8 @@ class RedisCache: behaves, and should be familiar to Python users. The biggest difference is that all the public methods in this class are coroutines, and must be awaited. - Because of limitations in Redis, this cache will only accept strings, integers and - floats both for keys and values. + Because of limitations in Redis, this cache will only accept strings and integers for keys, + and strings, integers, floats and booleans for values. Please note that this class MUST be created as a class attribute, and that that class must also contain an attribute with an instance of our Bot. See `__get__` and `__set_name__` -- cgit v1.2.3 From 2426ea2141dd75aba21562d925244c1a43af94fc Mon Sep 17 00:00:00 2001 From: kosayoda Date: Tue, 16 Jun 2020 21:55:14 +0800 Subject: Revise inaccurate typehint for Optional reason --- bot/cogs/moderation/infractions.py | 49 ++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f685f6991..3db788eb9 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -53,7 +53,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Permanent infractions @command() - async def warn(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def warn(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Warn a user for the given reason.""" infraction = await utils.post_infraction(ctx, user, "warning", reason, active=False) if infraction is None: @@ -62,12 +62,12 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user) @command() - async def kick(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def kick(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Kick a user for the given reason.""" await self.apply_kick(ctx, user, reason, active=False) @command() - async def ban(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def ban(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Permanently ban a user for the given reason and stop watching them with Big Brother.""" await self.apply_ban(ctx, user, reason) @@ -75,7 +75,9 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Temporary infractions @command(aliases=["mute"]) - async def tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: str = None) -> None: + async def tempmute( + self, ctx: Context, user: Member, duration: Expiry, *, reason: t.Optional[str] = None + ) -> None: """ Temporarily mute a user for the given reason and duration. @@ -94,7 +96,9 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_mute(ctx, user, reason, expires_at=duration) @command() - async def tempban(self, ctx: Context, user: FetchedMember, duration: Expiry, *, reason: str = None) -> None: + async def tempban( + self, ctx: Context, user: FetchedMember, duration: Expiry, *, reason: t.Optional[str] = None + ) -> None: """ Temporarily ban a user for the given reason and duration. @@ -116,7 +120,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Permanent shadow infractions @command(hidden=True) - async def note(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def note(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Create a private note for a user with the given reason without notifying the user.""" infraction = await utils.post_infraction(ctx, user, "note", reason, hidden=True, active=False) if infraction is None: @@ -125,12 +129,14 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user) @command(hidden=True, aliases=['shadowkick', 'skick']) - async def shadow_kick(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def shadow_kick(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Kick a user for the given reason without notifying the user.""" await self.apply_kick(ctx, user, reason, hidden=True, active=False) @command(hidden=True, aliases=['shadowban', 'sban']) - async def shadow_ban(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def shadow_ban( + self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None + ) -> None: """Permanently ban a user for the given reason without notifying the user.""" await self.apply_ban(ctx, user, reason, hidden=True) @@ -138,7 +144,14 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Temporary shadow infractions @command(hidden=True, aliases=["shadowtempmute, stempmute", "shadowmute", "smute"]) - async def shadow_tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: str = None) -> None: + async def shadow_tempmute( + self, + ctx: Context, + user: Member, + duration: Expiry, + *, + reason: t.Optional[str] = None + ) -> None: """ Temporarily mute a user for the given reason and duration without notifying the user. @@ -158,12 +171,12 @@ class Infractions(InfractionScheduler, commands.Cog): @command(hidden=True, aliases=["shadowtempban, stempban"]) async def shadow_tempban( - self, - ctx: Context, - user: FetchedMember, - duration: Expiry, - *, - reason: str = None + self, + ctx: Context, + user: FetchedMember, + duration: Expiry, + *, + reason: t.Optional[str] = None ) -> None: """ Temporarily ban a user for the given reason and duration without notifying the user. @@ -198,7 +211,7 @@ class Infractions(InfractionScheduler, commands.Cog): # endregion # region: Base apply functions - async def apply_mute(self, ctx: Context, user: Member, reason: str, **kwargs) -> None: + async def apply_mute(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None: """Apply a mute infraction with kwargs passed to `post_infraction`.""" if await utils.get_active_infraction(ctx, user, "mute"): return @@ -218,7 +231,7 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user, action()) @respect_role_hierarchy() - async def apply_kick(self, ctx: Context, user: Member, reason: str, **kwargs) -> None: + async def apply_kick(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None: """Apply a kick infraction with kwargs passed to `post_infraction`.""" infraction = await utils.post_infraction(ctx, user, "kick", reason, active=False, **kwargs) if infraction is None: @@ -233,7 +246,7 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() - async def apply_ban(self, ctx: Context, user: UserSnowflake, reason: str, **kwargs) -> None: + async def apply_ban(self, ctx: Context, user: UserSnowflake, reason: t.Optional[str], **kwargs) -> None: """ Apply a ban infraction with kwargs passed to `post_infraction`. -- cgit v1.2.3 From 6fe0bc1c9ce35459b7d9b5bd2309b41dcc4c0dcc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 16 Jun 2020 12:40:12 -0700 Subject: Add optional type annotations to reason in pardon funcs --- bot/cogs/moderation/infractions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 3db788eb9..f7747e7f8 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -298,7 +298,7 @@ class Infractions(InfractionScheduler, commands.Cog): # endregion # region: Base pardon functions - async def pardon_mute(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + async def pardon_mute(self, user_id: int, guild: discord.Guild, reason: t.Optional[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 = {} @@ -324,7 +324,7 @@ class Infractions(InfractionScheduler, commands.Cog): return log_text - async def pardon_ban(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + async def pardon_ban(self, user_id: int, guild: discord.Guild, reason: t.Optional[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 = {} -- cgit v1.2.3 From 20a8b6fe92c398fdc246d78591600bb7bde78bca Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 16 Jun 2020 12:48:13 -0700 Subject: Format parameters with a more consistent style --- bot/cogs/moderation/infractions.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f7747e7f8..3b28526b2 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -75,9 +75,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Temporary infractions @command(aliases=["mute"]) - async def tempmute( - self, ctx: Context, user: Member, duration: Expiry, *, reason: t.Optional[str] = None - ) -> None: + async def tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: t.Optional[str] = None) -> None: """ Temporarily mute a user for the given reason and duration. @@ -97,7 +95,12 @@ class Infractions(InfractionScheduler, commands.Cog): @command() async def tempban( - self, ctx: Context, user: FetchedMember, duration: Expiry, *, reason: t.Optional[str] = None + self, + ctx: Context, + user: FetchedMember, + duration: Expiry, + *, + reason: t.Optional[str] = None ) -> None: """ Temporarily ban a user for the given reason and duration. @@ -134,9 +137,7 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_kick(ctx, user, reason, hidden=True, active=False) @command(hidden=True, aliases=['shadowban', 'sban']) - async def shadow_ban( - self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None - ) -> None: + async def shadow_ban(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Permanently ban a user for the given reason without notifying the user.""" await self.apply_ban(ctx, user, reason, hidden=True) @@ -145,12 +146,11 @@ class Infractions(InfractionScheduler, commands.Cog): @command(hidden=True, aliases=["shadowtempmute, stempmute", "shadowmute", "smute"]) async def shadow_tempmute( - self, - ctx: Context, - user: Member, - duration: Expiry, - *, - reason: t.Optional[str] = None + self, ctx: Context, + user: Member, + duration: Expiry, + *, + reason: t.Optional[str] = None ) -> None: """ Temporarily mute a user for the given reason and duration without notifying the user. @@ -171,12 +171,12 @@ class Infractions(InfractionScheduler, commands.Cog): @command(hidden=True, aliases=["shadowtempban, stempban"]) async def shadow_tempban( - self, - ctx: Context, - user: FetchedMember, - duration: Expiry, - *, - reason: t.Optional[str] = None + self, + ctx: Context, + user: FetchedMember, + duration: Expiry, + *, + reason: t.Optional[str] = None ) -> None: """ Temporarily ban a user for the given reason and duration without notifying the user. -- cgit v1.2.3 From 778635241bf6c1a97f60f48a2bc9b40791a524e9 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Wed, 17 Jun 2020 19:15:31 +0100 Subject: Add LMGTFY to domain blacklist --- config-default.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config-default.yml b/config-default.yml index aff5fb2e1..f111c64f5 100644 --- a/config-default.yml +++ b/config-default.yml @@ -331,6 +331,7 @@ filter: - ssteam.site - steamwalletgift.com - discord.gift + - lmgtfy.com word_watchlist: - goo+ks* -- cgit v1.2.3 From 47b6f65e231305c2ceb4f48a2a772a734ae190db Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Wed, 17 Jun 2020 21:20:27 +0100 Subject: Update deletion scheduler to use latest watchlist configuration --- bot/cogs/filtering.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index f7cf4c3ea..76ea68660 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -7,7 +7,7 @@ from typing import List, Mapping, Optional, Union import dateutil import discord.errors from dateutil.relativedelta import relativedelta -from discord import Colour, DMChannel, HTTPException, Member, Message, NotFound, TextChannel +from discord import Colour, HTTPException, Member, Message, NotFound, TextChannel from discord.ext.commands import Cog from discord.utils import escape_markdown @@ -56,6 +56,7 @@ def expand_spoilers(text: str) -> str: split_text[0::2] + split_text[1::2] + split_text ) + OFFENSIVE_MSG_DELETE_TIME = timedelta(days=Filter.offensive_msg_delete_days) @@ -113,6 +114,7 @@ class Filtering(Cog, Scheduler): "function": self._has_watch_regex_match, "type": "watchlist", "content_only": True, + "schedule_deletion": True }, "watch_rich_embeds": { "enabled": Filter.watch_rich_embeds, @@ -120,21 +122,7 @@ class Filtering(Cog, Scheduler): "type": "watchlist", "content_only": False, "schedule_deletion": False - }, - "watch_words": { - "enabled": Filter.watch_words, - "function": self._has_watchlist_words, - "type": "watchlist", - "content_only": True, - "schedule_deletion": True - }, - "watch_tokens": { - "enabled": Filter.watch_tokens, - "function": self._has_watchlist_tokens, - "type": "watchlist", - "content_only": True, - "schedule_deletion": True - }, + } } self.bot.loop.create_task(self.reschedule_offensive_msg_deletion()) @@ -481,7 +469,7 @@ class Filtering(Cog, Scheduler): await self.bot.wait_until_ready() response = await self.bot.api_client.get('bot/offensive-messages',) - now = datetime.datetime.utcnow() + now = datetime.utcnow() for msg in response: delete_at = dateutil.parser.isoparse(msg['delete_date']).replace(tzinfo=None) -- cgit v1.2.3 From ebc0eae42c1da67f61f040a67bc1b70e53a6f97e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 17 Jun 2020 16:46:43 -0700 Subject: Sync: fix guild ID check Need to compare the IDs against each other rather than the Guild object against the ID. --- bot/cogs/sync/cog.py | 12 ++++++------ tests/bot/cogs/sync/test_cog.py | 35 +++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 578cccfc9..5ace957e7 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -47,7 +47,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: """Adds newly create role to the database table over the API.""" - if role.guild != constants.Guild.id: + if role.guild.id != constants.Guild.id: return await self.bot.api_client.post( @@ -64,7 +64,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_delete(self, role: Role) -> None: """Deletes role from the database when it's deleted from the guild.""" - if role.guild != constants.Guild.id: + if role.guild.id != constants.Guild.id: return await self.bot.api_client.delete(f'bot/roles/{role.id}') @@ -72,7 +72,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_update(self, before: Role, after: Role) -> None: """Syncs role with the database if any of the stored attributes were updated.""" - if after.guild != constants.Guild.id: + if after.guild.id != constants.Guild.id: return was_updated = ( @@ -103,7 +103,7 @@ class Sync(Cog): previously left), it will update the user's information. If the user is not yet known by the database, the user is added. """ - if member.guild != constants.Guild.id: + if member.guild.id != constants.Guild.id: return packed = { @@ -135,7 +135,7 @@ class Sync(Cog): @Cog.listener() async def on_member_remove(self, member: Member) -> None: """Set the in_guild field to False when a member leaves the guild.""" - if member.guild != constants.Guild.id: + if member.guild.id != constants.Guild.id: return await self.patch_user(member.id, json={"in_guild": False}) @@ -143,7 +143,7 @@ class Sync(Cog): @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """Update the roles of the member in the database if a change is detected.""" - if after.guild != constants.Guild.id: + if after.guild.id != constants.Guild.id: return if before.roles != after.roles: diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index e5be14391..120bc991d 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -134,6 +134,9 @@ class SyncCogListenerTests(SyncCogTestCase): self.guild_id_patcher = mock.patch("bot.cogs.sync.cog.constants.Guild.id", 5) self.guild_id = self.guild_id_patcher.start() + self.guild = helpers.MockGuild(id=self.guild_id) + self.other_guild = helpers.MockGuild(id=0) + def tearDown(self): self.guild_id_patcher.stop() @@ -148,14 +151,14 @@ class SyncCogListenerTests(SyncCogTestCase): "permissions": 8, "position": 23, } - role = helpers.MockRole(**role_data, guild=self.guild_id) + role = helpers.MockRole(**role_data, guild=self.guild) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_called_once_with("bot/roles", json=role_data) async def test_sync_cog_on_guild_role_create_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_not_awaited() @@ -163,14 +166,14 @@ class SyncCogListenerTests(SyncCogTestCase): """A DELETE request should be sent.""" self.assertTrue(self.cog.on_guild_role_delete.__cog_listener__) - role = helpers.MockRole(id=99, guild=self.guild_id) + role = helpers.MockRole(id=99, guild=self.guild) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_called_once_with("bot/roles/99") async def test_sync_cog_on_guild_role_delete_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_not_awaited() @@ -198,8 +201,8 @@ class SyncCogListenerTests(SyncCogTestCase): after_role_data = role_data.copy() after_role_data[attribute] = 876 - before_role = helpers.MockRole(**role_data, guild=self.guild_id) - after_role = helpers.MockRole(**after_role_data, guild=self.guild_id) + before_role = helpers.MockRole(**role_data, guild=self.guild) + after_role = helpers.MockRole(**after_role_data, guild=self.guild) await self.cog.on_guild_role_update(before_role, after_role) @@ -213,7 +216,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_guild_role_update_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_update(role, role) self.bot.api_client.put.assert_not_awaited() @@ -221,7 +224,7 @@ class SyncCogListenerTests(SyncCogTestCase): """Member should be patched to set in_guild as False.""" self.assertTrue(self.cog.on_member_remove.__cog_listener__) - member = helpers.MockMember(guild=self.guild_id) + member = helpers.MockMember(guild=self.guild) await self.cog.on_member_remove(member) self.cog.patch_user.assert_called_once_with( @@ -231,7 +234,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_remove_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_remove(member) self.cog.patch_user.assert_not_awaited() @@ -241,8 +244,8 @@ class SyncCogListenerTests(SyncCogTestCase): # Roles are intentionally unsorted. before_roles = [helpers.MockRole(id=12), helpers.MockRole(id=30), helpers.MockRole(id=20)] - before_member = helpers.MockMember(roles=before_roles, guild=self.guild_id) - after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild_id) + before_member = helpers.MockMember(roles=before_roles, guild=self.guild) + after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild) await self.cog.on_member_update(before_member, after_member) @@ -263,8 +266,8 @@ class SyncCogListenerTests(SyncCogTestCase): with self.subTest(attribute=attribute): self.cog.patch_user.reset_mock() - before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild_id) - after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild_id) + before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild) + after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild) await self.cog.on_member_update(before_member, after_member) @@ -272,7 +275,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_update_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_update(member, member) self.cog.patch_user.assert_not_awaited() @@ -329,7 +332,7 @@ class SyncCogListenerTests(SyncCogTestCase): member = helpers.MockMember( discriminator="1234", roles=[helpers.MockRole(id=22), helpers.MockRole(id=12)], - guild=self.guild_id, + guild=self.guild, ) data = { @@ -376,7 +379,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_join_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_join(member) self.bot.api_client.post.assert_not_awaited() self.bot.api_client.put.assert_not_awaited() -- cgit v1.2.3 From 311326b21fe887063f0d4f757b9624f41ed28418 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 17 Jun 2020 17:04:48 -0700 Subject: Make sub_clyde case-sensitive and use Cyrillic e's The Cyrillic characters are more likely to be rendered similarly to their Latin counterparts than the math sans-serif characters. --- bot/utils/messages.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index 6ad9351cc..c7d756708 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -119,10 +119,14 @@ async def send_attachments( def sub_clyde(username: Optional[str]) -> Optional[str]: """ - Replace "e" in any "clyde" in `username` with a similar Unicode char and return the new string. + Replace "e"/"E" in any "clyde" in `username` with a Cyrillic "е"/"E" and return the new string. Discord disallows "clyde" anywhere in the username for webhooks. It will return a 400. Return None only if `username` is None. """ + def replace_e(match: re.Match) -> str: + char = "е" if match[2] == "e" else "Е" + return match[1] + char + if username: - return re.sub(r"(clyd)e", r"\1𝖾", username, flags=re.I) + return re.sub(r"(clyd)(e)", replace_e, username, flags=re.I) -- cgit v1.2.3 From c8cbd1e744c5c48490be321b19ef4f062443ed6d Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 19 Jun 2020 18:28:29 +0200 Subject: Pipenv: add script for html coverage report Similarly to the `report` script, this removes the need to invoke coverage when generating the html report. --- Pipfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Pipfile b/Pipfile index b42ca6d58..33be99587 100644 --- a/Pipfile +++ b/Pipfile @@ -50,4 +50,5 @@ precommit = "pre-commit install" build = "docker build -t pythondiscord/bot:latest -f Dockerfile ." push = "docker push pythondiscord/bot:latest" test = "coverage run -m unittest" +html = "coverage html" report = "coverage report" -- cgit v1.2.3 From 55db81a8c089c96a2e5e96110e3c80f0d36ebb58 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 19 Jun 2020 15:16:09 -0700 Subject: Preserve empty string when substituting clyde --- bot/utils/messages.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index c7d756708..a40a12e98 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -130,3 +130,5 @@ def sub_clyde(username: Optional[str]) -> Optional[str]: if username: return re.sub(r"(clyd)(e)", replace_e, username, flags=re.I) + else: + return username # Empty string or None -- cgit v1.2.3 From 581573f2ece96a9ec666795431ff21068e949a63 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 20 Jun 2020 01:20:35 +0200 Subject: Write unit test for `sub_clyde` --- tests/bot/utils/test_messages.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/bot/utils/test_messages.py diff --git a/tests/bot/utils/test_messages.py b/tests/bot/utils/test_messages.py new file mode 100644 index 000000000..9c22c9751 --- /dev/null +++ b/tests/bot/utils/test_messages.py @@ -0,0 +1,27 @@ +import unittest + +from bot.utils import messages + + +class TestMessages(unittest.TestCase): + """Tests for functions in the `bot.utils.messages` module.""" + + def test_sub_clyde(self): + """Uppercase E's and lowercase e's are substituted with their cyrillic counterparts.""" + sub_e = "\u0435" + sub_E = "\u0415" # noqa: N806: Uppercase E in variable name + + test_cases = ( + (None, None), + ("", ""), + ("clyde", f"clyd{sub_e}"), + ("CLYDE", f"CLYD{sub_E}"), + ("cLyDe", f"cLyD{sub_e}"), + ("BIGclyde", f"BIGclyd{sub_e}"), + ("small clydeus the unholy", f"small clyd{sub_e}us the unholy"), + ("BIGCLYDE, babyclyde", f"BIGCLYD{sub_E}, babyclyd{sub_e}"), + ) + + for username_in, username_out in test_cases: + with self.subTest(input=username_in, expected_output=username_out): + self.assertEqual(messages.sub_clyde(username_in), username_out) -- cgit v1.2.3