From ab0d44b8e013694c2e92af51c6fdb8ed9239c331 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Wed, 25 Dec 2019 18:20:16 +0100 Subject: Hardcode SIGKILL value It allows the cog to also work on Windows, because of Signals.SIGKILL not being defined on this platform --- bot/cogs/snekbox.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index da33e27b2..e9e5465ad 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -36,6 +36,8 @@ RAW_CODE_REGEX = re.compile( MAX_PASTE_LEN = 1000 EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.rockstars, Roles.partners) +SIGKILL = 9 + class Snekbox(Cog): """Safe evaluation of Python code using Snekbox.""" @@ -101,7 +103,7 @@ class Snekbox(Cog): if returncode is None: msg = "Your eval job has failed" error = stdout.strip() - elif returncode == 128 + Signals.SIGKILL: + elif returncode == 128 + SIGKILL: msg = "Your eval job timed out or ran out of memory" elif returncode == 255: msg = "Your eval job has failed" -- cgit v1.2.3 From e53492ed9485e169b2fa471635c2ea624ec1d532 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Thu, 26 Dec 2019 11:17:48 +0100 Subject: Correct eval output to include the 11th line --- bot/cogs/snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index e9e5465ad..00b8618e2 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -154,7 +154,7 @@ class Snekbox(Cog): lines = output.count("\n") if lines > 0: - output = output.split("\n")[:10] # Only first 10 cause the rest is truncated anyway + output = output.split("\n")[:11] # Only first 11 cause the rest is truncated anyway output = (f"{i:03d} | {line}" for i, line in enumerate(output, 1)) output = "\n".join(output) -- cgit v1.2.3 From b5730e0b07a4eb886710d71648ba4c0ffb4ebf79 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 28 Jan 2020 14:50:58 +0000 Subject: Don't strip whitespaces during snekbox formatting It could lead to a misleading result if it is stripped. --- bot/cogs/snekbox.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 00b8618e2..81951efd3 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -137,7 +137,7 @@ class Snekbox(Cog): """ log.trace("Formatting output...") - output = output.strip(" \n") + output = output.rstrip("\n") original_output = output # To be uploaded to a pasting service if needed paste_link = None @@ -171,7 +171,6 @@ class Snekbox(Cog): if truncated: paste_link = await self.upload_output(original_output) - output = output.strip() if not output: output = "[No output]" -- cgit v1.2.3 From 6991e5fab13893c05d6f220e71f6ffc71509c1aa Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 29 Jan 2020 19:19:58 +0000 Subject: Re-eval snippet with emoji reaction If the eval message is edited after less than 10 seconds, an emoji is added to the message, and if the user adds the same, the snippet is re-evaluated. This make easier to correct snipper mistakes. --- bot/cogs/snekbox.py | 69 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 81951efd3..1688c0278 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -1,3 +1,4 @@ +import asyncio import datetime import logging import re @@ -200,32 +201,54 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") - self.jobs[ctx.author.id] = datetime.datetime.now() - code = self.prepare_input(code) + while True: + self.jobs[ctx.author.id] = datetime.datetime.now() + code = self.prepare_input(code) - try: - async with ctx.typing(): - results = await self.post_eval(code) - msg, error = self.get_results_message(results) - - if error: - output, paste_link = error, None - else: - output, paste_link = await self.format_output(results["stdout"]) - - icon = self.get_status_emoji(results) - msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" - if paste_link: - msg = f"{msg}\nFull output: {paste_link}" - - response = await ctx.send(msg) - self.bot.loop.create_task( - wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + try: + async with ctx.typing(): + results = await self.post_eval(code) + msg, error = self.get_results_message(results) + + if error: + output, paste_link = error, None + else: + output, paste_link = await self.format_output(results["stdout"]) + + icon = self.get_status_emoji(results) + msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" + if paste_link: + msg = f"{msg}\nFull output: {paste_link}" + + response = await ctx.send(msg) + self.bot.loop.create_task( + wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + ) + + log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + finally: + del self.jobs[ctx.author.id] + + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=lambda o, n: n.id == ctx.message.id and o.content != n.content, + timeout=10 + ) + await ctx.message.add_reaction('🔁') + await self.bot.wait_for( + 'reaction_add', + check=lambda r, u: r.message.id == ctx.message.id and u.id == ctx.author.id and str(r) == '🔁', + timeout=10 ) - log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") - finally: - del self.jobs[ctx.author.id] + log.info(f"Re-evaluating message {ctx.message.id}") + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + await response.delete() + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return def setup(bot: Bot) -> None: -- cgit v1.2.3 From 032e1b80934194b85c43d67f3a26cf51b972696d Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:03:35 +0100 Subject: Use actual functions instead of lambdas for bot.wait_for The use of lambdas made the functions hard to test, this new format allows us to easily test those functions and document them. --- bot/cogs/snekbox.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 1688c0278..3fc8d9937 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -3,9 +3,11 @@ import datetime import logging import re import textwrap +from functools import partial from signal import Signals from typing import Optional, Tuple +from discord import Message, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -232,13 +234,13 @@ class Snekbox(Cog): try: _, new_message = await self.bot.wait_for( 'message_edit', - check=lambda o, n: n.id == ctx.message.id and o.content != n.content, + check=partial(predicate_eval_message_edit, ctx), timeout=10 ) await ctx.message.add_reaction('🔁') await self.bot.wait_for( 'reaction_add', - check=lambda r, u: r.message.id == ctx.message.id and u.id == ctx.author.id and str(r) == '🔁', + check=partial(predicate_eval_emoji_reaction, ctx), timeout=10 ) @@ -251,6 +253,16 @@ class Snekbox(Cog): return +def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message) -> bool: + """Return True if the edited message is the context message and the content was indeed modified.""" + return new_msg.id == ctx.message.id and old_msg.content != new_msg.content + + +def predicate_eval_emoji_reaction(ctx: Context, reaction: Reaction, user: User) -> bool: + """Return True if the reaction 🔁 was added by the context message author on this message.""" + return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == '🔁' + + def setup(bot: Bot) -> None: """Load the Snekbox cog.""" bot.add_cog(Snekbox(bot)) -- cgit v1.2.3 From adf63e65a0b861cb02a6e8cc1e5b2c2c09e57726 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:05:09 +0100 Subject: Create an AsyncContextManagerMock mock for testing asynchronous context managers It can be used to test aiohttp request functions, since they are async context managers --- tests/helpers.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 5df796c23..6aee8623f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -127,6 +127,18 @@ class AsyncMock(CustomMockMixin, unittest.mock.MagicMock): return super().__call__(*args, **kwargs) +class AsyncContextManagerMock(unittest.mock.MagicMock): + def __init__(self, return_value: Any): + super().__init__() + self._return_value = return_value + + async def __aenter__(self): + return self._return_value + + async def __aexit__(self, *args): + pass + + class AsyncIteratorMock: """ A class to mock asynchronous iterators. -- cgit v1.2.3 From 8c8f8dd50f376c3398c04e7bbe76b5028b69ff83 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:11:47 +0100 Subject: Write tests for bot/cogs/test_snekbox.py --- tests/bot/cogs/test_snekbox.py | 363 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 363 insertions(+) create mode 100644 tests/bot/cogs/test_snekbox.py diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py new file mode 100644 index 000000000..293efed0f --- /dev/null +++ b/tests/bot/cogs/test_snekbox.py @@ -0,0 +1,363 @@ +import asyncio +import logging +import unittest +from unittest.mock import MagicMock, Mock, call, patch + +from bot.cogs import snekbox +from bot.cogs.snekbox import Snekbox +from bot.constants import URLs +from tests.helpers import ( + AsyncContextManagerMock, AsyncMock, MockBot, MockContext, MockMessage, MockReaction, MockUser, async_test +) + + +class SnekboxTests(unittest.TestCase): + def setUp(self): + """Add mocked bot and cog to the instance.""" + self.bot = MockBot() + + self.mocked_post = MagicMock() + self.mocked_post.json = AsyncMock() + self.bot.http_session.post = MagicMock(return_value=AsyncContextManagerMock(self.mocked_post)) + + self.cog = Snekbox(bot=self.bot) + + @async_test + async def test_post_eval(self): + """Post the eval code to the URLs.snekbox_eval_api endpoint.""" + await self.cog.post_eval("import random") + self.bot.http_session.post.assert_called_once_with( + URLs.snekbox_eval_api, + json={"input": "import random"}, + raise_for_status=True + ) + + @async_test + async def test_upload_output_reject_too_long(self): + """Reject output longer than MAX_PASTE_LEN.""" + self.assertEqual(await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)), "too long to upload") + + @async_test + async def test_upload_output(self): + """Upload the eval output to the URLs.paste_service.format(key="documents") endpoint.""" + key = "RainbowDash" + self.mocked_post.json.return_value = {"key": key} + + self.assertEqual( + await self.cog.upload_output("My awesome output"), + URLs.paste_service.format(key=key) + ) + self.bot.http_session.post.assert_called_once_with( + URLs.paste_service.format(key="documents"), + data="My awesome output", + raise_for_status=True + ) + + @async_test + async def test_upload_output_gracefully_fallback_if_exception_during_request(self): + """Output upload gracefully fallback if the upload fail.""" + self.mocked_post.json.side_effect = Exception + log = logging.getLogger("bot.cogs.snekbox") + with self.assertLogs(logger=log, level='ERROR'): + await self.cog.upload_output('My awesome output!') + + @async_test + async def test_upload_output_gracefully_fallback_if_no_key_in_response(self): + """Output upload gracefully fallback if there is no key entry in the response body.""" + self.mocked_post.json.return_value = {} + self.assertEqual((await self.cog.upload_output('My awesome output!')), None) + + def test_prepare_input(self): + cases = ( + ('print("Hello world!")', 'print("Hello world!")', 'non-formatted'), + ('`print("Hello world!")`', 'print("Hello world!")', 'one line code block'), + ('```\nprint("Hello world!")```', 'print("Hello world!")', 'multiline code block'), + ('```py\nprint("Hello world!")```', 'print("Hello world!")', 'multiline python code block'), + ) + for case, expected, testname in cases: + with self.subTest(msg=f'Extract code from {testname}.', case=case, expected=expected): + self.assertEqual(self.cog.prepare_input(case), expected) + + def test_get_results_message(self): + """Return error and message according to the eval result.""" + cases = ( + ('ERROR', None, ('Your eval job has failed', 'ERROR')), + ('', 128 + snekbox.SIGKILL, ('Your eval job timed out or ran out of memory', '')), + ('', 255, ('Your eval job has failed', 'A fatal NsJail error occurred')) + ) + for stdout, returncode, expected in cases: + with self.subTest(stdout=stdout, returncode=returncode, expected=expected): + self.assertEqual(self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}), expected) + + @patch('bot.cogs.snekbox.Signals', side_effect=ValueError) + def test_get_results_message_invalid_signal(self, mock_Signals: Mock): + self.assertEqual( + self.cog.get_results_message({'stdout': '', 'returncode': 127}), + ('Your eval job has completed with return code 127', '') + ) + + @patch('bot.cogs.snekbox.Signals') + def test_get_results_message_valid_signal(self, mock_Signals: Mock): + mock_Signals.return_value.name = 'SIGTEST' + self.assertEqual( + self.cog.get_results_message({'stdout': '', 'returncode': 127}), + ('Your eval job has completed with return code 127 (SIGTEST)', '') + ) + + def test_get_status_emoji(self): + """Return emoji according to the eval result.""" + cases = ( + ('', -1, ':warning:'), + ('Hello world!', 0, ':white_check_mark:'), + ('Invalid beard size', -1, ':x:') + ) + for stdout, returncode, expected in cases: + with self.subTest(stdout=stdout, returncode=returncode, expected=expected): + self.assertEqual(self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}), expected) + + @async_test + async def test_format_output(self): + """Test output formatting.""" + self.cog.upload_output = AsyncMock(return_value='https://testificate.com/') + + too_many_lines = ( + '001 | v\n002 | e\n003 | r\n004 | y\n005 | l\n006 | o\n' + '007 | n\n008 | g\n009 | b\n010 | e\n011 | a\n... (truncated - too many lines)' + ) + too_long_too_many_lines = ( + "\n".join( + f"{i:03d} | {line}" for i, line in enumerate(['verylongbeard' * 10] * 15, 1) + )[:1000] + "\n... (truncated - too long, too many lines)" + ) + + cases = ( + ('', ('[No output]', None), 'No output'), + ('My awesome output', ('My awesome output', None), 'One line output'), + ('<@', ("<@\u200B", None), r'Convert <@ to <@\u200B'), + (' Date: Sun, 16 Feb 2020 11:41:56 -0800 Subject: Moderation: fix member not found error not being shown --- bot/cogs/moderation/infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f4e296df9..9ea17b2b3 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -313,6 +313,6 @@ class Infractions(InfractionScheduler, commands.Cog): async def cog_command_error(self, ctx: Context, error: Exception) -> None: """Send a notification to the invoking context on a Union failure.""" if isinstance(error, commands.BadUnionArgument): - if discord.User in error.converters: + if discord.User in error.converters or discord.Member in error.converters: await ctx.send(str(error.errors[0])) error.handled = True -- cgit v1.2.3 From a83d2683f72a750b1946df913749a5c4257ebb16 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 11:52:54 -0800 Subject: Error handler: create separate function to handle CheckFailure --- bot/cogs/error_handler.py | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 52893b2ee..bd47eecf8 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -100,21 +100,9 @@ class ErrorHandler(Cog): f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" ) - elif isinstance(e, NoPrivateMessage): - await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, BotMissingPermissions): - await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") - log.warning( - f"The bot is missing permissions to execute command {command}: {e.missing_perms}" - ) - elif isinstance(e, MissingPermissions): - log.debug( - f"{ctx.message.author} is missing permissions to invoke command {command}: " - f"{e.missing_perms}" - ) - elif isinstance(e, InChannelCheckFailure): - await ctx.send(e) - elif isinstance(e, (CheckFailure, CommandOnCooldown, DisabledCommand)): + elif isinstance(e, CheckFailure): + await self.handle_check_failure(ctx, e) + elif isinstance(e, (CommandOnCooldown, DisabledCommand)): log.debug( f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" @@ -138,8 +126,34 @@ class ErrorHandler(Cog): else: await self.handle_unexpected_error(ctx, e.original) else: + # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + @staticmethod + async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: + """Handle CheckFailure exceptions and its children.""" + command = ctx.command + + if isinstance(e, NoPrivateMessage): + await ctx.send("Sorry, this command can't be used in a private message!") + elif isinstance(e, BotMissingPermissions): + await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") + log.warning( + f"The bot is missing permissions to execute command {command}: {e.missing_perms}" + ) + elif isinstance(e, MissingPermissions): + log.debug( + f"{ctx.message.author} is missing permissions to invoke command {command}: " + f"{e.missing_perms}" + ) + elif isinstance(e, InChannelCheckFailure): + await ctx.send(e) + else: + log.debug( + f"Command {command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) + @staticmethod async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: """Generic handler for errors without an explicit handler.""" -- cgit v1.2.3 From eab4b16ccb2d5b6f3d0a8765e8741fe88fb03e27 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 11:58:48 -0800 Subject: Error handler: create separate function to handle ResponseCodeError --- bot/cogs/error_handler.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index bd47eecf8..97124cb15 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -109,20 +109,7 @@ class ErrorHandler(Cog): ) elif isinstance(e, CommandInvokeError): if isinstance(e.original, ResponseCodeError): - status = e.original.response.status - - if status == 404: - await ctx.send("There does not seem to be anything matching your query.") - elif status == 400: - content = await e.original.response.json() - log.debug(f"API responded with 400 for command {command}: %r.", content) - await ctx.send("According to the API, your request is malformed.") - elif 500 <= status < 600: - await ctx.send("Sorry, there seems to be an internal issue with the API.") - log.warning(f"API responded with {status} for command {command}") - else: - await ctx.send(f"Got an unexpected status code from the API (`{status}`).") - log.warning(f"Unexpected API response for command {command}: {status}") + await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) else: @@ -154,6 +141,22 @@ class ErrorHandler(Cog): f"{e.__class__.__name__}: {e}" ) + @staticmethod + async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: + """Handle ResponseCodeError exceptions.""" + if e.status == 404: + await ctx.send("There does not seem to be anything matching your query.") + elif e.status == 400: + content = await e.response.json() + log.debug(f"API responded with 400 for command {ctx.command}: %r.", content) + await ctx.send("According to the API, your request is malformed.") + elif 500 <= e.status < 600: + await ctx.send("Sorry, there seems to be an internal issue with the API.") + log.warning(f"API responded with {e.status} for command {ctx.command}") + else: + await ctx.send(f"Got an unexpected status code from the API (`{e.status}`).") + log.warning(f"Unexpected API response for command {ctx.command}: {e.status}") + @staticmethod async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: """Generic handler for errors without an explicit handler.""" -- cgit v1.2.3 From 29e3c3e46242866820b9c4461378ed4b2e3afb47 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:09:24 -0800 Subject: Error handler: log unhandled exceptions instead of re-raising --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 97124cb15..5eef045e8 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -165,9 +165,9 @@ class ErrorHandler(Cog): f"```{e.__class__.__name__}: {e}```" ) log.error( - f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" + f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", + exc_info=e ) - raise e def setup(bot: Bot) -> None: -- cgit v1.2.3 From 806c69f78c5751f6dc93bd8dcc6fff95436fe0ed Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:19:52 -0800 Subject: Error handler: move tag retrieval to a separate function --- bot/cogs/error_handler.py | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 5eef045e8..7078d425d 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -72,24 +72,8 @@ class ErrorHandler(Cog): # Try to look for a tag with the command's name if the command isn't found. if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): - if not ctx.channel.id == Channels.verification: - tags_get_command = self.bot.get_command("tags get") - ctx.invoked_from_error_handler = True - - log_msg = "Cancelling attempt to fall back to a tag due to failed checks." - try: - if not await tags_get_command.can_run(ctx): - log.debug(log_msg) - return - except CommandError as tag_error: - log.debug(log_msg) - await self.on_command_error(ctx, tag_error) - return - - # Return to not raise the exception - with contextlib.suppress(ResponseCodeError): - await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) - return + if ctx.channel.id != Channels.verification: + await self.try_get_tag(ctx) elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) @@ -116,6 +100,32 @@ class ErrorHandler(Cog): # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + async def try_get_tag(self, ctx: Context) -> None: + """ + Attempt to display a tag by interpreting the command name as a tag name. + + The invocation of tags get respects its checks. Any CommandErrors raised will be handled + by `on_command_error`, but the `invoked_from_error_handler` attribute will be added to + the context to prevent infinite recursion in the case of a CommandNotFound exception. + """ + tags_get_command = self.bot.get_command("tags get") + ctx.invoked_from_error_handler = True + + log_msg = "Cancelling attempt to fall back to a tag due to failed checks." + try: + if not await tags_get_command.can_run(ctx): + log.debug(log_msg) + return + except CommandError as tag_error: + log.debug(log_msg) + await self.on_command_error(ctx, tag_error) + return + + # Return to not raise the exception + with contextlib.suppress(ResponseCodeError): + await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) + return + @staticmethod async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" -- cgit v1.2.3 From fb30fb1427fa26d6cfd54fdb6a80e4e7552d808f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:31:42 -0800 Subject: Error handler: move help command retrieval to a separate function --- bot/cogs/error_handler.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 7078d425d..6a0aef13e 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -1,10 +1,12 @@ import contextlib import logging +import typing as t from discord.ext.commands import ( BadArgument, BotMissingPermissions, CheckFailure, + Command, CommandError, CommandInvokeError, CommandNotFound, @@ -53,18 +55,9 @@ class ErrorHandler(Cog): 10. Otherwise, handling is deferred to `handle_unexpected_error` """ command = ctx.command - parent = None - if command is not None: - parent = command.parent - - # Retrieve the help command for the invoked command. - if parent and command: - help_command = (self.bot.get_command("help"), parent.name, command.name) - elif command: - help_command = (self.bot.get_command("help"), command.name) - else: - help_command = (self.bot.get_command("help"),) + # TODO: use ctx.send_help() once PR #519 is merged. + help_command = await self.get_help_command(command) if hasattr(e, "handled"): log.trace(f"Command {command} had its error already handled locally; ignoring.") @@ -100,6 +93,20 @@ class ErrorHandler(Cog): # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + async def get_help_command(self, command: t.Optional[Command]) -> t.Tuple: + """Return the help command invocation args to display help for `command`.""" + parent = None + if command is not None: + parent = command.parent + + # Retrieve the help command for the invoked command. + if parent and command: + return self.bot.get_command("help"), parent.name, command.name + elif command: + return self.bot.get_command("help"), command.name + else: + return self.bot.get_command("help") + async def try_get_tag(self, ctx: Context) -> None: """ Attempt to display a tag by interpreting the command name as a tag name. -- cgit v1.2.3 From d263f948e57a71e23cf4e04d678a880a130f3884 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:44:58 -0800 Subject: Error handler: create separate function to handle UserInputError --- bot/cogs/error_handler.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 6a0aef13e..c7758d946 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -56,9 +56,6 @@ class ErrorHandler(Cog): """ command = ctx.command - # TODO: use ctx.send_help() once PR #519 is merged. - help_command = await self.get_help_command(command) - if hasattr(e, "handled"): log.trace(f"Command {command} had its error already handled locally; ignoring.") return @@ -67,16 +64,8 @@ class ErrorHandler(Cog): if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) - elif isinstance(e, BadArgument): - await ctx.send(f"Bad argument: {e}\n") - await ctx.invoke(*help_command) elif isinstance(e, UserInputError): - await ctx.send("Something about your input seems off. Check the arguments:") - await ctx.invoke(*help_command) - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) + await self.handle_user_input_error(ctx, e) elif isinstance(e, CheckFailure): await self.handle_check_failure(ctx, e) elif isinstance(e, (CommandOnCooldown, DisabledCommand)): @@ -133,6 +122,22 @@ class ErrorHandler(Cog): await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) return + async def handle_user_input_error(self, ctx: Context, e: UserInputError) -> None: + """Handle UserInputError exceptions and its children.""" + # TODO: use ctx.send_help() once PR #519 is merged. + help_command = await self.get_help_command(ctx.command) + + if isinstance(e, BadArgument): + await ctx.send(f"Bad argument: {e}\n") + await ctx.invoke(*help_command) + else: + await ctx.send("Something about your input seems off. Check the arguments:") + await ctx.invoke(*help_command) + log.debug( + f"Command {ctx.command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) + @staticmethod async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" -- cgit v1.2.3 From dbd879e715fe9eadee33d098282cb7b4d941df26 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:09:05 -0800 Subject: Error handler: simplify error imports Import the errors module and qualify the error types with it rather than importing a large list of error types. --- bot/cogs/error_handler.py | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index c7758d946..c65ada344 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -2,21 +2,7 @@ import contextlib import logging import typing as t -from discord.ext.commands import ( - BadArgument, - BotMissingPermissions, - CheckFailure, - Command, - CommandError, - CommandInvokeError, - CommandNotFound, - CommandOnCooldown, - DisabledCommand, - MissingPermissions, - NoPrivateMessage, - UserInputError, -) -from discord.ext.commands import Cog, Context +from discord.ext.commands import Cog, Command, Context, errors from bot.api import ResponseCodeError from bot.bot import Bot @@ -33,7 +19,7 @@ class ErrorHandler(Cog): self.bot = bot @Cog.listener() - async def on_command_error(self, ctx: Context, e: CommandError) -> None: + async def on_command_error(self, ctx: Context, e: errors.CommandError) -> None: """ Provide generic command error handling. @@ -61,19 +47,19 @@ class ErrorHandler(Cog): return # Try to look for a tag with the command's name if the command isn't found. - if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): + if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) - elif isinstance(e, UserInputError): + elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) - elif isinstance(e, CheckFailure): + elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) - elif isinstance(e, (CommandOnCooldown, DisabledCommand)): + elif isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): log.debug( f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" ) - elif isinstance(e, CommandInvokeError): + elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: @@ -112,7 +98,7 @@ class ErrorHandler(Cog): if not await tags_get_command.can_run(ctx): log.debug(log_msg) return - except CommandError as tag_error: + except errors.CommandError as tag_error: log.debug(log_msg) await self.on_command_error(ctx, tag_error) return @@ -122,12 +108,12 @@ class ErrorHandler(Cog): await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) return - async def handle_user_input_error(self, ctx: Context, e: UserInputError) -> None: + async def handle_user_input_error(self, ctx: Context, e: errors.UserInputError) -> None: """Handle UserInputError exceptions and its children.""" # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) - if isinstance(e, BadArgument): + if isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) else: @@ -139,18 +125,18 @@ class ErrorHandler(Cog): ) @staticmethod - async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: + async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" command = ctx.command - if isinstance(e, NoPrivateMessage): + if isinstance(e, errors.NoPrivateMessage): await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, BotMissingPermissions): + elif isinstance(e, errors.BotMissingPermissions): await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") log.warning( f"The bot is missing permissions to execute command {command}: {e.missing_perms}" ) - elif isinstance(e, MissingPermissions): + elif isinstance(e, errors.MissingPermissions): log.debug( f"{ctx.message.author} is missing permissions to invoke command {command}: " f"{e.missing_perms}" @@ -180,7 +166,7 @@ class ErrorHandler(Cog): log.warning(f"Unexpected API response for command {ctx.command}: {e.status}") @staticmethod - async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: + async def handle_unexpected_error(ctx: Context, e: errors.CommandError) -> None: """Generic handler for errors without an explicit handler.""" await ctx.send( f"Sorry, an unexpected error occurred. Please let us know!\n\n" -- cgit v1.2.3 From d2f94f4c1280716e32e13611f6c778f9d9d4efd3 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:15:55 -0800 Subject: Error handler: handle MissingRequiredArgument Send a message indicating which argument is missing. --- bot/cogs/error_handler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index c65ada344..ffb36d10a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -113,7 +113,10 @@ class ErrorHandler(Cog): # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) - if isinstance(e, errors.BadArgument): + if isinstance(e, errors.MissingRequiredArgument): + await ctx.send(f"Missing required argument `{e.param.name}`.") + await ctx.invoke(*help_command) + elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) else: -- cgit v1.2.3 From 6fa0ba18a6b4daa265e6716f9d360117378c67ab Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:17:03 -0800 Subject: Error handler: handle TooManyArguments Send a message specifying the error reason. --- bot/cogs/error_handler.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index ffb36d10a..5cf95e71a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -116,6 +116,9 @@ class ErrorHandler(Cog): if isinstance(e, errors.MissingRequiredArgument): await ctx.send(f"Missing required argument `{e.param.name}`.") await ctx.invoke(*help_command) + elif isinstance(e, errors.TooManyArguments): + await ctx.send(f"Too many arguments provided.") + await ctx.invoke(*help_command) elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) -- cgit v1.2.3 From 8cdbec386d50e5866dcfd7cc0aeee359bb182317 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:23:56 -0800 Subject: Error handler: handle BadUnionArgument Send a message specifying the parameter name, the converters used, and the last error message from the converters. --- bot/cogs/error_handler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 5cf95e71a..d67261fc6 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -122,6 +122,8 @@ class ErrorHandler(Cog): elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) + elif isinstance(e, errors.BadUnionArgument): + await ctx.send(f"Bad argument: {e}\n```{e.errors[-1]}```") else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) -- cgit v1.2.3 From 4116aca0218138dd1a97db39b942a886945fa05b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 17:23:43 -0800 Subject: Error handler: handle ArgumentParsingError Simply send the error message with the help command. --- bot/cogs/error_handler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index d67261fc6..07b93283d 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -124,6 +124,8 @@ class ErrorHandler(Cog): await ctx.invoke(*help_command) elif isinstance(e, errors.BadUnionArgument): await ctx.send(f"Bad argument: {e}\n```{e.errors[-1]}```") + elif isinstance(e, errors.ArgumentParsingError): + await ctx.send(f"Argument parsing error: {e}") else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) -- cgit v1.2.3 From 476d5e7851f5b53ece319f023eeca88ae5c345eb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 17:38:41 -0800 Subject: Error handler: (almost) always log the error being handled The log level is debug for most errors and it's mainly useful for precisely that - debugging. This is why some "useless" errors are also logged e.g. CommandNotFound. Unexpected errors and some API errors will still have higher levels. * Add a single log statement to the end of the handler to cover UserInputError, CheckFailure, and CommandNotFound (when it's not trying to get a tag) * Log 404s from API --- bot/cogs/error_handler.py | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 07b93283d..ff8b36ddc 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -50,23 +50,26 @@ class ErrorHandler(Cog): if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) + return # Exit early to avoid logging. elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) - elif isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) - else: - # MaxConcurrencyReached, ExtensionError + return # Exit early to avoid logging. + elif not isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): + # ConversionError, MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + return # Exit early to avoid logging. + + log.debug( + f"Command {command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) async def get_help_command(self, command: t.Optional[Command]) -> t.Tuple: """Return the help command invocation args to display help for `command`.""" @@ -129,10 +132,6 @@ class ErrorHandler(Cog): else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) - log.debug( - f"Command {ctx.command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: @@ -153,17 +152,13 @@ class ErrorHandler(Cog): ) elif isinstance(e, InChannelCheckFailure): await ctx.send(e) - else: - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) @staticmethod async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: """Handle ResponseCodeError exceptions.""" if e.status == 404: await ctx.send("There does not seem to be anything matching your query.") + log.debug(f"API responded with 404 for command {ctx.command}") elif e.status == 400: content = await e.response.json() log.debug(f"API responded with 400 for command {ctx.command}: %r.", content) -- cgit v1.2.3 From 9bfdf7e3e95c07a1b0369c9fa8bdb4c91339732f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 18:09:08 -0800 Subject: Error handler: simplify check failure handler & handle bot missing roles discord.py's default error messages are quite descriptive already so there really isn't a need to write our own. Therefore, the log calls were removed so that the generic debug log message is used in the on_command_error. In addition to handling missing bot permissions, missing bot roles are also handled. The message doesn't specify which because it doesn't really matter to the end-user. The logs will use the default error messages as described above, and those will contain the specific roles or permissions that are missing. --- bot/cogs/error_handler.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index ff8b36ddc..6c4074e3a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -136,21 +136,17 @@ class ErrorHandler(Cog): @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" - command = ctx.command + bot_missing_errors = ( + errors.BotMissingPermissions, + errors.BotMissingRole, + errors.BotMissingAnyRole + ) - if isinstance(e, errors.NoPrivateMessage): - await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, errors.BotMissingPermissions): - await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") - log.warning( - f"The bot is missing permissions to execute command {command}: {e.missing_perms}" - ) - elif isinstance(e, errors.MissingPermissions): - log.debug( - f"{ctx.message.author} is missing permissions to invoke command {command}: " - f"{e.missing_perms}" + if isinstance(e, bot_missing_errors): + await ctx.send( + f"Sorry, it looks like I don't have the permissions or roles I need to do that." ) - elif isinstance(e, InChannelCheckFailure): + elif isinstance(e, (InChannelCheckFailure, errors.NoPrivateMessage)): await ctx.send(e) @staticmethod -- cgit v1.2.3 From aa84854f942d68f5245d2ca99612dfdd6ad167ce Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 19:01:42 -0800 Subject: Error handler: update docstrings to reflect recent changes --- bot/cogs/error_handler.py | 59 +++++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 6c4074e3a..d2c806566 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -23,22 +23,22 @@ class ErrorHandler(Cog): """ Provide generic command error handling. - Error handling is deferred to any local error handler, if present. - - Error handling emits a single error response, prioritized as follows: - 1. If the name fails to match a command but matches a tag, the tag is invoked - 2. Send a BadArgument error message to the invoking context & invoke the command's help - 3. Send a UserInputError error message to the invoking context & invoke the command's help - 4. Send a NoPrivateMessage error message to the invoking context - 5. Send a BotMissingPermissions error message to the invoking context - 6. Log a MissingPermissions error, no message is sent - 7. Send a InChannelCheckFailure error message to the invoking context - 8. Log CheckFailure, CommandOnCooldown, and DisabledCommand errors, no message is sent - 9. For CommandInvokeErrors, response is based on the type of error: - * 404: Error message is sent to the invoking context - * 400: Log the resopnse JSON, no message is sent - * 500 <= status <= 600: Error message is sent to the invoking context - 10. Otherwise, handling is deferred to `handle_unexpected_error` + Error handling is deferred to any local error handler, if present. This is done by + checking for the presence of a `handled` attribute on the error. + + Error handling emits a single error message in the invoking context `ctx` and a log message, + prioritised as follows: + + 1. If the name fails to match a command but matches a tag, the tag is invoked + * If CommandNotFound is raised when invoking the tag (determined by the presence of the + `invoked_from_error_handler` attribute), this error is treated as being unexpected + and therefore sends an error message + * Commands in the verification channel are ignored + 2. UserInputError: see `handle_user_input_error` + 3. CheckFailure: see `handle_check_failure` + 4. ResponseCodeError: see `handle_api_error` + 5. Otherwise, if not a CommandOnCooldown or DisabledCommand, handling is deferred to + `handle_unexpected_error` """ command = ctx.command @@ -112,7 +112,16 @@ class ErrorHandler(Cog): return async def handle_user_input_error(self, ctx: Context, e: errors.UserInputError) -> None: - """Handle UserInputError exceptions and its children.""" + """ + Send an error message in `ctx` for UserInputError, sometimes invoking the help command too. + + * MissingRequiredArgument: send an error message with arg name and the help command + * TooManyArguments: send an error message and the help command + * BadArgument: send an error message and the help command + * BadUnionArgument: send an error message including the error produced by the last converter + * ArgumentParsingError: send an error message + * Other: send an error message and the help command + """ # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) @@ -135,7 +144,17 @@ class ErrorHandler(Cog): @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: - """Handle CheckFailure exceptions and its children.""" + """ + Send an error message in `ctx` for certain types of CheckFailure. + + The following types are handled: + + * BotMissingPermissions + * BotMissingRole + * BotMissingAnyRole + * NoPrivateMessage + * InChannelCheckFailure + """ bot_missing_errors = ( errors.BotMissingPermissions, errors.BotMissingRole, @@ -151,7 +170,7 @@ class ErrorHandler(Cog): @staticmethod async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: - """Handle ResponseCodeError exceptions.""" + """Send an error message in `ctx` for ResponseCodeError and log it.""" if e.status == 404: await ctx.send("There does not seem to be anything matching your query.") log.debug(f"API responded with 404 for command {ctx.command}") @@ -168,7 +187,7 @@ class ErrorHandler(Cog): @staticmethod async def handle_unexpected_error(ctx: Context, e: errors.CommandError) -> None: - """Generic handler for errors without an explicit handler.""" + """Send a generic error message in `ctx` and log the exception as an error with exc_info.""" await ctx.send( f"Sorry, an unexpected error occurred. Please let us know!\n\n" f"```{e.__class__.__name__}: {e}```" -- cgit v1.2.3 From b3fdfa71ceeecbd9fe62cadc240fcad27bad7a32 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 19:06:22 -0800 Subject: Error handler: handle CommandOnCooldown errors Simply send the error's default message to the invoking context. --- bot/cogs/error_handler.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index d2c806566..347ce93ae 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -36,9 +36,9 @@ class ErrorHandler(Cog): * Commands in the verification channel are ignored 2. UserInputError: see `handle_user_input_error` 3. CheckFailure: see `handle_check_failure` - 4. ResponseCodeError: see `handle_api_error` - 5. Otherwise, if not a CommandOnCooldown or DisabledCommand, handling is deferred to - `handle_unexpected_error` + 4. CommandOnCooldown: send an error message in the invoking context + 5. ResponseCodeError: see `handle_api_error` + 6. Otherwise, if not a DisabledCommand, handling is deferred to `handle_unexpected_error` """ command = ctx.command @@ -55,13 +55,15 @@ class ErrorHandler(Cog): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) + elif isinstance(e, errors.CommandOnCooldown): + await ctx.send(e) elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) return # Exit early to avoid logging. - elif not isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): + elif not isinstance(e, errors.DisabledCommand): # ConversionError, MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) return # Exit early to avoid logging. -- cgit v1.2.3 From 68cf5d8eb9721cb1d91ba004409b82b0a283782d Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 17:48:53 +0100 Subject: Use pregenerated partials This avoid recreating partials for each re-eval --- bot/cogs/snekbox.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 3fc8d9937..d075c4fd5 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -203,6 +203,9 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") + _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) + _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) + while True: self.jobs[ctx.author.id] = datetime.datetime.now() code = self.prepare_input(code) @@ -234,13 +237,13 @@ class Snekbox(Cog): try: _, new_message = await self.bot.wait_for( 'message_edit', - check=partial(predicate_eval_message_edit, ctx), + check=_predicate_eval_message_edit, timeout=10 ) await ctx.message.add_reaction('🔁') await self.bot.wait_for( 'reaction_add', - check=partial(predicate_eval_emoji_reaction, ctx), + check=_predicate_emoji_reaction, timeout=10 ) -- cgit v1.2.3 From e1e68ad561513cc02eac0eab59990430fbcfe516 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:00:09 +0100 Subject: Suppress HTTPException while deleting bot output It was triggering an error if the user deleted the output before re-evaluating --- bot/cogs/snekbox.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index d075c4fd5..42830fb58 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import datetime import logging import re @@ -7,7 +8,7 @@ from functools import partial from signal import Signals from typing import Optional, Tuple -from discord import Message, Reaction, User +from discord import HTTPException, Message, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -250,7 +251,8 @@ class Snekbox(Cog): log.info(f"Re-evaluating message {ctx.message.id}") code = new_message.content.split(' ', maxsplit=1)[1] await ctx.message.clear_reactions() - await response.delete() + with contextlib.suppress(HTTPException): + await response.delete() except asyncio.TimeoutError: await ctx.message.clear_reactions() return -- cgit v1.2.3 From 8b386b533662e7f4be44cc57b9d9c63fde8a7ebf Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:14:31 +0100 Subject: Snekbox small refactoring Makes the code a bit clearer Co-authored-by: Shirayuki Nekomata --- bot/cogs/snekbox.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 42830fb58..efa4696b5 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -158,8 +158,8 @@ class Snekbox(Cog): lines = output.count("\n") if lines > 0: - output = output.split("\n")[:11] # Only first 11 cause the rest is truncated anyway - output = (f"{i:03d} | {line}" for i, line in enumerate(output, 1)) + output = [f"{i:03d} | {line}" for i, line in enumerate(output.split('\n'), 1)] + output = output[:11] # Limiting to only 11 lines output = "\n".join(output) if lines > 10: @@ -175,8 +175,7 @@ class Snekbox(Cog): if truncated: paste_link = await self.upload_output(original_output) - if not output: - output = "[No output]" + output = output or "[No output]" return output, paste_link -- cgit v1.2.3 From 33769405549efc3c0571cd1b2da5fa0b59ec742a Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:23:38 +0100 Subject: Split assertion onto separate lines Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 293efed0f..b5190cd7f 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -35,7 +35,8 @@ class SnekboxTests(unittest.TestCase): @async_test async def test_upload_output_reject_too_long(self): """Reject output longer than MAX_PASTE_LEN.""" - self.assertEqual(await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)), "too long to upload") + result = await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)) + self.assertEqual(result, "too long to upload") @async_test async def test_upload_output(self): -- cgit v1.2.3 From 2974d489494370f364c44899a529f5e93c89bedc Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:25:55 +0100 Subject: Split assertions onto separate lines Reads better as separate lines Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index b5190cd7f..01525110d 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -88,7 +88,8 @@ class SnekboxTests(unittest.TestCase): ) for stdout, returncode, expected in cases: with self.subTest(stdout=stdout, returncode=returncode, expected=expected): - self.assertEqual(self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}), expected) + actual = self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}) + self.assertEqual(actual, expected) @patch('bot.cogs.snekbox.Signals', side_effect=ValueError) def test_get_results_message_invalid_signal(self, mock_Signals: Mock): @@ -114,7 +115,8 @@ class SnekboxTests(unittest.TestCase): ) for stdout, returncode, expected in cases: with self.subTest(stdout=stdout, returncode=returncode, expected=expected): - self.assertEqual(self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}), expected) + actual = self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}) + self.assertEqual(actual, expected) @async_test async def test_format_output(self): @@ -321,7 +323,8 @@ class SnekboxTests(unittest.TestCase): with self.subTest(msg=f'Messages with {testname} return {expected}'): ctx = MockContext() ctx.message = ctx_msg - self.assertEqual(snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg), expected) + actual = snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg) + self.assertEqual(actual, expected) def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" @@ -351,7 +354,8 @@ class SnekboxTests(unittest.TestCase): ) for reaction, user, expected, testname in cases: with self.subTest(msg=f'Test with {testname} and expected return {expected}'): - self.assertEqual(snekbox.predicate_eval_emoji_reaction(valid_ctx, reaction, user), expected) + actual = snekbox.predicate_eval_emoji_reaction(valid_ctx, reaction, user) + self.assertEqual(actual, expected) class SnekboxSetupTests(unittest.TestCase): -- cgit v1.2.3 From dd9c250253a7bb24751f2de274dfa168efafb717 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:29:22 +0100 Subject: Delete additional informations from subtest Reduce visual clutter Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 01525110d..f1f03ab2f 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -76,7 +76,7 @@ class SnekboxTests(unittest.TestCase): ('```py\nprint("Hello world!")```', 'print("Hello world!")', 'multiline python code block'), ) for case, expected, testname in cases: - with self.subTest(msg=f'Extract code from {testname}.', case=case, expected=expected): + with self.subTest(msg=f'Extract code from {testname}.'): self.assertEqual(self.cog.prepare_input(case), expected) def test_get_results_message(self): -- cgit v1.2.3 From b5a1bf7c6ca467cef40a429cc8ca02314526801c Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:30:39 +0100 Subject: Use a space instead of an empty string in test_get_status_emoji Because of the stripping, it should still be considered as empty Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index f1f03ab2f..94ff685c4 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -109,7 +109,7 @@ class SnekboxTests(unittest.TestCase): def test_get_status_emoji(self): """Return emoji according to the eval result.""" cases = ( - ('', -1, ':warning:'), + (' ', -1, ':warning:'), ('Hello world!', 0, ':white_check_mark:'), ('Invalid beard size', -1, ':x:') ) -- cgit v1.2.3 From c1a998ca246e153333438ff91bce200bf74cc0f5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:38:24 +0100 Subject: Assert return value of Snekbox.post_eval --- tests/bot/cogs/test_snekbox.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 94ff685c4..cf07aad71 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -25,7 +25,9 @@ class SnekboxTests(unittest.TestCase): @async_test async def test_post_eval(self): """Post the eval code to the URLs.snekbox_eval_api endpoint.""" - await self.cog.post_eval("import random") + self.mocked_post.json.return_value = {'lemon': 'AI'} + + self.assertEqual(await self.cog.post_eval("import random"), {'lemon': 'AI'}) self.bot.http_session.post.assert_called_once_with( URLs.snekbox_eval_api, json={"input": "import random"}, -- cgit v1.2.3 From 12f43fc09406dee8cb1b36757fc2af7ae799a9d5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:44:00 +0100 Subject: Use kwargs to set mock attributes --- tests/bot/cogs/test_snekbox.py | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index cf07aad71..112c923c8 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -306,15 +306,9 @@ class SnekboxTests(unittest.TestCase): def test_predicate_eval_message_edit(self): """Test the predicate_eval_message_edit function.""" - msg0 = MockMessage() - msg0.id = 1 - msg0.content = 'abc' - msg1 = MockMessage() - msg1.id = 2 - msg1.content = 'abcdef' - msg2 = MockMessage() - msg2.id = 1 - msg2.content = 'abcdef' + msg0 = MockMessage(id=1, content='abc') + msg1 = MockMessage(id=2, content='abcdef') + msg2 = MockMessage(id=1, content='abcdef') cases = ( (msg0, msg0, False, 'same ID, same content'), @@ -323,29 +317,21 @@ class SnekboxTests(unittest.TestCase): ) for ctx_msg, new_msg, expected, testname in cases: with self.subTest(msg=f'Messages with {testname} return {expected}'): - ctx = MockContext() - ctx.message = ctx_msg + ctx = MockContext(message=ctx_msg) actual = snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg) self.assertEqual(actual, expected) def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" - valid_reaction = MockReaction() - valid_reaction.message.id = 1 + valid_reaction = MockReaction(message=MockMessage(id=1)) valid_reaction.__str__.return_value = '🔁' - valid_ctx = MockContext() - valid_ctx.message.id = 1 - valid_ctx.author.id = 2 - valid_user = MockUser() - valid_user.id = 2 - - invalid_reaction_id = MockReaction() - invalid_reaction_id.message.id = 42 + valid_ctx = MockContext(message=MockMessage(id=1), author=MockUser(id=2)) + valid_user = MockUser(id=2) + + invalid_reaction_id = MockReaction(message=MockMessage(id=42)) invalid_reaction_id.__str__.return_value = '🔁' - invalid_user_id = MockUser() - invalid_user_id.id = 42 - invalid_reaction_str = MockReaction() - invalid_reaction_str.message.id = 1 + invalid_user_id = MockUser(id=42) + invalid_reaction_str = MockReaction(message=MockMessage(id=1)) invalid_reaction_str.__str__.return_value = ':longbeard:' cases = ( -- cgit v1.2.3 From f12d76dca1073b489b4a407a17dbab623aa0dce5 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:18:01 -0800 Subject: Config: rename roles to match their names in the guild --- config-default.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/config-default.yml b/config-default.yml index 379475907..5755b2191 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,20 +160,20 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - admin: &ADMIN_ROLE 267628507062992896 - announcements: 463658397560995840 - champion: 430492892331769857 - contributor: 295488872404484098 - core_developer: 587606783669829632 - helpers: 267630620367257601 - jammer: 591786436651646989 - moderator: &MOD_ROLE 267629731250176001 - muted: &MUTED_ROLE 277914926603829249 - owner: &OWNER_ROLE 267627879762755584 - partners: 323426753857191936 - rockstars: &ROCKSTARS_ROLE 458226413825294336 - team_leader: 501324292341104650 - verified: 352427296948486144 + admins: &ADMINS_ROLE 267628507062992896 + announcements: 463658397560995840 + code_jam_champions: 430492892331769857 + contributors: 295488872404484098 + core_developers: 587606783669829632 + developers: 352427296948486144 + helpers: 267630620367257601 + jammers: 591786436651646989 + moderators: &MODS_ROLE 267629731250176001 + muted: &MUTED_ROLE 277914926603829249 + owners: &OWNERS_ROLE 267627879762755584 + partners: 323426753857191936 + python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 + team_leaders: 501324292341104650 webhooks: talent_pool: 569145364800602132 @@ -267,10 +267,10 @@ filter: - *USER_EVENT_A role_whitelist: - - *ADMIN_ROLE - - *MOD_ROLE - - *OWNER_ROLE - - *ROCKSTARS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + - *OWNERS_ROLE + - *PYTHON_COMMUNITY_ROLE keys: -- cgit v1.2.3 From ffdde0f8a2f14590baf47462143fbd685d851fad Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:20:54 -0800 Subject: Config: split roles into categories --- config-default.yml | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/config-default.yml b/config-default.yml index 5755b2191..203f9e64e 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,26 +160,30 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - admins: &ADMINS_ROLE 267628507062992896 announcements: 463658397560995840 - code_jam_champions: 430492892331769857 contributors: 295488872404484098 - core_developers: 587606783669829632 developers: 352427296948486144 - helpers: 267630620367257601 - jammers: 591786436651646989 - moderators: &MODS_ROLE 267629731250176001 muted: &MUTED_ROLE 277914926603829249 - owners: &OWNERS_ROLE 267627879762755584 partners: 323426753857191936 python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 - team_leaders: 501324292341104650 + + # Staff + admins: &ADMINS_ROLE 267628507062992896 + core_developers: 587606783669829632 + helpers: 267630620367257601 + moderators: &MODS_ROLE 267629731250176001 + owners: &OWNERS_ROLE 267627879762755584 + + # Code Jam + code_jam_champions: 430492892331769857 + jammers: 591786436651646989 + team_leaders: 501324292341104650 webhooks: - talent_pool: 569145364800602132 - big_brother: 569133704568373283 - reddit: 635408384794951680 - duck_pond: 637821475327311927 + talent_pool: 569145364800602132 + big_brother: 569133704568373283 + reddit: 635408384794951680 + duck_pond: 637821475327311927 filter: -- cgit v1.2.3 From bf7dd8c17dea3d0c12139893c210500fdad820f9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:46:12 -0800 Subject: Config: split channels into categories --- config-default.yml | 60 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/config-default.yml b/config-default.yml index 203f9e64e..415ed13f4 100644 --- a/config-default.yml +++ b/config-default.yml @@ -113,19 +113,32 @@ guild: python_help: 356013061213126657 channels: - admins: &ADMINS 365960823622991872 - admin_spam: &ADMIN_SPAM 563594791770914816 - admins_voice: &ADMINS_VOICE 500734494840717332 announcements: 354619224620138496 - attachment_log: &ATTCH_LOG 649243850006855680 - big_brother_logs: &BBLOGS 468507907357409333 - bot: &BOT_CMD 267659945086812160 checkpoint_test: 422077681434099723 - defcon: &DEFCON 464469101889454091 + user_event_a: &USER_EVENT_A 592000283102674944 + + # Development devcontrib: &DEV_CONTRIB 635950537262759947 devlog: &DEVLOG 622895325144940554 devtest: &DEVTEST 414574275865870337 - esoteric: 470884583684964352 + + # Discussion + meta: 429409067623251969 + python: 267624335836053506 + + # Logs + attachment_log: &ATTCH_LOG 649243850006855680 + message_log: &MESSAGE_LOG 467752170159079424 + modlog: &MODLOG 282638479504965634 + userlog: 528976905546760203 + voice_log: 640292421988646961 + + # Off-topic + off_topic_0: 291284109232308226 + off_topic_1: 463035241142026251 + off_topic_2: 463035268514185226 + + # Python Help help_0: 303906576991780866 help_1: 303906556754395136 help_2: 303906514266226689 @@ -134,26 +147,31 @@ guild: help_5: 454941769734422538 help_6: 587375753306570782 help_7: 587375768556797982 + + # Special + bot: &BOT_CMD 267659945086812160 + esoteric: 470884583684964352 + reddit: 458224812528238616 + verification: 352442727016693763 + + # Staff + admins: &ADMINS 365960823622991872 + admin_spam: &ADMIN_SPAM 563594791770914816 + defcon: &DEFCON 464469101889454091 helpers: &HELPERS 385474242440986624 - message_log: &MESSAGE_LOG 467752170159079424 - meta: 429409067623251969 - mod_spam: &MOD_SPAM 620607373828030464 mods: &MODS 305126844661760000 mod_alerts: 473092532147060736 - modlog: &MODLOG 282638479504965634 - off_topic_0: 291284109232308226 - off_topic_1: 463035241142026251 - off_topic_2: 463035268514185226 + mod_spam: &MOD_SPAM 620607373828030464 organisation: &ORGANISATION 551789653284356126 - python: 267624335836053506 - reddit: 458224812528238616 staff_lounge: &STAFF_LOUNGE 464905259261755392 + + # Voice + admins_voice: &ADMINS_VOICE 500734494840717332 staff_voice: &STAFF_VOICE 412375055910043655 + + # Watch + big_brother_logs: &BBLOGS 468507907357409333 talent_pool: &TALENT_POOL 534321732593647616 - userlog: 528976905546760203 - user_event_a: &USER_EVENT_A 592000283102674944 - verification: 352442727016693763 - voice_log: 640292421988646961 staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] -- cgit v1.2.3 From 707d2f7208e7008f94bfff1a1e13664dc38c6d6c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:47:11 -0800 Subject: Config: shorten name of PYTHON_COMMUNITY_ROLE --- config-default.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/config-default.yml b/config-default.yml index 415ed13f4..1e478154f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -178,12 +178,12 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - announcements: 463658397560995840 - contributors: 295488872404484098 - developers: 352427296948486144 - muted: &MUTED_ROLE 277914926603829249 - partners: 323426753857191936 - python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 + announcements: 463658397560995840 + contributors: 295488872404484098 + developers: 352427296948486144 + muted: &MUTED_ROLE 277914926603829249 + partners: 323426753857191936 + python_community: &PY_COMMUNITY_ROLE 458226413825294336 # Staff admins: &ADMINS_ROLE 267628507062992896 @@ -292,7 +292,7 @@ filter: - *ADMINS_ROLE - *MODS_ROLE - *OWNERS_ROLE - - *PYTHON_COMMUNITY_ROLE + - *PY_COMMUNITY_ROLE keys: -- cgit v1.2.3 From 8c1a31b7d043facd876b22800e8cb44ae15d9492 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:50:21 -0800 Subject: Config: remove checkpoint_test and devtest They no longer exist in the guild. * Move devlog under the "Logs" category --- bot/cogs/bot.py | 1 - bot/cogs/tags.py | 1 - bot/constants.py | 2 -- config-default.yml | 2 -- 4 files changed, 6 deletions(-) diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index 73b1e8f41..74e882e0e 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -40,7 +40,6 @@ class BotCog(Cog, name="Bot"): # These channels will also work, but will not be subject to cooldown self.channel_whitelist = ( Channels.bot, - Channels.devtest, ) # Stores improperly formatted Python codeblock message ids and the corresponding bot message diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index b6360dfae..a38f5617f 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -15,7 +15,6 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) TEST_CHANNELS = ( - Channels.devtest, Channels.bot, Channels.helpers ) diff --git a/bot/constants.py b/bot/constants.py index 681d8da49..15f078cbf 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -363,11 +363,9 @@ class Channels(metaclass=YAMLGetter): attachment_log: int big_brother_logs: int bot: int - checkpoint_test: int defcon: int devcontrib: int devlog: int - devtest: int esoteric: int help_0: int help_1: int diff --git a/config-default.yml b/config-default.yml index 1e478154f..058317262 100644 --- a/config-default.yml +++ b/config-default.yml @@ -114,13 +114,11 @@ guild: channels: announcements: 354619224620138496 - checkpoint_test: 422077681434099723 user_event_a: &USER_EVENT_A 592000283102674944 # Development devcontrib: &DEV_CONTRIB 635950537262759947 devlog: &DEVLOG 622895325144940554 - devtest: &DEVTEST 414574275865870337 # Discussion meta: 429409067623251969 -- cgit v1.2.3 From a850a32135ef3b454abfe2ad017a46badd73d3c1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:54:58 -0800 Subject: Constants: rename roles to match their names in the guild --- bot/cogs/defcon.py | 12 ++++++------ bot/cogs/eval.py | 4 ++-- bot/cogs/extensions.py | 2 +- bot/cogs/jams.py | 6 +++--- bot/cogs/moderation/scheduler.py | 2 +- bot/cogs/snekbox.py | 2 +- bot/cogs/tags.py | 2 +- bot/cogs/verification.py | 2 +- bot/constants.py | 22 +++++++++++----------- tests/bot/cogs/test_information.py | 2 +- 10 files changed, 28 insertions(+), 28 deletions(-) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index a0d8fedd5..c7ea1f2bf 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -69,7 +69,7 @@ class Defcon(Cog): except Exception: # Yikes! log.exception("Unable to get DEFCON settings!") await self.bot.get_channel(Channels.devlog).send( - f"<@&{Roles.admin}> **WARNING**: Unable to get DEFCON settings!" + f"<@&{Roles.admins}> **WARNING**: Unable to get DEFCON settings!" ) else: @@ -118,7 +118,7 @@ class Defcon(Cog): ) @group(name='defcon', aliases=('dc',), invoke_without_command=True) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def defcon_group(self, ctx: Context) -> None: """Check the DEFCON status or run a subcommand.""" await ctx.invoke(self.bot.get_command("help"), "defcon") @@ -146,7 +146,7 @@ class Defcon(Cog): await self.send_defcon_log(action, ctx.author, error) @defcon_group.command(name='enable', aliases=('on', 'e')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def enable_command(self, ctx: Context) -> None: """ Enable DEFCON mode. Useful in a pinch, but be sure you know what you're doing! @@ -159,7 +159,7 @@ class Defcon(Cog): await self.update_channel_topic() @defcon_group.command(name='disable', aliases=('off', 'd')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def disable_command(self, ctx: Context) -> None: """Disable DEFCON mode. Useful in a pinch, but be sure you know what you're doing!""" self.enabled = False @@ -167,7 +167,7 @@ class Defcon(Cog): await self.update_channel_topic() @defcon_group.command(name='status', aliases=('s',)) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def status_command(self, ctx: Context) -> None: """Check the current status of DEFCON mode.""" embed = Embed( @@ -179,7 +179,7 @@ class Defcon(Cog): await ctx.send(embed=embed) @defcon_group.command(name='days') - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def days_command(self, ctx: Context, days: int) -> None: """Set how old an account must be to join the server, in days, with DEFCON mode enabled.""" self.days = timedelta(days=days) diff --git a/bot/cogs/eval.py b/bot/cogs/eval.py index 9c729f28a..52136fc8d 100644 --- a/bot/cogs/eval.py +++ b/bot/cogs/eval.py @@ -174,14 +174,14 @@ async def func(): # (None,) -> Any await ctx.send(f"```py\n{out}```", embed=embed) @group(name='internal', aliases=('int',)) - @with_role(Roles.owner, Roles.admin) + @with_role(Roles.owners, Roles.admins) async def internal_group(self, ctx: Context) -> None: """Internal commands. Top secret!""" if not ctx.invoked_subcommand: await ctx.invoke(self.bot.get_command("help"), "internal") @internal_group.command(name='eval', aliases=('e',)) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def eval(self, ctx: Context, *, code: str) -> None: """Run eval in a REPL-like format.""" code = code.strip("`") diff --git a/bot/cogs/extensions.py b/bot/cogs/extensions.py index f16e79fb7..b312e1a1d 100644 --- a/bot/cogs/extensions.py +++ b/bot/cogs/extensions.py @@ -221,7 +221,7 @@ class Extensions(commands.Cog): # This cannot be static (must have a __func__ attribute). def cog_check(self, ctx: Context) -> bool: """Only allow moderators and core developers to invoke the commands in this cog.""" - return with_role_check(ctx, *MODERATION_ROLES, Roles.core_developer) + return with_role_check(ctx, *MODERATION_ROLES, Roles.core_developers) # This cannot be static (must have a __func__ attribute). async def cog_command_error(self, ctx: Context, error: Exception) -> None: diff --git a/bot/cogs/jams.py b/bot/cogs/jams.py index 985f28ce5..1d062b0c2 100644 --- a/bot/cogs/jams.py +++ b/bot/cogs/jams.py @@ -18,7 +18,7 @@ class CodeJams(commands.Cog): self.bot = bot @commands.command() - @with_role(Roles.admin) + @with_role(Roles.admins) async def createteam(self, ctx: commands.Context, team_name: str, members: commands.Greedy[Member]) -> None: """ Create team channels (voice and text) in the Code Jams category, assign roles, and add overwrites for the team. @@ -95,10 +95,10 @@ class CodeJams(commands.Cog): ) # Assign team leader role - await members[0].add_roles(ctx.guild.get_role(Roles.team_leader)) + await members[0].add_roles(ctx.guild.get_role(Roles.team_leaders)) # Assign rest of roles - jammer_role = ctx.guild.get_role(Roles.jammer) + jammer_role = ctx.guild.get_role(Roles.jammers) for member in members: await member.add_roles(jammer_role) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index c0de0e4da..db1a3030e 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -307,7 +307,7 @@ class InfractionScheduler(Scheduler): Infractions of unsupported types will raise a ValueError. """ guild = self.bot.get_guild(constants.Guild.id) - mod_role = guild.get_role(constants.Roles.moderator) + mod_role = guild.get_role(constants.Roles.moderators) user_id = infraction["user"] actor = infraction["actor"] type_ = infraction["type"] diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index da33e27b2..84457e38f 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -34,7 +34,7 @@ RAW_CODE_REGEX = re.compile( ) MAX_PASTE_LEN = 1000 -EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.rockstars, Roles.partners) +EVAL_ROLES = (Roles.helpers, Roles.moderators, Roles.admins, Roles.owners, Roles.python_community, Roles.partners) class Snekbox(Cog): diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index a38f5617f..2c4fa02bd 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -220,7 +220,7 @@ class Tags(Cog): )) @tags_group.command(name='delete', aliases=('remove', 'rm', 'd')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def delete_command(self, ctx: Context, *, tag_name: TagNameConverter) -> None: """Remove a tag from the database.""" await self.bot.api_client.delete(f'bot/tags/{tag_name}') diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 582237374..09bef80c4 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -38,7 +38,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! PERIODIC_PING = ( f"@everyone To verify that you have read our rules, please type `{BotConfig.prefix}accept`." - f" If you encounter any problems during the verification process, ping the <@&{Roles.admin}> role in this channel." + f" If you encounter any problems during the verification process, ping the <@&{Roles.admins}> role in this channel." ) BOT_MESSAGE_DELETE_DELAY = 10 diff --git a/bot/constants.py b/bot/constants.py index 15f078cbf..03578fefd 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -409,19 +409,19 @@ class Roles(metaclass=YAMLGetter): section = "guild" subsection = "roles" - admin: int + admins: int announcements: int - champion: int - contributor: int - core_developer: int + code_jam_champions: int + contributors: int + core_developers: int helpers: int - jammer: int - moderator: int + jammers: int + moderators: int muted: int - owner: int + owners: int partners: int - rockstars: int - team_leader: int + python_community: int + team_leaders: int verified: int # This is the Developers role on PyDis, here named verified for readability reasons. @@ -570,8 +570,8 @@ BOT_DIR = os.path.dirname(__file__) PROJECT_ROOT = os.path.abspath(os.path.join(BOT_DIR, os.pardir)) # Default role combinations -MODERATION_ROLES = Roles.moderator, Roles.admin, Roles.owner -STAFF_ROLES = Roles.helpers, Roles.moderator, Roles.admin, Roles.owner +MODERATION_ROLES = Roles.moderators, Roles.admins, Roles.owners +STAFF_ROLES = Roles.helpers, Roles.moderators, Roles.admins, Roles.owners # Roles combinations STAFF_CHANNELS = Guild.staff_channels diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index deae7ebad..38293269f 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -19,7 +19,7 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderators) def setUp(self): """Sets up fresh objects for each test.""" -- cgit v1.2.3 From ed25a7ae240cce1162a0a67fa6451363671cc7de Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:58:51 -0800 Subject: Constants: rename developers role back to verified It makes the code which uses it more readable. A comment was added to explain the discrepancy between the constant's name and the name in the guild. --- config-default.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config-default.yml b/config-default.yml index 058317262..dc5be5d47 100644 --- a/config-default.yml +++ b/config-default.yml @@ -178,11 +178,13 @@ guild: roles: announcements: 463658397560995840 contributors: 295488872404484098 - developers: 352427296948486144 muted: &MUTED_ROLE 277914926603829249 partners: 323426753857191936 python_community: &PY_COMMUNITY_ROLE 458226413825294336 + # This is the Developers role on PyDis, here named verified for readability reasons + verified: 352427296948486144 + # Staff admins: &ADMINS_ROLE 267628507062992896 core_developers: 587606783669829632 -- cgit v1.2.3 From e38990dc26fd77323e33da267ab8f16538f32438 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:59:38 -0800 Subject: Constants: remove code jam champions role Nothing was using it. --- bot/constants.py | 1 - config-default.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 03578fefd..98914fb9d 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -411,7 +411,6 @@ class Roles(metaclass=YAMLGetter): admins: int announcements: int - code_jam_champions: int contributors: int core_developers: int helpers: int diff --git a/config-default.yml b/config-default.yml index dc5be5d47..10cc34816 100644 --- a/config-default.yml +++ b/config-default.yml @@ -193,7 +193,6 @@ guild: owners: &OWNERS_ROLE 267627879762755584 # Code Jam - code_jam_champions: 430492892331769857 jammers: 591786436651646989 team_leaders: 501324292341104650 -- cgit v1.2.3 From 734fd8ff1a0e5bc309b0d84e34b2b6a1d0c204d9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:16:32 -0800 Subject: Config: rename channels to match their names in the guild --- bot/cogs/bot.py | 4 +- bot/cogs/clean.py | 2 +- bot/cogs/defcon.py | 2 +- bot/cogs/free.py | 2 +- bot/cogs/help.py | 2 +- bot/cogs/information.py | 6 +-- bot/cogs/logging.py | 2 +- bot/cogs/moderation/modlog.py | 12 +++--- bot/cogs/snekbox.py | 2 +- bot/cogs/tags.py | 2 +- bot/cogs/utils.py | 2 +- bot/cogs/verification.py | 9 +++-- bot/constants.py | 12 +++--- config-default.yml | 93 +++++++++++++++++++++---------------------- 14 files changed, 76 insertions(+), 76 deletions(-) diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index 74e882e0e..f17135877 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -34,12 +34,12 @@ class BotCog(Cog, name="Bot"): Channels.help_5: 0, Channels.help_6: 0, Channels.help_7: 0, - Channels.python: 0, + Channels.python_discussion: 0, } # These channels will also work, but will not be subject to cooldown self.channel_whitelist = ( - Channels.bot, + Channels.bot_commands, ) # Stores improperly formatted Python codeblock message ids and the corresponding bot message diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 2104efe57..5cdf0b048 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -173,7 +173,7 @@ class Clean(Cog): colour=Colour(Colours.soft_red), title="Bulk message delete", text=message, - channel_id=Channels.modlog, + channel_id=Channels.mod_log, ) @group(invoke_without_command=True, name="clean", aliases=["purge"]) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index c7ea1f2bf..050760a71 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -68,7 +68,7 @@ class Defcon(Cog): except Exception: # Yikes! log.exception("Unable to get DEFCON settings!") - await self.bot.get_channel(Channels.devlog).send( + await self.bot.get_channel(Channels.dev_log).send( f"<@&{Roles.admins}> **WARNING**: Unable to get DEFCON settings!" ) diff --git a/bot/cogs/free.py b/bot/cogs/free.py index 49cab6172..02c02d067 100644 --- a/bot/cogs/free.py +++ b/bot/cogs/free.py @@ -22,7 +22,7 @@ class Free(Cog): PYTHON_HELP_ID = Categories.python_help @command(name="free", aliases=('f',)) - @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES) + @redirect_output(destination_channel=Channels.bot_commands, bypass_roles=STAFF_ROLES) async def free(self, ctx: Context, user: Member = None, seek: int = 2) -> None: """ Lists free help channels by likeliness of availability. diff --git a/bot/cogs/help.py b/bot/cogs/help.py index fd5bbc3ca..744722220 100644 --- a/bot/cogs/help.py +++ b/bot/cogs/help.py @@ -507,7 +507,7 @@ class Help(DiscordCog): """Custom Embed Pagination Help feature.""" @commands.command('help') - @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES) + @redirect_output(destination_channel=Channels.bot_commands, bypass_roles=STAFF_ROLES) async def new_help(self, ctx: Context, *commands) -> None: """Shows Command Help.""" try: diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 13c8aabaa..49beca15b 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -152,8 +152,8 @@ class Information(Cog): # Non-staff may only do this in #bot-commands if not with_role_check(ctx, *constants.STAFF_ROLES): - if not ctx.channel.id == constants.Channels.bot: - raise InChannelCheckFailure(constants.Channels.bot) + if not ctx.channel.id == constants.Channels.bot_commands: + raise InChannelCheckFailure(constants.Channels.bot_commands) embed = await self.create_user_embed(ctx, user) @@ -332,7 +332,7 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_channel(constants.Channels.bot, bypass_roles=constants.STAFF_ROLES) + @in_channel(constants.Channels.bot_commands, bypass_roles=constants.STAFF_ROLES) async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None: """Shows information about the raw API response.""" # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling diff --git a/bot/cogs/logging.py b/bot/cogs/logging.py index d1b7dcab3..9dcb1456b 100644 --- a/bot/cogs/logging.py +++ b/bot/cogs/logging.py @@ -34,7 +34,7 @@ class Logging(Cog): ) if not DEBUG_MODE: - await self.bot.get_channel(Channels.devlog).send(embed=embed) + await self.bot.get_channel(Channels.dev_log).send(embed=embed) def setup(bot: Bot) -> None: diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index e8ae0dbe6..94e646248 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -87,7 +87,7 @@ class ModLog(Cog, name="ModLog"): title: t.Optional[str], text: str, thumbnail: t.Optional[t.Union[str, discord.Asset]] = None, - channel_id: int = Channels.modlog, + channel_id: int = Channels.mod_log, ping_everyone: bool = False, files: t.Optional[t.List[discord.File]] = None, content: t.Optional[str] = None, @@ -377,7 +377,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_ban, Colours.soft_red, "User banned", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -399,7 +399,7 @@ class ModLog(Cog, name="ModLog"): Icons.sign_in, Colours.soft_green, "User joined", message, thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -416,7 +416,7 @@ class ModLog(Cog, name="ModLog"): Icons.sign_out, Colours.soft_red, "User left", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -433,7 +433,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_unban, Colour.blurple(), "User unbanned", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.modlog + channel_id=Channels.mod_log ) @Cog.listener() @@ -529,7 +529,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_update, Colour.blurple(), "Member updated", message, thumbnail=after.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 84457e38f..aef12546d 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -177,7 +177,7 @@ class Snekbox(Cog): @command(name="eval", aliases=("e",)) @guild_only() - @in_channel(Channels.bot, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) + @in_channel(Channels.bot_commands, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) async def eval_command(self, ctx: Context, *, code: str = None) -> None: """ Run Python code and get the results. diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index 2c4fa02bd..5da9a4148 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -15,7 +15,7 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) TEST_CHANNELS = ( - Channels.bot, + Channels.bot_commands, Channels.helpers ) diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index da278011a..94b9d6b5a 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -89,7 +89,7 @@ class Utils(Cog): await ctx.message.channel.send(embed=pep_embed) @command() - @in_channel(Channels.bot, bypass_roles=STAFF_ROLES) + @in_channel(Channels.bot_commands, bypass_roles=STAFF_ROLES) async def charinfo(self, ctx: Context, *, characters: str) -> None: """Shows you information on up to 25 unicode characters.""" match = re.match(r"<(a?):(\w+):(\d+)>", characters) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 09bef80c4..94bef3188 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -30,10 +30,11 @@ your information removed here as well. Feel free to review them at any point! Additionally, if you'd like to receive notifications for the announcements we post in <#{Channels.announcements}> \ -from time to time, you can send `!subscribe` to <#{Channels.bot}> at any time to assign yourself the \ +from time to time, you can send `!subscribe` to <#{Channels.bot_commands}> at any time to assign yourself the \ **Announcements** role. We'll mention this role every time we make an announcement. -If you'd like to unsubscribe from the announcement notifications, simply send `!unsubscribe` to <#{Channels.bot}>. +If you'd like to unsubscribe from the announcement notifications, simply send `!unsubscribe` to \ +<#{Channels.bot_commands}>. """ PERIODIC_PING = ( @@ -136,7 +137,7 @@ class Verification(Cog): await ctx.message.delete() @command(name='subscribe') - @in_channel(Channels.bot) + @in_channel(Channels.bot_commands) async def subscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Subscribe to announcement notifications by assigning yourself the role.""" has_role = False @@ -160,7 +161,7 @@ class Verification(Cog): ) @command(name='unsubscribe') - @in_channel(Channels.bot) + @in_channel(Channels.bot_commands) async def unsubscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Unsubscribe from announcement notifications by removing the role from yourself.""" has_role = False diff --git a/bot/constants.py b/bot/constants.py index 98914fb9d..f35d608da 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -362,10 +362,10 @@ class Channels(metaclass=YAMLGetter): announcements: int attachment_log: int big_brother_logs: int - bot: int + bot_commands: int defcon: int devcontrib: int - devlog: int + dev_log: int esoteric: int help_0: int help_1: int @@ -381,16 +381,16 @@ class Channels(metaclass=YAMLGetter): mod_spam: int mods: int mod_alerts: int - modlog: int + mod_log: int off_topic_0: int off_topic_1: int off_topic_2: int organisation: int - python: int + python_discussion: int reddit: int talent_pool: int - userlog: int - user_event_a: int + user_log: int + user_event_announcements: int verification: int voice_log: int diff --git a/config-default.yml b/config-default.yml index 10cc34816..44da694c2 100644 --- a/config-default.yml +++ b/config-default.yml @@ -110,69 +110,69 @@ guild: id: 267624335836053506 categories: - python_help: 356013061213126657 + python_help: 356013061213126657 channels: - announcements: 354619224620138496 - user_event_a: &USER_EVENT_A 592000283102674944 + announcements: 354619224620138496 + user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 - devlog: &DEVLOG 622895325144940554 + devcontrib: &DEV_CONTRIB 635950537262759947 + dev_log: &DEVLOG 622895325144940554 # Discussion - meta: 429409067623251969 - python: 267624335836053506 + meta: 429409067623251969 + python_discussion: 267624335836053506 # Logs - attachment_log: &ATTCH_LOG 649243850006855680 - message_log: &MESSAGE_LOG 467752170159079424 - modlog: &MODLOG 282638479504965634 - userlog: 528976905546760203 - voice_log: 640292421988646961 + attachment_log: &ATTACH_LOG 649243850006855680 + message_log: &MESSAGE_LOG 467752170159079424 + mod_log: &MOD_LOG 282638479504965634 + user_log: 528976905546760203 + voice_log: 640292421988646961 # Off-topic - off_topic_0: 291284109232308226 - off_topic_1: 463035241142026251 - off_topic_2: 463035268514185226 + off_topic_0: 291284109232308226 + off_topic_1: 463035241142026251 + off_topic_2: 463035268514185226 # Python Help - help_0: 303906576991780866 - help_1: 303906556754395136 - help_2: 303906514266226689 - help_3: 439702951246692352 - help_4: 451312046647148554 - help_5: 454941769734422538 - help_6: 587375753306570782 - help_7: 587375768556797982 + help_0: 303906576991780866 + help_1: 303906556754395136 + help_2: 303906514266226689 + help_3: 439702951246692352 + help_4: 451312046647148554 + help_5: 454941769734422538 + help_6: 587375753306570782 + help_7: 587375768556797982 # Special - bot: &BOT_CMD 267659945086812160 - esoteric: 470884583684964352 - reddit: 458224812528238616 - verification: 352442727016693763 + bot_commands: &BOT_CMD 267659945086812160 + esoteric: 470884583684964352 + reddit: 458224812528238616 + verification: 352442727016693763 # Staff - admins: &ADMINS 365960823622991872 - admin_spam: &ADMIN_SPAM 563594791770914816 - defcon: &DEFCON 464469101889454091 - helpers: &HELPERS 385474242440986624 - mods: &MODS 305126844661760000 - mod_alerts: 473092532147060736 - mod_spam: &MOD_SPAM 620607373828030464 - organisation: &ORGANISATION 551789653284356126 - staff_lounge: &STAFF_LOUNGE 464905259261755392 + admins: &ADMINS 365960823622991872 + admin_spam: &ADMIN_SPAM 563594791770914816 + defcon: &DEFCON 464469101889454091 + helpers: &HELPERS 385474242440986624 + mods: &MODS 305126844661760000 + mod_alerts: 473092532147060736 + mod_spam: &MOD_SPAM 620607373828030464 + organisation: &ORGANISATION 551789653284356126 + staff_lounge: &STAFF_LOUNGE 464905259261755392 # Voice - admins_voice: &ADMINS_VOICE 500734494840717332 - staff_voice: &STAFF_VOICE 412375055910043655 + admins_voice: &ADMINS_VOICE 500734494840717332 + staff_voice: &STAFF_VOICE 412375055910043655 # Watch - big_brother_logs: &BBLOGS 468507907357409333 - talent_pool: &TALENT_POOL 534321732593647616 + big_brother_logs: &BB_LOGS 468507907357409333 + talent_pool: &TALENT_POOL 534321732593647616 staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] + ignored: [*ADMINS, *MESSAGE_LOG, *MOD_LOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTACH_LOG] reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: @@ -193,8 +193,8 @@ guild: owners: &OWNERS_ROLE 267627879762755584 # Code Jam - jammers: 591786436651646989 - team_leaders: 501324292341104650 + jammers: 591786436651646989 + team_leaders: 501324292341104650 webhooks: talent_pool: 569145364800602132 @@ -278,12 +278,11 @@ filter: # Censor doesn't apply to these channel_whitelist: - *ADMINS - - *MODLOG + - *MOD_LOG - *MESSAGE_LOG - - *DEVLOG - - *BBLOGS + - *DEV_LOG + - *BB_LOGS - *STAFF_LOUNGE - - *DEVTEST - *TALENT_POOL - *USER_EVENT_A -- cgit v1.2.3 From b387b2ec3201a33a0f2ba46a032477b126479671 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:23:25 -0800 Subject: Always load doc and verification extensions They used to only be loaded in "debug mode" because the main guild was used to test the bot. However, we have since moved to using a separate test guild so it's no longer a concern if these cogs get loaded. It was confusing to some contributors as to why these cogs were not being loaded since the debug mode isn't really documented anywhere. --- bot/__main__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 490163739..0079a9381 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -7,7 +7,7 @@ from sentry_sdk.integrations.logging import LoggingIntegration from bot import patches from bot.bot import Bot -from bot.constants import Bot as BotConfig, DEBUG_MODE +from bot.constants import Bot as BotConfig sentry_logging = LoggingIntegration( level=logging.TRACE, @@ -40,10 +40,8 @@ bot.load_extension("bot.cogs.clean") bot.load_extension("bot.cogs.extensions") bot.load_extension("bot.cogs.help") -# Only load this in production -if not DEBUG_MODE: - bot.load_extension("bot.cogs.doc") - bot.load_extension("bot.cogs.verification") +bot.load_extension("bot.cogs.doc") +bot.load_extension("bot.cogs.verification") # Feature cogs bot.load_extension("bot.cogs.alias") -- cgit v1.2.3 From 92e4e6d250d6de1b164d36b8a16c62dd4c71fa4f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:40:21 -0800 Subject: Config: fix DEV_LOG variable thingy --- config-default.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config-default.yml b/config-default.yml index 44da694c2..51efe4d9a 100644 --- a/config-default.yml +++ b/config-default.yml @@ -117,8 +117,8 @@ guild: user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 - dev_log: &DEVLOG 622895325144940554 + devcontrib: &DEV_CONTRIB 635950537262759947 + dev_log: &DEV_LOG 622895325144940554 # Discussion meta: 429409067623251969 -- cgit v1.2.3 From 14536f873d5d233880a88c9f71710ef7c2061625 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:40:57 -0800 Subject: Config: add underscore to devcontrib --- bot/constants.py | 2 +- config-default.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index f35d608da..63f7b15ee 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -364,7 +364,7 @@ class Channels(metaclass=YAMLGetter): big_brother_logs: int bot_commands: int defcon: int - devcontrib: int + dev_contrib: int dev_log: int esoteric: int help_0: int diff --git a/config-default.yml b/config-default.yml index 51efe4d9a..a43610562 100644 --- a/config-default.yml +++ b/config-default.yml @@ -117,7 +117,7 @@ guild: user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 + dev_contrib: &DEV_CONTRIB 635950537262759947 dev_log: &DEV_LOG 622895325144940554 # Discussion -- cgit v1.2.3 From 7c14787a9b1550328180ac3ce3da4d9faa65f41e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:05:36 -0800 Subject: Config: replace abbreviated lists with normal ones Lists were getting too long to be readable as one line. Having each element on a separate line also reduces merge conflicts. --- config-default.yml | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/config-default.yml b/config-default.yml index a43610562..05059fbee 100644 --- a/config-default.yml +++ b/config-default.yml @@ -171,9 +171,26 @@ guild: big_brother_logs: &BB_LOGS 468507907357409333 talent_pool: &TALENT_POOL 534321732593647616 - staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MOD_LOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTACH_LOG] - reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] + staff_channels: + - *ADMINS + - *ADMIN_SPAM + - *DEFCON + - *HELPERS + - *MODS + - *MOD_SPAM + - *ORGANISATION + + ignored: + - *ADMINS + - *ADMINS_VOICE + - *ATTACH_LOG + - *MESSAGE_LOG + - *MOD_LOG + - *STAFF_VOICE + + reminder_whitelist: + - *BOT_CMD + - *DEV_CONTRIB roles: announcements: 463658397560995840 @@ -454,7 +471,20 @@ redirect_output: duck_pond: threshold: 5 - custom_emojis: [*DUCKY_YELLOW, *DUCKY_BLURPLE, *DUCKY_CAMO, *DUCKY_DEVIL, *DUCKY_NINJA, *DUCKY_REGAL, *DUCKY_TUBE, *DUCKY_HUNT, *DUCKY_WIZARD, *DUCKY_PARTY, *DUCKY_ANGEL, *DUCKY_MAUL, *DUCKY_SANTA] + custom_emojis: + - *DUCKY_YELLOW + - *DUCKY_BLURPLE + - *DUCKY_CAMO + - *DUCKY_DEVIL + - *DUCKY_NINJA + - *DUCKY_REGAL + - *DUCKY_TUBE + - *DUCKY_HUNT + - *DUCKY_WIZARD + - *DUCKY_PARTY + - *DUCKY_ANGEL + - *DUCKY_MAUL + - *DUCKY_SANTA config: required_keys: ['bot.token'] -- cgit v1.2.3 From 66fa960fcf0f1d11af20ec1c77039e9ca791f4dc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:08:40 -0800 Subject: Constants: rename Guild.Constant.ignored to modlog_blacklist This name better explains what the list is for. --- bot/cogs/moderation/modlog.py | 10 +++++----- bot/constants.py | 2 +- config-default.yml | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 94e646248..59ae6b587 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -538,7 +538,7 @@ class ModLog(Cog, name="ModLog"): channel = message.channel author = message.author - if message.guild.id != GuildConstant.id or channel.id in GuildConstant.ignored: + if message.guild.id != GuildConstant.id or channel.id in GuildConstant.modlog_blacklist: return self._cached_deletes.append(message.id) @@ -591,7 +591,7 @@ class ModLog(Cog, name="ModLog"): @Cog.listener() async def on_raw_message_delete(self, event: discord.RawMessageDeleteEvent) -> None: """Log raw message delete event to message change log.""" - if event.guild_id != GuildConstant.id or event.channel_id in GuildConstant.ignored: + if event.guild_id != GuildConstant.id or event.channel_id in GuildConstant.modlog_blacklist: return await asyncio.sleep(1) # Wait here in case the normal event was fired @@ -635,7 +635,7 @@ class ModLog(Cog, name="ModLog"): if ( not msg_before.guild or msg_before.guild.id != GuildConstant.id - or msg_before.channel.id in GuildConstant.ignored + or msg_before.channel.id in GuildConstant.modlog_blacklist or msg_before.author.bot ): return @@ -717,7 +717,7 @@ class ModLog(Cog, name="ModLog"): if ( not message.guild or message.guild.id != GuildConstant.id - or message.channel.id in GuildConstant.ignored + or message.channel.id in GuildConstant.modlog_blacklist or message.author.bot ): return @@ -769,7 +769,7 @@ class ModLog(Cog, name="ModLog"): """Log member voice state changes to the voice log channel.""" if ( member.guild.id != GuildConstant.id - or (before.channel and before.channel.id in GuildConstant.ignored) + or (before.channel and before.channel.id in GuildConstant.modlog_blacklist) ): return diff --git a/bot/constants.py b/bot/constants.py index 63f7b15ee..9855421c9 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -428,7 +428,7 @@ class Guild(metaclass=YAMLGetter): section = "guild" id: int - ignored: List[int] + modlog_blacklist: List[int] staff_channels: List[int] reminder_whitelist: List[int] diff --git a/config-default.yml b/config-default.yml index 05059fbee..b253f32e8 100644 --- a/config-default.yml +++ b/config-default.yml @@ -180,7 +180,8 @@ guild: - *MOD_SPAM - *ORGANISATION - ignored: + # Modlog cog ignores events which occur in these channels + modlog_blacklist: - *ADMINS - *ADMINS_VOICE - *ATTACH_LOG -- cgit v1.2.3 From 12c7d2794e8e9d086f5d52c8916df26bb9de5979 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:29:08 -0800 Subject: Tests: fix setting bot-commands ID in information tests --- tests/bot/cogs/test_information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 38293269f..8443cfe71 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -521,7 +521,7 @@ class UserCommandTests(unittest.TestCase): """A regular user should not be able to use this command outside of bot-commands.""" constants.MODERATION_ROLES = [self.moderator_role.id] constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) @@ -533,7 +533,7 @@ class UserCommandTests(unittest.TestCase): def test_regular_user_may_use_command_in_bot_commands_channel(self, create_embed, constants): """A regular user should be allowed to use `!user` targeting themselves in bot-commands.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) @@ -546,7 +546,7 @@ class UserCommandTests(unittest.TestCase): def test_regular_user_can_explicitly_target_themselves(self, create_embed, constants): """A user should target itself with `!user` when a `user` argument was not provided.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) @@ -559,7 +559,7 @@ class UserCommandTests(unittest.TestCase): def test_staff_members_can_bypass_channel_restriction(self, create_embed, constants): """Staff members should be able to bypass the bot-commands channel restriction.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200)) -- cgit v1.2.3 From 3b3206471c028f87685c4c07db0c167a7066ced2 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 25 Feb 2020 12:28:10 -0500 Subject: Configure staff role & channel groupings in YAML Delete duplicate keys that were missed in the merge --- bot/constants.py | 11 +++++++---- config-default.yml | 26 ++++++++++++++++++++------ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 285761055..b1713aa60 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -431,9 +431,12 @@ class Guild(metaclass=YAMLGetter): section = "guild" id: int + moderation_channels: List[int] + moderation_roles: List[int] modlog_blacklist: List[int] - staff_channels: List[int] reminder_whitelist: List[int] + staff_channels: List[int] + staff_roles: List[int] class Keys(metaclass=YAMLGetter): section = "keys" @@ -579,14 +582,14 @@ BOT_DIR = os.path.dirname(__file__) PROJECT_ROOT = os.path.abspath(os.path.join(BOT_DIR, os.pardir)) # Default role combinations -MODERATION_ROLES = Roles.moderators, Roles.admins, Roles.owners -STAFF_ROLES = Roles.helpers, Roles.moderators, Roles.admins, Roles.owners +MODERATION_ROLES = Guild.moderation_roles +STAFF_ROLES = Guild.staff_roles # Roles combinations STAFF_CHANNELS = Guild.staff_channels # Default Channel combinations -MODERATION_CHANNELS = Channels.admins, Channels.admin_spam, Channels.mod_alerts, Channels.mods, Channels.mod_spam +MODERATION_CHANNELS = Guild.moderation_channels # Bot replies diff --git a/config-default.yml b/config-default.yml index dca00500e..ab237423f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,7 +160,7 @@ guild: defcon: &DEFCON 464469101889454091 helpers: &HELPERS 385474242440986624 mods: &MODS 305126844661760000 - mod_alerts: 473092532147060736 + mod_alerts: &MOD_ALERTS 473092532147060736 mod_spam: &MOD_SPAM 620607373828030464 organisation: &ORGANISATION 551789653284356126 staff_lounge: &STAFF_LOUNGE 464905259261755392 @@ -182,6 +182,13 @@ guild: - *MOD_SPAM - *ORGANISATION + moderation_channels: + - *ADMINS + - *ADMIN_SPAM + - *MOD_ALERTS + - *MODS + - *MOD_SPAM + # Modlog cog ignores events which occur in these channels modlog_blacklist: - *ADMINS @@ -195,10 +202,6 @@ guild: - *BOT_CMD - *DEV_CONTRIB - staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] - reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] - roles: announcements: 463658397560995840 contributors: 295488872404484098 @@ -212,7 +215,7 @@ guild: # Staff admins: &ADMINS_ROLE 267628507062992896 core_developers: 587606783669829632 - helpers: 267630620367257601 + helpers: &HELPERS_ROLE 267630620367257601 moderators: &MODS_ROLE 267629731250176001 owners: &OWNERS_ROLE 267627879762755584 @@ -220,6 +223,17 @@ guild: jammers: 591786436651646989 team_leaders: 501324292341104650 + moderation_roles: + - *OWNERS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + + staff_roles: + - *OWNERS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + - *HELPERS_ROLE + webhooks: talent_pool: 569145364800602132 big_brother: 569133704568373283 -- cgit v1.2.3 From 284c1de321fea5927dafc1ac3192ad763bda3203 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 25 Feb 2020 12:47:09 -0500 Subject: Fix mismatched constant names in syncer tests --- bot/cogs/sync/syncers.py | 8 ++++---- tests/bot/cogs/sync/test_base.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py index 6715ad6fb..d6891168f 100644 --- a/bot/cogs/sync/syncers.py +++ b/bot/cogs/sync/syncers.py @@ -23,7 +23,7 @@ _Diff = namedtuple('Diff', ('created', 'updated', 'deleted')) class Syncer(abc.ABC): """Base class for synchronising the database with objects in the Discord cache.""" - _CORE_DEV_MENTION = f"<@&{constants.Roles.core_developer}> " + _CORE_DEV_MENTION = f"<@&{constants.Roles.core_developers}> " _REACTION_EMOJIS = (constants.Emojis.check_mark, constants.Emojis.cross_mark) def __init__(self, bot: Bot) -> None: @@ -54,12 +54,12 @@ class Syncer(abc.ABC): # Send to core developers if it's an automatic sync. if not message: log.trace("Message not provided for confirmation; creating a new one in dev-core.") - channel = self.bot.get_channel(constants.Channels.devcore) + channel = self.bot.get_channel(constants.Channels.dev_core) if not channel: log.debug("Failed to get the dev-core channel from cache; attempting to fetch it.") try: - channel = await self.bot.fetch_channel(constants.Channels.devcore) + channel = await self.bot.fetch_channel(constants.Channels.dev_core) except HTTPException: log.exception( f"Failed to fetch channel for sending sync confirmation prompt; " @@ -93,7 +93,7 @@ class Syncer(abc.ABC): `author` of the prompt. """ # For automatic syncs, check for the core dev role instead of an exact author - has_role = any(constants.Roles.core_developer == role.id for role in user.roles) + has_role = any(constants.Roles.core_developers == role.id for role in user.roles) return ( reaction.message.id == message.id and not user.bot diff --git a/tests/bot/cogs/sync/test_base.py b/tests/bot/cogs/sync/test_base.py index e6a6f9688..c2e143865 100644 --- a/tests/bot/cogs/sync/test_base.py +++ b/tests/bot/cogs/sync/test_base.py @@ -84,7 +84,7 @@ class SyncerSendPromptTests(unittest.TestCase): mock_() await self.syncer._send_prompt() - method.assert_called_once_with(constants.Channels.devcore) + method.assert_called_once_with(constants.Channels.dev_core) @helpers.async_test async def test_send_prompt_returns_None_if_channel_fetch_fails(self): @@ -135,7 +135,7 @@ class SyncerConfirmationTests(unittest.TestCase): def setUp(self): self.bot = helpers.MockBot() self.syncer = TestSyncer(self.bot) - self.core_dev_role = helpers.MockRole(id=constants.Roles.core_developer) + self.core_dev_role = helpers.MockRole(id=constants.Roles.core_developers) @staticmethod def get_message_reaction(emoji): -- cgit v1.2.3 From f87f7559db8b352490324a535fb77e88f2f68b41 Mon Sep 17 00:00:00 2001 From: Matteo Date: Thu, 27 Feb 2020 11:37:02 +0100 Subject: Split the eval command procedure into two functions. Two functions were created: send_eval and continue_eval, in order to facilitate testing. The corresponding tests are also changed in this commit. --- bot/cogs/snekbox.py | 112 +++++++++++++++++------------- tests/bot/cogs/test_snekbox.py | 150 ++++++++++++++++++++++------------------- 2 files changed, 148 insertions(+), 114 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index efa4696b5..25b2455e8 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -179,6 +179,68 @@ class Snekbox(Cog): return output, paste_link + async def send_eval(self, ctx: Context, code: str) -> Message: + """ + Evaluate code, format it, and send the output to the corresponding channel. + + Return the bot response. + """ + async with ctx.typing(): + results = await self.post_eval(code) + msg, error = self.get_results_message(results) + + if error: + output, paste_link = error, None + else: + output, paste_link = await self.format_output(results["stdout"]) + + icon = self.get_status_emoji(results) + msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" + if paste_link: + msg = f"{msg}\nFull output: {paste_link}" + + response = await ctx.send(msg) + self.bot.loop.create_task( + wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + ) + + log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + return response + + async def continue_eval(self, ctx: Context, response: Message) -> Tuple[bool, Optional[str]]: + """ + Check if the eval session should continue. + + First item of the returned tuple is if the eval session should continue, + the second is the new code to evaluate. + """ + _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) + _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) + + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=_predicate_eval_message_edit, + timeout=10 + ) + await ctx.message.add_reaction('🔁') + await self.bot.wait_for( + 'reaction_add', + check=_predicate_emoji_reaction, + timeout=10 + ) + + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + with contextlib.suppress(HTTPException): + await response.delete() + + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return False, None + + return True, code + @command(name="eval", aliases=("e",)) @guild_only() @in_channel(Channels.bot, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) @@ -203,58 +265,18 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") - _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) - _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) - while True: self.jobs[ctx.author.id] = datetime.datetime.now() code = self.prepare_input(code) - try: - async with ctx.typing(): - results = await self.post_eval(code) - msg, error = self.get_results_message(results) - - if error: - output, paste_link = error, None - else: - output, paste_link = await self.format_output(results["stdout"]) - - icon = self.get_status_emoji(results) - msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" - if paste_link: - msg = f"{msg}\nFull output: {paste_link}" - - response = await ctx.send(msg) - self.bot.loop.create_task( - wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) - ) - - log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + response = await self.send_eval(ctx, code) finally: del self.jobs[ctx.author.id] - try: - _, new_message = await self.bot.wait_for( - 'message_edit', - check=_predicate_eval_message_edit, - timeout=10 - ) - await ctx.message.add_reaction('🔁') - await self.bot.wait_for( - 'reaction_add', - check=_predicate_emoji_reaction, - timeout=10 - ) - - log.info(f"Re-evaluating message {ctx.message.id}") - code = new_message.content.split(' ', maxsplit=1)[1] - await ctx.message.clear_reactions() - with contextlib.suppress(HTTPException): - await response.delete() - except asyncio.TimeoutError: - await ctx.message.clear_reactions() - return + continue_eval, code = await self.continue_eval(ctx, response) + if not continue_eval: + break + log.info(f"Re-evaluating message {ctx.message.id}") def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message) -> bool: diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 112c923c8..c1c0f8d47 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -1,6 +1,7 @@ import asyncio import logging import unittest +from functools import partial from unittest.mock import MagicMock, Mock, call, patch from bot.cogs import snekbox @@ -175,28 +176,33 @@ class SnekboxTests(unittest.TestCase): async def test_eval_command_evaluate_once(self): """Test the eval command procedure.""" ctx = MockContext() - ctx.message = MockMessage() - ctx.send = AsyncMock() - ctx.author.mention = '@LemonLemonishBeard#0042' - ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) - self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) - self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('[No output]', None)) - self.bot.wait_for.side_effect = asyncio.TimeoutError + response = MockMessage() + self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') + self.cog.send_eval = AsyncMock(return_value=response) + self.cog.continue_eval = AsyncMock(return_value=(False, None)) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.prepare_input.assert_called_once_with('MyAwesomeCode') + self.cog.send_eval.assert_called_once_with(ctx, 'MyAwesomeFormattedCode') + self.cog.continue_eval.assert_called_once_with(ctx, response) - ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```' - ) - self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_status_emoji.assert_called_once_with({'stdout': '', 'returncode': 0}) - self.cog.get_results_message.assert_called_once_with({'stdout': '', 'returncode': 0}) - self.cog.format_output.assert_called_once_with('') + @async_test + async def test_eval_command_evaluate_twice(self): + """Test the eval and re-eval command procedure.""" + ctx = MockContext() + response = MockMessage() + self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') + self.cog.send_eval = AsyncMock(return_value=response) + self.cog.continue_eval = AsyncMock() + self.cog.continue_eval.side_effect = ((True, 'MyAwesomeCode-2'), (False, None)) + + await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.prepare_input.has_calls(call('MyAwesomeCode'), call('MyAwesomeCode-2')) + self.cog.send_eval.assert_called_with(ctx, 'MyAwesomeFormattedCode') + self.cog.continue_eval.assert_called_with(ctx, response) @async_test - async def test_eval_command_reject_two_eval(self): + async def test_eval_command_reject_two_eval_at_the_same_time(self): """Test if the eval command rejects an eval if the author already have a running eval.""" ctx = MockContext() ctx.author.id = 42 @@ -217,92 +223,98 @@ class SnekboxTests(unittest.TestCase): ctx.invoke.assert_called_once_with(self.bot.get_command("help"), "eval") @async_test - async def test_eval_command_return_error(self): - """Test the eval command error handling.""" + async def test_send_eval(self): + """Test the send_eval function.""" ctx = MockContext() ctx.message = MockMessage() ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': 'ERROR', 'returncode': 127}) - self.cog.get_results_message = MagicMock(return_value=('Return code 127', 'Error occurred')) - self.cog.get_status_emoji = MagicMock(return_value=':nope!:') - self.cog.format_output = AsyncMock() - self.bot.wait_for.side_effect = asyncio.TimeoutError - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) + self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) + self.cog.get_status_emoji = MagicMock(return_value=':yay!:') + self.cog.format_output = AsyncMock(return_value=('[No output]', None)) + await self.cog.send_eval(ctx, 'MyAwesomeCode') ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```py\nError occurred\n```' + '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```' ) self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_results_message.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) - self.cog.get_status_emoji.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) - self.cog.format_output.assert_not_called() + self.cog.get_status_emoji.assert_called_once_with({'stdout': '', 'returncode': 0}) + self.cog.get_results_message.assert_called_once_with({'stdout': '', 'returncode': 0}) + self.cog.format_output.assert_called_once_with('') @async_test - async def test_eval_command_with_paste_link(self): - """Test the eval command procedure with the use of a paste link.""" + async def test_send_eval_with_paste_link(self): + """Test the send_eval function with a too long output that generate a paste link.""" ctx = MockContext() ctx.message = MockMessage() ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': 'SuperLongBeard', 'returncode': 0}) + self.cog.post_eval = AsyncMock(return_value={'stdout': 'Way too long beard', 'returncode': 0}) self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('Truncated - too long beard', 'https://testificate.com/')) - self.bot.wait_for.side_effect = asyncio.TimeoutError - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.format_output = AsyncMock(return_value=('Way too long beard', 'lookatmybeard.com')) + await self.cog.send_eval(ctx, 'MyAwesomeCode') ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n' - 'Truncated - too long beard\n```\nFull output: https://testificate.com/' + '@LemonLemonishBeard#0042 :yay!: Return code 0.' + '\n\n```py\nWay too long beard\n```\nFull output: lookatmybeard.com' ) self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_status_emoji.assert_called_once_with({'stdout': 'SuperLongBeard', 'returncode': 0}) - self.cog.get_results_message.assert_called_once_with({'stdout': 'SuperLongBeard', 'returncode': 0}) - self.cog.format_output.assert_called_with('SuperLongBeard') + self.cog.get_status_emoji.assert_called_once_with({'stdout': 'Way too long beard', 'returncode': 0}) + self.cog.get_results_message.assert_called_once_with({'stdout': 'Way too long beard', 'returncode': 0}) + self.cog.format_output.assert_called_once_with('Way too long beard') @async_test - async def test_eval_command_evaluate_twice(self): - """Test the eval command re-evaluation procedure.""" + async def test_send_eval_with_non_zero_eval(self): + """Test the send_eval function with a code returning a non-zero code.""" ctx = MockContext() ctx.message = MockMessage() - ctx.message.content = '!e MyAwesomeCode' - updated_msg = MockMessage() - updated_msg .content = '!e MyAwesomeCode-2' - response_msg = MockMessage() - response_msg.delete = AsyncMock() - ctx.send = AsyncMock(return_value=response_msg) + ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) - self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) - self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('[No output]', None)) - self.bot.wait_for.side_effect = ((None, updated_msg), None, asyncio.TimeoutError) - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') - - self.cog.post_eval.assert_has_calls((call('MyAwesomeCode'), call('MyAwesomeCode-2'))) + self.cog.post_eval = AsyncMock(return_value={'stdout': 'ERROR', 'returncode': 127}) + self.cog.get_results_message = MagicMock(return_value=('Return code 127', 'Beard got stuck in the eval')) + self.cog.get_status_emoji = MagicMock(return_value=':nope!:') + self.cog.format_output = AsyncMock() # This function isn't called - # Multiplied by 2 because we expect it to be called twice - ctx.send.assert_has_calls( - [call('@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```')] * 2 + await self.cog.send_eval(ctx, 'MyAwesomeCode') + ctx.send.assert_called_once_with( + '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```py\nBeard got stuck in the eval\n```' ) - self.cog.get_status_emoji.assert_has_calls([call({'stdout': '', 'returncode': 0})] * 2) - self.cog.get_results_message.assert_has_calls([call({'stdout': '', 'returncode': 0})] * 2) - self.cog.format_output.assert_has_calls([call('')] * 2) + self.cog.post_eval.assert_called_once_with('MyAwesomeCode') + self.cog.get_status_emoji.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) + self.cog.get_results_message.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) + self.cog.format_output.assert_not_called() + @async_test + async def test_continue_eval_does_continue(self): + """Test that the continue_eval function does continue if required conditions are met.""" + ctx = MockContext(message=MockMessage(add_reaction=AsyncMock(), clear_reactions=AsyncMock())) + response = MockMessage(delete=AsyncMock()) + new_msg = MockMessage(content='!e NewCode') + self.bot.wait_for.side_effect = ((None, new_msg), None) + + actual = await self.cog.continue_eval(ctx, response) + self.assertEqual(actual, (True, 'NewCode')) self.bot.wait_for.has_calls( - call('message_edit', check=snekbox.predicate_eval_message_edit, timeout=10), - call('reaction_add', check=snekbox.predicate_eval_emoji_reaction, timeout=10) + call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), + call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) ) ctx.message.add_reaction.assert_called_once_with('🔁') - ctx.message.clear_reactions.assert_called() - response_msg.delete.assert_called_once() + ctx.message.clear_reactions.assert_called_once() + response.delete.assert_called_once() + + @async_test + async def test_continue_eval_does_not_continue(self): + ctx = MockContext(message=MockMessage(clear_reactions=AsyncMock())) + self.bot.wait_for.side_effect = asyncio.TimeoutError + + actual = await self.cog.continue_eval(ctx, MockMessage()) + self.assertEqual(actual, (False, None)) + ctx.message.clear_reactions.assert_called_once() def test_predicate_eval_message_edit(self): """Test the predicate_eval_message_edit function.""" -- cgit v1.2.3 From afc74faadd5fb4d3fd3003bed9f2a1f241c0dc58 Mon Sep 17 00:00:00 2001 From: Matteo Date: Thu, 27 Feb 2020 11:47:01 +0100 Subject: Use unicode code point instead of literal for the snekbox re-eval emoji Unicode literals aren't really safe compared to code points --- bot/cogs/snekbox.py | 8 +++++--- tests/bot/cogs/test_snekbox.py | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 25b2455e8..52d830fa8 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -42,6 +42,8 @@ EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.ro SIGKILL = 9 +REEVAL_EMOJI = '\U0001f501' # :repeat: + class Snekbox(Cog): """Safe evaluation of Python code using Snekbox.""" @@ -223,7 +225,7 @@ class Snekbox(Cog): check=_predicate_eval_message_edit, timeout=10 ) - await ctx.message.add_reaction('🔁') + await ctx.message.add_reaction(REEVAL_EMOJI) await self.bot.wait_for( 'reaction_add', check=_predicate_emoji_reaction, @@ -285,8 +287,8 @@ def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message def predicate_eval_emoji_reaction(ctx: Context, reaction: Reaction, user: User) -> bool: - """Return True if the reaction 🔁 was added by the context message author on this message.""" - return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == '🔁' + """Return True if the reaction REEVAL_EMOJI was added by the context message author on this message.""" + return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == REEVAL_EMOJI def setup(bot: Bot) -> None: diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index c1c0f8d47..e7a1e3362 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -303,7 +303,7 @@ class SnekboxTests(unittest.TestCase): call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) ) - ctx.message.add_reaction.assert_called_once_with('🔁') + ctx.message.add_reaction.assert_called_once_with(snekbox.REEVAL_EMOJI) ctx.message.clear_reactions.assert_called_once() response.delete.assert_called_once() @@ -336,12 +336,12 @@ class SnekboxTests(unittest.TestCase): def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" valid_reaction = MockReaction(message=MockMessage(id=1)) - valid_reaction.__str__.return_value = '🔁' + valid_reaction.__str__.return_value = snekbox.REEVAL_EMOJI valid_ctx = MockContext(message=MockMessage(id=1), author=MockUser(id=2)) valid_user = MockUser(id=2) invalid_reaction_id = MockReaction(message=MockMessage(id=42)) - invalid_reaction_id.__str__.return_value = '🔁' + invalid_reaction_id.__str__.return_value = snekbox.REEVAL_EMOJI invalid_user_id = MockUser(id=42) invalid_reaction_str = MockReaction(message=MockMessage(id=1)) invalid_reaction_str.__str__.return_value = ':longbeard:' -- cgit v1.2.3 From b62e15f835ff4b6c808a9b571919bcfa479d004b Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 28 Feb 2020 09:41:12 +0100 Subject: Return only the new code in continue_eval and check for truthiness instead --- bot/cogs/snekbox.py | 13 ++++++------- tests/bot/cogs/test_snekbox.py | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 52d830fa8..381b309e0 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -209,12 +209,11 @@ class Snekbox(Cog): log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") return response - async def continue_eval(self, ctx: Context, response: Message) -> Tuple[bool, Optional[str]]: + async def continue_eval(self, ctx: Context, response: Message) -> Optional[str]: """ Check if the eval session should continue. - First item of the returned tuple is if the eval session should continue, - the second is the new code to evaluate. + Return the new code to evaluate or None if the eval session should be terminated. """ _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) @@ -239,9 +238,9 @@ class Snekbox(Cog): except asyncio.TimeoutError: await ctx.message.clear_reactions() - return False, None + return None - return True, code + return code @command(name="eval", aliases=("e",)) @guild_only() @@ -275,8 +274,8 @@ class Snekbox(Cog): finally: del self.jobs[ctx.author.id] - continue_eval, code = await self.continue_eval(ctx, response) - if not continue_eval: + code = await self.continue_eval(ctx, response) + if not code: break log.info(f"Re-evaluating message {ctx.message.id}") diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index e7a1e3362..985bc66a1 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -179,7 +179,7 @@ class SnekboxTests(unittest.TestCase): response = MockMessage() self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') self.cog.send_eval = AsyncMock(return_value=response) - self.cog.continue_eval = AsyncMock(return_value=(False, None)) + self.cog.continue_eval = AsyncMock(return_value=None) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') self.cog.prepare_input.assert_called_once_with('MyAwesomeCode') @@ -194,7 +194,7 @@ class SnekboxTests(unittest.TestCase): self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') self.cog.send_eval = AsyncMock(return_value=response) self.cog.continue_eval = AsyncMock() - self.cog.continue_eval.side_effect = ((True, 'MyAwesomeCode-2'), (False, None)) + self.cog.continue_eval.side_effect = ('MyAwesomeCode-2', None) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') self.cog.prepare_input.has_calls(call('MyAwesomeCode'), call('MyAwesomeCode-2')) @@ -298,7 +298,7 @@ class SnekboxTests(unittest.TestCase): self.bot.wait_for.side_effect = ((None, new_msg), None) actual = await self.cog.continue_eval(ctx, response) - self.assertEqual(actual, (True, 'NewCode')) + self.assertEqual(actual, 'NewCode') self.bot.wait_for.has_calls( call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) @@ -313,7 +313,7 @@ class SnekboxTests(unittest.TestCase): self.bot.wait_for.side_effect = asyncio.TimeoutError actual = await self.cog.continue_eval(ctx, MockMessage()) - self.assertEqual(actual, (False, None)) + self.assertEqual(actual, None) ctx.message.clear_reactions.assert_called_once() def test_predicate_eval_message_edit(self): -- cgit v1.2.3 From 77ee577598ae0ed9d49a0771f682e5d5a96fc7e5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 28 Feb 2020 09:46:51 +0100 Subject: Ignore NotFound errors inside continue_eval It could have caused some errors if the user delete his own message --- bot/cogs/snekbox.py | 49 +++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 381b309e0..d52027ac6 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -8,7 +8,7 @@ from functools import partial from signal import Signals from typing import Optional, Tuple -from discord import HTTPException, Message, Reaction, User +from discord import HTTPException, Message, NotFound, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -218,29 +218,30 @@ class Snekbox(Cog): _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) - try: - _, new_message = await self.bot.wait_for( - 'message_edit', - check=_predicate_eval_message_edit, - timeout=10 - ) - await ctx.message.add_reaction(REEVAL_EMOJI) - await self.bot.wait_for( - 'reaction_add', - check=_predicate_emoji_reaction, - timeout=10 - ) - - code = new_message.content.split(' ', maxsplit=1)[1] - await ctx.message.clear_reactions() - with contextlib.suppress(HTTPException): - await response.delete() - - except asyncio.TimeoutError: - await ctx.message.clear_reactions() - return None - - return code + with contextlib.suppress(NotFound): + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=_predicate_eval_message_edit, + timeout=10 + ) + await ctx.message.add_reaction(REEVAL_EMOJI) + await self.bot.wait_for( + 'reaction_add', + check=_predicate_emoji_reaction, + timeout=10 + ) + + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + with contextlib.suppress(HTTPException): + await response.delete() + + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return None + + return code @command(name="eval", aliases=("e",)) @guild_only() -- cgit v1.2.3 From b8769e036e9247be47cea1b2073e92ea48724ca8 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 28 Feb 2020 09:41:37 -0800 Subject: Snekbox: mention re-evaluation feature in the command's docstring --- bot/cogs/snekbox.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 44764e7e9..cff7c5786 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -251,7 +251,10 @@ class Snekbox(Cog): Run Python code and get the results. This command supports multiple lines of code, including code wrapped inside a formatted code - block. We've done our best to make this safe, but do let us know if you manage to find an + block. Code can be re-evaluated by editing the original message within 10 seconds and + clicking the reaction that subsequently appears. + + We've done our best to make this sandboxed, but do let us know if you manage to find an issue with it! """ if ctx.author.id in self.jobs: -- cgit v1.2.3