From 78b00dc8f43d7cb25ce72bc97d6236ec9944cd1f Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:10:35 +0300 Subject: Remove WatchChannel parent of TalentPool and migrate first commands --- bot/exts/recruitment/talentpool/_cog.py | 80 +++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 28 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index b809cea17..09f2fecbd 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -1,6 +1,6 @@ import logging import textwrap -from collections import ChainMap +from collections import ChainMap, defaultdict from typing import Union from discord import Color, Embed, Member, User @@ -8,35 +8,48 @@ from discord.ext.commands import Cog, Context, group, has_any_role from bot.api import ResponseCodeError from bot.bot import Bot -from bot.constants import Channels, Guild, MODERATION_ROLES, STAFF_ROLES, Webhooks +from bot.constants import Guild, MODERATION_ROLES, STAFF_ROLES from bot.converters import FetchedMember -from bot.exts.moderation.watchchannels._watchchannel import WatchChannel from bot.exts.recruitment.talentpool._review import Reviewer from bot.pagination import LinePaginator from bot.utils import time +from bot.utils.time import get_time_delta REASON_MAX_CHARS = 1000 log = logging.getLogger(__name__) -class TalentPool(WatchChannel, Cog, name="Talentpool"): +class TalentPool(Cog, name="Talentpool"): """Relays messages of helper candidates to a watch channel to observe them.""" def __init__(self, bot: Bot) -> None: - super().__init__( - bot, - destination=Channels.talent_pool, - webhook_id=Webhooks.talent_pool, - api_endpoint='bot/nominations', - api_default_params={'active': 'true', 'ordering': '-inserted_at'}, - logger=log, - disable_header=True, - ) - + self.bot = bot self.reviewer = Reviewer(self.__class__.__name__, bot, self) self.bot.loop.create_task(self.reviewer.reschedule_reviews()) + # Stores talentpool users in cache + self.cache = defaultdict(dict) + + async def refresh_cache(self) -> bool: + """Updates TalentPool users cache.""" + try: + data = await self.bot.api_client.get( + 'bot/nominations', + params={'active': 'true', 'ordering': '-inserted_at'} + ) + except ResponseCodeError as err: + log.exception("Failed to fetch the watched users from the API", exc_info=err) + return False + + self.cache = defaultdict(dict) + + for entry in data: + user_id = entry.pop('user') + self.cache[user_id] = entry + + return True + @group(name='talentpool', aliases=('tp', 'talent', 'nomination', 'n'), invoke_without_command=True) @has_any_role(*MODERATION_ROLES) async def nomination_group(self, ctx: Context) -> None: @@ -78,26 +91,37 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): The optional kwarg `update_cache` specifies whether the cache should be refreshed by polling the API. """ - # TODO Once the watch channel is removed, this can be done in a smarter way, without splitting and overriding - # the list_watched_users function. - watched_data = await self.prepare_watched_users_data(ctx, oldest_first, update_cache) + successful_update = False + if update_cache: + if not (successful_update := await self.refresh_cache()): + await ctx.send(":warning: Unable to update cache. Data may be inaccurate.") - if update_cache and not watched_data["updated"]: - await ctx.send(f":x: Failed to update {self.__class__.__name__} user cache, serving from cache") + nominations = self.cache.items() + if oldest_first: + nominations = reversed(nominations) lines = [] - for user_id, line in watched_data["info"].items(): - if self.watched_users[user_id]['reviewed']: + + for user_id, user_data in nominations: + member = ctx.guild.get_member(user_id) + line = f"• `{user_id}`" + if member: + line += f" ({member.name}#{member.discriminator})" + inserted_at = user_data['inserted_at'] + line += f", added {get_time_delta(inserted_at)}" + if not member: # Cross off users who left the server. + line = f"~~{line}~~" + if user_data['reviewed']: line += " *(reviewed)*" elif user_id in self.reviewer: - line += " *(scheduled)*" + line += " *(scheduled)" lines.append(line) if not lines: lines = ("There's nothing here yet.",) embed = Embed( - title=watched_data["title"], + title=f"Talent Pool active nominations ({'updated' if update_cache and successful_update else 'cached'})", color=Color.blue() ) await LinePaginator.paginate(lines, ctx, embed, empty=False) @@ -130,8 +154,8 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): await ctx.send(":x: Nominating staff members, eh? Here's a cookie :cookie:") return - if not await self.fetch_user_cache(): - await ctx.send(f":x: Failed to update the user cache; can't add {user}") + if not await self.refresh_cache(): + await ctx.send(f":x: Failed to update the cache; can't add {user}") return if len(reason) > REASON_MAX_CHARS: @@ -140,7 +164,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): # Manual request with `raise_for_status` as False because we want the actual response session = self.bot.api_client.session - url = self.bot.api_client._url_for(self.api_endpoint) + url = self.bot.api_client._url_for('bot/nominations') kwargs = { 'json': { 'actor': ctx.author.id, @@ -162,13 +186,13 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): else: resp.raise_for_status() - self.watched_users[user.id] = response_data + self.cache[user.id] = response_data if user.id not in self.reviewer: self.reviewer.schedule_review(user.id) history = await self.bot.api_client.get( - self.api_endpoint, + 'bot/nominations', params={ "user__id": str(user.id), "active": "false", -- cgit v1.2.3 From dda3490a0a1a55bb3ccb16b82afc0dd131f335b5 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:19:51 +0300 Subject: Use more accurate command names and docstring for talent pool commands --- bot/exts/recruitment/talentpool/_cog.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 09f2fecbd..40fb90f11 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -56,25 +56,25 @@ class TalentPool(Cog, name="Talentpool"): """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" await ctx.send_help(ctx.command) - @nomination_group.command(name='watched', aliases=('all', 'list'), root_aliases=("nominees",)) + @nomination_group.command(name='list', aliases=('all', 'watched'), root_aliases=("nominees",)) @has_any_role(*MODERATION_ROLES) - async def watched_command( + async def list_command( self, ctx: Context, oldest_first: bool = False, update_cache: bool = True ) -> None: """ - Shows the users that are currently being monitored in the talent pool. + Shows the users that are currently in the talent pool. The optional kwarg `oldest_first` can be used to order the list by oldest nomination. The optional kwarg `update_cache` can be used to update the user cache using the API before listing the users. """ - await self.list_watched_users(ctx, oldest_first=oldest_first, update_cache=update_cache) + await self.list_users(ctx, oldest_first=oldest_first, update_cache=update_cache) - async def list_watched_users( + async def list_users( self, ctx: Context, oldest_first: bool = False, @@ -130,21 +130,20 @@ class TalentPool(Cog, name="Talentpool"): @has_any_role(*MODERATION_ROLES) async def oldest_command(self, ctx: Context, update_cache: bool = True) -> None: """ - Shows talent pool monitored users ordered by oldest nomination. + Shows talent pool users ordered by oldest nomination. The optional kwarg `update_cache` can be used to update the user cache using the API before listing the users. """ - await ctx.invoke(self.watched_command, oldest_first=True, update_cache=update_cache) + await ctx.invoke(self.list_command, oldest_first=True, update_cache=update_cache) - @nomination_group.command(name='watch', aliases=('w', 'add', 'a'), root_aliases=("nominate",)) + @nomination_group.command(name='add', aliases=('w', 'a', 'watch'), root_aliases=("nominate",)) @has_any_role(*STAFF_ROLES) - async def watch_command(self, ctx: Context, user: FetchedMember, *, reason: str = '') -> None: + async def add_command(self, ctx: Context, user: FetchedMember, *, reason: str = '') -> None: """ - Relay messages sent by the given `user` to the `#talent-pool` channel. + Adds user nomination (or nomination entry) to Talent Pool. - A `reason` for adding the user to the talent pool is optional. - If given, it will be displayed in the header when relaying messages of this user to the channel. + If user already have nomination, then entry associated with existing nomination will be created. """ if user.bot: await ctx.send(f":x: I'm sorry {ctx.author}, I'm afraid I can't do that. I only watch humans.") @@ -235,9 +234,9 @@ class TalentPool(Cog, name="Talentpool"): max_size=1000 ) - @nomination_group.command(name='unwatch', aliases=('end', ), root_aliases=("unnominate",)) + @nomination_group.command(name='end', aliases=('unwatch',), root_aliases=("unnominate",)) @has_any_role(*MODERATION_ROLES) - async def unwatch_command(self, ctx: Context, user: FetchedMember, *, reason: str) -> None: + async def end_command(self, ctx: Context, user: FetchedMember, *, reason: str) -> None: """ Ends the active nomination of the specified user with the given reason. -- cgit v1.2.3 From f144bace9243659887cbb125ea915f809736fc97 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:22:13 +0300 Subject: Migrate nominations history command to non-watchchannel system --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 40fb90f11..ee5dc5dae 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -210,7 +210,7 @@ class TalentPool(Cog, name="Talentpool"): async def history_command(self, ctx: Context, user: FetchedMember) -> None: """Shows the specified user's nomination history.""" result = await self.bot.api_client.get( - self.api_endpoint, + 'bot/nominations', params={ 'user__id': str(user.id), 'ordering': "-active,-inserted_at" -- cgit v1.2.3 From 807a27ec112684a2a2b8cfd2b470ab8378f0da98 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:30:46 +0300 Subject: Migrate Talent Pool Reviewer class to non-watchchannel Talent Pool --- bot/exts/recruitment/talentpool/_review.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index fb3461238..bc2878451 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -47,9 +47,9 @@ class Reviewer: log.trace("Rescheduling reviews") await self.bot.wait_until_guild_available() # TODO Once the watch channel is removed, this can be done in a smarter way, e.g create a sync function. - await self._pool.fetch_user_cache() + await self._pool.refresh_cache() - for user_id, user_data in self._pool.watched_users.items(): + for user_id, user_data in self._pool.cache.items(): if not user_data["reviewed"]: self.schedule_review(user_id) @@ -57,7 +57,7 @@ class Reviewer: """Schedules a single user for review.""" log.trace(f"Scheduling review of user with ID {user_id}") - user_data = self._pool.watched_users[user_id] + user_data = self._pool.cache[user_id] inserted_at = isoparse(user_data['inserted_at']).replace(tzinfo=None) review_at = inserted_at + timedelta(days=MAX_DAYS_IN_POOL) @@ -69,7 +69,7 @@ class Reviewer: """Format a generic review of a user and post it to the nomination voting channel.""" log.trace(f"Posting the review of {user_id}") - nomination = self._pool.watched_users[user_id] + nomination = self._pool.cache[user_id] if not nomination: log.trace(f"There doesn't appear to be an active nomination for {user_id}") return @@ -79,7 +79,7 @@ class Reviewer: member = guild.get_member(user_id) if update_database: - await self.bot.api_client.patch(f"{self._pool.api_endpoint}/{nomination['id']}", json={"reviewed": True}) + await self.bot.api_client.patch(f"bot/nominations/{nomination['id']}", json={"reviewed": True}) if not member: await channel.send( @@ -228,7 +228,7 @@ class Reviewer: """ log.trace(f"Fetching the nomination history data for {member.id}'s review") history = await self.bot.api_client.get( - self._pool.api_endpoint, + "bot/nominations", params={ "user__id": str(member.id), "active": "false", @@ -286,18 +286,18 @@ class Reviewer: Returns True if the user was successfully marked as reviewed, False otherwise. """ log.trace(f"Updating user {user_id} as reviewed") - await self._pool.fetch_user_cache() - if user_id not in self._pool.watched_users: + await self._pool.refresh_cache() + if user_id not in self._pool.cache: log.trace(f"Can't find a nominated user with id {user_id}") await ctx.send(f"❌ Can't find a currently nominated user with id `{user_id}`") return False - nomination = self._pool.watched_users[user_id] + nomination = self._pool.cache[user_id] if nomination["reviewed"]: await ctx.send("❌ This nomination was already reviewed, but here's a cookie 🍪") return False - await self.bot.api_client.patch(f"{self._pool.api_endpoint}/{nomination['id']}", json={"reviewed": True}) + await self.bot.api_client.patch(f"bot/nominations/{nomination['id']}", json={"reviewed": True}) if user_id in self._review_scheduler: self._review_scheduler.cancel(user_id) -- cgit v1.2.3 From 2d4764041da02ccee64a93c02c0dd392c31e80c4 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:31:24 +0300 Subject: Migrate unnominate command to non-watchchannel --- bot/exts/recruitment/talentpool/_cog.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index ee5dc5dae..1ad40812a 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -30,13 +30,14 @@ class TalentPool(Cog, name="Talentpool"): # Stores talentpool users in cache self.cache = defaultdict(dict) + self.api_default_params = {'active': 'true', 'ordering': '-inserted_at'} async def refresh_cache(self) -> bool: """Updates TalentPool users cache.""" try: data = await self.bot.api_client.get( 'bot/nominations', - params={'active': 'true', 'ordering': '-inserted_at'} + params=self.api_default_params ) except ResponseCodeError as err: log.exception("Failed to fetch the watched users from the API", exc_info=err) @@ -243,11 +244,11 @@ class TalentPool(Cog, name="Talentpool"): Providing a `reason` is required. """ if len(reason) > REASON_MAX_CHARS: - await ctx.send(f":x: Maxiumum allowed characters for the end reason is {REASON_MAX_CHARS}.") + await ctx.send(f":x: Maximum allowed characters for the end reason is {REASON_MAX_CHARS}.") return if await self.unwatch(user.id, reason): - await ctx.send(f":white_check_mark: Messages sent by {user} will no longer be relayed") + await ctx.send(f":white_check_mark: Successfully un-nominated {user}") else: await ctx.send(":x: The specified user does not have an active nomination") @@ -349,7 +350,7 @@ class TalentPool(Cog, name="Talentpool"): async def unwatch(self, user_id: int, reason: str) -> bool: """End the active nomination of a user with the given reason and return True on success.""" active_nomination = await self.bot.api_client.get( - self.api_endpoint, + 'bot/nominations', params=ChainMap( {"user__id": str(user_id)}, self.api_default_params, @@ -364,10 +365,9 @@ class TalentPool(Cog, name="Talentpool"): nomination = active_nomination[0] await self.bot.api_client.patch( - f"{self.api_endpoint}/{nomination['id']}", + f"bot/nominations/{nomination['id']}", json={'end_reason': reason, 'active': False} ) - self._remove_user(user_id) self.reviewer.cancel(user_id) -- cgit v1.2.3 From ae3c91082f1a456d37c8ce72c65509315e4a2137 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:32:58 +0300 Subject: Add missing asterisk to nominees list --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 1ad40812a..4ef3af3d1 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -115,7 +115,7 @@ class TalentPool(Cog, name="Talentpool"): if user_data['reviewed']: line += " *(reviewed)*" elif user_id in self.reviewer: - line += " *(scheduled)" + line += " *(scheduled)*" lines.append(line) if not lines: -- cgit v1.2.3 From 4716b8eba3d25f21e277ee20cff2741cece2e0ed Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 09:52:53 +0300 Subject: Migrate nomination reasons editing commands --- bot/exts/recruitment/talentpool/_cog.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 4ef3af3d1..7824d22d7 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -267,10 +267,10 @@ class TalentPool(Cog, name="Talentpool"): return try: - nomination = await self.bot.api_client.get(f"{self.api_endpoint}/{nomination_id}") + nomination = await self.bot.api_client.get(f"bot/nominations/{nomination_id}") except ResponseCodeError as e: if e.response.status == 404: - self.log.trace(f"Nomination API 404: Can't find a nomination with id {nomination_id}") + log.trace(f"Nomination API 404: Can't find a nomination with id {nomination_id}") await ctx.send(f":x: Can't find a nomination with id `{nomination_id}`") return else: @@ -284,13 +284,13 @@ class TalentPool(Cog, name="Talentpool"): await ctx.send(f":x: {actor} doesn't have an entry in this nomination.") return - self.log.trace(f"Changing reason for nomination with id {nomination_id} of actor {actor} to {repr(reason)}") + log.trace(f"Changing reason for nomination with id {nomination_id} of actor {actor} to {repr(reason)}") await self.bot.api_client.patch( - f"{self.api_endpoint}/{nomination_id}", + f"bot/nominations/{nomination_id}", json={"actor": actor.id, "reason": reason} ) - await self.fetch_user_cache() # Update cache + await self.refresh_cache() # Update cache await ctx.send(":white_check_mark: Successfully updated nomination reason.") @nomination_edit_group.command(name='end_reason') @@ -302,10 +302,10 @@ class TalentPool(Cog, name="Talentpool"): return try: - nomination = await self.bot.api_client.get(f"{self.api_endpoint}/{nomination_id}") + nomination = await self.bot.api_client.get(f"bot/nominations/{nomination_id}") except ResponseCodeError as e: if e.response.status == 404: - self.log.trace(f"Nomination API 404: Can't find a nomination with id {nomination_id}") + log.trace(f"Nomination API 404: Can't find a nomination with id {nomination_id}") await ctx.send(f":x: Can't find a nomination with id `{nomination_id}`") return else: @@ -315,13 +315,13 @@ class TalentPool(Cog, name="Talentpool"): await ctx.send(":x: Can't edit the end reason of an active nomination.") return - self.log.trace(f"Changing end reason for nomination with id {nomination_id} to {repr(reason)}") + log.trace(f"Changing end reason for nomination with id {nomination_id} to {repr(reason)}") await self.bot.api_client.patch( - f"{self.api_endpoint}/{nomination_id}", + f"bot/nominations/{nomination_id}", json={"end_reason": reason} ) - await self.fetch_user_cache() # Update cache. + await self.refresh_cache() # Update cache. await ctx.send(":white_check_mark: Updated the end reason of the nomination!") @nomination_group.command(aliases=('mr',)) -- cgit v1.2.3 From e9a233f1f2fbc9b0204041dffa0f3ad6a1f7e8cf Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Sun, 28 Mar 2021 10:17:47 +0300 Subject: Remove talentpool channel constants --- bot/constants.py | 2 -- config-default.yml | 3 --- 2 files changed, 5 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 467a4a2c4..840f6fbf6 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -457,7 +457,6 @@ class Channels(metaclass=YAMLGetter): voice_chat: int big_brother_logs: int - talent_pool: int class Webhooks(metaclass=YAMLGetter): @@ -470,7 +469,6 @@ class Webhooks(metaclass=YAMLGetter): duck_pond: int incidents_archive: int reddit: int - talent_pool: int class Roles(metaclass=YAMLGetter): diff --git a/config-default.yml b/config-default.yml index 502f0f861..8fc1f6718 100644 --- a/config-default.yml +++ b/config-default.yml @@ -223,7 +223,6 @@ guild: # Watch big_brother_logs: &BB_LOGS 468507907357409333 - talent_pool: &TALENT_POOL 534321732593647616 moderation_categories: - *MODMAIL @@ -292,7 +291,6 @@ guild: incidents_archive: 720671599790915702 python_news: &PYNEWS_WEBHOOK 704381182279942324 reddit: 635408384794951680 - talent_pool: 569145364800602132 filter: @@ -323,7 +321,6 @@ filter: - *MESSAGE_LOG - *MOD_LOG - *STAFF_LOUNGE - - *TALENT_POOL - *USER_EVENT_A role_whitelist: -- cgit v1.2.3 From d1a2d9965fadc5292685d3ca78a75fd16f07af26 Mon Sep 17 00:00:00 2001 From: Xithrius Date: Sat, 12 Jun 2021 14:08:52 -0700 Subject: Switched back to default value getting. --- bot/exts/recruitment/talentpool/_review.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index 585640699..4e61ecb3e 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -65,7 +65,7 @@ class Reviewer: """Schedules a single user for review.""" log.trace(f"Scheduling review of user with ID {user_id}") - user_data = self._pool.cache[user_id] + user_data = self._pool.cache.get(user_id) inserted_at = isoparse(user_data['inserted_at']).replace(tzinfo=None) review_at = inserted_at + timedelta(days=MAX_DAYS_IN_POOL) @@ -93,7 +93,7 @@ class Reviewer: await last_message.add_reaction(reaction) if update_database: - nomination = self._pool.cache[user_id] + nomination = self._pool.cache.get(user_id) await self.bot.api_client.patch(f"{self._pool.api_endpoint}/{nomination['id']}", json={"reviewed": True}) async def make_review(self, user_id: int) -> typing.Tuple[str, Optional[Emoji]]: @@ -104,7 +104,7 @@ class Reviewer: # not to accidentally insert the IDs of users that have no # active nominated by using the `watched_users.get(user_id)` # instead of `watched_users[user_id]`. - nomination = self._pool.cache[user_id] + nomination = self._pool.cache.get(user_id) if not nomination: log.trace(f"There doesn't appear to be an active nomination for {user_id}") return "", None @@ -393,7 +393,7 @@ class Reviewer: await ctx.send(f":x: Can't find a currently nominated user with id `{user_id}`") return False - nomination = self._pool.cache[user_id] + nomination = self._pool.cache.get(user_id) if nomination["reviewed"]: await ctx.send(":x: This nomination was already reviewed, but here's a cookie :cookie:") return False -- cgit v1.2.3 From 2a83c5f8f7b9bfd60a50e67f20c04f31651bbcaa Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sat, 28 Aug 2021 11:38:38 +0100 Subject: Only check URL-like objects against domain filters Previously a message such as 'https://google.com hello! flask.request.method' would be filtered due to us filtering the url shortener t.me. This commit changes to logic so that we only check parts of the messages that matched the URL regex against our blacklist, to avoid these false-positives. --- bot/exts/filters/filtering.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index 10cc7885d..b7e91395e 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -478,15 +478,16 @@ class Filtering(Cog): Second return value is a reason of URL blacklisting (can be None). """ text = self.clean_input(text) - if not URL_RE.search(text): + matches = URL_RE.findall(text) + if not matches: return False, None - text = text.lower() domain_blacklist = self._get_filterlist_items("domain_name", allowed=False) for url in domain_blacklist: - if url.lower() in text: - return True, self._get_filterlist_value("domain_name", url, allowed=False)["comment"] + for match in matches: + if url.lower() in match.lower(): + return True, self._get_filterlist_value("domain_name", url, allowed=False)["comment"] return False, None -- cgit v1.2.3 From dc01b874db2f2909dd90f62847d995e7c94a17eb Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 19:47:36 +0100 Subject: Add concurrency rules to all GitHub workflows This concurrency rule means that workflow actions will be cancelled if a new set of actions are triggered on the same ref. A ref is either a PR or a branch itself. Doing this means that if someone pushes to a PR multiple times in quick succession, workflows won't get queued up. Instead, only the workflows from the most recent changes will run, as this is all we really care about anyway. I see the benefits of this as twofold. 1. The author gets faster feedback about the most recent change pushed, rather than have to wait for all previous runs to completed 2. Other contributors don't need to wait for a queue of redundant workflows to finish. --- .github/workflows/build.yml | 4 ++++ .github/workflows/deploy.yml | 4 ++++ .github/workflows/lint-test.yml | 3 +++ .github/workflows/sentry_release.yml | 4 ++++ .github/workflows/status_embed.yaml | 4 ++++ 5 files changed, 19 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 84a671917..f8f2c8888 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,6 +8,10 @@ on: types: - completed +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: if: github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push' diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 8b809b777..0f030ebd7 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -8,6 +8,10 @@ on: types: - completed +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: environment: production diff --git a/.github/workflows/lint-test.yml b/.github/workflows/lint-test.yml index e99e6d181..c4f87d522 100644 --- a/.github/workflows/lint-test.yml +++ b/.github/workflows/lint-test.yml @@ -6,6 +6,9 @@ on: - main pull_request: +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: lint-test: diff --git a/.github/workflows/sentry_release.yml b/.github/workflows/sentry_release.yml index f6a1e1f0e..48f5e50f4 100644 --- a/.github/workflows/sentry_release.yml +++ b/.github/workflows/sentry_release.yml @@ -5,6 +5,10 @@ on: branches: - main +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: create_sentry_release: runs-on: ubuntu-latest diff --git a/.github/workflows/status_embed.yaml b/.github/workflows/status_embed.yaml index b6a71b887..4178c366d 100644 --- a/.github/workflows/status_embed.yaml +++ b/.github/workflows/status_embed.yaml @@ -9,6 +9,10 @@ on: types: - completed +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: status_embed: # We need to send a status embed whenever the workflow -- cgit v1.2.3 From c2d1c974bb95d75b275260e869ec67d3e92d1ae1 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 21:23:54 +0100 Subject: Change all references to watch to nominate instead --- bot/exts/recruitment/talentpool/_cog.py | 42 ++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 8db2d7eac..99567d55e 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -25,7 +25,7 @@ log = logging.getLogger(__name__) class TalentPool(Cog, name="Talentpool"): - """Relays messages of helper candidates to a watch channel to observe them.""" + """Used to nominate potential helper candidates.""" # RedisCache[str, bool] # Can contain a single key, "autoreview_enabled", with the value a bool indicating if autoreview is enabled. @@ -59,7 +59,7 @@ class TalentPool(Cog, name="Talentpool"): params=self.api_default_params ) except ResponseCodeError as err: - log.exception("Failed to fetch the watched users from the API", exc_info=err) + log.exception("Failed to fetch the currently nominated users from the API", exc_info=err) return False self.cache = defaultdict(dict) @@ -120,7 +120,11 @@ class TalentPool(Cog, name="Talentpool"): else: await ctx.send("Autoreview is currently disabled") - @nomination_group.command(name='watched', aliases=('all', 'list'), root_aliases=("nominees",)) + @nomination_group.command( + name="nominess", + aliases=("nominated", "all", "list", "watched"), + root_aliases=("nominees",) + ) @has_any_role(*MODERATION_ROLES) async def list_command( self, @@ -201,19 +205,23 @@ class TalentPool(Cog, name="Talentpool"): """ await ctx.invoke(self.list_command, oldest_first=True, update_cache=update_cache) - @nomination_group.command(name='forcewatch', aliases=('fw', 'forceadd', 'fa'), root_aliases=("forcenominate",)) + @nomination_group.command( + name="forcenominate", + aliases=("fw", "forceadd", "fa", "fn", "forcewatch"), + root_aliases=("forcenominate",) + ) @has_any_role(*MODERATION_ROLES) - async def force_watch_command(self, ctx: Context, user: MemberOrUser, *, reason: str = '') -> None: + async def force_nominate_command(self, ctx: Context, user: MemberOrUser, *, reason: str = '') -> None: """ Adds the given `user` to the talent pool, from any channel. A `reason` for adding the user to the talent pool is optional. """ - await self._watch_user(ctx, user, reason) + await self._nominate_user(ctx, user, reason) - @nomination_group.command(name='watch', aliases=('w', 'add', 'a'), root_aliases=("nominate",)) + @nomination_group.command(name='nominate', aliases=("w", "add", "a", "watch"), root_aliases=("nominate",)) @has_any_role(*STAFF_ROLES) - async def watch_command(self, ctx: Context, user: MemberOrUser, *, reason: str = '') -> None: + async def nominate_command(self, ctx: Context, user: MemberOrUser, *, reason: str = '') -> None: """ Adds the given `user` to the talent pool. @@ -224,18 +232,18 @@ class TalentPool(Cog, name="Talentpool"): if any(role.id in MODERATION_ROLES for role in ctx.author.roles): await ctx.send( f":x: Nominations should be run in the <#{Channels.nominations}> channel. " - "Use `!tp forcewatch` to override this check." + "Use `!tp forcenominate` to override this check." ) else: await ctx.send(f":x: Nominations must be run in the <#{Channels.nominations}> channel") return - await self._watch_user(ctx, user, reason) + await self._nominate_user(ctx, user, reason) - async def _watch_user(self, ctx: Context, user: MemberOrUser, reason: str) -> None: + async def _nominate_user(self, ctx: Context, user: MemberOrUser, reason: str) -> None: """Adds the given user to the talent pool.""" if user.bot: - await ctx.send(f":x: I'm sorry {ctx.author}, I'm afraid I can't do that. I only watch humans.") + await ctx.send(f":x: I'm sorry {ctx.author}, I'm afraid I can't do that. Only humans can be nominated.") return if isinstance(user, Member) and any(role.id in STAFF_ROLES for role in user.roles): @@ -325,7 +333,7 @@ class TalentPool(Cog, name="Talentpool"): @nomination_group.command(name='end', aliases=('unwatch',), root_aliases=("unnominate",)) @has_any_role(*MODERATION_ROLES) - async def unwatch_command(self, ctx: Context, user: MemberOrUser, *, reason: str) -> None: + async def end_nomination_command(self, ctx: Context, user: MemberOrUser, *, reason: str) -> None: """ Ends the active nomination of the specified user with the given reason. @@ -335,7 +343,7 @@ class TalentPool(Cog, name="Talentpool"): await ctx.send(f":x: Maximum allowed characters for the end reason is {REASON_MAX_CHARS}.") return - if await self.unwatch(user.id, reason): + if await self.end_nomination(user.id, reason): await ctx.send(f":white_check_mark: Messages sent by {user.mention} will no longer be relayed") else: await ctx.send(":x: The specified user does not have an active nomination") @@ -444,7 +452,7 @@ class TalentPool(Cog, name="Talentpool"): @Cog.listener() async def on_member_ban(self, guild: Guild, user: Union[MemberOrUser]) -> None: """Remove `user` from the talent pool after they are banned.""" - await self.unwatch(user.id, "User was banned.") + await self.end_nomination(user.id, "User was banned.") @Cog.listener() async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: @@ -466,7 +474,7 @@ class TalentPool(Cog, name="Talentpool"): log.info(f"Archiving nomination {message.id}") await self.reviewer.archive_vote(message, emoji == Emojis.incident_actioned) - async def unwatch(self, user_id: int, reason: str) -> bool: + async def end_nomination(self, user_id: int, reason: str) -> bool: """End the active nomination of a user with the given reason and return True on success.""" active_nomination = await self.bot.api_client.get( 'bot/nominations', @@ -536,7 +544,7 @@ class TalentPool(Cog, name="Talentpool"): {entries_string} End date: {end_date} - Unwatch reason: {nomination_object["end_reason"]} + Unnominate reason: {nomination_object["end_reason"]} =============== """ ) -- cgit v1.2.3 From 20aa8b23a386ad2d570a1d771825331b07074bb1 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 21:27:54 +0100 Subject: Remove code that is no longer hit --- bot/exts/recruitment/talentpool/_cog.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 99567d55e..78f9b189b 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -47,10 +47,6 @@ class TalentPool(Cog, name="Talentpool"): """Return whether automatic posting of nomination reviews is enabled.""" return await self.talentpool_settings.get(AUTOREVIEW_ENABLED_KEY, True) - # Stores talentpool users in cache - self.cache = defaultdict(dict) - self.api_default_params = {'active': 'true', 'ordering': '-inserted_at'} - async def refresh_cache(self) -> bool: """Updates TalentPool users cache.""" try: -- cgit v1.2.3 From ffed769fbb1f450ac966564cc0ba75c2406e18ae Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 21:39:20 +0100 Subject: Update comment to reference the new TalentPool cache name --- bot/exts/recruitment/talentpool/_review.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index dfdcdcffe..1cc9c900c 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -103,10 +103,10 @@ class Reviewer: """Format a generic review of a user and return it with the reviewed emoji.""" log.trace(f"Formatting the review of {user_id}") - # Since `watched_users` is a defaultdict, we should take care + # Since `cache` is a defaultdict, we should take care # not to accidentally insert the IDs of users that have no - # active nominated by using the `watched_users.get(user_id)` - # instead of `watched_users[user_id]`. + # active nominated by using the `cache.get(user_id)` + # instead of `cache[user_id]`. nomination = self._pool.cache.get(user_id) if not nomination: log.trace(f"There doesn't appear to be an active nomination for {user_id}") -- cgit v1.2.3 From 6a35307ada47a1c3091b23fb3a33dea78fe63c15 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 21:44:48 +0100 Subject: Update api endpoint name in talent pool review --- bot/exts/recruitment/talentpool/_review.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index 1cc9c900c..3ffbf93f3 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -97,7 +97,7 @@ class Reviewer: if update_database: nomination = self._pool.cache.get(user_id) - await self.bot.api_client.patch(f"{self._pool.api_endpoint}/{nomination['id']}", json={"reviewed": True}) + await self.bot.api_client.patch(f"bot/nominations/{nomination['id']}", json={"reviewed": True}) async def make_review(self, user_id: int) -> typing.Tuple[str, Optional[Emoji]]: """Format a generic review of a user and return it with the reviewed emoji.""" -- cgit v1.2.3 From 892fafcd653b35c53ba48d7af42decd50b7fdda9 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 21:58:59 +0100 Subject: Update unnominate message to remove reference to relaying messages. --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 78f9b189b..4a77de24c 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -340,7 +340,7 @@ class TalentPool(Cog, name="Talentpool"): return if await self.end_nomination(user.id, reason): - await ctx.send(f":white_check_mark: Messages sent by {user.mention} will no longer be relayed") + await ctx.send(f":white_check_mark: Successfully un-nominated {user}") else: await ctx.send(":x: The specified user does not have an active nomination") -- cgit v1.2.3 From bcb664e19c3e417c17f00562180393f601c43a5a Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 22:04:09 +0100 Subject: Add back missing default params for TalentPool cog --- bot/exts/recruitment/talentpool/_cog.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 4a77de24c..38fefddd4 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -34,6 +34,7 @@ class TalentPool(Cog, name="Talentpool"): def __init__(self, bot: Bot) -> None: self.bot = bot self.reviewer = Reviewer(self.__class__.__name__, bot, self) + self.api_default_params = {'active': 'true', 'ordering': '-inserted_at'} self.bot.loop.create_task(self.schedule_autoreviews()) async def schedule_autoreviews(self) -> None: -- cgit v1.2.3 From ea6bd12f09c7978db24184cf09b91e1d643cffba Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 22:04:46 +0100 Subject: use global logger in talent pool, over a self var that has been deleted --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 38fefddd4..f932fc003 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -42,7 +42,7 @@ class TalentPool(Cog, name="Talentpool"): if await self.autoreview_enabled(): await self.reviewer.reschedule_reviews() else: - self.log.trace("Not scheduling reviews as autoreview is disabled.") + log.trace("Not scheduling reviews as autoreview is disabled.") async def autoreview_enabled(self) -> bool: """Return whether automatic posting of nomination reviews is enabled.""" -- cgit v1.2.3 From 5ca1fec1e9a2bb767863701bee1ffcad729331ec Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 22:46:26 +0100 Subject: Rename list nominated user command for clarity --- bot/exts/recruitment/talentpool/_cog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index f932fc003..6dc36058b 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -137,9 +137,9 @@ class TalentPool(Cog, name="Talentpool"): The optional kwarg `update_cache` can be used to update the user cache using the API before listing the users. """ - await self.list_users(ctx, oldest_first=oldest_first, update_cache=update_cache) + await self.list_nominated_users(ctx, oldest_first=oldest_first, update_cache=update_cache) - async def list_users( + async def list_nominated_users( self, ctx: Context, oldest_first: bool = False, -- cgit v1.2.3 From 1b04bdfec2ebf1399c93bf43b9bce735b47a8279 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 30 Aug 2021 23:47:52 +0200 Subject: Make the base_url argument in doc set optional --- bot/exts/info/doc/_cog.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/bot/exts/info/doc/_cog.py b/bot/exts/info/doc/_cog.py index fb9b2584a..a2119a53d 100644 --- a/bot/exts/info/doc/_cog.py +++ b/bot/exts/info/doc/_cog.py @@ -150,6 +150,8 @@ class DocCog(commands.Cog): self.update_or_reschedule_inventory(api_package_name, base_url, inventory_url), ) else: + if not base_url: + base_url = self.base_url_from_inventory_url(inventory_url) self.update_single(api_package_name, base_url, package) def ensure_unique_symbol_name(self, package_name: str, group_name: str, symbol_name: str) -> str: @@ -352,6 +354,11 @@ class DocCog(commands.Cog): msg = await ctx.send(embed=doc_embed) await wait_for_deletion(msg, (ctx.author.id,)) + @staticmethod + def base_url_from_inventory_url(inventory_url: str) -> str: + """Get a base url from the url to an objects inventory by removing the last path segment.""" + return inventory_url.removesuffix("/").rsplit("/", maxsplit=1)[0] + "/" + @docs_group.command(name="setdoc", aliases=("s",)) @commands.has_any_role(*MODERATION_ROLES) @lock(NAMESPACE, COMMAND_LOCK_SINGLETON, raise_error=True) @@ -359,21 +366,21 @@ class DocCog(commands.Cog): self, ctx: commands.Context, package_name: PackageName, - base_url: ValidURL, inventory: Inventory, + base_url: ValidURL = "", ) -> None: """ Adds a new documentation metadata object to the site's database. The database will update the object, should an existing item with the specified `package_name` already exist. + If the base url is not specified, a default created by removing the last segment of the inventory url is used. Example: !docs setdoc \ python \ - https://docs.python.org/3/ \ https://docs.python.org/3/objects.inv """ - if not base_url.endswith("/"): + if base_url and not base_url.endswith("/"): raise commands.BadArgument("The base url must end with a slash.") inventory_url, inventory_dict = inventory body = { @@ -388,6 +395,8 @@ class DocCog(commands.Cog): + "\n".join(f"{key}: {value}" for key, value in body.items()) ) + if not base_url: + base_url = self.base_url_from_inventory_url(inventory_url) self.update_single(package_name, base_url, inventory_dict) await ctx.send(f"Added the package `{package_name}` to the database and updated the inventories.") -- cgit v1.2.3 From 9ef8306c2bc8ab0285da62ba2c7c42e003f015e2 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 30 Aug 2021 22:51:47 +0100 Subject: Fix spelling of a TalentPool command name --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 6dc36058b..c0b3f8348 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -118,7 +118,7 @@ class TalentPool(Cog, name="Talentpool"): await ctx.send("Autoreview is currently disabled") @nomination_group.command( - name="nominess", + name="nominees", aliases=("nominated", "all", "list", "watched"), root_aliases=("nominees",) ) -- cgit v1.2.3 From 990bc67c7d02027d1eb3fc30654585bf77bc0d17 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 1 Sep 2021 10:23:05 +0100 Subject: Add an extra alias to unnominate --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index c0b3f8348..477fff2fd 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -328,7 +328,7 @@ class TalentPool(Cog, name="Talentpool"): max_size=1000 ) - @nomination_group.command(name='end', aliases=('unwatch',), root_aliases=("unnominate",)) + @nomination_group.command(name="end", aliases=("unwatch", "unnominate"), root_aliases=("unnominate",)) @has_any_role(*MODERATION_ROLES) async def end_nomination_command(self, ctx: Context, user: MemberOrUser, *, reason: str) -> None: """ -- cgit v1.2.3 From 97017f00ecdb70359ff8cf6f4a8fe970c6aa7ad5 Mon Sep 17 00:00:00 2001 From: Xithrius Date: Fri, 3 Sep 2021 00:50:50 -0700 Subject: `Unnominate reason` to `Unnomination reason` --- bot/exts/recruitment/talentpool/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 477fff2fd..c2257c84b 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -541,7 +541,7 @@ class TalentPool(Cog, name="Talentpool"): {entries_string} End date: {end_date} - Unnominate reason: {nomination_object["end_reason"]} + Unnomination reason: {nomination_object["end_reason"]} =============== """ ) -- cgit v1.2.3 From a06367e1691d8ef39e2cf8f8e0d15e5922d29d01 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Fri, 3 Sep 2021 09:42:44 +0100 Subject: Pop user from talent pool cache when unnominated --- bot/exts/recruitment/talentpool/_cog.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index c2257c84b..a317c6645 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -493,6 +493,7 @@ class TalentPool(Cog, name="Talentpool"): json={'end_reason': reason, 'active': False} ) + self.cache.pop(user_id) if await self.autoreview_enabled(): self.reviewer.cancel(user_id) -- cgit v1.2.3 From da697eef39bf6a786d8293312d41b9bd634c542d Mon Sep 17 00:00:00 2001 From: Joe Banks Date: Sat, 4 Sep 2021 09:41:47 +0100 Subject: Update in accordance with python-discord/kubernetes#95 --- .github/workflows/deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 8b809b777..f8a8292a4 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -38,6 +38,6 @@ jobs: uses: Azure/k8s-deploy@v1 with: manifests: | - bot/deployment.yaml + namespaces/default/bot/deployment.yaml images: 'ghcr.io/python-discord/bot:${{ steps.sha_tag.outputs.tag }}' kubectl-version: 'latest' -- cgit v1.2.3 From 66f1ca3ca60b4db46b58dcc8d30a4f45e1f8d122 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sat, 4 Sep 2021 14:49:30 +0100 Subject: Pin platform in Dockerfile Some of our deps don't have wheels for atm processors. With Mac's M1 chips becomming more common, we should make it easier for those users to build our environments. --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 4d8592590..30bf8a361 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.9-slim +FROM --platform=linux/amd64 python:3.9-slim # Set pip to have no saved cache ENV PIP_NO_CACHE_DIR=false \ -- cgit v1.2.3 From b4c7d61361f7ba567e6638d3721f7b3bbcf334d9 Mon Sep 17 00:00:00 2001 From: ChrisJL Date: Sat, 4 Sep 2021 23:14:15 +0100 Subject: Server command now uses correct TalentPool attr (#1810) This was attr changed when we removed the concept of a user being 'watched' while removing the talentpool. --- bot/exts/info/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index bcf8c10d2..51d47b75c 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -72,7 +72,7 @@ class Information(Cog): """Return additional server info only visible in moderation channels.""" talentpool_info = "" if cog := self.bot.get_cog("Talentpool"): - talentpool_info = f"Nominated: {len(cog.watched_users)}\n" + talentpool_info = f"Nominated: {len(cog.cache)}\n" bb_info = "" if cog := self.bot.get_cog("Big Brother"): -- cgit v1.2.3 From 6bb0dba2230e9b3fcab2caaf559b749543c26abb Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sun, 5 Sep 2021 09:34:54 +0100 Subject: Declare and refresh TalentPool.cache on init of cog This avoids issues in the server cog trying to access it before it's assigned and refreshed. I also migrated to the tasks to `scheduling.create_task()` as the created tasks currently don't have any error handling they can hide errors in development until the task object is destroyed (if that occurs at all) which logs the exception. The scheduling alternative attaches a callback which logs exceptions to prevent this. --- bot/exts/info/information.py | 3 ++- bot/exts/recruitment/talentpool/_cog.py | 9 ++++++--- bot/exts/recruitment/talentpool/_review.py | 2 -- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 51d47b75c..d44886969 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -72,7 +72,8 @@ class Information(Cog): """Return additional server info only visible in moderation channels.""" talentpool_info = "" if cog := self.bot.get_cog("Talentpool"): - talentpool_info = f"Nominated: {len(cog.cache)}\n" + num_nominated = len(cog.cache) if cog.cache else 0 + talentpool_info = f"Nominated: {num_nominated}\n" bb_info = "" if cog := self.bot.get_cog("Big Brother"): diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index a317c6645..bea5ff72c 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -2,7 +2,7 @@ import logging import textwrap from collections import ChainMap, defaultdict from io import StringIO -from typing import Union +from typing import Optional, Union import discord from async_rediscache import RedisCache @@ -15,7 +15,7 @@ from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles, STAF from bot.converters import MemberOrUser from bot.exts.recruitment.talentpool._review import Reviewer from bot.pagination import LinePaginator -from bot.utils import time +from bot.utils import scheduling, time from bot.utils.time import get_time_delta AUTOREVIEW_ENABLED_KEY = "autoreview_enabled" @@ -34,8 +34,11 @@ class TalentPool(Cog, name="Talentpool"): def __init__(self, bot: Bot) -> None: self.bot = bot self.reviewer = Reviewer(self.__class__.__name__, bot, self) + self.cache: Optional[defaultdict[dict]] = None self.api_default_params = {'active': 'true', 'ordering': '-inserted_at'} - self.bot.loop.create_task(self.schedule_autoreviews()) + + scheduling.create_task(self.refresh_cache(), event_loop=self.bot.loop) + scheduling.create_task(self.schedule_autoreviews(), event_loop=self.bot.loop) async def schedule_autoreviews(self) -> None: """Reschedule reviews for active nominations if autoreview is enabled.""" diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index 3ffbf93f3..f4aa73e75 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -57,8 +57,6 @@ class Reviewer: """Reschedule all active nominations to be reviewed at the appropriate time.""" log.trace("Rescheduling reviews") await self.bot.wait_until_guild_available() - # TODO Once the watch channel is removed, this can be done in a smarter way, e.g create a sync function. - await self._pool.refresh_cache() for user_id, user_data in self._pool.cache.items(): if not user_data["reviewed"]: -- cgit v1.2.3 From fddd34158b3fda284bd39970297c00e7d54d122a Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 6 Sep 2021 13:36:42 +0100 Subject: Refactor & simplifiy domain filter check --- bot/exts/filters/filtering.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index b7e91395e..7e698880f 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -478,17 +478,12 @@ class Filtering(Cog): Second return value is a reason of URL blacklisting (can be None). """ text = self.clean_input(text) - matches = URL_RE.findall(text) - if not matches: - return False, None domain_blacklist = self._get_filterlist_items("domain_name", allowed=False) - - for url in domain_blacklist: - for match in matches: - if url.lower() in match.lower(): + for match in URL_RE.finditer(text): + for url in domain_blacklist: + if url.lower() in match.group(1).lower(): return True, self._get_filterlist_value("domain_name", url, allowed=False)["comment"] - return False, None @staticmethod -- cgit v1.2.3 From cace28f844fe6ff022dfff7e2409d34fbc3d8bf4 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 6 Sep 2021 14:18:17 +0100 Subject: Add .gg/ to invite filter regex --- bot/utils/regex.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/utils/regex.py b/bot/utils/regex.py index a8efe1446..7bad1e627 100644 --- a/bot/utils/regex.py +++ b/bot/utils/regex.py @@ -6,7 +6,8 @@ INVITE_RE = re.compile( r"discordapp(?:[\.,]|dot)com(?:\/|slash)invite|" # or discordapp.com/invite/ r"discord(?:[\.,]|dot)me|" # or discord.me r"discord(?:[\.,]|dot)li|" # or discord.li - r"discord(?:[\.,]|dot)io" # or discord.io. + r"discord(?:[\.,]|dot)io|" # or discord.io. + r"(?:[\.,]|dot)gg" # or .gg/ r")(?:[\/]|slash)" # / or 'slash' r"([a-zA-Z0-9\-]+)", # the invite code itself flags=re.IGNORECASE -- cgit v1.2.3 From 160f0ecb125fe317abc644100e58ab9557ea7faa Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Wed, 8 Sep 2021 09:48:22 +0100 Subject: Add support for `!infraction last` (#1804) * Add support for `!infraction last` `!infraction last` will view details of the last infraction the user made. Also changes the `Infraction` converter to use the `expanded` endpoint 1) Added `InvalidInfraction` to `bot.errors` - called when value passed to `Infraction` converter is invalid 2) Improved error messages for when an invalid infraction is passed to `!infraction {infr_id}` 3) Improved some other messages/docstrings etc. Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com> --- bot/converters.py | 14 +++++++++-- bot/errors.py | 19 ++++++++++++++- bot/exts/moderation/infraction/management.py | 36 +++++++++++++++------------- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index bd4044c7e..18bb6e4e5 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -17,6 +17,7 @@ from discord.utils import DISCORD_EPOCH, escape_markdown, snowflake_time from bot import exts from bot.api import ResponseCodeError from bot.constants import URLs +from bot.errors import InvalidInfraction from bot.exts.info.doc import _inventory_parser from bot.utils.extensions import EXTENSIONS, unqualify from bot.utils.regex import INVITE_RE @@ -558,7 +559,7 @@ class Infraction(Converter): "ordering": "-inserted_at" } - infractions = await ctx.bot.api_client.get("bot/infractions", params=params) + infractions = await ctx.bot.api_client.get("bot/infractions/expanded", params=params) if not infractions: raise BadArgument( @@ -568,7 +569,16 @@ class Infraction(Converter): return infractions[0] else: - return await ctx.bot.api_client.get(f"bot/infractions/{arg}") + try: + return await ctx.bot.api_client.get(f"bot/infractions/{arg}/expanded") + except ResponseCodeError as e: + if e.status == 404: + raise InvalidInfraction( + converter=Infraction, + original=e, + infraction_arg=arg + ) + raise e if t.TYPE_CHECKING: diff --git a/bot/errors.py b/bot/errors.py index 2633390a8..078b645f1 100644 --- a/bot/errors.py +++ b/bot/errors.py @@ -1,6 +1,9 @@ from __future__ import annotations -from typing import Hashable, TYPE_CHECKING +from typing import Hashable, TYPE_CHECKING, Union + +from discord.ext.commands import ConversionError, Converter + if TYPE_CHECKING: from bot.converters import MemberOrUser @@ -40,6 +43,20 @@ class InvalidInfractedUserError(Exception): super().__init__(reason) +class InvalidInfraction(ConversionError): + """ + Raised by the Infraction converter when trying to fetch an invalid infraction id. + + Attributes: + `infraction_arg` -- the value that we attempted to convert into an Infraction + """ + + def __init__(self, converter: Converter, original: Exception, infraction_arg: Union[int, str]): + + self.infraction_arg = infraction_arg + super().__init__(converter, original) + + class BrandingMisconfiguration(RuntimeError): """Raised by the Branding cog when a misconfigured event is encountered.""" diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index 223a124d8..d72cf8f89 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -11,9 +11,9 @@ from discord.ext.commands import Context from discord.utils import escape_markdown from bot import constants -from bot.api import ResponseCodeError from bot.bot import Bot from bot.converters import Expiry, Infraction, MemberOrUser, Snowflake, UnambiguousUser, allowed_strings +from bot.errors import InvalidInfraction from bot.exts.moderation.infraction.infractions import Infractions from bot.exts.moderation.modlog import ModLog from bot.pagination import LinePaginator @@ -45,25 +45,22 @@ class ModManagement(commands.Cog): # region: Edit infraction commands @commands.group(name='infraction', aliases=('infr', 'infractions', 'inf', 'i'), invoke_without_command=True) - async def infraction_group(self, ctx: Context, infr_id: int = None) -> None: - """Infraction manipulation commands. If `infr_id` is passed then this command fetches that infraction.""" - if infr_id is None: + async def infraction_group(self, ctx: Context, infraction: Infraction = None) -> None: + """ + Infraction manipulation commands. + + If `infraction` is passed then this command fetches that infraction. The `Infraction` converter + supports 'l', 'last' and 'recent' to get the most recent infraction made by `ctx.author`. + """ + if infraction is None: await ctx.send_help(ctx.command) return - try: - infraction_list = [await self.bot.api_client.get(f"bot/infractions/{infr_id}/expanded")] - except ResponseCodeError as e: - if e.status == 404: - await ctx.send(f":x: No infraction with ID `{infr_id}` could be found.") - return - raise e - embed = discord.Embed( - title=f"Infraction #{infr_id}", + title=f"Infraction #{infraction['id']}", colour=discord.Colour.orange() ) - await self.send_infraction_list(ctx, embed, infraction_list) + await self.send_infraction_list(ctx, embed, [infraction]) @infraction_group.command(name="append", aliases=("amend", "add", "a")) async def infraction_append( @@ -348,13 +345,20 @@ class ModManagement(commands.Cog): return all(checks) # This cannot be static (must have a __func__ attribute). - async def cog_command_error(self, ctx: Context, error: Exception) -> None: - """Send a notification to the invoking context on a Union failure.""" + async def cog_command_error(self, ctx: Context, error: commands.CommandError) -> None: + """Handles errors for commands within this cog.""" if isinstance(error, commands.BadUnionArgument): if discord.User in error.converters: await ctx.send(str(error.errors[0])) error.handled = True + elif isinstance(error, InvalidInfraction): + if error.infraction_arg.isdigit(): + await ctx.send(f":x: Could not find an infraction with id `{error.infraction_arg}`.") + else: + await ctx.send(f":x: `{error.infraction_arg}` is not a valid integer infraction id.") + error.handled = True + def setup(bot: Bot) -> None: """Load the ModManagement cog.""" -- cgit v1.2.3 From b412451d38a419c2b66815729cbc724e6fea0ab6 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 6 Sep 2021 13:08:04 +0100 Subject: Wait until login before trying to use the bot api client --- bot/exts/recruitment/talentpool/_cog.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index bea5ff72c..01a2f20e2 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -37,12 +37,14 @@ class TalentPool(Cog, name="Talentpool"): self.cache: Optional[defaultdict[dict]] = None self.api_default_params = {'active': 'true', 'ordering': '-inserted_at'} - scheduling.create_task(self.refresh_cache(), event_loop=self.bot.loop) + self.initial_refresh_task = scheduling.create_task(self.refresh_cache(), event_loop=self.bot.loop) scheduling.create_task(self.schedule_autoreviews(), event_loop=self.bot.loop) async def schedule_autoreviews(self) -> None: """Reschedule reviews for active nominations if autoreview is enabled.""" if await self.autoreview_enabled(): + # Wait for a populated cache first + await self.initial_refresh_task await self.reviewer.reschedule_reviews() else: log.trace("Not scheduling reviews as autoreview is disabled.") @@ -53,6 +55,8 @@ class TalentPool(Cog, name="Talentpool"): async def refresh_cache(self) -> bool: """Updates TalentPool users cache.""" + # Wait until logged in to ensure bot api client exists + await self.bot.wait_until_guild_available() try: data = await self.bot.api_client.get( 'bot/nominations', -- cgit v1.2.3 From 2a2b0cd34807b9a0d5292129c81d08b64ce476be Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 7 Sep 2021 12:30:46 +0100 Subject: Remove previous nominations output This raised questions from helpers when they saw that someone had many previous nominations. There is no reason why a helper needs to see this information. --- bot/exts/recruitment/talentpool/_cog.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 01a2f20e2..aaafff973 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -291,18 +291,7 @@ class TalentPool(Cog, name="Talentpool"): if await self.autoreview_enabled() and user.id not in self.reviewer: self.reviewer.schedule_review(user.id) - history = await self.bot.api_client.get( - 'bot/nominations', - params={ - "user__id": str(user.id), - "active": "false", - "ordering": "-inserted_at" - } - ) - msg = f"✅ The nomination for {user.mention} has been added to the talent pool" - if history: - msg += f"\n\n({len(history)} previous nominations in total)" await ctx.send(msg) -- cgit v1.2.3 From f699c56d8321004adee51df00b5105a4d8b1d3b7 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Wed, 8 Sep 2021 15:20:54 +0100 Subject: Use - rather than 0 for number of nominees when cache isn't ready --- bot/exts/info/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index d44886969..be67910a6 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -72,7 +72,7 @@ class Information(Cog): """Return additional server info only visible in moderation channels.""" talentpool_info = "" if cog := self.bot.get_cog("Talentpool"): - num_nominated = len(cog.cache) if cog.cache else 0 + num_nominated = len(cog.cache) if cog.cache else "-" talentpool_info = f"Nominated: {num_nominated}\n" bb_info = "" -- cgit v1.2.3 From 108fb5d3dfc76c78d51126660ce891f9d1ee10eb Mon Sep 17 00:00:00 2001 From: Janine vN Date: Thu, 9 Sep 2021 10:31:48 -0400 Subject: Add string-formatting tag Adds a tag to show the string formatting mini language --- bot/resources/tags/string-formatting.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 bot/resources/tags/string-formatting.md diff --git a/bot/resources/tags/string-formatting.md b/bot/resources/tags/string-formatting.md new file mode 100644 index 000000000..fba5577b8 --- /dev/null +++ b/bot/resources/tags/string-formatting.md @@ -0,0 +1,27 @@ +**String Formatting Mini Language** +The String Formatting Language in Python is a powerful way to tailor the display of strings and other data structures. This string formatting mini language works for f-strings and .format(). + +Take a look at some of these examples! +```py +my_num = 2134234523 +print(f"{my_num:,}") + +my_smaller_num = -30.0532234 +print(f"{my_smaller_num:=09.2f}") + +my_str = "Center me!" +print(f"{my_str:^20}") + +repr_str = "Spam \t Ham" +print(f"{repr_str!r}") +``` +Results: +``` +2,134,234,523 +-00030.05 + Center me! +'Spam \t Ham' +``` +**Full Specification & Resources** +[String Formatting Mini Language Specification](https://docs.python.org/3/library/string.html#format-specification-mini-language) +[pyformat.info](https://pyformat.info/) \ No newline at end of file -- cgit v1.2.3 From 693a91ea12b6f856da6c3a2ad6e17bcf12779d53 Mon Sep 17 00:00:00 2001 From: Janine vN Date: Thu, 9 Sep 2021 10:52:14 -0400 Subject: Add required newline to end of file --- bot/resources/tags/string-formatting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/resources/tags/string-formatting.md b/bot/resources/tags/string-formatting.md index fba5577b8..272b96af3 100644 --- a/bot/resources/tags/string-formatting.md +++ b/bot/resources/tags/string-formatting.md @@ -24,4 +24,4 @@ Results: ``` **Full Specification & Resources** [String Formatting Mini Language Specification](https://docs.python.org/3/library/string.html#format-specification-mini-language) -[pyformat.info](https://pyformat.info/) \ No newline at end of file +[pyformat.info](https://pyformat.info/) -- cgit v1.2.3 From 3d6ba16691512292c239e76c0a9ae373b3982bc2 Mon Sep 17 00:00:00 2001 From: Janine vN Date: Thu, 9 Sep 2021 12:28:13 -0400 Subject: Change formatting of examples After some discussion back and forth, I've adjusted how to display the examples and code bock to be of a more REPL-style. Additionally, a filler character for the "Center Me!" string is added to illustrate how exactly in centers it. This commit also adds some small styling changes. --- bot/resources/tags/string-formatting.md | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/bot/resources/tags/string-formatting.md b/bot/resources/tags/string-formatting.md index 272b96af3..707d19c90 100644 --- a/bot/resources/tags/string-formatting.md +++ b/bot/resources/tags/string-formatting.md @@ -1,25 +1,22 @@ -**String Formatting Mini Language** -The String Formatting Language in Python is a powerful way to tailor the display of strings and other data structures. This string formatting mini language works for f-strings and .format(). +**String Formatting Mini-Language** +The String Formatting Language in Python is a powerful way to tailor the display of strings and other data structures. This string formatting mini language works for f-strings and `.format()`. Take a look at some of these examples! ```py -my_num = 2134234523 -print(f"{my_num:,}") +>>> my_num = 2134234523 +>>> print(f"{my_num:,}") +2,134,234,523 -my_smaller_num = -30.0532234 -print(f"{my_smaller_num:=09.2f}") +>>> my_smaller_num = -30.0532234 +>>> print(f"{my_smaller_num:=09.2f}") +-00030.05 -my_str = "Center me!" -print(f"{my_str:^20}") +>>> my_str = "Center me!" +>>> print(f"{my_str:-^20}") +-----Center me!----- -repr_str = "Spam \t Ham" -print(f"{repr_str!r}") -``` -Results: -``` -2,134,234,523 --00030.05 - Center me! +>>> repr_str = "Spam \t Ham" +>>> print(f"{repr_str!r}") 'Spam \t Ham' ``` **Full Specification & Resources** -- cgit v1.2.3 From 13e53790515da14703c204908513be33ab28539c Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sat, 11 Sep 2021 15:09:33 +0200 Subject: Raise InvalidHeaderError if inventory has invalid header If the requested inventory file has an invalid header, the resource has most likely moved or the url was invalid. In case of adding a new inventory, this error can be handled and displayed to the user. For automatic fetching it'll stop the rescheduling as the request succeeded but returned invalid content. --- bot/exts/info/doc/_cog.py | 9 +++++++-- bot/exts/info/doc/_inventory_parser.py | 22 +++++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/bot/exts/info/doc/_cog.py b/bot/exts/info/doc/_cog.py index a2119a53d..ce3f4c95b 100644 --- a/bot/exts/info/doc/_cog.py +++ b/bot/exts/info/doc/_cog.py @@ -21,7 +21,7 @@ from bot.utils.lock import SharedEvent, lock from bot.utils.messages import send_denial, wait_for_deletion from bot.utils.scheduling import Scheduler from . import NAMESPACE, PRIORITY_PACKAGES, _batch_parser, doc_cache -from ._inventory_parser import InventoryDict, fetch_inventory +from ._inventory_parser import InvalidHeaderError, InventoryDict, fetch_inventory log = logging.getLogger(__name__) @@ -135,7 +135,12 @@ class DocCog(commands.Cog): The first attempt is rescheduled to execute in `FETCH_RESCHEDULE_DELAY.first` minutes, the subsequent attempts in `FETCH_RESCHEDULE_DELAY.repeated` minutes. """ - package = await fetch_inventory(inventory_url) + try: + package = await fetch_inventory(inventory_url) + except InvalidHeaderError as e: + # Do not reschedule if the header is invalid, as the request went through but the contents are invalid. + log.warning(f"Invalid inventory header at {inventory_url}. Reason: {e}") + return if not package: if api_package_name in self.inventory_scheduler: diff --git a/bot/exts/info/doc/_inventory_parser.py b/bot/exts/info/doc/_inventory_parser.py index 80d5841a0..61924d070 100644 --- a/bot/exts/info/doc/_inventory_parser.py +++ b/bot/exts/info/doc/_inventory_parser.py @@ -16,6 +16,10 @@ _V2_LINE_RE = re.compile(r'(?x)(.+?)\s+(\S*:\S*)\s+(-?\d+)\s+?(\S*)\s+(.*)') InventoryDict = DefaultDict[str, List[Tuple[str, str]]] +class InvalidHeaderError(Exception): + """Raised when an inventory file has an invalid header.""" + + class ZlibStreamReader: """Class used for decoding zlib data of a stream line by line.""" @@ -80,19 +84,25 @@ async def _fetch_inventory(url: str) -> InventoryDict: stream = response.content inventory_header = (await stream.readline()).decode().rstrip() - inventory_version = int(inventory_header[-1:]) - await stream.readline() # skip project name - await stream.readline() # skip project version + try: + inventory_version = int(inventory_header[-1:]) + except ValueError: + raise InvalidHeaderError("Unable to convert inventory version header.") + + has_project_header = (await stream.readline()).startswith(b"# Project") + has_version_header = (await stream.readline()).startswith(b"# Version") + if not (has_project_header and has_version_header): + raise InvalidHeaderError("Inventory missing project or version header.") if inventory_version == 1: return await _load_v1(stream) elif inventory_version == 2: if b"zlib" not in await stream.readline(): - raise ValueError(f"Invalid inventory file at url {url}.") + raise InvalidHeaderError("'zlib' not found in header of compressed inventory.") return await _load_v2(stream) - raise ValueError(f"Invalid inventory file at url {url}.") + raise InvalidHeaderError("Incompatible inventory version.") async def fetch_inventory(url: str) -> Optional[InventoryDict]: @@ -115,6 +125,8 @@ async def fetch_inventory(url: str) -> Optional[InventoryDict]: f"Failed to get inventory from {url}; " f"trying again ({attempt}/{FAILED_REQUEST_ATTEMPTS})." ) + except InvalidHeaderError: + raise except Exception: log.exception( f"An unexpected error has occurred during fetching of {url}; " -- cgit v1.2.3 From ba33608246a4262463f90dd04c91b5fb1acb99c2 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sat, 11 Sep 2021 15:34:05 +0200 Subject: Display BadArgument to user when invalid header is raised in converter --- bot/converters.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 18bb6e4e5..d7946b86c 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -235,11 +235,16 @@ class Inventory(Converter): async def convert(ctx: Context, url: str) -> t.Tuple[str, _inventory_parser.InventoryDict]: """Convert url to Intersphinx inventory URL.""" await ctx.trigger_typing() - if (inventory := await _inventory_parser.fetch_inventory(url)) is None: - raise BadArgument( - f"Failed to fetch inventory file after {_inventory_parser.FAILED_REQUEST_ATTEMPTS} attempts." - ) - return url, inventory + try: + inventory = await _inventory_parser.fetch_inventory(url) + except _inventory_parser.InvalidHeaderError: + raise BadArgument("Unable to parse inventory because of invalid header, check if URL is correct.") + else: + if inventory is None: + raise BadArgument( + f"Failed to fetch inventory file after {_inventory_parser.FAILED_REQUEST_ATTEMPTS} attempts." + ) + return url, inventory class Snowflake(IDConverter): -- cgit v1.2.3 From 6fb6967783bff17f2b248098f803bb21b1f770d9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sat, 11 Sep 2021 19:06:13 +0200 Subject: Use scheduling create_task util instead of creating from loop directly The util attaches an error logging callback instead of relying on python's exception logging which only occurs when the task is destroyed --- bot/async_stats.py | 4 +++- bot/exts/backend/branding/_cog.py | 3 ++- bot/exts/backend/config_verifier.py | 4 ++-- bot/exts/backend/logging.py | 4 ++-- bot/exts/backend/sync/_cog.py | 3 ++- bot/exts/filters/antispam.py | 6 +++++- bot/exts/filters/filter_lists.py | 3 ++- bot/exts/filters/filtering.py | 6 +++--- bot/exts/fun/duck_pond.py | 3 ++- bot/exts/fun/off_topic_names.py | 5 +++-- bot/exts/help_channels/_cog.py | 2 +- bot/exts/info/codeblock/_cog.py | 4 ++-- bot/exts/info/doc/_batch_parser.py | 5 +++-- bot/exts/info/doc/_cog.py | 6 ++++-- bot/exts/info/pep.py | 3 ++- bot/exts/info/python_news.py | 5 +++-- bot/exts/moderation/defcon.py | 3 ++- bot/exts/moderation/incidents.py | 5 +++-- bot/exts/moderation/infraction/_scheduler.py | 2 +- bot/exts/moderation/metabase.py | 4 ++-- bot/exts/moderation/modpings.py | 7 ++++++- bot/exts/moderation/silence.py | 3 ++- bot/exts/moderation/stream.py | 6 +++--- bot/exts/moderation/watchchannels/_watchchannel.py | 11 +++++++---- bot/exts/utils/reminders.py | 3 ++- bot/exts/utils/snekbox.py | 4 ++-- tests/bot/exts/backend/sync/test_cog.py | 7 ++++--- tests/helpers.py | 5 ++++- 28 files changed, 79 insertions(+), 47 deletions(-) diff --git a/bot/async_stats.py b/bot/async_stats.py index 58a80f528..2af832e5b 100644 --- a/bot/async_stats.py +++ b/bot/async_stats.py @@ -3,6 +3,8 @@ import socket from statsd.client.base import StatsClientBase +from bot.utils import scheduling + class AsyncStatsClient(StatsClientBase): """An async transport method for statsd communication.""" @@ -32,7 +34,7 @@ class AsyncStatsClient(StatsClientBase): def _send(self, data: str) -> None: """Start an async task to send data to statsd.""" - self._loop.create_task(self._async_send(data)) + scheduling.create_task(self._async_send(data), event_loop=self._loop) async def _async_send(self, data: str) -> None: """Send data to the statsd server using the async transport.""" diff --git a/bot/exts/backend/branding/_cog.py b/bot/exts/backend/branding/_cog.py index 0ba146635..ab0a761ff 100644 --- a/bot/exts/backend/branding/_cog.py +++ b/bot/exts/backend/branding/_cog.py @@ -17,6 +17,7 @@ from bot.bot import Bot from bot.constants import Branding as BrandingConfig, Channels, Colours, Guild, MODERATION_ROLES from bot.decorators import mock_in_debug from bot.exts.backend.branding._repository import BrandingRepository, Event, RemoteObject +from bot.utils import scheduling log = logging.getLogger(__name__) @@ -126,7 +127,7 @@ class Branding(commands.Cog): self.bot = bot self.repository = BrandingRepository(bot) - self.bot.loop.create_task(self.maybe_start_daemon()) # Start depending on cache. + scheduling.create_task(self.maybe_start_daemon(), event_loop=self.bot.loop) # Start depending on cache. # region: Internal logic & state management diff --git a/bot/exts/backend/config_verifier.py b/bot/exts/backend/config_verifier.py index d72c6c22e..c24cb324f 100644 --- a/bot/exts/backend/config_verifier.py +++ b/bot/exts/backend/config_verifier.py @@ -4,7 +4,7 @@ from discord.ext.commands import Cog from bot import constants from bot.bot import Bot - +from bot.utils import scheduling log = logging.getLogger(__name__) @@ -14,7 +14,7 @@ class ConfigVerifier(Cog): def __init__(self, bot: Bot): self.bot = bot - self.channel_verify_task = self.bot.loop.create_task(self.verify_channels()) + self.channel_verify_task = scheduling.create_task(self.verify_channels(), event_loop=self.bot.loop) async def verify_channels(self) -> None: """ diff --git a/bot/exts/backend/logging.py b/bot/exts/backend/logging.py index 823f14ea4..8f1b8026f 100644 --- a/bot/exts/backend/logging.py +++ b/bot/exts/backend/logging.py @@ -5,7 +5,7 @@ from discord.ext.commands import Cog from bot.bot import Bot from bot.constants import Channels, DEBUG_MODE - +from bot.utils import scheduling log = logging.getLogger(__name__) @@ -16,7 +16,7 @@ class Logging(Cog): def __init__(self, bot: Bot): self.bot = bot - self.bot.loop.create_task(self.startup_greeting()) + scheduling.create_task(self.startup_greeting(), event_loop=self.bot.loop) async def startup_greeting(self) -> None: """Announce our presence to the configured devlog channel.""" diff --git a/bot/exts/backend/sync/_cog.py b/bot/exts/backend/sync/_cog.py index 48d2b6f02..f88dcf538 100644 --- a/bot/exts/backend/sync/_cog.py +++ b/bot/exts/backend/sync/_cog.py @@ -9,6 +9,7 @@ from bot import constants from bot.api import ResponseCodeError from bot.bot import Bot from bot.exts.backend.sync import _syncers +from bot.utils import scheduling log = logging.getLogger(__name__) @@ -18,7 +19,7 @@ class Sync(Cog): def __init__(self, bot: Bot) -> None: self.bot = bot - self.bot.loop.create_task(self.sync_guild()) + scheduling.create_task(self.sync_guild(), event_loop=self.bot.loop) async def sync_guild(self) -> None: """Syncs the roles/users of the guild with the database.""" diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 8c075fa95..6808bfa03 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -129,7 +129,11 @@ class AntiSpam(Cog): self.max_interval = max_interval_config['interval'] self.cache = MessageCache(AntiSpamConfig.cache_size, newest_first=True) - self.bot.loop.create_task(self.alert_on_validation_error(), name="AntiSpam.alert_on_validation_error") + scheduling.create_task( + self.alert_on_validation_error(), + name="AntiSpam.alert_on_validation_error", + event_loop=self.bot.loop, + ) @property def mod_log(self) -> ModLog: diff --git a/bot/exts/filters/filter_lists.py b/bot/exts/filters/filter_lists.py index 232c1e48b..a06437f3d 100644 --- a/bot/exts/filters/filter_lists.py +++ b/bot/exts/filters/filter_lists.py @@ -9,6 +9,7 @@ from bot.api import ResponseCodeError from bot.bot import Bot from bot.converters import ValidDiscordServerInvite, ValidFilterListType from bot.pagination import LinePaginator +from bot.utils import scheduling log = logging.getLogger(__name__) @@ -27,7 +28,7 @@ class FilterLists(Cog): def __init__(self, bot: Bot) -> None: self.bot = bot - self.bot.loop.create_task(self._amend_docstrings()) + scheduling.create_task(self._amend_docstrings(), event_loop=self.bot.loop) async def _amend_docstrings(self) -> None: """Add the valid FilterList types to the docstrings, so they'll appear in !help invocations.""" diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index 7e698880f..64f3b82af 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -21,9 +21,9 @@ from bot.constants import ( ) from bot.exts.events.code_jams._channels import CATEGORY_NAME as JAM_CATEGORY_NAME from bot.exts.moderation.modlog import ModLog +from bot.utils import scheduling from bot.utils.messages import format_user from bot.utils.regex import INVITE_RE -from bot.utils.scheduling import Scheduler log = logging.getLogger(__name__) @@ -64,7 +64,7 @@ class Filtering(Cog): def __init__(self, bot: Bot): self.bot = bot - self.scheduler = Scheduler(self.__class__.__name__) + self.scheduler = scheduling.Scheduler(self.__class__.__name__) self.name_lock = asyncio.Lock() staff_mistake_str = "If you believe this was a mistake, please let staff know!" @@ -133,7 +133,7 @@ class Filtering(Cog): }, } - self.bot.loop.create_task(self.reschedule_offensive_msg_deletion()) + scheduling.create_task(self.reschedule_offensive_msg_deletion(), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel scheduled tasks.""" diff --git a/bot/exts/fun/duck_pond.py b/bot/exts/fun/duck_pond.py index 7f7e4585c..8ced6922c 100644 --- a/bot/exts/fun/duck_pond.py +++ b/bot/exts/fun/duck_pond.py @@ -9,6 +9,7 @@ from discord.ext.commands import Cog, Context, command from bot import constants from bot.bot import Bot from bot.converters import MemberOrUser +from bot.utils import scheduling from bot.utils.checks import has_any_role from bot.utils.messages import count_unique_users_reaction, send_attachments from bot.utils.webhooks import send_webhook @@ -24,7 +25,7 @@ class DuckPond(Cog): self.webhook_id = constants.Webhooks.duck_pond self.webhook = None self.ducked_messages = [] - self.bot.loop.create_task(self.fetch_webhook()) + scheduling.create_task(self.fetch_webhook(), event_loop=self.bot.loop) self.relay_lock = None async def fetch_webhook(self) -> None: diff --git a/bot/exts/fun/off_topic_names.py b/bot/exts/fun/off_topic_names.py index 845b8175c..2f56aa5ba 100644 --- a/bot/exts/fun/off_topic_names.py +++ b/bot/exts/fun/off_topic_names.py @@ -11,6 +11,7 @@ from bot.bot import Bot from bot.constants import Channels, MODERATION_ROLES from bot.converters import OffTopicName from bot.pagination import LinePaginator +from bot.utils import scheduling CHANNELS = (Channels.off_topic_0, Channels.off_topic_1, Channels.off_topic_2) log = logging.getLogger(__name__) @@ -50,7 +51,7 @@ class OffTopicNames(Cog): self.bot = bot self.updater_task = None - self.bot.loop.create_task(self.init_offtopic_updater()) + scheduling.create_task(self.init_offtopic_updater(), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel any running updater tasks on cog unload.""" @@ -62,7 +63,7 @@ class OffTopicNames(Cog): await self.bot.wait_until_guild_available() if self.updater_task is None: coro = update_names(self.bot) - self.updater_task = self.bot.loop.create_task(coro) + self.updater_task = scheduling.create_task(coro, event_loop=self.bot.loop) @group(name='otname', aliases=('otnames', 'otn'), invoke_without_command=True) @has_any_role(*MODERATION_ROLES) diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py index cfc9cf477..40fb9429c 100644 --- a/bot/exts/help_channels/_cog.py +++ b/bot/exts/help_channels/_cog.py @@ -82,7 +82,7 @@ class HelpChannels(commands.Cog): # Asyncio stuff self.queue_tasks: t.List[asyncio.Task] = [] - self.init_task = self.bot.loop.create_task(self.init_cog()) + self.init_task = scheduling.create_task(self.init_cog(), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel the init task and scheduled tasks when the cog unloads.""" diff --git a/bot/exts/info/codeblock/_cog.py b/bot/exts/info/codeblock/_cog.py index 9a0705d2b..f63a459ff 100644 --- a/bot/exts/info/codeblock/_cog.py +++ b/bot/exts/info/codeblock/_cog.py @@ -11,7 +11,7 @@ from bot.bot import Bot from bot.exts.filters.token_remover import TokenRemover from bot.exts.filters.webhook_remover import WEBHOOK_URL_RE from bot.exts.info.codeblock._instructions import get_instructions -from bot.utils import has_lines +from bot.utils import has_lines, scheduling from bot.utils.channel import is_help_channel from bot.utils.messages import wait_for_deletion @@ -114,7 +114,7 @@ class CodeBlockCog(Cog, name="Code Block"): bot_message = await message.channel.send(f"Hey {message.author.mention}!", embed=embed) self.codeblock_message_ids[message.id] = bot_message.id - self.bot.loop.create_task(wait_for_deletion(bot_message, (message.author.id,))) + scheduling.create_task(wait_for_deletion(bot_message, (message.author.id,)), event_loop=self.bot.loop) # Increase amount of codeblock correction in stats self.bot.stats.incr("codeblock_corrections") diff --git a/bot/exts/info/doc/_batch_parser.py b/bot/exts/info/doc/_batch_parser.py index 369bb462c..51ee29b68 100644 --- a/bot/exts/info/doc/_batch_parser.py +++ b/bot/exts/info/doc/_batch_parser.py @@ -24,9 +24,10 @@ class StaleInventoryNotifier: """Handle sending notifications about stale inventories through `DocItem`s to dev log.""" def __init__(self): - self._init_task = bot.instance.loop.create_task( + self._init_task = scheduling.create_task( self._init_channel(), - name="StaleInventoryNotifier channel init" + name="StaleInventoryNotifier channel init", + event_loop=bot.instance.loop, ) self._warned_urls = set() diff --git a/bot/exts/info/doc/_cog.py b/bot/exts/info/doc/_cog.py index a2119a53d..6cc1723cd 100644 --- a/bot/exts/info/doc/_cog.py +++ b/bot/exts/info/doc/_cog.py @@ -17,6 +17,7 @@ from bot.bot import Bot from bot.constants import MODERATION_ROLES, RedirectOutput from bot.converters import Inventory, PackageName, ValidURL, allowed_strings from bot.pagination import LinePaginator +from bot.utils import scheduling from bot.utils.lock import SharedEvent, lock from bot.utils.messages import send_denial, wait_for_deletion from bot.utils.scheduling import Scheduler @@ -75,9 +76,10 @@ class DocCog(commands.Cog): self.refresh_event.set() self.symbol_get_event = SharedEvent() - self.init_refresh_task = self.bot.loop.create_task( + self.init_refresh_task = scheduling.create_task( self.init_refresh_inventory(), - name="Doc inventory init" + name="Doc inventory init", + event_loop=self.bot.loop, ) @lock(NAMESPACE, COMMAND_LOCK_SINGLETON, raise_error=True) diff --git a/bot/exts/info/pep.py b/bot/exts/info/pep.py index b11b34db0..bbd112911 100644 --- a/bot/exts/info/pep.py +++ b/bot/exts/info/pep.py @@ -9,6 +9,7 @@ from discord.ext.commands import Cog, Context, command from bot.bot import Bot from bot.constants import Keys +from bot.utils import scheduling from bot.utils.caching import AsyncCache log = logging.getLogger(__name__) @@ -32,7 +33,7 @@ class PythonEnhancementProposals(Cog): self.peps: Dict[int, str] = {} # To avoid situations where we don't have last datetime, set this to now. self.last_refreshed_peps: datetime = datetime.now() - self.bot.loop.create_task(self.refresh_peps_urls()) + scheduling.create_task(self.refresh_peps_urls(), event_loop=self.bot.loop) async def refresh_peps_urls(self) -> None: """Refresh PEP URLs listing in every 3 hours.""" diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 63eb4ac17..58dcd3a02 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -11,6 +11,7 @@ from discord.ext.tasks import loop from bot import constants from bot.bot import Bot +from bot.utils import scheduling from bot.utils.webhooks import send_webhook PEPS_RSS_URL = "https://www.python.org/dev/peps/peps.rss/" @@ -33,8 +34,8 @@ class PythonNews(Cog): self.webhook_names = {} self.webhook: t.Optional[discord.Webhook] = None - self.bot.loop.create_task(self.get_webhook_names()) - self.bot.loop.create_task(self.get_webhook_and_channel()) + scheduling.create_task(self.get_webhook_names(), event_loop=self.bot.loop) + scheduling.create_task(self.get_webhook_and_channel(), event_loop=self.bot.loop) async def start_tasks(self) -> None: """Start the tasks for fetching new PEPs and mailing list messages.""" diff --git a/bot/exts/moderation/defcon.py b/bot/exts/moderation/defcon.py index 6ac077b93..053e8ae57 100644 --- a/bot/exts/moderation/defcon.py +++ b/bot/exts/moderation/defcon.py @@ -17,6 +17,7 @@ from bot.bot import Bot from bot.constants import Channels, Colours, Emojis, Event, Icons, MODERATION_ROLES, Roles from bot.converters import DurationDelta, Expiry from bot.exts.moderation.modlog import ModLog +from bot.utils import scheduling from bot.utils.messages import format_user from bot.utils.scheduling import Scheduler from bot.utils.time import ( @@ -69,7 +70,7 @@ class Defcon(Cog): self.scheduler = Scheduler(self.__class__.__name__) - self.bot.loop.create_task(self._sync_settings()) + scheduling.create_task(self._sync_settings(), event_loop=self.bot.loop) @property def mod_log(self) -> ModLog: diff --git a/bot/exts/moderation/incidents.py b/bot/exts/moderation/incidents.py index 561e0251e..a3d90e3fe 100644 --- a/bot/exts/moderation/incidents.py +++ b/bot/exts/moderation/incidents.py @@ -9,6 +9,7 @@ from discord.ext.commands import Cog from bot.bot import Bot from bot.constants import Channels, Colours, Emojis, Guild, Webhooks +from bot.utils import scheduling from bot.utils.messages import sub_clyde log = logging.getLogger(__name__) @@ -190,7 +191,7 @@ class Incidents(Cog): self.bot = bot self.event_lock = asyncio.Lock() - self.crawl_task = self.bot.loop.create_task(self.crawl_incidents()) + self.crawl_task = scheduling.create_task(self.crawl_incidents(), event_loop=self.bot.loop) async def crawl_incidents(self) -> None: """ @@ -275,7 +276,7 @@ class Incidents(Cog): return payload.message_id == incident.id coroutine = self.bot.wait_for(event="raw_message_delete", check=check, timeout=timeout) - return self.bot.loop.create_task(coroutine) + return scheduling.create_task(coroutine, event_loop=self.bot.loop) async def process_event(self, reaction: str, incident: discord.Message, member: discord.Member) -> None: """ diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 6ba4e74e9..8e844822d 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -29,7 +29,7 @@ class InfractionScheduler: self.bot = bot self.scheduler = scheduling.Scheduler(self.__class__.__name__) - self.bot.loop.create_task(self.reschedule_infractions(supported_infractions)) + scheduling.create_task(self.reschedule_infractions(supported_infractions), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel scheduled tasks.""" diff --git a/bot/exts/moderation/metabase.py b/bot/exts/moderation/metabase.py index 9eeeec074..6eadd4bad 100644 --- a/bot/exts/moderation/metabase.py +++ b/bot/exts/moderation/metabase.py @@ -14,7 +14,7 @@ from discord.ext.commands import Cog, Context, group, has_any_role from bot.bot import Bot from bot.constants import Metabase as MetabaseConfig, Roles from bot.converters import allowed_strings -from bot.utils import send_to_paste_service +from bot.utils import scheduling, send_to_paste_service from bot.utils.channel import is_mod_channel from bot.utils.scheduling import Scheduler @@ -40,7 +40,7 @@ class Metabase(Cog): self.exports: Dict[int, List[Dict]] = {} # Saves the output of each question, so internal eval can access it - self.init_task = self.bot.loop.create_task(self.init_cog()) + self.init_task = scheduling.create_task(self.init_cog(), event_loop=self.bot.loop) async def cog_command_error(self, ctx: Context, error: Exception) -> None: """Handle ClientResponseError errors locally to invalidate token if needed.""" diff --git a/bot/exts/moderation/modpings.py b/bot/exts/moderation/modpings.py index 80c9f0c38..d775cdedf 100644 --- a/bot/exts/moderation/modpings.py +++ b/bot/exts/moderation/modpings.py @@ -9,6 +9,7 @@ from discord.ext.commands import Cog, Context, group, has_any_role from bot.bot import Bot from bot.constants import Colours, Emojis, Guild, Icons, MODERATION_ROLES, Roles from bot.converters import Expiry +from bot.utils import scheduling from bot.utils.scheduling import Scheduler log = logging.getLogger(__name__) @@ -29,7 +30,11 @@ class ModPings(Cog): self.guild = None self.moderators_role = None - self.reschedule_task = self.bot.loop.create_task(self.reschedule_roles(), name="mod-pings-reschedule") + self.reschedule_task = scheduling.create_task( + self.reschedule_roles(), + name="mod-pings-reschedule", + event_loop=self.bot.loop, + ) async def reschedule_roles(self) -> None: """Reschedule moderators role re-apply times.""" diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 95e2792c3..2ee6496df 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -13,6 +13,7 @@ from discord.ext.commands import Context from bot import constants from bot.bot import Bot from bot.converters import HushDurationConverter +from bot.utils import scheduling from bot.utils.lock import LockedResourceError, lock, lock_arg from bot.utils.scheduling import Scheduler @@ -104,7 +105,7 @@ class Silence(commands.Cog): self.bot = bot self.scheduler = Scheduler(self.__class__.__name__) - self._init_task = self.bot.loop.create_task(self._async_init()) + self._init_task = scheduling.create_task(self._async_init(), event_loop=self.bot.loop) async def _async_init(self) -> None: """Set instance attributes once the guild is available and reschedule unsilences.""" diff --git a/bot/exts/moderation/stream.py b/bot/exts/moderation/stream.py index 01d2614b0..b5bd62a71 100644 --- a/bot/exts/moderation/stream.py +++ b/bot/exts/moderation/stream.py @@ -15,7 +15,7 @@ from bot.constants import ( ) from bot.converters import Expiry from bot.pagination import LinePaginator -from bot.utils.scheduling import Scheduler +from bot.utils import scheduling from bot.utils.time import discord_timestamp, format_infraction_with_duration log = logging.getLogger(__name__) @@ -30,8 +30,8 @@ class Stream(commands.Cog): def __init__(self, bot: Bot): self.bot = bot - self.scheduler = Scheduler(self.__class__.__name__) - self.reload_task = self.bot.loop.create_task(self._reload_tasks_from_redis()) + self.scheduler = scheduling.Scheduler(self.__class__.__name__) + self.reload_task = scheduling.create_task(self._reload_tasks_from_redis(), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel all scheduled tasks.""" diff --git a/bot/exts/moderation/watchchannels/_watchchannel.py b/bot/exts/moderation/watchchannels/_watchchannel.py index 146426569..a42e1f518 100644 --- a/bot/exts/moderation/watchchannels/_watchchannel.py +++ b/bot/exts/moderation/watchchannels/_watchchannel.py @@ -18,7 +18,7 @@ from bot.exts.filters.token_remover import TokenRemover from bot.exts.filters.webhook_remover import WEBHOOK_URL_RE from bot.exts.moderation.modlog import ModLog from bot.pagination import LinePaginator -from bot.utils import CogABCMeta, messages +from bot.utils import CogABCMeta, messages, scheduling from bot.utils.time import get_time_delta log = logging.getLogger(__name__) @@ -69,7 +69,7 @@ class WatchChannel(metaclass=CogABCMeta): self.message_history = MessageHistory() self.disable_header = disable_header - self._start = self.bot.loop.create_task(self.start_watchchannel()) + self._start = scheduling.create_task(self.start_watchchannel(), event_loop=self.bot.loop) @property def modlog(self) -> ModLog: @@ -169,7 +169,7 @@ class WatchChannel(metaclass=CogABCMeta): """Queues up messages sent by watched users.""" if msg.author.id in self.watched_users: if not self.consuming_messages: - self._consume_task = self.bot.loop.create_task(self.consume_messages()) + self._consume_task = scheduling.create_task(self.consume_messages(), event_loop=self.bot.loop) self.log.trace(f"Received message: {msg.content} ({len(msg.attachments)} attachments)") self.message_queue[msg.author.id][msg.channel.id].append(msg) @@ -199,7 +199,10 @@ class WatchChannel(metaclass=CogABCMeta): if self.message_queue: self.log.trace("Channel queue not empty: Continuing consuming queues") - self._consume_task = self.bot.loop.create_task(self.consume_messages(delay_consumption=False)) + self._consume_task = scheduling.create_task( + self.consume_messages(delay_consumption=False), + event_loop=self.bot.loop, + ) else: self.log.trace("Done consuming messages.") diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 41b6cac5c..00eb930b5 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -17,6 +17,7 @@ from bot.constants import ( ) from bot.converters import Duration, UnambiguousUser from bot.pagination import LinePaginator +from bot.utils import scheduling from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial @@ -40,7 +41,7 @@ class Reminders(Cog): self.bot = bot self.scheduler = Scheduler(self.__class__.__name__) - self.bot.loop.create_task(self.reschedule_reminders()) + scheduling.create_task(self.reschedule_reminders(), event_loop=self.bot.loop) def cog_unload(self) -> None: """Cancel scheduled tasks.""" diff --git a/bot/exts/utils/snekbox.py b/bot/exts/utils/snekbox.py index b1f1ba6a8..5fb10a25b 100644 --- a/bot/exts/utils/snekbox.py +++ b/bot/exts/utils/snekbox.py @@ -14,7 +14,7 @@ from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot from bot.constants import Categories, Channels, Roles, URLs from bot.decorators import redirect_output -from bot.utils import send_to_paste_service +from bot.utils import scheduling, send_to_paste_service from bot.utils.messages import wait_for_deletion log = logging.getLogger(__name__) @@ -219,7 +219,7 @@ class Snekbox(Cog): response = await ctx.send("Attempt to circumvent filter detected. Moderator team has been alerted.") else: response = await ctx.send(msg) - self.bot.loop.create_task(wait_for_deletion(response, (ctx.author.id,))) + scheduling.create_task(wait_for_deletion(response, (ctx.author.id,)), event_loop=self.bot.loop) log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") return response diff --git a/tests/bot/exts/backend/sync/test_cog.py b/tests/bot/exts/backend/sync/test_cog.py index 22a07313e..fdd0ab74a 100644 --- a/tests/bot/exts/backend/sync/test_cog.py +++ b/tests/bot/exts/backend/sync/test_cog.py @@ -60,13 +60,13 @@ class SyncCogTestCase(unittest.IsolatedAsyncioTestCase): class SyncCogTests(SyncCogTestCase): """Tests for the Sync cog.""" + @mock.patch("bot.utils.scheduling.create_task") @mock.patch.object(Sync, "sync_guild", new_callable=mock.MagicMock) - def test_sync_cog_init(self, sync_guild): + def test_sync_cog_init(self, sync_guild, create_task): """Should instantiate syncers and run a sync for the guild.""" # Reset because a Sync cog was already instantiated in setUp. self.RoleSyncer.reset_mock() self.UserSyncer.reset_mock() - self.bot.loop.create_task = mock.MagicMock() mock_sync_guild_coro = mock.MagicMock() sync_guild.return_value = mock_sync_guild_coro @@ -74,7 +74,8 @@ class SyncCogTests(SyncCogTestCase): Sync(self.bot) sync_guild.assert_called_once_with() - self.bot.loop.create_task.assert_called_once_with(mock_sync_guild_coro) + create_task.assert_called_once() + self.assertEqual(create_task.call_args.args[0], mock_sync_guild_coro) async def test_sync_cog_sync_guild(self): """Roles and users should be synced only if a guild is successfully retrieved.""" diff --git a/tests/helpers.py b/tests/helpers.py index 3978076ed..47f06f292 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -278,7 +278,10 @@ def _get_mock_loop() -> unittest.mock.Mock: # Since calling `create_task` on our MockBot does not actually schedule the coroutine object # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object # to prevent "has not been awaited"-warnings. - loop.create_task.side_effect = lambda coroutine: coroutine.close() + def mock_create_task(coroutine, **kwargs): + coroutine.close() + return unittest.mock.Mock() + loop.create_task.side_effect = mock_create_task return loop -- cgit v1.2.3 From f6a02d8435048d00e1bda31085ea25557f08a228 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sat, 11 Sep 2021 19:11:12 +0200 Subject: Use create_task util instead of asyncio.create_task The util attaches an error logging callback instead of relying on python's exception logging which only occurs when the task is destroyed --- bot/decorators.py | 4 ++-- bot/exts/help_channels/_cog.py | 2 +- bot/exts/info/doc/_cog.py | 2 +- bot/exts/moderation/defcon.py | 3 +-- bot/exts/utils/reminders.py | 3 +-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/bot/decorators.py b/bot/decorators.py index f65ec4103..ee210be26 100644 --- a/bot/decorators.py +++ b/bot/decorators.py @@ -10,7 +10,7 @@ from discord.ext import commands from discord.ext.commands import Cog, Context from bot.constants import Channels, DEBUG_MODE, RedirectOutput -from bot.utils import function +from bot.utils import function, scheduling from bot.utils.checks import ContextCheckFailure, in_whitelist_check from bot.utils.function import command_wraps @@ -154,7 +154,7 @@ def redirect_output( if ping_user: await ctx.send(f"Here's the output of your command, {ctx.author.mention}") - asyncio.create_task(func(self, ctx, *args, **kwargs)) + scheduling.create_task(func(self, ctx, *args, **kwargs)) message = await old_channel.send( f"Hey, {ctx.author.mention}, you can find the output of your command here: " diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py index 40fb9429c..8612f9866 100644 --- a/bot/exts/help_channels/_cog.py +++ b/bot/exts/help_channels/_cog.py @@ -507,7 +507,7 @@ class HelpChannels(commands.Cog): """Wait for a dormant channel to become available in the queue and return it.""" log.trace("Waiting for a dormant channel.") - task = asyncio.create_task(self.channel_queue.get()) + task = scheduling.create_task(self.channel_queue.get()) self.queue_tasks.append(task) channel = await task diff --git a/bot/exts/info/doc/_cog.py b/bot/exts/info/doc/_cog.py index 6cc1723cd..1624c50f6 100644 --- a/bot/exts/info/doc/_cog.py +++ b/bot/exts/info/doc/_cog.py @@ -458,4 +458,4 @@ class DocCog(commands.Cog): """Clear scheduled inventories, queued symbols and cleanup task on cog unload.""" self.inventory_scheduler.cancel_all() self.init_refresh_task.cancel() - asyncio.create_task(self.item_fetcher.clear(), name="DocCog.item_fetcher unload clear") + scheduling.create_task(self.item_fetcher.clear(), name="DocCog.item_fetcher unload clear") diff --git a/bot/exts/moderation/defcon.py b/bot/exts/moderation/defcon.py index 053e8ae57..ac813d6ba 100644 --- a/bot/exts/moderation/defcon.py +++ b/bot/exts/moderation/defcon.py @@ -1,4 +1,3 @@ -import asyncio import logging import traceback from collections import namedtuple @@ -206,7 +205,7 @@ class Defcon(Cog): new_topic = f"{BASE_CHANNEL_TOPIC}\n(Threshold: {humanize_delta(self.threshold) if self.threshold else '-'})" self.mod_log.ignore(Event.guild_channel_update, Channels.defcon) - asyncio.create_task(self.channel.edit(topic=new_topic)) + scheduling.create_task(self.channel.edit(topic=new_topic)) @defcon_settings.atomic_transaction async def _update_threshold(self, author: User, threshold: relativedelta, expiry: Optional[Expiry] = None) -> None: diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 00eb930b5..1030357fd 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -1,4 +1,3 @@ -import asyncio import logging import random import textwrap @@ -81,7 +80,7 @@ class Reminders(Cog): f"Reminder {reminder['id']} invalid: " f"User {reminder['author']}={user}, Channel {reminder['channel_id']}={channel}." ) - asyncio.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}")) + scheduling.create_task(self.bot.api_client.delete(f"bot/reminders/{reminder['id']}")) return is_valid, user, channel -- cgit v1.2.3 From 71ce990ba28dca3edf713ac28eff81065155f793 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 11 Sep 2021 23:39:41 +0400 Subject: Adds Core Dev Voting To Changelog Blacklist Adds the channel used for voting on contributors to the message changelog blacklist. Signed-off-by: Hassan Abouelela --- config-default.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/config-default.yml b/config-default.yml index a18fdafa5..3405934e0 100644 --- a/config-default.yml +++ b/config-default.yml @@ -157,9 +157,10 @@ guild: reddit: &REDDIT_CHANNEL 458224812528238616 # Development - dev_contrib: &DEV_CONTRIB 635950537262759947 - dev_core: &DEV_CORE 411200599653351425 - dev_log: &DEV_LOG 622895325144940554 + dev_contrib: &DEV_CONTRIB 635950537262759947 + dev_core: &DEV_CORE 411200599653351425 + dev_voting: &DEV_CORE_VOTING 839162966519447552 + dev_log: &DEV_LOG 622895325144940554 # Discussion meta: 429409067623251969 @@ -251,6 +252,7 @@ guild: - *MESSAGE_LOG - *MOD_LOG - *STAFF_VOICE + - *DEV_CORE_VOTING reminder_whitelist: - *BOT_CMD -- cgit v1.2.3 From 3c1cc75371fa8be3e1de3affa7b5a1f54b55da6b Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Fri, 17 Sep 2021 13:34:15 +0100 Subject: Add metricity to docker-compose By adding metricity to the compose, we allow it to migrate itself, rather than needing the site to do it. Defaulting 'USE_METRICITY' to false means that it will run migrations, but not actually start the bot. This means we don't add another service that needs to run all the time, which could impact some contribs on lower powered hardware. --- .gitignore | 1 + docker-compose.yml | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f74a142f3..177345908 100644 --- a/.gitignore +++ b/.gitignore @@ -116,6 +116,7 @@ log.* # Custom user configuration config.yml docker-compose.override.yml +metricity-config.toml # xmlrunner unittest XML reports TEST-**.xml diff --git a/docker-compose.yml b/docker-compose.yml index 0f0355dac..b3ca6baa4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,6 +23,11 @@ services: POSTGRES_DB: pysite POSTGRES_PASSWORD: pysite POSTGRES_USER: pysite + healthcheck: + test: ["CMD-SHELL", "pg_isready -U pysite"] + interval: 2s + timeout: 1s + retries: 5 redis: << : *logging @@ -31,6 +36,21 @@ services: ports: - "127.0.0.1:6379:6379" + metricity: + << : *logging + restart: on-failure # USE_METRICITY=false will stop the container, so this ensures it only restarts on error + depends_on: + postgres: + condition: service_healthy + image: ghcr.io/python-discord/metricity:latest + env_file: + - .env + environment: + DATABASE_URI: postgres://pysite:pysite@postgres/metricity + USE_METRICITY: ${USE_METRICITY-false} + volumes: + - .:/tmp/bot:ro + snekbox: << : *logging << : *restart_policy @@ -56,7 +76,7 @@ services: - "127.0.0.1:8000:8000" tty: true depends_on: - - postgres + - metricity environment: DATABASE_URL: postgres://pysite:pysite@postgres:5432/pysite METRICITY_DB_URL: postgres://pysite:pysite@postgres:5432/metricity -- cgit v1.2.3 From fe248cc05a3c50beb064c43aef224c0204512df9 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Fri, 17 Sep 2021 13:35:38 +0100 Subject: Remove duplicate roles when defining allowed_mentions --- bot/bot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/bot.py b/bot/bot.py index 914da9c98..db3d651a3 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -109,7 +109,7 @@ class Bot(commands.Bot): def create(cls) -> "Bot": """Create and return an instance of a Bot.""" loop = asyncio.get_event_loop() - allowed_roles = [discord.Object(id_) for id_ in constants.MODERATION_ROLES] + allowed_roles = list({discord.Object(id_) for id_ in constants.MODERATION_ROLES}) intents = discord.Intents.all() intents.presences = False -- cgit v1.2.3 From 802fa0dd29f35d4cf02c7be88dcec01d9ab67729 Mon Sep 17 00:00:00 2001 From: Izan Date: Mon, 20 Sep 2021 18:55:49 +0100 Subject: Remove coveralls from lint-test --- .github/workflows/lint-test.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/lint-test.yml b/.github/workflows/lint-test.yml index 619544e1a..2f42f1895 100644 --- a/.github/workflows/lint-test.yml +++ b/.github/workflows/lint-test.yml @@ -121,13 +121,6 @@ jobs: - name: Run tests and generate coverage report run: pytest -n auto --cov --disable-warnings -q - # This step will publish the coverage reports coveralls.io and - # print a "job" link in the output of the GitHub Action - - name: Publish coverage report to coveralls.io - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: coveralls - # Prepare the Pull Request Payload artifact. If this fails, we # we fail silently using the `continue-on-error` option. It's # nice if this succeeds, but if it fails for any reason, it -- cgit v1.2.3 From eff7d7f2cf3f2cd91ed6654b97edd95db786fd47 Mon Sep 17 00:00:00 2001 From: Boris Muratov <8bee278@gmail.com> Date: Tue, 21 Sep 2021 23:39:52 +0300 Subject: Add `cog` as an alias to extensions command --- bot/exts/utils/extensions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/utils/extensions.py b/bot/exts/utils/extensions.py index f78664527..309126d0e 100644 --- a/bot/exts/utils/extensions.py +++ b/bot/exts/utils/extensions.py @@ -36,7 +36,7 @@ class Extensions(commands.Cog): def __init__(self, bot: Bot): self.bot = bot - @group(name="extensions", aliases=("ext", "exts", "c", "cogs"), invoke_without_command=True) + @group(name="extensions", aliases=("ext", "exts", "c", "cog", "cogs"), invoke_without_command=True) async def extensions_group(self, ctx: Context) -> None: """Load, unload, reload, and list loaded extensions.""" await ctx.send_help(ctx.command) -- cgit v1.2.3 From 18b5fd814377ba059a5ca2f8f17475a59d87fd8f Mon Sep 17 00:00:00 2001 From: Izan Date: Thu, 23 Sep 2021 10:29:04 +0100 Subject: Use full paste link --- bot/resources/tags/paste.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/resources/tags/paste.md b/bot/resources/tags/paste.md index 2ed51def7..8c3c2985d 100644 --- a/bot/resources/tags/paste.md +++ b/bot/resources/tags/paste.md @@ -1,6 +1,6 @@ **Pasting large amounts of code** If your code is too long to fit in a codeblock in discord, you can paste your code here: -https://paste.pydis.com/ +https://paste.pythondiscord.com/ After pasting your code, **save** it by clicking the floppy disk icon in the top right, or by typing `ctrl + S`. After doing that, the URL should **change**. Copy the URL and post it here so others can see it. -- cgit v1.2.3 From 650e739fbb66eb30502675cc3b8bd257e1ee825d Mon Sep 17 00:00:00 2001 From: Johannes Christ Date: Fri, 24 Sep 2021 19:53:58 +0200 Subject: Suppress NotFound for batch deletion. I will copy my comment from the code, as it explains why we want this: In the rare case where we found messages matching the spam filter across multiple channels, it is possible that a single channel will only contain a single message to delete. If that should be the case, discord.py will use the "delete single message" endpoint instead of the bulk delete endpoint, and the single message deletion endpoint will complain if you give it that does not exist. As this means that we have no other message to delete in this channel (and message deletes work per-channel), we can just log an exception and carry on with business. --- bot/exts/filters/antispam.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 8c075fa95..72103c9fb 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -250,7 +250,20 @@ class AntiSpam(Cog): for message in messages: channel_messages[message.channel].append(message) for channel, messages in channel_messages.items(): - await channel.delete_messages(messages) + try: + await channel.delete_messages(messages) + except NotFound: + # In the rare case where we found messages matching the + # spam filter across multiple channels, it is possible + # that a single channel will only contain a single message + # to delete. If that should be the case, discord.py will + # use the "delete single message" endpoint instead of the + # bulk delete endpoint, and the single message deletion + # endpoint will complain if you give it that does not exist. + # As this means that we have no other message to delete in + # this channel (and message deletes work per-channel), + # we can just log an exception and carry on with business. + log.info(f"Tried to delete message `{messages[0].id}`, but message could not be found.") # Otherwise, the bulk delete endpoint will throw up. # Delete the message directly instead. -- cgit v1.2.3 From c04832f3cc5867de285f9e5c97d0c0c5191ac66d Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 27 Sep 2021 21:34:03 +0100 Subject: Add LT and GT to supported otn chars --- bot/converters.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 18bb6e4e5..8b0890830 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -392,7 +392,8 @@ class Duration(DurationDelta): class OffTopicName(Converter): """A converter that ensures an added off-topic name is valid.""" - ALLOWED_CHARACTERS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ!?'`-" + ALLOWED_CHARACTERS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ!?'`-<>" + TRANSLATED_CHARACTERS = "𝖠𝖡𝖢𝖣𝖤𝖥𝖦𝖧𝖨𝖩𝖪𝖫𝖬𝖭𝖮𝖯𝖰𝖱𝖲𝖳𝖴𝖵𝖶𝖷𝖸𝖹ǃ?’’-<>" @classmethod def translate_name(cls, name: str, *, from_unicode: bool = True) -> str: @@ -402,9 +403,9 @@ class OffTopicName(Converter): If `from_unicode` is True, the name is translated from a discord-safe format, back to normalized text. """ if from_unicode: - table = str.maketrans(cls.ALLOWED_CHARACTERS, '𝖠𝖡𝖢𝖣𝖤𝖥𝖦𝖧𝖨𝖩𝖪𝖫𝖬𝖭𝖮𝖯𝖰𝖱𝖲𝖳𝖴𝖵𝖶𝖷𝖸𝖹ǃ?’’-') + table = str.maketrans(cls.ALLOWED_CHARACTERS, cls.TRANSLATED_CHARACTERS) else: - table = str.maketrans('𝖠𝖡𝖢𝖣𝖤𝖥𝖦𝖧𝖨𝖩𝖪𝖫𝖬𝖭𝖮𝖯𝖰𝖱𝖲𝖳𝖴𝖵𝖶𝖷𝖸𝖹ǃ?’’-', cls.ALLOWED_CHARACTERS) + table = str.maketrans(cls.TRANSLATED_CHARACTERS, cls.ALLOWED_CHARACTERS) return name.translate(table) -- cgit v1.2.3 From b9f41c5f54fc1f506561430cdafb07f14966cf68 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 20 Sep 2021 14:14:39 +0100 Subject: Add get_or_fetch_member util This is now needed, as we're a large server it's not guaranteed that the member cache will always be fully populated. --- bot/utils/members.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 bot/utils/members.py diff --git a/bot/utils/members.py b/bot/utils/members.py new file mode 100644 index 000000000..302fe6d63 --- /dev/null +++ b/bot/utils/members.py @@ -0,0 +1,24 @@ +import logging +import typing as t + +import discord + +log = logging.getLogger(__name__) + + +async def get_or_fetch_member(guild: discord.Guild, member_id: int) -> t.Optional[discord.Member]: + """ + Attempt to get a member from cache; on failure fetch from the API. + + Return `None` to indicate the member could not be found. + """ + if member := guild.get_member(member_id): + log.trace("%s retrieved from cache.", member) + else: + try: + member = await guild.fetch_member(member_id) + except discord.errors.NotFound: + log.trace("Failed to fetch %d from API.", member_id) + return None + log.trace("%s fetched from API.", member) + return member -- cgit v1.2.3 From 15a8e7e8f8b5682838613a035012a49f5be5034e Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 20 Sep 2021 14:30:39 +0100 Subject: Migrate all Guild.get_member calls to get_or_fetch_member This protects us against the guild cache not being fully populated with members. --- bot/exts/backend/sync/_syncers.py | 3 ++- bot/exts/events/code_jams/_cog.py | 7 ++++--- bot/exts/filters/token_remover.py | 7 ++++--- bot/exts/help_channels/_cog.py | 4 ++-- bot/exts/info/information.py | 9 +++++---- bot/exts/moderation/infraction/infractions.py | 5 +++-- bot/exts/moderation/infraction/management.py | 3 ++- bot/exts/moderation/infraction/superstarify.py | 3 ++- bot/exts/moderation/stream.py | 23 +++++++++------------- bot/exts/moderation/watchchannels/_watchchannel.py | 5 +++-- bot/exts/recruitment/talentpool/_cog.py | 9 +++++---- bot/exts/recruitment/talentpool/_review.py | 3 ++- bot/exts/utils/reminders.py | 18 +++++++++-------- 13 files changed, 53 insertions(+), 46 deletions(-) diff --git a/bot/exts/backend/sync/_syncers.py b/bot/exts/backend/sync/_syncers.py index c9f2d2da8..50016df0c 100644 --- a/bot/exts/backend/sync/_syncers.py +++ b/bot/exts/backend/sync/_syncers.py @@ -9,6 +9,7 @@ from more_itertools import chunked import bot from bot.api import ResponseCodeError +from bot.utils.members import get_or_fetch_member log = logging.getLogger(__name__) @@ -156,7 +157,7 @@ class UserSyncer(Syncer): if db_user[db_field] != guild_value: updated_fields[db_field] = guild_value - if guild_user := guild.get_member(db_user["id"]): + if guild_user := await get_or_fetch_member(guild, db_user["id"]): seen_guild_users.add(guild_user.id) maybe_update("name", guild_user.name) diff --git a/bot/exts/events/code_jams/_cog.py b/bot/exts/events/code_jams/_cog.py index e099f7dfa..7b0831ab4 100644 --- a/bot/exts/events/code_jams/_cog.py +++ b/bot/exts/events/code_jams/_cog.py @@ -11,6 +11,7 @@ from discord.ext import commands from bot.bot import Bot from bot.constants import Emojis, Roles from bot.exts.events.code_jams import _channels +from bot.utils.members import get_or_fetch_member from bot.utils.services import send_to_paste_service log = logging.getLogger(__name__) @@ -59,7 +60,7 @@ class CodeJams(commands.Cog): reader = csv.DictReader(csv_file.splitlines()) for row in reader: - member = ctx.guild.get_member(int(row["Team Member Discord ID"])) + member = await get_or_fetch_member(ctx.guild, int(row["Team Member Discord ID"])) if member is None: log.trace(f"Got an invalid member ID: {row['Team Member Discord ID']}") @@ -69,8 +70,8 @@ class CodeJams(commands.Cog): team_leaders = await ctx.guild.create_role(name="Code Jam Team Leaders", colour=TEAM_LEADERS_COLOUR) - for team_name, members in teams.items(): - await _channels.create_team_channel(ctx.guild, team_name, members, team_leaders) + for team_name, team_members in teams.items(): + await _channels.create_team_channel(ctx.guild, team_name, team_members, team_leaders) await _channels.create_team_leader_channel(ctx.guild, team_leaders) await ctx.send(f"{Emojis.check_mark} Created Code Jam with {len(teams)} teams.") diff --git a/bot/exts/filters/token_remover.py b/bot/exts/filters/token_remover.py index 93f1f3c33..6c86ff849 100644 --- a/bot/exts/filters/token_remover.py +++ b/bot/exts/filters/token_remover.py @@ -11,6 +11,7 @@ from bot import utils from bot.bot import Bot from bot.constants import Channels, Colours, Event, Icons from bot.exts.moderation.modlog import ModLog +from bot.utils.members import get_or_fetch_member from bot.utils.messages import format_user log = logging.getLogger(__name__) @@ -99,7 +100,7 @@ class TokenRemover(Cog): await msg.channel.send(DELETION_MESSAGE_TEMPLATE.format(mention=msg.author.mention)) log_message = self.format_log_message(msg, found_token) - userid_message, mention_everyone = self.format_userid_log_message(msg, found_token) + userid_message, mention_everyone = await self.format_userid_log_message(msg, found_token) log.debug(log_message) # Send pretty mod log embed to mod-alerts @@ -116,7 +117,7 @@ class TokenRemover(Cog): self.bot.stats.incr("tokens.removed_tokens") @classmethod - def format_userid_log_message(cls, msg: Message, token: Token) -> t.Tuple[str, bool]: + async def format_userid_log_message(cls, msg: Message, token: Token) -> t.Tuple[str, bool]: """ Format the portion of the log message that includes details about the detected user ID. @@ -128,7 +129,7 @@ class TokenRemover(Cog): Returns a tuple of (log_message, mention_everyone) """ user_id = cls.extract_user_id(token.user_id) - user = msg.guild.get_member(user_id) + user = await get_or_fetch_member(msg.guild, user_id) if user: return KNOWN_USER_LOG_MESSAGE.format( diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py index 8612f9866..a64ceac3a 100644 --- a/bot/exts/help_channels/_cog.py +++ b/bot/exts/help_channels/_cog.py @@ -14,7 +14,7 @@ from bot import constants from bot.bot import Bot from bot.constants import Channels, RedirectOutput from bot.exts.help_channels import _caches, _channel, _message, _name, _stats -from bot.utils import channel as channel_utils, lock, scheduling +from bot.utils import channel as channel_utils, lock, members, scheduling log = logging.getLogger(__name__) @@ -434,7 +434,7 @@ class HelpChannels(commands.Cog): await _caches.claimants.delete(channel.id) await _caches.session_participants.delete(channel.id) - claimant = self.bot.get_guild(constants.Guild.id).get_member(claimant_id) + claimant = await members.get_or_fetch_member(self.bot.get_guild(constants.Guild.id), claimant_id) if claimant is None: log.info(f"{claimant_id} left the guild during their help session; the cooldown role won't be removed") else: diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index be67910a6..c60fd2127 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -19,6 +19,7 @@ from bot.errors import NonExistentRoleError from bot.pagination import LinePaginator from bot.utils.channel import is_mod_channel, is_staff_channel from bot.utils.checks import cooldown_with_role_bypass, has_no_roles_check, in_whitelist_check +from bot.utils.members import get_or_fetch_member from bot.utils.time import TimestampFormats, discord_timestamp, humanize_delta log = logging.getLogger(__name__) @@ -46,13 +47,13 @@ class Information(Cog): @staticmethod def join_role_stats(role_ids: list[int], guild: Guild, name: Optional[str] = None) -> dict[str, int]: """Return a dictionary with the number of `members` of each role given, and the `name` for this joined group.""" - members = 0 + member_count = 0 for role_id in role_ids: if (role := guild.get_role(role_id)) is not None: - members += len(role.members) + member_count += len(role.members) else: raise NonExistentRoleError(role_id) - return {name or role.name.title(): members} + return {name or role.name.title(): member_count} @staticmethod def get_member_counts(guild: Guild) -> dict[str, int]: @@ -244,7 +245,7 @@ class Information(Cog): async def create_user_embed(self, ctx: Context, user: MemberOrUser) -> Embed: """Creates an embed containing information on the `user`.""" - on_server = bool(ctx.guild.get_member(user.id)) + on_server = bool(await get_or_fetch_member(ctx.guild, user.id)) created = discord_timestamp(user.created_at, TimestampFormats.RELATIVE) diff --git a/bot/exts/moderation/infraction/infractions.py b/bot/exts/moderation/infraction/infractions.py index eaba97703..b58b09250 100644 --- a/bot/exts/moderation/infraction/infractions.py +++ b/bot/exts/moderation/infraction/infractions.py @@ -14,6 +14,7 @@ from bot.converters import Duration, Expiry, MemberOrUser, UnambiguousMemberOrUs from bot.decorators import respect_role_hierarchy from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler +from bot.utils.members import get_or_fetch_member from bot.utils.messages import format_user log = logging.getLogger(__name__) @@ -422,7 +423,7 @@ class Infractions(InfractionScheduler, commands.Cog): notify: bool = True ) -> t.Dict[str, str]: """Remove a user's muted role, optionally DM them a notification, and return a log dict.""" - user = guild.get_member(user_id) + user = await get_or_fetch_member(guild, user_id) log_text = {} if user: @@ -470,7 +471,7 @@ class Infractions(InfractionScheduler, commands.Cog): notify: bool = True ) -> t.Dict[str, str]: """Optionally DM the user a pardon notification and return a log dict.""" - user = guild.get_member(user_id) + user = await get_or_fetch_member(guild, user_id) log_text = {} if user: diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index d72cf8f89..0cb2a8b60 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -19,6 +19,7 @@ from bot.exts.moderation.modlog import ModLog from bot.pagination import LinePaginator from bot.utils import messages, time from bot.utils.channel import is_mod_channel +from bot.utils.members import get_or_fetch_member from bot.utils.time import humanize_delta, until_expiration log = logging.getLogger(__name__) @@ -190,7 +191,7 @@ class ModManagement(commands.Cog): # Get information about the infraction's user user_id = new_infraction['user'] - user = ctx.guild.get_member(user_id) + user = await get_or_fetch_member(ctx.guild, user_id) if user: user_text = messages.format_user(user) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 05a2bbe10..aa2fd367b 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -14,6 +14,7 @@ from bot.bot import Bot from bot.converters import Duration, Expiry from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction._scheduler import InfractionScheduler +from bot.utils.members import get_or_fetch_member from bot.utils.messages import format_user from bot.utils.time import format_infraction @@ -198,7 +199,7 @@ class Superstarify(InfractionScheduler, Cog): return guild = self.bot.get_guild(constants.Guild.id) - user = guild.get_member(infraction["user"]) + user = await get_or_fetch_member(guild, infraction["user"]) # Don't bother sending a notification if the user left the guild. if not user: diff --git a/bot/exts/moderation/stream.py b/bot/exts/moderation/stream.py index b5bd62a71..a179a9acc 100644 --- a/bot/exts/moderation/stream.py +++ b/bot/exts/moderation/stream.py @@ -16,6 +16,7 @@ from bot.constants import ( from bot.converters import Expiry from bot.pagination import LinePaginator from bot.utils import scheduling +from bot.utils.members import get_or_fetch_member from bot.utils.time import discord_timestamp, format_infraction_with_duration log = logging.getLogger(__name__) @@ -47,23 +48,17 @@ class Stream(commands.Cog): """Reload outstanding tasks from redis on startup, delete the task if the member has since left the server.""" await self.bot.wait_until_guild_available() items = await self.task_cache.items() + guild = self.bot.get_guild(Guild.id) for key, value in items: - member = self.bot.get_guild(Guild.id).get_member(key) + member = await get_or_fetch_member(guild, key) if not member: - # Member isn't found in the cache - try: - member = await self.bot.get_guild(Guild.id).fetch_member(key) - except discord.errors.NotFound: - log.debug( - f"Member {key} left the guild before we could schedule " - "the revoking of their streaming permissions." - ) - await self.task_cache.delete(key) - continue - except discord.HTTPException: - log.exception(f"Exception while trying to retrieve member {key} from Discord.") - continue + log.debug( + "User with ID %d left the guild before their streaming permissions could be revoked.", + key + ) + await self.task_cache.delete(key) + continue revoke_time = Arrow.utcfromtimestamp(value) log.debug(f"Scheduling {member} ({member.id}) to have streaming permission revoked at {revoke_time}") diff --git a/bot/exts/moderation/watchchannels/_watchchannel.py b/bot/exts/moderation/watchchannels/_watchchannel.py index a42e1f518..3fafd097b 100644 --- a/bot/exts/moderation/watchchannels/_watchchannel.py +++ b/bot/exts/moderation/watchchannels/_watchchannel.py @@ -19,6 +19,7 @@ from bot.exts.filters.webhook_remover import WEBHOOK_URL_RE from bot.exts.moderation.modlog import ModLog from bot.pagination import LinePaginator from bot.utils import CogABCMeta, messages, scheduling +from bot.utils.members import get_or_fetch_member from bot.utils.time import get_time_delta log = logging.getLogger(__name__) @@ -281,7 +282,7 @@ class WatchChannel(metaclass=CogABCMeta): user_id = msg.author.id guild = self.bot.get_guild(GuildConfig.id) - actor = guild.get_member(self.watched_users[user_id]['actor']) + actor = await get_or_fetch_member(guild, self.watched_users[user_id]['actor']) actor = actor.display_name if actor else self.watched_users[user_id]['actor'] inserted_at = self.watched_users[user_id]['inserted_at'] @@ -355,7 +356,7 @@ class WatchChannel(metaclass=CogABCMeta): list_data["info"] = {} for user_id, user_data in watched_iter: - member = ctx.guild.get_member(user_id) + member = await get_or_fetch_member(ctx.guild, user_id) line = f"• `{user_id}`" if member: line += f" ({member.name}#{member.discriminator})" diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index aaafff973..38d7cfca1 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -16,6 +16,7 @@ from bot.converters import MemberOrUser from bot.exts.recruitment.talentpool._review import Reviewer from bot.pagination import LinePaginator from bot.utils import scheduling, time +from bot.utils.members import get_or_fetch_member from bot.utils.time import get_time_delta AUTOREVIEW_ENABLED_KEY = "autoreview_enabled" @@ -175,7 +176,7 @@ class TalentPool(Cog, name="Talentpool"): lines = [] for user_id, user_data in nominations: - member = ctx.guild.get_member(user_id) + member = await get_or_fetch_member(ctx.guild, user_id) line = f"• `{user_id}`" if member: line += f" ({member.name}#{member.discriminator})" @@ -314,7 +315,7 @@ class TalentPool(Cog, name="Talentpool"): title=f"Nominations for {user.display_name} `({user.id})`", color=Color.blue() ) - lines = [self._nomination_to_string(nomination) for nomination in result] + lines = [await self._nomination_to_string(nomination) for nomination in result] await LinePaginator.paginate( lines, ctx=ctx, @@ -495,13 +496,13 @@ class TalentPool(Cog, name="Talentpool"): return True - def _nomination_to_string(self, nomination_object: dict) -> str: + async def _nomination_to_string(self, nomination_object: dict) -> str: """Creates a string representation of a nomination.""" guild = self.bot.get_guild(Guild.id) entries = [] for site_entry in nomination_object["entries"]: actor_id = site_entry["actor"] - actor = guild.get_member(actor_id) + actor = await get_or_fetch_member(guild, actor_id) reason = site_entry["reason"] or "*None*" created = time.format_infraction(site_entry["inserted_at"]) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index f4aa73e75..14a8dd4c0 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -16,6 +16,7 @@ from discord.ext.commands import Context from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, Colours, Emojis, Guild +from bot.utils.members import get_or_fetch_member from bot.utils.messages import count_unique_users_reaction, pin_no_system_message from bot.utils.scheduling import Scheduler from bot.utils.time import get_time_delta, time_since @@ -111,7 +112,7 @@ class Reviewer: return "", None guild = self.bot.get_guild(Guild.id) - member = guild.get_member(user_id) + member = await get_or_fetch_member(guild, user_id) if not member: return ( diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 1030357fd..95f3661af 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -19,6 +19,7 @@ from bot.pagination import LinePaginator from bot.utils import scheduling from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg +from bot.utils.members import get_or_fetch_member from bot.utils.messages import send_denial from bot.utils.scheduling import Scheduler from bot.utils.time import TimestampFormats, discord_timestamp @@ -136,11 +137,12 @@ class Reminders(Cog): await send_denial(ctx, f"You can't mention other {disallowed_mentions} in your reminder!") return False - def get_mentionables(self, mention_ids: t.List[int]) -> t.Iterator[Mentionable]: + async def get_mentionables(self, mention_ids: t.List[int]) -> t.Iterator[Mentionable]: """Converts Role and Member ids to their corresponding objects if possible.""" guild = self.bot.get_guild(Guild.id) for mention_id in mention_ids: - if mentionable := (guild.get_member(mention_id) or guild.get_role(mention_id)): + member = await get_or_fetch_member(guild, mention_id) + if mentionable := (member or guild.get_role(mention_id)): yield mentionable def schedule_reminder(self, reminder: dict) -> None: @@ -194,9 +196,9 @@ class Reminders(Cog): embed.description = f"Here's your reminder: {reminder['content']}" # Here the jump URL is in the format of base_url/guild_id/channel_id/message_id - additional_mentions = ' '.join( - mentionable.mention for mentionable in self.get_mentionables(reminder["mentions"]) - ) + additional_mentions = ' '.join([ + mentionable.mention async for mentionable in self.get_mentionables(reminder["mentions"]) + ]) jump_url = reminder.get("jump_url") embed.description += f"\n[Jump back to when you created the reminder]({jump_url})" @@ -337,10 +339,10 @@ class Reminders(Cog): remind_datetime = isoparse(remind_at).replace(tzinfo=None) time = discord_timestamp(remind_datetime, TimestampFormats.RELATIVE) - mentions = ", ".join( + mentions = ", ".join([ # Both Role and User objects have the `name` attribute - mention.name for mention in self.get_mentionables(mentions) - ) + mention.name async for mention in self.get_mentionables(mentions) + ]) mention_string = f"\n**Mentions:** {mentions}" if mentions else "" text = textwrap.dedent(f""" -- cgit v1.2.3 From 7a9828baf440155cdba92045b0903a76a18f2b2a Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 21 Sep 2021 18:33:27 +0100 Subject: Mock fetch_member in tests that user get_or_fetch now --- tests/bot/exts/backend/sync/test_users.py | 5 +++++ tests/bot/exts/filters/test_token_remover.py | 13 +++++++------ tests/bot/exts/moderation/infraction/test_infractions.py | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/bot/exts/backend/sync/test_users.py b/tests/bot/exts/backend/sync/test_users.py index 27932be95..88f1b2f52 100644 --- a/tests/bot/exts/backend/sync/test_users.py +++ b/tests/bot/exts/backend/sync/test_users.py @@ -1,6 +1,8 @@ import unittest from unittest import mock +from discord.errors import NotFound + from bot.exts.backend.sync._syncers import UserSyncer, _Diff from tests import helpers @@ -134,6 +136,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): self.get_mock_member(fake_user()), None ] + guild.fetch_member.side_effect = NotFound(mock.Mock(status=404), "Not found") actual_diff = await UserSyncer._get_diff(guild) expected_diff = ([], [{"id": 63, "in_guild": False}], None) @@ -158,6 +161,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): self.get_mock_member(updated_user), None ] + guild.fetch_member.side_effect = NotFound(mock.Mock(status=404), "Not found") actual_diff = await UserSyncer._get_diff(guild) expected_diff = ([new_user], [{"id": 55, "name": "updated"}, {"id": 63, "in_guild": False}], None) @@ -177,6 +181,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase): self.get_mock_member(fake_user()), None ] + guild.fetch_member.side_effect = NotFound(mock.Mock(status=404), "Not found") actual_diff = await UserSyncer._get_diff(guild) expected_diff = ([], [], None) diff --git a/tests/bot/exts/filters/test_token_remover.py b/tests/bot/exts/filters/test_token_remover.py index 51feae9cb..05e790723 100644 --- a/tests/bot/exts/filters/test_token_remover.py +++ b/tests/bot/exts/filters/test_token_remover.py @@ -295,20 +295,21 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): ) @autospec("bot.exts.filters.token_remover", "UNKNOWN_USER_LOG_MESSAGE") - def test_format_userid_log_message_unknown(self, unknown_user_log_message): + async def test_format_userid_log_message_unknown(self, unknown_user_log_message,): """Should correctly format the user ID portion when the actual user it belongs to is unknown.""" token = Token("NDcyMjY1OTQzMDYyNDEzMzMy", "XsySD_", "s45jqDV_Iisn-symw0yDRrk_jf4") unknown_user_log_message.format.return_value = " Partner" msg = MockMessage(id=555, content="hello world") msg.guild.get_member.return_value = None + msg.guild.fetch_member.side_effect = NotFound(mock.Mock(status=404), "Not found") - return_value = TokenRemover.format_userid_log_message(msg, token) + return_value = await TokenRemover.format_userid_log_message(msg, token) self.assertEqual(return_value, (unknown_user_log_message.format.return_value, False)) unknown_user_log_message.format.assert_called_once_with(user_id=472265943062413332) @autospec("bot.exts.filters.token_remover", "KNOWN_USER_LOG_MESSAGE") - def test_format_userid_log_message_bot(self, known_user_log_message): + async def test_format_userid_log_message_bot(self, known_user_log_message): """Should correctly format the user ID portion when the ID belongs to a known bot.""" token = Token("NDcyMjY1OTQzMDYyNDEzMzMy", "XsySD_", "s45jqDV_Iisn-symw0yDRrk_jf4") known_user_log_message.format.return_value = " Partner" @@ -316,7 +317,7 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): msg.guild.get_member.return_value.__str__.return_value = "Sam" msg.guild.get_member.return_value.bot = True - return_value = TokenRemover.format_userid_log_message(msg, token) + return_value = await TokenRemover.format_userid_log_message(msg, token) self.assertEqual(return_value, (known_user_log_message.format.return_value, True)) @@ -327,12 +328,12 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): ) @autospec("bot.exts.filters.token_remover", "KNOWN_USER_LOG_MESSAGE") - def test_format_log_message_user_token_user(self, user_token_message): + async def test_format_log_message_user_token_user(self, user_token_message): """Should correctly format the user ID portion when the ID belongs to a known user.""" token = Token("NDY3MjIzMjMwNjUwNzc3NjQx", "XsySD_", "s45jqDV_Iisn-symw0yDRrk_jf4") user_token_message.format.return_value = "Partner" - return_value = TokenRemover.format_userid_log_message(self.msg, token) + return_value = await TokenRemover.format_userid_log_message(self.msg, token) self.assertEqual(return_value, (user_token_message.format.return_value, True)) user_token_message.format.assert_called_once_with( diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index f844a9181..aeff734dc 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -3,6 +3,8 @@ import textwrap import unittest from unittest.mock import ANY, AsyncMock, MagicMock, Mock, patch +from discord.errors import NotFound + from bot.constants import Event from bot.exts.moderation.infraction import _utils from bot.exts.moderation.infraction.infractions import Infractions @@ -195,6 +197,7 @@ class VoiceBanTests(unittest.IsolatedAsyncioTestCase): async def test_voice_unban_user_not_found(self): """Should include info to return dict when user was not found from guild.""" self.guild.get_member.return_value = None + self.guild.fetch_member.side_effect = NotFound(Mock(status=404), "Not found") result = await self.cog.pardon_voice_ban(self.user.id, self.guild) self.assertEqual(result, {"Info": "User was not found in the guild."}) -- cgit v1.2.3 From 6cf9f54c2f2fe51ca08ae44e98dc5dcea97b7ad8 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 21 Sep 2021 20:21:12 +0100 Subject: Rename channel helper to be consistent with other helpers --- bot/exts/help_channels/_channel.py | 4 ++-- bot/exts/help_channels/_cog.py | 6 +++--- bot/utils/channel.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index 0846b28c8..f1bcea171 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -10,7 +10,7 @@ from arrow import Arrow import bot from bot import constants from bot.exts.help_channels import _caches, _message -from bot.utils.channel import try_get_channel +from bot.utils.channel import get_or_fetch_channel log = logging.getLogger(__name__) @@ -133,7 +133,7 @@ async def move_to_bottom(channel: discord.TextChannel, category_id: int, **optio options should be avoided, as it may interfere with the category move we perform. """ # Get a fresh copy of the category from the bot to avoid the cache mismatch issue we had. - category = await try_get_channel(category_id) + category = await get_or_fetch_channel(category_id) payload = [{"id": c.id, "position": c.position} for c in category.channels] diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py index a64ceac3a..7c39bc132 100644 --- a/bot/exts/help_channels/_cog.py +++ b/bot/exts/help_channels/_cog.py @@ -278,13 +278,13 @@ class HelpChannels(commands.Cog): log.trace("Getting the CategoryChannel objects for the help categories.") try: - self.available_category = await channel_utils.try_get_channel( + self.available_category = await channel_utils.get_or_fetch_channel( constants.Categories.help_available ) - self.in_use_category = await channel_utils.try_get_channel( + self.in_use_category = await channel_utils.get_or_fetch_channel( constants.Categories.help_in_use ) - self.dormant_category = await channel_utils.try_get_channel( + self.dormant_category = await channel_utils.get_or_fetch_channel( constants.Categories.help_dormant ) except discord.HTTPException: diff --git a/bot/utils/channel.py b/bot/utils/channel.py index 72603c521..6d2356679 100644 --- a/bot/utils/channel.py +++ b/bot/utils/channel.py @@ -53,7 +53,7 @@ def is_in_category(channel: discord.TextChannel, category_id: int) -> bool: return getattr(channel, "category_id", None) == category_id -async def try_get_channel(channel_id: int) -> discord.abc.GuildChannel: +async def get_or_fetch_channel(channel_id: int) -> discord.abc.GuildChannel: """Attempt to get or fetch a channel and return it.""" log.trace(f"Getting the channel {channel_id}.") -- cgit v1.2.3 From e52db754d8758458fdbaff41eb2ac5dbaf83c9f7 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 21 Sep 2021 18:35:35 +0100 Subject: Add new poetry tasks for pytest Updated the test task to now run with --ff which runs failed tests from the last run first Added retest, which runs pytest with --lf this only runs the failed tests from the last test run --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 23cbba19b..4431a41c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,7 +62,8 @@ precommit = "pre-commit install" build = "docker build -t ghcr.io/python-discord/bot:latest -f Dockerfile ." push = "docker push ghcr.io/python-discord/bot:latest" test-nocov = "pytest -n auto" -test = "pytest -n auto --cov-report= --cov" +test = "pytest -n auto --cov-report= --cov --ff" +retest = "pytest -n auto --cov-report= --cov --lf" html = "coverage html" report = "coverage report" -- cgit v1.2.3 From 58098b5121a4ebb06b6f5ed411143ec8d5766266 Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Wed, 29 Sep 2021 23:14:13 +0100 Subject: python-news escape fixes (#1822) * Fix escapes in python-news posts No longer escapes markdown inside of codeblocks or pre-escaped markdown. Co-authored-by: Ryu18 * Add escaping to title of py-news posts * Fix typo Co-authored-by: Ryu18 --- bot/exts/info/python_news.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 58dcd3a02..2a8b64f32 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -23,6 +23,14 @@ THREAD_URL = "https://mail.python.org/archives/list/{list}@python.org/thread/{id AVATAR_URL = "https://www.python.org/static/opengraph-icon-200x200.png" +# By first matching everything within a codeblock, +# when matching markdown it won't be within a codeblock +MARKDOWN_REGEX = re.compile( + r"(?P`.*?`)" # matches everything within a codeblock + r"|(?P(? str: - """Escape the markdown underlines and spoilers.""" - return re.sub(r"[_|]", lambda match: "\\" + match[0], content) + """Escape the markdown underlines and spoilers that aren't in codeblocks.""" + return MARKDOWN_REGEX.sub( + lambda match: match.group("codeblock") or "\\" + match.group("markdown"), + content + ) async def post_pep_news(self) -> None: """Fetch new PEPs and when they don't have announcement in #python-news, create it.""" @@ -109,7 +120,7 @@ class PythonNews(Cog): # Build an embed and send a webhook embed = discord.Embed( - title=new["title"], + title=self.escape_markdown(new["title"]), description=self.escape_markdown(new["summary"]), timestamp=new_datetime, url=new["link"], @@ -129,7 +140,7 @@ class PythonNews(Cog): self.bot.stats.incr("python_news.posted.pep") if msg.channel.is_news(): - log.trace("Publishing PEP annnouncement because it was in a news channel") + log.trace("Publishing PEP announcement because it was in a news channel") await msg.publish() # Apply new sent news to DB to avoid duplicate sending @@ -179,7 +190,7 @@ class PythonNews(Cog): # Build an embed and send a message to the webhook embed = discord.Embed( - title=thread_information["subject"], + title=self.escape_markdown(thread_information["subject"]), description=content[:1000] + f"... [continue reading]({link})" if len(content) > 1000 else content, timestamp=new_date, url=link, -- cgit v1.2.3 From eeb70eff84530ca0939ec94e1343b7ad6bd703b7 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 21 Sep 2021 22:53:07 +0100 Subject: Allow helpers to edit their own nomination reason This change will allow helpers to run the edit reason command in the Talentpool cog. To ensure that non-mod helpers can only edit their own reasons the interface for the reason command has been changed slightly. If nominee_or_nomination_id is a member or user, then the command edits the currently active nomination for that person. If it's an int, then use this to look up that nomination ID to edit. If no nominator is specified, assume the invoker is editing their own nomination reason. Otherwise, edit the reason from that specific nominator. Raise a permission error if a non-mod staff member invokes this command on a specific nomination ID, or with an nominator other than themselves. --- bot/exts/recruitment/talentpool/_cog.py | 68 +++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index aaafff973..0b56ea8b8 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -7,12 +7,12 @@ from typing import Optional, Union import discord from async_rediscache import RedisCache from discord import Color, Embed, Member, PartialMessage, RawReactionActionEvent -from discord.ext.commands import Cog, Context, group, has_any_role +from discord.ext.commands import BadArgument, Cog, Context, group, has_any_role from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles, STAFF_ROLES -from bot.converters import MemberOrUser +from bot.converters import MemberOrUser, UnambiguousMemberOrUser from bot.exts.recruitment.talentpool._review import Reviewer from bot.pagination import LinePaginator from bot.utils import scheduling, time @@ -75,7 +75,7 @@ class TalentPool(Cog, name="Talentpool"): return True @group(name='talentpool', aliases=('tp', 'talent', 'nomination', 'n'), invoke_without_command=True) - @has_any_role(*MODERATION_ROLES) + @has_any_role(*STAFF_ROLES) async def nomination_group(self, ctx: Context) -> None: """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" await ctx.send_help(ctx.command) @@ -342,18 +342,70 @@ class TalentPool(Cog, name="Talentpool"): await ctx.send(":x: The specified user does not have an active nomination") @nomination_group.group(name='edit', aliases=('e',), invoke_without_command=True) - @has_any_role(*MODERATION_ROLES) + @has_any_role(*STAFF_ROLES) async def nomination_edit_group(self, ctx: Context) -> None: """Commands to edit nominations.""" await ctx.send_help(ctx.command) @nomination_edit_group.command(name='reason') - @has_any_role(*MODERATION_ROLES) - async def edit_reason_command(self, ctx: Context, nomination_id: int, actor: MemberOrUser, *, reason: str) -> None: - """Edits the reason of a specific nominator in a specific active nomination.""" + @has_any_role(*STAFF_ROLES) + async def edit_reason_command( + self, + ctx: Context, + nominee_or_nomination_id: Union[UnambiguousMemberOrUser, int], + nominator: Optional[UnambiguousMemberOrUser] = None, + *, + reason: str + ) -> None: + """ + Edit the nomination reason of a specific nominator for a given nomination. + + If nominee_or_nomination_id resolves to a member or user, edit the currently active nomination for that person. + Otherwise, if it's an int, look up that nomination ID to edit. + + If no nominator is specified, assume the invoker is editing their own nomination reason. + Otherwise, edit the reason from that specific nominator. + + Raise a permission error if a non-mod staff member invokes this command on a + specific nomination ID, or with an nominator other than themselves. + """ + # If not specified, assume the invoker is editing their own nomination reason. + nominator = nominator or ctx.author + + if nominator != ctx.author or isinstance(nominee_or_nomination_id, int): + # Invoker has specified another nominator, or a specific nomination id + if not any(role.id in MODERATION_ROLES for role in ctx.author.roles): + raise BadArgument( + "Only moderators can edit specific nomination IDs, " + "or the reason of a nominator other than themselves." + ) + + await self._edit_nomination_reason( + ctx, + target=nominee_or_nomination_id, + actor=nominator, + reason=reason + ) + + async def _edit_nomination_reason( + self, + ctx: Context, + target: Union[int, Member], + actor: MemberOrUser, + reason: str, + ) -> None: + """Edits a nomination reason in the database after validating the input.""" if len(reason) > REASON_MAX_CHARS: - await ctx.send(f":x: Maxiumum allowed characters for the reason is {REASON_MAX_CHARS}.") + await ctx.send(f":x: Maximum allowed characters for the reason is {REASON_MAX_CHARS}.") return + if isinstance(target, Member): + if nomination := self.cache.get(target.id): + nomination_id = nomination["id"] + else: + await ctx.send("No active nomination found for that member.") + return + else: + nomination_id = target try: nomination = await self.bot.api_client.get(f"bot/nominations/{nomination_id}") -- cgit v1.2.3 From 77f76cb85f97e619509d121ba2147c54b8d822e8 Mon Sep 17 00:00:00 2001 From: ChrisJL Date: Wed, 22 Sep 2021 18:59:14 +0100 Subject: Force kwargs when calling helper util Co-authored-by: Bluenix --- bot/exts/recruitment/talentpool/_cog.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 0b56ea8b8..70535095d 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -390,11 +390,12 @@ class TalentPool(Cog, name="Talentpool"): async def _edit_nomination_reason( self, ctx: Context, + *, target: Union[int, Member], actor: MemberOrUser, reason: str, ) -> None: - """Edits a nomination reason in the database after validating the input.""" + """Edit a nomination reason in the database after validating the input.""" if len(reason) > REASON_MAX_CHARS: await ctx.send(f":x: Maximum allowed characters for the reason is {REASON_MAX_CHARS}.") return -- cgit v1.2.3 From 2f0b4215f0be46c62ea529f20ccba0ea967dcd21 Mon Sep 17 00:00:00 2001 From: wookie184 Date: Sun, 3 Oct 2021 14:51:01 +0100 Subject: Handle channel category being None in antispam/malware checks The code already handled the attribute not existing (e.g from a DM channel), but didn't handle TextChannels not in a category --- bot/exts/filters/antimalware.py | 2 +- bot/exts/filters/antispam.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/exts/filters/antimalware.py b/bot/exts/filters/antimalware.py index 0eedeb0fb..e708e5149 100644 --- a/bot/exts/filters/antimalware.py +++ b/bot/exts/filters/antimalware.py @@ -63,7 +63,7 @@ class AntiMalware(Cog): return # Ignore code jam channels - if hasattr(message.channel, "category") and message.channel.category.name == JAM_CATEGORY_NAME: + if getattr(message.channel, "category", None) and message.channel.category.name == JAM_CATEGORY_NAME: return # Check if user is staff, if is, return diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index fe79a5d62..70c1168bf 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -166,7 +166,7 @@ class AntiSpam(Cog): not message.guild or message.guild.id != GuildConfig.id or message.author.bot - or (hasattr(message.channel, "category") and message.channel.category.name == JAM_CATEGORY_NAME) + or (getattr(message.channel, "category", None) and message.channel.category.name == JAM_CATEGORY_NAME) or (message.channel.id in Filter.channel_whitelist and not DEBUG_MODE) or (any(role.id in Filter.role_whitelist for role in message.author.roles) and not DEBUG_MODE) ): -- cgit v1.2.3 From 0bd2f1e8ac7bc3daaad6a0151cec19eaeae6c758 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sun, 3 Oct 2021 17:19:35 +0100 Subject: Block helpers from editing nomination reasons in channels other than #nominations --- bot/exts/recruitment/talentpool/_cog.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 70535095d..98e83f309 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -372,9 +372,13 @@ class TalentPool(Cog, name="Talentpool"): # If not specified, assume the invoker is editing their own nomination reason. nominator = nominator or ctx.author - if nominator != ctx.author or isinstance(nominee_or_nomination_id, int): - # Invoker has specified another nominator, or a specific nomination id - if not any(role.id in MODERATION_ROLES for role in ctx.author.roles): + if not any(role.id in MODERATION_ROLES for role in ctx.author.roles): + if ctx.channel.id != Channels.nominations: + await ctx.send(f":x: Nomination edits must be run in the <#{Channels.nominations}> channel") + return + + if nominator != ctx.author or isinstance(nominee_or_nomination_id, int): + # Invoker has specified another nominator, or a specific nomination id raise BadArgument( "Only moderators can edit specific nomination IDs, " "or the reason of a nominator other than themselves." -- cgit v1.2.3 From d444ebd8c3f9b10e021ecc94da8bbac482a1b236 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sun, 3 Oct 2021 22:57:32 +0100 Subject: Check cache for Members and Users By swapping the isinstance to check for int, the else block now catches the case where target is Member or User, this allows for editting the nomination reason of members that are off server. --- bot/exts/recruitment/talentpool/_cog.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 98e83f309..193be2095 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -6,7 +6,7 @@ from typing import Optional, Union import discord from async_rediscache import RedisCache -from discord import Color, Embed, Member, PartialMessage, RawReactionActionEvent +from discord import Color, Embed, Member, PartialMessage, RawReactionActionEvent, User from discord.ext.commands import BadArgument, Cog, Context, group, has_any_role from bot.api import ResponseCodeError @@ -395,7 +395,7 @@ class TalentPool(Cog, name="Talentpool"): self, ctx: Context, *, - target: Union[int, Member], + target: Union[int, Member, User], actor: MemberOrUser, reason: str, ) -> None: @@ -403,14 +403,14 @@ class TalentPool(Cog, name="Talentpool"): if len(reason) > REASON_MAX_CHARS: await ctx.send(f":x: Maximum allowed characters for the reason is {REASON_MAX_CHARS}.") return - if isinstance(target, Member): + if isinstance(target, int): + nomination_id = target + else: if nomination := self.cache.get(target.id): nomination_id = nomination["id"] else: await ctx.send("No active nomination found for that member.") return - else: - nomination_id = target try: nomination = await self.bot.api_client.get(f"bot/nominations/{nomination_id}") -- cgit v1.2.3 From 17d100e2207b6b98c7b0cb6d9c378b15b1bb4c4e Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 4 Oct 2021 19:30:15 +0100 Subject: Monkey patch http.send_typing to catch 403s Sometimes discord turns off typing events by throwing 403's, so we should catch those --- bot/__init__.py | 4 +++- bot/typing.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 bot/typing.py diff --git a/bot/__init__.py b/bot/__init__.py index 8f880b8e6..70ff03fd4 100644 --- a/bot/__init__.py +++ b/bot/__init__.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING from discord.ext import commands -from bot import log +from bot import log, typing from bot.command import Command if TYPE_CHECKING: @@ -17,6 +17,8 @@ log.setup() if os.name == "nt": asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) +typing.patch_typing() + # Monkey-patch discord.py decorators to use the Command subclass which supports root aliases. # Must be patched before any cogs are added. commands.command = partial(commands.command, cls=Command) diff --git a/bot/typing.py b/bot/typing.py new file mode 100644 index 000000000..4b1df3f2f --- /dev/null +++ b/bot/typing.py @@ -0,0 +1,32 @@ +import logging +from datetime import datetime, timedelta + +from discord import Forbidden, http + +log = logging.getLogger(__name__) + + +def patch_typing() -> None: + """ + Sometimes discord turns off typing events by throwing 403's. + + Handle those issues by patching the trigger_typing method so it ignores 403's in general. + """ + log.info("Patching send_typing, which should fix things breaking when discord disables typing events. Stay safe!") + + original = http.HTTPClient.send_typing + last_403 = None + + async def honeybadger_type(self, channel_id: int) -> None: # noqa: ANN001 + nonlocal last_403 + if last_403 and (datetime.now() - last_403) < timedelta(minutes=5): + log.warning("Not sending typing event, we got a 403 less than 5 minutes ago.") + return + try: + await original(self, channel_id) + except Forbidden: + last_403 = datetime.now() + log.warning("Got a 403 from typing event!") + pass + + http.HTTPClient.send_typing = honeybadger_type -- cgit v1.2.3 From 66dea7644d2f0572607105f85d80af9f39398e61 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 4 Oct 2021 19:37:33 +0100 Subject: Move all monkey patches to their own file --- bot/__init__.py | 9 ++++----- bot/command.py | 18 ------------------ bot/monkey_patches.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ bot/typing.py | 32 -------------------------------- 4 files changed, 54 insertions(+), 55 deletions(-) delete mode 100644 bot/command.py create mode 100644 bot/monkey_patches.py delete mode 100644 bot/typing.py diff --git a/bot/__init__.py b/bot/__init__.py index 70ff03fd4..a1c4466f1 100644 --- a/bot/__init__.py +++ b/bot/__init__.py @@ -5,8 +5,7 @@ from typing import TYPE_CHECKING from discord.ext import commands -from bot import log, typing -from bot.command import Command +from bot import log, monkey_patches if TYPE_CHECKING: from bot.bot import Bot @@ -17,11 +16,11 @@ log.setup() if os.name == "nt": asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) -typing.patch_typing() +monkey_patches.patch_typing() # Monkey-patch discord.py decorators to use the Command subclass which supports root aliases. # Must be patched before any cogs are added. -commands.command = partial(commands.command, cls=Command) -commands.GroupMixin.command = partialmethod(commands.GroupMixin.command, cls=Command) +commands.command = partial(commands.command, cls=monkey_patches.Command) +commands.GroupMixin.command = partialmethod(commands.GroupMixin.command, cls=monkey_patches.Command) instance: "Bot" = None # Global Bot instance. diff --git a/bot/command.py b/bot/command.py deleted file mode 100644 index 0fb900f7b..000000000 --- a/bot/command.py +++ /dev/null @@ -1,18 +0,0 @@ -from discord.ext import commands - - -class Command(commands.Command): - """ - A `discord.ext.commands.Command` subclass which supports root aliases. - - A `root_aliases` keyword argument is added, which is a sequence of alias names that will act as - top-level commands rather than being aliases of the command's group. It's stored as an attribute - also named `root_aliases`. - """ - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.root_aliases = kwargs.get("root_aliases", []) - - if not isinstance(self.root_aliases, (list, tuple)): - raise TypeError("Root aliases of a command must be a list or a tuple of strings.") diff --git a/bot/monkey_patches.py b/bot/monkey_patches.py new file mode 100644 index 000000000..9c0a22bfb --- /dev/null +++ b/bot/monkey_patches.py @@ -0,0 +1,50 @@ +import logging +from datetime import datetime, timedelta + +from discord import Forbidden, http +from discord.ext import commands + +log = logging.getLogger(__name__) + + +class Command(commands.Command): + """ + A `discord.ext.commands.Command` subclass which supports root aliases. + + A `root_aliases` keyword argument is added, which is a sequence of alias names that will act as + top-level commands rather than being aliases of the command's group. It's stored as an attribute + also named `root_aliases`. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.root_aliases = kwargs.get("root_aliases", []) + + if not isinstance(self.root_aliases, (list, tuple)): + raise TypeError("Root aliases of a command must be a list or a tuple of strings.") + + +def patch_typing() -> None: + """ + Sometimes discord turns off typing events by throwing 403's. + + Handle those issues by patching the trigger_typing method so it ignores 403's in general. + """ + log.info("Patching send_typing, which should fix things breaking when discord disables typing events. Stay safe!") + + original = http.HTTPClient.send_typing + last_403 = None + + async def honeybadger_type(self, channel_id: int) -> None: # noqa: ANN001 + nonlocal last_403 + if last_403 and (datetime.now() - last_403) < timedelta(minutes=5): + log.warning("Not sending typing event, we got a 403 less than 5 minutes ago.") + return + try: + await original(self, channel_id) + except Forbidden: + last_403 = datetime.now() + log.warning("Got a 403 from typing event!") + pass + + http.HTTPClient.send_typing = honeybadger_type diff --git a/bot/typing.py b/bot/typing.py deleted file mode 100644 index 4b1df3f2f..000000000 --- a/bot/typing.py +++ /dev/null @@ -1,32 +0,0 @@ -import logging -from datetime import datetime, timedelta - -from discord import Forbidden, http - -log = logging.getLogger(__name__) - - -def patch_typing() -> None: - """ - Sometimes discord turns off typing events by throwing 403's. - - Handle those issues by patching the trigger_typing method so it ignores 403's in general. - """ - log.info("Patching send_typing, which should fix things breaking when discord disables typing events. Stay safe!") - - original = http.HTTPClient.send_typing - last_403 = None - - async def honeybadger_type(self, channel_id: int) -> None: # noqa: ANN001 - nonlocal last_403 - if last_403 and (datetime.now() - last_403) < timedelta(minutes=5): - log.warning("Not sending typing event, we got a 403 less than 5 minutes ago.") - return - try: - await original(self, channel_id) - except Forbidden: - last_403 = datetime.now() - log.warning("Got a 403 from typing event!") - pass - - http.HTTPClient.send_typing = honeybadger_type -- cgit v1.2.3 From 44e5f472d3338d4573f69354f8aa49023063d90a Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 4 Oct 2021 20:42:11 +0100 Subject: Use utcnow() and lower a logging level in patch_typing --- bot/monkey_patches.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/monkey_patches.py b/bot/monkey_patches.py index 9c0a22bfb..4dbdb5eab 100644 --- a/bot/monkey_patches.py +++ b/bot/monkey_patches.py @@ -30,20 +30,20 @@ def patch_typing() -> None: Handle those issues by patching the trigger_typing method so it ignores 403's in general. """ - log.info("Patching send_typing, which should fix things breaking when discord disables typing events. Stay safe!") + log.debug("Patching send_typing, which should fix things breaking when discord disables typing events. Stay safe!") original = http.HTTPClient.send_typing last_403 = None async def honeybadger_type(self, channel_id: int) -> None: # noqa: ANN001 nonlocal last_403 - if last_403 and (datetime.now() - last_403) < timedelta(minutes=5): + if last_403 and (datetime.utcnow() - last_403) < timedelta(minutes=5): log.warning("Not sending typing event, we got a 403 less than 5 minutes ago.") return try: await original(self, channel_id) except Forbidden: - last_403 = datetime.now() + last_403 = datetime.utcnow() log.warning("Got a 403 from typing event!") pass -- cgit v1.2.3 From 269ab7e1a0f35c4ecf14bffba93ab1695841364b Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Mon, 4 Oct 2021 21:00:59 +0100 Subject: Improvements to anti-spam log message - Removed redundant f-string - Made it so that messages with attachments will *always* upload to the log site. - Made it so that truncates messages will also upload to the paste site, for full viewing. - Fixed a previously unnoticed bug where truncating didn't account for extra characters of the ellipses. --- bot/exts/filters/antispam.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 70c1168bf..dc8d24bba 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -83,7 +83,11 @@ class DeletionContext: ) # For multiple messages or those with excessive newlines, use the logs API - if len(self.messages) > 1 or 'newlines' in self.rules: + if any(( + len(self.messages) > 1, + self.messages[0].attachments, + self.messages[0].count('\n') > 15 + )): url = await modlog.upload_log(self.messages.values(), actor_id, self.attachments) mod_alert_message += f"A complete log of the offending messages can be found [here]({url})" else: @@ -93,9 +97,11 @@ class DeletionContext: remaining_chars = 4080 - len(mod_alert_message) if len(content) > remaining_chars: - content = content[:remaining_chars] + "..." + url = await modlog.upload_log([message], actor_id, self.attachments) + log_site_msg = f"The full message can be found [here]({url})" + content = content[:remaining_chars - (3 + len(log_site_msg))] + "..." - mod_alert_message += f"{content}" + mod_alert_message += content *_, last_message = self.messages.values() await modlog.send_log_message( -- cgit v1.2.3 From 74493f202d3478d6159dcfc0dbe4ee27b6334fc6 Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Tue, 5 Oct 2021 11:52:32 +0100 Subject: Make attachments check explicit Co-authored-by: Matteo Bertucci --- bot/exts/filters/antispam.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index dc8d24bba..7f4b5a627 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -85,7 +85,7 @@ class DeletionContext: # For multiple messages or those with excessive newlines, use the logs API if any(( len(self.messages) > 1, - self.messages[0].attachments, + len(self.messages[0].attachments) > 0, self.messages[0].count('\n') > 15 )): url = await modlog.upload_log(self.messages.values(), actor_id, self.attachments) -- cgit v1.2.3 From 62ec6f6c52921854e00e0af71490f050adce0749 Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Tue, 5 Oct 2021 14:00:10 +0100 Subject: Fix if statement error Adds `messages_as_list` for easy indexing of `self.messages.values()` Also updated the comment above if statement and tidied some code --- bot/exts/filters/antispam.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 7f4b5a627..3b30817df 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -46,7 +46,6 @@ RULE_FUNCTION_MAPPING = { @dataclass class DeletionContext: """Represents a Deletion Context for a single spam event.""" - members: frozenset[Member] triggered_in: TextChannel channels: set[TextChannel] = field(default_factory=set) @@ -82,34 +81,34 @@ class DeletionContext: f"**Rules:** {', '.join(rule for rule in self.rules)}\n" ) - # For multiple messages or those with excessive newlines, use the logs API + messages_as_list = list(self.messages.values()) + first_message = messages_as_list[0] + # For multiple messages and those with attachments or excessive newlines, use the logs API if any(( - len(self.messages) > 1, - len(self.messages[0].attachments) > 0, - self.messages[0].count('\n') > 15 + len(messages_as_list) > 1, + len(first_message.attachments) > 0, + first_message.content.count('\n') > 15 )): url = await modlog.upload_log(self.messages.values(), actor_id, self.attachments) mod_alert_message += f"A complete log of the offending messages can be found [here]({url})" else: mod_alert_message += "Message:\n" - [message] = self.messages.values() - content = message.clean_content + content = first_message.clean_content remaining_chars = 4080 - len(mod_alert_message) if len(content) > remaining_chars: - url = await modlog.upload_log([message], actor_id, self.attachments) + url = await modlog.upload_log([first_message], actor_id, self.attachments) log_site_msg = f"The full message can be found [here]({url})" content = content[:remaining_chars - (3 + len(log_site_msg))] + "..." mod_alert_message += content - *_, last_message = self.messages.values() await modlog.send_log_message( icon_url=Icons.filtering, colour=Colour(Colours.soft_red), title="Spam detected!", text=mod_alert_message, - thumbnail=last_message.author.avatar_url_as(static_format="png"), + thumbnail=first_message.author.avatar_url_as(static_format="png"), channel_id=Channels.mod_alerts, ping_everyone=AntiSpamConfig.ping_everyone ) -- cgit v1.2.3 From 025efcde7c5c478ecdc91ecd4c73f13bdfa6bdad Mon Sep 17 00:00:00 2001 From: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Date: Tue, 5 Oct 2021 16:17:20 +0100 Subject: Fix linting Reverted accidental line deletion --- bot/exts/filters/antispam.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 3b30817df..8bae159d2 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -46,6 +46,7 @@ RULE_FUNCTION_MAPPING = { @dataclass class DeletionContext: """Represents a Deletion Context for a single spam event.""" + members: frozenset[Member] triggered_in: TextChannel channels: set[TextChannel] = field(default_factory=set) -- cgit v1.2.3