From 1231f384ef9cf3aac6a4318b2ca4c465f2552aa8 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 2 Mar 2020 13:57:33 +0100 Subject: Add HushDurationConverter. --- bot/converters.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/bot/converters.py b/bot/converters.py index 1945e1da3..976376fce 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -262,6 +262,34 @@ class ISODateTime(Converter): return dt +class HushDurationConverter(Converter): + """Convert passed duration to `int` minutes or `None`.""" + + MINUTES_RE = re.compile(r"(\d+)(?:M|m|$)") + + async def convert(self, ctx: Context, argument: str) -> t.Optional[int]: + """ + Convert `argument` to a duration that's max 15 minutes or None. + + If `"forever"` is passed, None is returned; otherwise an int of the extracted time. + Accepted formats are: + , + m, + M, + forever. + """ + if argument == "forever": + return None + match = self.MINUTES_RE.match(argument) + if not match: + raise BadArgument(f"{argument} is not a valid minutes duration.") + + duration = int(match.group(1)) + if duration > 15: + raise BadArgument("Duration must be below 15 minutes.") + return duration + + def proxy_user(user_id: str) -> discord.Object: """ Create a proxy user object from the given id. -- cgit v1.2.3 From be6738983e5150ca24c20cbd7b482002ab9d69e6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 2 Mar 2020 23:24:05 +0100 Subject: Add Silence cog. FirstHash is used for handling channels in `loop_alert_channels` set as tuples without considering other elements. --- bot/cogs/moderation/__init__.py | 2 + bot/cogs/moderation/silence.py | 141 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 bot/cogs/moderation/silence.py diff --git a/bot/cogs/moderation/__init__.py b/bot/cogs/moderation/__init__.py index 5243cb92d..0349fe4b1 100644 --- a/bot/cogs/moderation/__init__.py +++ b/bot/cogs/moderation/__init__.py @@ -2,6 +2,7 @@ from bot.bot import Bot from .infractions import Infractions from .management import ModManagement from .modlog import ModLog +from .silence import Silence from .superstarify import Superstarify @@ -10,4 +11,5 @@ def setup(bot: Bot) -> None: bot.add_cog(Infractions(bot)) bot.add_cog(ModLog(bot)) bot.add_cog(ModManagement(bot)) + bot.add_cog(Silence(bot)) bot.add_cog(Superstarify(bot)) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py new file mode 100644 index 000000000..f37196744 --- /dev/null +++ b/bot/cogs/moderation/silence.py @@ -0,0 +1,141 @@ +import asyncio +import logging +from contextlib import suppress +from typing import Optional + +from discord import PermissionOverwrite, TextChannel +from discord.ext import commands, tasks +from discord.ext.commands import Context, TextChannelConverter + +from bot.bot import Bot +from bot.constants import Channels, Emojis, Guild, Roles +from bot.converters import HushDurationConverter + +log = logging.getLogger(__name__) + + +class FirstHash(tuple): + """Tuple with only first item used for hash and eq.""" + + def __new__(cls, *args): + """Construct tuple from `args`.""" + return super().__new__(cls, args) + + def __hash__(self): + return hash((self[0],)) + + def __eq__(self, other: "FirstHash"): + return self[0] == other[0] + + +class Silence(commands.Cog): + """Commands for stopping channel messages for `verified` role in a channel.""" + + def __init__(self, bot: Bot): + self.bot = bot + self.loop_alert_channels = set() + self.bot.loop.create_task(self._get_server_values()) + + async def _get_server_values(self) -> None: + """Fetch required internal values after they're available.""" + await self.bot.wait_until_guild_available() + guild = self.bot.get_guild(Guild.id) + self._verified_role = guild.get_role(Roles.verified) + self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + self._mod_log_channel = self.bot.get_channel(Channels.mod_log) + + @commands.command(aliases=("hush",)) + async def silence( + self, + ctx: Context, + duration: HushDurationConverter = 10, + channel: TextChannelConverter = None + ) -> None: + """ + Silence `channel` for `duration` minutes or `"forever"`. + + If duration is forever, start a notifier loop that triggers every 15 minutes. + """ + channel = channel or ctx.channel + + if not await self._silence(channel, persistent=(duration is None), duration=duration): + await ctx.send(f"{Emojis.cross_mark} {channel.mention} is already silenced.") + return + if duration is None: + await ctx.send(f"{Emojis.check_mark} Channel {channel.mention} silenced indefinitely.") + return + + await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced for {duration} minute(s).") + await asyncio.sleep(duration*60) + await self.unsilence(ctx, channel) + + @commands.command(aliases=("unhush",)) + async def unsilence(self, ctx: Context, channel: TextChannelConverter = None) -> None: + """ + Unsilence `channel`. + + Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. + """ + channel = channel or ctx.channel + alert_channel = self._mod_log_channel if ctx.invoked_with == "hush" else ctx.channel + + if await self._unsilence(channel): + await alert_channel.send(f"{Emojis.check_mark} Unsilenced {channel.mention}.") + + async def _silence(self, channel: TextChannel, persistent: bool, duration: Optional[int]) -> bool: + """ + Silence `channel` for `self._verified_role`. + + If `persistent` is `True` add `channel` with current iteration of `self._notifier` + to `self.self.loop_alert_channels` and attempt to start notifier. + `duration` is only used for logging; if None is passed `persistent` should be True to not log None. + """ + if channel.overwrites_for(self._verified_role).send_messages is False: + log.debug(f"Tried to silence channel #{channel} ({channel.id}) but the channel was already silenced.") + return False + await channel.set_permissions(self._verified_role, overwrite=PermissionOverwrite(send_messages=False)) + if persistent: + log.debug(f"Silenced #{channel} ({channel.id}) indefinitely.") + self.loop_alert_channels.add(FirstHash(channel, self._notifier.current_loop)) + with suppress(RuntimeError): + self._notifier.start() + return True + + log.debug(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") + return True + + async def _unsilence(self, channel: TextChannel) -> bool: + """ + Unsilence `channel`. + + Check if `channel` is silenced through a `PermissionOverwrite`, + if it is unsilence it, attempt to remove it from `self.loop_alert_channels` + and if `self.loop_alert_channels` are left empty, stop the `self._notifier` + """ + if channel.overwrites_for(self._verified_role).send_messages is False: + await channel.set_permissions(self._verified_role, overwrite=None) + log.debug(f"Unsilenced channel #{channel} ({channel.id}).") + + with suppress(KeyError): + self.loop_alert_channels.remove(FirstHash(channel)) + if not self.loop_alert_channels: + self._notifier.cancel() + return True + log.debug(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") + return False + + @tasks.loop() + async def _notifier(self) -> None: + """Post notice of permanently silenced channels to `mod_alerts` periodically.""" + # Wait for 15 minutes between notices with pause at start of loop. + await asyncio.sleep(15*60) + current_iter = self._notifier.current_loop+1 + channels_text = ', '.join( + f"{channel.mention} for {current_iter-start} min" + for channel, start in self.loop_alert_channels + ) + channels_log_text = ', '.join( + f'#{channel} ({channel.id})' for channel, _ in self.loop_alert_channels + ) + log.debug(f"Sending notice with channels: {channels_log_text}") + await self._mod_alerts_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") -- cgit v1.2.3 From 1f8933f5551185aa60dc11807778c0bf7f78b0c3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 2 Mar 2020 23:25:36 +0100 Subject: Add logging to loop start and loop end. --- bot/cogs/moderation/silence.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index f37196744..560a0a15c 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -139,3 +139,11 @@ class Silence(commands.Cog): ) log.debug(f"Sending notice with channels: {channels_log_text}") await self._mod_alerts_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") + + @_notifier.before_loop + async def _log_notifier_start(self) -> None: + log.trace("Starting notifier loop.") + + @_notifier.after_loop + async def _log_notifier_end(self) -> None: + log.trace("Stopping notifier loop.") -- cgit v1.2.3 From 4254053af6980c75a845ddcc7f1701f7b86a42a9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:15:17 +0100 Subject: Restrict cog to moderators. --- bot/cogs/moderation/silence.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 560a0a15c..0081a420e 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -8,8 +8,9 @@ from discord.ext import commands, tasks from discord.ext.commands import Context, TextChannelConverter from bot.bot import Bot -from bot.constants import Channels, Emojis, Guild, Roles +from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles from bot.converters import HushDurationConverter +from bot.utils.checks import with_role_check log = logging.getLogger(__name__) @@ -147,3 +148,8 @@ class Silence(commands.Cog): @_notifier.after_loop async def _log_notifier_end(self) -> None: log.trace("Stopping notifier loop.") + + # This cannot be static (must have a __func__ attribute). + def cog_check(self, ctx: Context) -> bool: + """Only allow moderators to invoke the commands in this cog.""" + return with_role_check(ctx, *MODERATION_ROLES) -- cgit v1.2.3 From 8be3c7d1a65bf5d0e0bc07267e7c45f143fae2e6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:39:53 +0100 Subject: Add handling for shh/unshh for `CommandNotFound`. --- bot/cogs/error_handler.py | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 261769efc..45ab1f326 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -31,7 +31,9 @@ class ErrorHandler(Cog): 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 + 1. If the name fails to match a command: + If it matches shh+ or unshh+, the channel is silenced or unsilenced respectively. + otherwise if it 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 @@ -49,10 +51,14 @@ class ErrorHandler(Cog): return # Try to look for a tag with the command's name if the command isn't found. - if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): - if ctx.channel.id != Channels.verification: + if isinstance(e, errors.CommandNotFound): + if ( + not await self.try_silence(ctx) + and not hasattr(ctx, "invoked_from_error_handler") + and ctx.channel.id != Channels.verification + ): await self.try_get_tag(ctx) - return # Exit early to avoid logging. + return # Exit early to avoid logging. elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): @@ -89,6 +95,28 @@ class ErrorHandler(Cog): else: return self.bot.get_command("help") + async def try_silence(self, ctx: Context) -> bool: + """ + Attempt to invoke the silence or unsilence command if invoke with matches a pattern. + + Respecting the checks if: + invoked with `shh+` silence channel for amount of h's*2 with max of 15. + invoked with `unshh+` unsilence channel + Return bool depending on success of command. + """ + command = ctx.invoked_with.lower() + silence_command = self.bot.get_command("silence") + if not await silence_command.can_run(ctx): + log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") + return False + if command.startswith("shh"): + await ctx.invoke(silence_command, duration=min(command.count("h")*2, 15)) + return True + elif command.startswith("unshh"): + await ctx.invoke(self.bot.get_command("unsilence")) + return True + return False + 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 d4253e106771f90a983717a994349d52337b2de9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:40:34 +0100 Subject: Add tests for FirstHash class. --- tests/bot/cogs/moderation/__init__.py | 0 tests/bot/cogs/moderation/test_silence.py | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/bot/cogs/moderation/__init__.py create mode 100644 tests/bot/cogs/moderation/test_silence.py diff --git a/tests/bot/cogs/moderation/__init__.py b/tests/bot/cogs/moderation/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py new file mode 100644 index 000000000..2a06f5944 --- /dev/null +++ b/tests/bot/cogs/moderation/test_silence.py @@ -0,0 +1,25 @@ +import unittest + +from bot.cogs.moderation.silence import FirstHash + + +class FirstHashTests(unittest.TestCase): + def setUp(self) -> None: + self.test_cases = ( + (FirstHash(0, 4), FirstHash(0, 5)), + (FirstHash("string", None), FirstHash("string", True)) + ) + + def test_hashes_equal(self): + """Check hashes equal with same first item.""" + + for tuple1, tuple2 in self.test_cases: + with self.subTest(tuple1=tuple1, tuple2=tuple2): + self.assertEqual(hash(tuple1), hash(tuple2)) + + def test_eq(self): + """Check objects are equal with same first item.""" + + for tuple1, tuple2 in self.test_cases: + with self.subTest(tuple1=tuple1, tuple2=tuple2): + self.assertTrue(tuple1 == tuple2) -- cgit v1.2.3 From e872176b452ceca1b639ef42d640e18656c7c0c9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:42:18 +0100 Subject: Add test case for Silence cog. --- tests/bot/cogs/moderation/test_silence.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 2a06f5944..1db2b6eec 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,7 @@ import unittest -from bot.cogs.moderation.silence import FirstHash +from bot.cogs.moderation.silence import FirstHash, Silence +from tests.helpers import MockBot, MockContext class FirstHashTests(unittest.TestCase): @@ -23,3 +24,11 @@ class FirstHashTests(unittest.TestCase): for tuple1, tuple2 in self.test_cases: with self.subTest(tuple1=tuple1, tuple2=tuple2): self.assertTrue(tuple1 == tuple2) + + +class SilenceTests(unittest.TestCase): + def setUp(self) -> None: + + self.bot = MockBot() + self.cog = Silence(self.bot) + self.ctx = MockContext() -- cgit v1.2.3 From 1d83a5752aae483224129ee798e529f3d7d8e132 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:42:51 +0100 Subject: Add test for `silence` discord output. --- tests/bot/cogs/moderation/test_silence.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 1db2b6eec..088410bee 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,10 @@ +import asyncio import unittest +from functools import partial +from unittest import mock from bot.cogs.moderation.silence import FirstHash, Silence +from bot.constants import Emojis from tests.helpers import MockBot, MockContext @@ -32,3 +36,23 @@ class SilenceTests(unittest.TestCase): self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() + + def test_silence_sent_correct_discord_message(self): + """Check if proper message was sent when called with duration in channel with previous state.""" + test_cases = ( + ((self.cog, self.ctx, 0.0001), f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), + ((self.cog, self.ctx, None), f"{Emojis.check_mark} #channel silenced indefinitely.", True,), + ((self.cog, self.ctx, 5), f"{Emojis.cross_mark} #channel is already silenced.", False,), + ) + for silence_call_args, result_message, _silence_patch_return in test_cases: + with self.subTest( + silence_duration=silence_call_args[-1], + result_message=result_message, + starting_unsilenced_state=_silence_patch_return + ): + with mock.patch( + "bot.cogs.moderation.silence.Silence._silence", + new_callable=partial(mock.AsyncMock, return_value=_silence_patch_return) + ): + asyncio.run(self.cog.silence.callback(*silence_call_args)) + self.ctx.send.call_args.assert_called_once_with(result_message) -- cgit v1.2.3 From cfbe3b9742b5531bdced1d5b099739f01033a6bb Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 22:20:00 +0100 Subject: Add test for `unsilence` discord output. --- tests/bot/cogs/moderation/test_silence.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 088410bee..17420ce7d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -56,3 +56,12 @@ class SilenceTests(unittest.TestCase): ): asyncio.run(self.cog.silence.callback(*silence_call_args)) self.ctx.send.call_args.assert_called_once_with(result_message) + + def test_unsilence_sent_correct_discord_message(self): + """Check if proper message was sent to `alert_chanel`.""" + with mock.patch( + "bot.cogs.moderation.silence.Silence._unsilence", + new_callable=partial(mock.AsyncMock, return_value=True) + ): + asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) + self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") -- cgit v1.2.3 From 7acaa717aab47f470353dcb49ee0202e86339d7c Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 22:20:43 +0100 Subject: Use `Context.invoke` instead of calling `unsilence` directly. Calling the command coro directly did unnecessary checks and made tests for the method harder to realize. --- bot/cogs/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 0081a420e..266d6dedd 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -68,7 +68,7 @@ class Silence(commands.Cog): await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced for {duration} minute(s).") await asyncio.sleep(duration*60) - await self.unsilence(ctx, channel) + await ctx.invoke(self.unsilence, channel=channel) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, channel: TextChannelConverter = None) -> None: -- cgit v1.2.3 From 01c7f193806e494408792eb3907280dccad3eacf Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 22:20:57 +0100 Subject: Remove "Channel" from output string for consistency. --- bot/cogs/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 266d6dedd..10185761c 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -63,7 +63,7 @@ class Silence(commands.Cog): await ctx.send(f"{Emojis.cross_mark} {channel.mention} is already silenced.") return if duration is None: - await ctx.send(f"{Emojis.check_mark} Channel {channel.mention} silenced indefinitely.") + await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced indefinitely.") return await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced for {duration} minute(s).") -- cgit v1.2.3 From 57e7fc0b02704dd65b3307e92be87237f806cb68 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 22:21:43 +0100 Subject: Move notifier to separate class. Separating the notifier allows us to keep the Silence class and its methods to be more focused on the class' purpose, handling the logic of adding/removing channels and the loop itself behind `SilenceNotifier`'s interface. --- bot/cogs/moderation/silence.py | 86 ++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 10185761c..e12b6c606 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -29,21 +29,59 @@ class FirstHash(tuple): return self[0] == other[0] +class SilenceNotifier(tasks.Loop): + """Loop notifier for posting notices to `alert_channel` containing added channels.""" + + def __init__(self, alert_channel: TextChannel): + super().__init__(self._notifier, seconds=1, minutes=0, hours=0, count=None, reconnect=True, loop=None) + self._silenced_channels = set() + self._alert_channel = alert_channel + + def add_channel(self, channel: TextChannel) -> None: + """Add channel to `_silenced_channels` and start loop if not launched.""" + if not self._silenced_channels: + self.start() + log.trace("Starting notifier loop.") + self._silenced_channels.add(FirstHash(channel, self._current_loop)) + + def remove_channel(self, channel: TextChannel) -> None: + """Remove channel from `_silenced_channels` and stop loop if no channels remain.""" + with suppress(KeyError): + self._silenced_channels.remove(FirstHash(channel)) + if not self._silenced_channels: + self.stop() + log.trace("Stopping notifier loop.") + + async def _notifier(self) -> None: + """Post notice of `_silenced_channels` with their silenced duration to `_alert_channel` periodically.""" + # Wait for 15 minutes between notices with pause at start of loop. + if self._current_loop and not self._current_loop/60 % 15: + log.debug( + f"Sending notice with channels: " + f"{', '.join(f'#{channel} ({channel.id})' for channel, _ in self._silenced_channels)}." + ) + channels_text = ', '.join( + f"{channel.mention} for {(self._current_loop-start)//60} min" + for channel, start in self._silenced_channels + ) + await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") + + class Silence(commands.Cog): """Commands for stopping channel messages for `verified` role in a channel.""" def __init__(self, bot: Bot): self.bot = bot - self.loop_alert_channels = set() - self.bot.loop.create_task(self._get_server_values()) + self.bot.loop.create_task(self._get_instance_vars()) - async def _get_server_values(self) -> None: - """Fetch required internal values after they're available.""" + async def _get_instance_vars(self) -> None: + """Get instance variables after they're available to get from the guild.""" await self.bot.wait_until_guild_available() guild = self.bot.get_guild(Guild.id) self._verified_role = guild.get_role(Roles.verified) self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) self._mod_log_channel = self.bot.get_channel(Channels.mod_log) + self.notifier = SilenceNotifier(self._mod_log_channel) @commands.command(aliases=("hush",)) async def silence( @@ -87,8 +125,7 @@ class Silence(commands.Cog): """ Silence `channel` for `self._verified_role`. - If `persistent` is `True` add `channel` with current iteration of `self._notifier` - to `self.self.loop_alert_channels` and attempt to start notifier. + If `persistent` is `True` add `channel` to notifier. `duration` is only used for logging; if None is passed `persistent` should be True to not log None. """ if channel.overwrites_for(self._verified_role).send_messages is False: @@ -97,9 +134,7 @@ class Silence(commands.Cog): await channel.set_permissions(self._verified_role, overwrite=PermissionOverwrite(send_messages=False)) if persistent: log.debug(f"Silenced #{channel} ({channel.id}) indefinitely.") - self.loop_alert_channels.add(FirstHash(channel, self._notifier.current_loop)) - with suppress(RuntimeError): - self._notifier.start() + self.notifier.add_channel(channel) return True log.debug(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") @@ -110,45 +145,16 @@ class Silence(commands.Cog): Unsilence `channel`. Check if `channel` is silenced through a `PermissionOverwrite`, - if it is unsilence it, attempt to remove it from `self.loop_alert_channels` - and if `self.loop_alert_channels` are left empty, stop the `self._notifier` + if it is unsilence it and remove it from the notifier. """ if channel.overwrites_for(self._verified_role).send_messages is False: await channel.set_permissions(self._verified_role, overwrite=None) log.debug(f"Unsilenced channel #{channel} ({channel.id}).") - - with suppress(KeyError): - self.loop_alert_channels.remove(FirstHash(channel)) - if not self.loop_alert_channels: - self._notifier.cancel() + self.notifier.remove_channel(channel) return True log.debug(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False - @tasks.loop() - async def _notifier(self) -> None: - """Post notice of permanently silenced channels to `mod_alerts` periodically.""" - # Wait for 15 minutes between notices with pause at start of loop. - await asyncio.sleep(15*60) - current_iter = self._notifier.current_loop+1 - channels_text = ', '.join( - f"{channel.mention} for {current_iter-start} min" - for channel, start in self.loop_alert_channels - ) - channels_log_text = ', '.join( - f'#{channel} ({channel.id})' for channel, _ in self.loop_alert_channels - ) - log.debug(f"Sending notice with channels: {channels_log_text}") - await self._mod_alerts_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") - - @_notifier.before_loop - async def _log_notifier_start(self) -> None: - log.trace("Starting notifier loop.") - - @_notifier.after_loop - async def _log_notifier_end(self) -> None: - log.trace("Stopping notifier loop.") - # This cannot be static (must have a __func__ attribute). def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" -- cgit v1.2.3 From ee94c38063981ee6770c1d263eab9c0d2e178380 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 9 Mar 2020 20:41:55 +0100 Subject: Use `patch.object` instead of patch with direct `return_value`. --- tests/bot/cogs/moderation/test_silence.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 17420ce7d..53b3fd388 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,5 @@ import asyncio import unittest -from functools import partial from unittest import mock from bot.cogs.moderation.silence import FirstHash, Silence @@ -50,18 +49,12 @@ class SilenceTests(unittest.TestCase): result_message=result_message, starting_unsilenced_state=_silence_patch_return ): - with mock.patch( - "bot.cogs.moderation.silence.Silence._silence", - new_callable=partial(mock.AsyncMock, return_value=_silence_patch_return) - ): + with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): asyncio.run(self.cog.silence.callback(*silence_call_args)) self.ctx.send.call_args.assert_called_once_with(result_message) def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" - with mock.patch( - "bot.cogs.moderation.silence.Silence._unsilence", - new_callable=partial(mock.AsyncMock, return_value=True) - ): + with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") -- cgit v1.2.3 From d9bf06e7b916a7214f00b43cb08b582485f86781 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 10 Mar 2020 00:38:43 +0100 Subject: Retain previous channel overwrites. Previously silencing a channel reset all overwrites excluding `send_messages` and unsilencing them removed all overwrites. This is prevented by getting the current overwrite and applying it with only send_messages changed. --- bot/cogs/moderation/silence.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index e12b6c606..626c1ecfb 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -128,10 +128,14 @@ class Silence(commands.Cog): If `persistent` is `True` add `channel` to notifier. `duration` is only used for logging; if None is passed `persistent` should be True to not log None. """ - if channel.overwrites_for(self._verified_role).send_messages is False: + current_overwrite = channel.overwrites_for(self._verified_role) + if current_overwrite.send_messages is False: log.debug(f"Tried to silence channel #{channel} ({channel.id}) but the channel was already silenced.") return False - await channel.set_permissions(self._verified_role, overwrite=PermissionOverwrite(send_messages=False)) + await channel.set_permissions( + self._verified_role, + overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=False)) + ) if persistent: log.debug(f"Silenced #{channel} ({channel.id}) indefinitely.") self.notifier.add_channel(channel) @@ -147,8 +151,12 @@ class Silence(commands.Cog): Check if `channel` is silenced through a `PermissionOverwrite`, if it is unsilence it and remove it from the notifier. """ - if channel.overwrites_for(self._verified_role).send_messages is False: - await channel.set_permissions(self._verified_role, overwrite=None) + current_overwrite = channel.overwrites_for(self._verified_role) + if current_overwrite.send_messages is False: + await channel.set_permissions( + self._verified_role, + overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=True)) + ) log.debug(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) return True -- cgit v1.2.3 From 90a2e14abb898f39000cf11cfd26f0d89abb4800 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 10 Mar 2020 19:43:57 +0100 Subject: Remove `channel` arg from commands. --- bot/cogs/moderation/silence.py | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 626c1ecfb..0fe720882 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -5,7 +5,7 @@ from typing import Optional from discord import PermissionOverwrite, TextChannel from discord.ext import commands, tasks -from discord.ext.commands import Context, TextChannelConverter +from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles @@ -84,42 +84,32 @@ class Silence(commands.Cog): self.notifier = SilenceNotifier(self._mod_log_channel) @commands.command(aliases=("hush",)) - async def silence( - self, - ctx: Context, - duration: HushDurationConverter = 10, - channel: TextChannelConverter = None - ) -> None: + async def silence(self, ctx: Context, duration: HushDurationConverter = 10) -> None: """ Silence `channel` for `duration` minutes or `"forever"`. If duration is forever, start a notifier loop that triggers every 15 minutes. """ - channel = channel or ctx.channel - - if not await self._silence(channel, persistent=(duration is None), duration=duration): - await ctx.send(f"{Emojis.cross_mark} {channel.mention} is already silenced.") + if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): + await ctx.send(f"{Emojis.cross_mark} {ctx.channel.mention} is already silenced.") return if duration is None: - await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced indefinitely.") + await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced indefinitely.") return - await ctx.send(f"{Emojis.check_mark} {channel.mention} silenced for {duration} minute(s).") + await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced for {duration} minute(s).") await asyncio.sleep(duration*60) - await ctx.invoke(self.unsilence, channel=channel) + await ctx.invoke(self.unsilence) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, channel: TextChannelConverter = None) -> None: + async def unsilence(self, ctx: Context) -> None: """ Unsilence `channel`. Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. """ - channel = channel or ctx.channel - alert_channel = self._mod_log_channel if ctx.invoked_with == "hush" else ctx.channel - - if await self._unsilence(channel): - await alert_channel.send(f"{Emojis.check_mark} Unsilenced {channel.mention}.") + if await self._unsilence(ctx.channel): + await ctx.send(f"{Emojis.check_mark} Unsilenced {ctx.channel.mention}.") async def _silence(self, channel: TextChannel, persistent: bool, duration: Optional[int]) -> bool: """ -- cgit v1.2.3 From 39b2319bff31a7b3e2cb17154bcbdad0a0e71fc7 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 10 Mar 2020 23:38:53 +0100 Subject: Add alert with silenced channels on `cog_unload`. --- bot/cogs/moderation/silence.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 0fe720882..76c5a171d 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -72,6 +72,7 @@ class Silence(commands.Cog): def __init__(self, bot: Bot): self.bot = bot + self.muted_channels = set() self.bot.loop.create_task(self._get_instance_vars()) async def _get_instance_vars(self) -> None: @@ -131,6 +132,7 @@ class Silence(commands.Cog): self.notifier.add_channel(channel) return True + self.muted_channels.add(channel) log.debug(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") return True @@ -149,10 +151,19 @@ class Silence(commands.Cog): ) log.debug(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) + with suppress(KeyError): + self.muted_channels.remove(channel) return True log.debug(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False + def cog_unload(self) -> None: + """Send alert with silenced channels on unload.""" + if self.muted_channels: + channels_string = ''.join(channel.mention for channel in self.muted_channels) + message = f"<@&{Roles.moderators}> channels left silenced on cog unload: {channels_string}" + asyncio.create_task(self._mod_alerts_channel.send(message)) + # This cannot be static (must have a __func__ attribute). def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" -- cgit v1.2.3 From 85c439fbf78f59ff314f4f9daef1467d486709c3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 00:01:58 +0100 Subject: Remove unnecessary args from test cases. Needless call args which were constant were kept in the test cases, resulting in redundant code, the args were moved directly into the function call. --- tests/bot/cogs/moderation/test_silence.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 53b3fd388..1341911d5 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -39,18 +39,18 @@ class SilenceTests(unittest.TestCase): def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( - ((self.cog, self.ctx, 0.0001), f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), - ((self.cog, self.ctx, None), f"{Emojis.check_mark} #channel silenced indefinitely.", True,), - ((self.cog, self.ctx, 5), f"{Emojis.cross_mark} #channel is already silenced.", False,), + (0.0001, f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), + (None, f"{Emojis.check_mark} #channel silenced indefinitely.", True,), + (5, f"{Emojis.cross_mark} #channel is already silenced.", False,), ) - for silence_call_args, result_message, _silence_patch_return in test_cases: + for duration, result_message, _silence_patch_return in test_cases: with self.subTest( - silence_duration=silence_call_args[-1], + silence_duration=duration, result_message=result_message, starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): - asyncio.run(self.cog.silence.callback(*silence_call_args)) + asyncio.run(self.cog.silence.callback(self.cog, self.ctx, duration)) self.ctx.send.call_args.assert_called_once_with(result_message) def test_unsilence_sent_correct_discord_message(self): -- cgit v1.2.3 From adaf456607ba2f2724c6fd34308cd170c81aa651 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 00:56:31 +0100 Subject: Remove channel mentions from output discord messages. With the removal of the channel args, it's no longer necessary to mention the channel in the command output. Tests adjusted accordingly --- bot/cogs/moderation/silence.py | 8 ++++---- tests/bot/cogs/moderation/test_silence.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 76c5a171d..68cad4062 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -92,13 +92,13 @@ class Silence(commands.Cog): If duration is forever, start a notifier loop that triggers every 15 minutes. """ if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): - await ctx.send(f"{Emojis.cross_mark} {ctx.channel.mention} is already silenced.") + await ctx.send(f"{Emojis.cross_mark} current channel is already silenced.") return if duration is None: - await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced indefinitely.") + await ctx.send(f"{Emojis.check_mark} silenced current channel indefinitely.") return - await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced for {duration} minute(s).") + await ctx.send(f"{Emojis.check_mark} silenced current channel for {duration} minute(s).") await asyncio.sleep(duration*60) await ctx.invoke(self.unsilence) @@ -110,7 +110,7 @@ class Silence(commands.Cog): Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. """ if await self._unsilence(ctx.channel): - await ctx.send(f"{Emojis.check_mark} Unsilenced {ctx.channel.mention}.") + await ctx.send(f"{Emojis.check_mark} unsilenced current channel.") async def _silence(self, channel: TextChannel, persistent: bool, duration: Optional[int]) -> bool: """ diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 1341911d5..6da374a8f 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -39,9 +39,9 @@ class SilenceTests(unittest.TestCase): def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( - (0.0001, f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), - (None, f"{Emojis.check_mark} #channel silenced indefinitely.", True,), - (5, f"{Emojis.cross_mark} #channel is already silenced.", False,), + (0.0001, f"{Emojis.check_mark} silenced current channel for 0.0001 minute(s).", True,), + (None, f"{Emojis.check_mark} silenced current channel indefinitely.", True,), + (5, f"{Emojis.cross_mark} current channel is already silenced.", False,), ) for duration, result_message, _silence_patch_return in test_cases: with self.subTest( @@ -57,4 +57,4 @@ class SilenceTests(unittest.TestCase): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) - self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") + self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From a24ffb3b53ca2043afe4d428e1456b6979c1f888 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 01:21:54 +0100 Subject: Move adding of channel to `muted_channels` up. Before the channel was not added if `persistent` was `True`. --- bot/cogs/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 68cad4062..4153b3439 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -127,12 +127,12 @@ class Silence(commands.Cog): self._verified_role, overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=False)) ) + self.muted_channels.add(channel) if persistent: log.debug(f"Silenced #{channel} ({channel.id}) indefinitely.") self.notifier.add_channel(channel) return True - self.muted_channels.add(channel) log.debug(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") return True -- cgit v1.2.3 From 68d43946d1dc6393a4f7b8b4812b5c4787842c12 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 01:28:26 +0100 Subject: Add test for `_silence` method. --- tests/bot/cogs/moderation/test_silence.py | 35 ++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6da374a8f..6a75db2a0 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,10 +1,11 @@ import asyncio import unittest from unittest import mock +from unittest.mock import Mock from bot.cogs.moderation.silence import FirstHash, Silence from bot.constants import Emojis -from tests.helpers import MockBot, MockContext +from tests.helpers import MockBot, MockContext, MockTextChannel class FirstHashTests(unittest.TestCase): @@ -35,6 +36,7 @@ class SilenceTests(unittest.TestCase): self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() + self.cog._verified_role = None def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" @@ -58,3 +60,34 @@ class SilenceTests(unittest.TestCase): with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") + + def test_silence_private_for_false(self): + """Permissions are not set and `False` is returned in an already silenced channel.""" + perm_overwrite = Mock(send_messages=False) + channel = Mock(overwrites_for=Mock(return_value=perm_overwrite)) + + self.assertFalse(asyncio.run(self.cog._silence(channel, True, None))) + channel.set_permissions.assert_not_called() + + def test_silence_private_silenced_channel(self): + """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" + channel = MockTextChannel() + muted_channels = Mock() + with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): + self.assertTrue(asyncio.run(self.cog._silence(channel, False, None))) + channel.set_permissions.assert_called_once() + self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + muted_channels.add.call_args.assert_called_once_with(channel) + + def test_silence_private_notifier(self): + """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" + channel = MockTextChannel() + with mock.patch.object(self.cog, "notifier", create=True): + with self.subTest(persistent=True): + asyncio.run(self.cog._silence(channel, True, None)) + self.cog.notifier.add_channel.assert_called_once() + + with mock.patch.object(self.cog, "notifier", create=True): + with self.subTest(persistent=False): + asyncio.run(self.cog._silence(channel, False, None)) + self.cog.notifier.add_channel.assert_not_called() -- cgit v1.2.3 From fef8c8e8504d8431ae7cad23128733d0b9039c7a Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:31:36 +0100 Subject: Use async test case. This allows us to use coroutines with await directly instead of asyncio.run --- tests/bot/cogs/moderation/test_silence.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6a75db2a0..33ff78ca6 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,4 +1,3 @@ -import asyncio import unittest from unittest import mock from unittest.mock import Mock @@ -30,15 +29,14 @@ class FirstHashTests(unittest.TestCase): self.assertTrue(tuple1 == tuple2) -class SilenceTests(unittest.TestCase): +class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: - self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() self.cog._verified_role = None - def test_silence_sent_correct_discord_message(self): + async def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( (0.0001, f"{Emojis.check_mark} silenced current channel for 0.0001 minute(s).", True,), @@ -52,42 +50,42 @@ class SilenceTests(unittest.TestCase): starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): - asyncio.run(self.cog.silence.callback(self.cog, self.ctx, duration)) + await self.cog.silence.callback(self.cog, self.ctx, duration) self.ctx.send.call_args.assert_called_once_with(result_message) - def test_unsilence_sent_correct_discord_message(self): + async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): - asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) + await self.cog.unsilence.callback(self.cog, self.ctx) self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") - def test_silence_private_for_false(self): + async def test_silence_private_for_false(self): """Permissions are not set and `False` is returned in an already silenced channel.""" perm_overwrite = Mock(send_messages=False) channel = Mock(overwrites_for=Mock(return_value=perm_overwrite)) - self.assertFalse(asyncio.run(self.cog._silence(channel, True, None))) + self.assertFalse(await self.cog._silence(channel, True, None)) channel.set_permissions.assert_not_called() - def test_silence_private_silenced_channel(self): + async def test_silence_private_silenced_channel(self): """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" channel = MockTextChannel() muted_channels = Mock() with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): - self.assertTrue(asyncio.run(self.cog._silence(channel, False, None))) + self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) muted_channels.add.call_args.assert_called_once_with(channel) - def test_silence_private_notifier(self): + async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" channel = MockTextChannel() with mock.patch.object(self.cog, "notifier", create=True): with self.subTest(persistent=True): - asyncio.run(self.cog._silence(channel, True, None)) + await self.cog._silence(channel, True, None) self.cog.notifier.add_channel.assert_called_once() with mock.patch.object(self.cog, "notifier", create=True): with self.subTest(persistent=False): - asyncio.run(self.cog._silence(channel, False, None)) + await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() -- cgit v1.2.3 From c575beccdbe5e4e715a4b11b378dd969a0327191 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:47:07 +0100 Subject: Add tests for `_unsilence` --- tests/bot/cogs/moderation/test_silence.py | 34 ++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 33ff78ca6..acfa3ffb8 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,6 @@ import unittest from unittest import mock -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence from bot.constants import Emojis @@ -89,3 +89,35 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(persistent=False): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() + + async def test_unsilence_private_for_false(self): + """Permissions are not set and `False` is returned in an unsilenced channel.""" + channel = Mock() + self.assertFalse(await self.cog._unsilence(channel)) + channel.set_permissions.assert_not_called() + + async def test_unsilence_private_unsilenced_channel(self): + """Channel had `send_message` permissions restored""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "notifier", create=True): + self.assertTrue(await self.cog._unsilence(channel)) + channel.set_permissions.assert_called_once() + self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + + async def test_unsilence_private_removed_notifier(self): + """Channel was removed from `notifier` on unsilence.""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "notifier", create=True): + await self.cog._unsilence(channel) + self.cog.notifier.remove_channel.call_args.assert_called_once_with(channel) + + async def test_unsilence_private_removed_muted_channel(self): + """Channel was removed from `muted_channels` on unsilence.""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "muted_channels", create=True),\ + mock.patch.object(self.cog, "notifier", create=True): # noqa E127 + await self.cog._unsilence(channel) + self.cog.muted_channels.remove.call_args.assert_called_once_with(channel) -- cgit v1.2.3 From a3f07589b215317d6a0fc16d982c3b645fe96151 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:54:35 +0100 Subject: Separate tests for permissions and `muted_channels.add` on `_silence`. --- tests/bot/cogs/moderation/test_silence.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index acfa3ffb8..3a513f3a7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -68,14 +68,11 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_not_called() async def test_silence_private_silenced_channel(self): - """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" + """Channel had `send_message` permissions revoked.""" channel = MockTextChannel() - muted_channels = Mock() - with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): - self.assertTrue(await self.cog._silence(channel, False, None)) + self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) - muted_channels.add.call_args.assert_called_once_with(channel) async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" @@ -90,6 +87,12 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() + async def test_silence_private_removed_muted_channel(self): + channel = MockTextChannel() + with mock.patch.object(self.cog, "muted_channels") as muted_channels: + await self.cog._silence(MockTextChannel(), False, None) + muted_channels.add.call_args.assert_called_once_with(channel) + async def test_unsilence_private_for_false(self): """Permissions are not set and `False` is returned in an unsilenced channel.""" channel = Mock() -- cgit v1.2.3 From c72d31f717ac5e755fe3848c99ebf426fcdf6d8b Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:58:09 +0100 Subject: Use patch decorators and assign names from `with` patches. --- tests/bot/cogs/moderation/test_silence.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 3a513f3a7..027508661 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -99,28 +99,28 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._unsilence(channel)) channel.set_permissions.assert_not_called() - async def test_unsilence_private_unsilenced_channel(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_unsilenced_channel(self, _): """Channel had `send_message` permissions restored""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "notifier", create=True): - self.assertTrue(await self.cog._unsilence(channel)) + self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) - async def test_unsilence_private_removed_notifier(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_removed_notifier(self, notifier): """Channel was removed from `notifier` on unsilence.""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "notifier", create=True): - await self.cog._unsilence(channel) - self.cog.notifier.remove_channel.call_args.assert_called_once_with(channel) + await self.cog._unsilence(channel) + notifier.remove_channel.call_args.assert_called_once_with(channel) - async def test_unsilence_private_removed_muted_channel(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_removed_muted_channel(self, _): """Channel was removed from `muted_channels` on unsilence.""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "muted_channels", create=True),\ - mock.patch.object(self.cog, "notifier", create=True): # noqa E127 + with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - self.cog.muted_channels.remove.call_args.assert_called_once_with(channel) + muted_channels.remove.call_args.assert_called_once_with(channel) -- cgit v1.2.3 From 44967038f39f4ecd1375fb9edff2b972becb5661 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 14:51:15 +0100 Subject: Add test for `cog_unload`. --- tests/bot/cogs/moderation/test_silence.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 027508661..fc2600f5c 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -3,7 +3,7 @@ from unittest import mock from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence -from bot.constants import Emojis +from bot.constants import Emojis, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -124,3 +124,19 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) muted_channels.remove.call_args.assert_called_once_with(channel) + + @mock.patch("bot.cogs.moderation.silence.asyncio") + @mock.patch.object(Silence, "_mod_alerts_channel", create=True) + def test_cog_unload(self, alert_channel, asyncio_mock): + """Task for sending an alert was created with present `muted_channels`.""" + with mock.patch.object(self.cog, "muted_channels"): + self.cog.cog_unload() + asyncio_mock.create_task.call_args.assert_called_once_with( + alert_channel.send(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") + ) + + @mock.patch("bot.cogs.moderation.silence.asyncio") + def test_cog_unload1(self, asyncio_mock): + """No task created with no channels.""" + self.cog.cog_unload() + asyncio_mock.create_task.assert_not_called() -- cgit v1.2.3 From cb9397ba9ef311917629c8904087c1b3c38cc2d3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:26:03 +0100 Subject: Add test for `cog_check`. --- tests/bot/cogs/moderation/test_silence.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index fc2600f5c..eaf897d1d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -140,3 +140,10 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """No task created with no channels.""" self.cog.cog_unload() asyncio_mock.create_task.assert_not_called() + + @mock.patch("bot.cogs.moderation.silence.with_role_check") + @mock.patch("bot.cogs.moderation.silence.MODERATION_ROLES", new=(1, 2, 3)) + def test_cog_check(self, role_check): + """Role check is called with `MODERATION_ROLES`""" + self.cog.cog_check(self.ctx) + role_check.assert_called_once_with(self.ctx, *(1, 2, 3)) -- cgit v1.2.3 From fbee48ee04dc6b44f97f229549c62cbfd5cef615 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:32:06 +0100 Subject: Fix erroneous `assert_called_once_with` calls. `assert_called_once_with` was being tested on call_args which always reported success.st. --- tests/bot/cogs/moderation/test_silence.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index eaf897d1d..4163a9af7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -51,13 +51,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) - self.ctx.send.call_args.assert_called_once_with(result_message) + self.ctx.send.assert_called_once_with(result_message) async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): await self.cog.unsilence.callback(self.cog, self.ctx) - self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") + self.ctx.send.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") async def test_silence_private_for_false(self): """Permissions are not set and `False` is returned in an already silenced channel.""" @@ -91,7 +91,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(MockTextChannel(), False, None) - muted_channels.add.call_args.assert_called_once_with(channel) + muted_channels.add.assert_called_once_with(channel) async def test_unsilence_private_for_false(self): """Permissions are not set and `False` is returned in an unsilenced channel.""" @@ -114,7 +114,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) await self.cog._unsilence(channel) - notifier.remove_channel.call_args.assert_called_once_with(channel) + notifier.remove_channel.assert_called_once_with(channel) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_muted_channel(self, _): @@ -123,7 +123,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - muted_channels.remove.call_args.assert_called_once_with(channel) + muted_channels.remove.assert_called_once_with(channel) @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) @@ -131,9 +131,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() - asyncio_mock.create_task.call_args.assert_called_once_with( - alert_channel.send(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") - ) + asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) + alert_channel.send.called_once_with(f"<@&{Roles.moderators}> chandnels left silenced on cog unload: ") @mock.patch("bot.cogs.moderation.silence.asyncio") def test_cog_unload1(self, asyncio_mock): -- cgit v1.2.3 From 64b27e557acf268a19246b2eb80ad6a743df95f4 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:32:24 +0100 Subject: Reset `self.ctx` call history after every subtest. --- tests/bot/cogs/moderation/test_silence.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 4163a9af7..ab2f091ec 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -52,6 +52,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) self.ctx.send.assert_called_once_with(result_message) + self.ctx.reset_mock() async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" -- cgit v1.2.3 From 10428d9a456c7bce533cda53100e4c35930211d6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:33:05 +0100 Subject: Pass created channel instead of new object. Creating a new object caused the assert to fail because different objects were used. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index ab2f091ec..23f8a84ab 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -91,7 +91,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_silence_private_removed_muted_channel(self): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: - await self.cog._silence(MockTextChannel(), False, None) + await self.cog._silence(channel, False, None) muted_channels.add.assert_called_once_with(channel) async def test_unsilence_private_for_false(self): -- cgit v1.2.3 From b2aa9af7f9f1485aa3ae8ed4d029fd2d72ea17ad Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 16:02:35 +0100 Subject: Add tests for `_get_instance_vars`. --- tests/bot/cogs/moderation/test_silence.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 23f8a84ab..c9aa7d84f 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -3,7 +3,7 @@ from unittest import mock from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence -from bot.constants import Emojis, Roles +from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -36,6 +36,33 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.ctx = MockContext() self.cog._verified_role = None + async def test_instance_vars_got_guild(self): + """Bot got guild after it became available.""" + await self.cog._get_instance_vars() + self.bot.wait_until_guild_available.assert_called_once() + self.bot.get_guild.assert_called_once_with(Guild.id) + + async def test_instance_vars_got_role(self): + """Got `Roles.verified` role from guild.""" + await self.cog._get_instance_vars() + guild = self.bot.get_guild() + guild.get_role.assert_called_once_with(Roles.verified) + + async def test_instance_vars_got_channels(self): + """Got channels from bot.""" + await self.cog._get_instance_vars() + self.bot.get_channel.called_once_with(Channels.mod_alerts) + self.bot.get_channel.called_once_with(Channels.mod_log) + + @mock.patch("bot.cogs.moderation.silence.SilenceNotifier") + async def test_instance_vars_got_notifier(self, notifier): + """Notifier was started with channel.""" + mod_log = MockTextChannel() + self.bot.get_channel.side_effect = (None, mod_log) + await self.cog._get_instance_vars() + notifier.assert_called_once_with(mod_log) + self.bot.get_channel.side_effect = None + async def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( -- cgit v1.2.3 From 8ee70ffe645621a6b97172176afe1ac63261df31 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 16:10:38 +0100 Subject: Create test case for `SilenceNotifier` --- tests/bot/cogs/moderation/test_silence.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c9aa7d84f..fc7734d45 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,7 +2,7 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock -from bot.cogs.moderation.silence import FirstHash, Silence +from bot.cogs.moderation.silence import FirstHash, Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -29,6 +29,12 @@ class FirstHashTests(unittest.TestCase): self.assertTrue(tuple1 == tuple2) +class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): + def setUp(self) -> None: + self.alert_channel = MockTextChannel() + self.notifier = SilenceNotifier(self.alert_channel) + + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.bot = MockBot() -- cgit v1.2.3 From fd75f10f3c8a588bd1763873baad08b8f90d58a3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 17:09:40 +0100 Subject: Add tests for `add_channel`. --- tests/bot/cogs/moderation/test_silence.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index fc7734d45..be5b8e550 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -33,6 +33,27 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.alert_channel = MockTextChannel() self.notifier = SilenceNotifier(self.alert_channel) + self.notifier.stop = self.notifier_stop_mock = Mock() + self.notifier.start = self.notifier_start_mock = Mock() + self.notifier._current_loop = self.current_loop_mock = Mock() + + def test_add_channel_adds_channel(self): + """Channel in FirstHash with current loop is added to internal set.""" + channel = Mock() + with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: + self.notifier.add_channel(channel) + silenced_channels.add.assert_called_with(FirstHash(channel, self.current_loop_mock)) + + def test_add_channel_starts_loop(self): + """Loop is started if `_silenced_channels` was empty.""" + self.notifier.add_channel(Mock()) + self.notifier_start_mock.assert_called_once() + + def test_add_channel_skips_start_with_channels(self): + """Loop start is not called when `_silenced_channels` is not empty.""" + with mock.patch.object(self.notifier, "_silenced_channels"): + self.notifier.add_channel(Mock()) + self.notifier_start_mock.assert_not_called() class SilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From d9c904164a9e54750ce8ee36535bceacfc4800f5 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 18:06:53 +0100 Subject: Remove `_current_loop` from setup. --- tests/bot/cogs/moderation/test_silence.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index be5b8e550..2e04dc407 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -35,14 +35,13 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier = SilenceNotifier(self.alert_channel) self.notifier.stop = self.notifier_stop_mock = Mock() self.notifier.start = self.notifier_start_mock = Mock() - self.notifier._current_loop = self.current_loop_mock = Mock() def test_add_channel_adds_channel(self): """Channel in FirstHash with current loop is added to internal set.""" channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.add_channel(channel) - silenced_channels.add.assert_called_with(FirstHash(channel, self.current_loop_mock)) + silenced_channels.add.assert_called_with(FirstHash(channel, self.notifier._current_loop)) def test_add_channel_starts_loop(self): """Loop is started if `_silenced_channels` was empty.""" -- cgit v1.2.3 From 4740c0fcdc6da6f164963fb34715e78c5d586cec Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 18:07:14 +0100 Subject: Add tests for `remove_channel`. --- tests/bot/cogs/moderation/test_silence.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 2e04dc407..c52ca2a2a 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -54,6 +54,24 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier.add_channel(Mock()) self.notifier_start_mock.assert_not_called() + def test_remove_channel_removes_channel(self): + """Channel in FirstHash is removed from `_silenced_channels`.""" + channel = Mock() + with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: + self.notifier.remove_channel(channel) + silenced_channels.remove.assert_called_with(FirstHash(channel)) + + def test_remove_channel_stops_loop(self): + """Notifier loop is stopped if `_silenced_channels` is empty after remove.""" + with mock.patch.object(self.notifier, "_silenced_channels", __bool__=lambda _: False): + self.notifier.remove_channel(Mock()) + self.notifier_stop_mock.assert_called_once() + + def test_remove_channel_skips_stop_with_channels(self): + """Notifier loop is not stopped if `_silenced_channels` is not empty after remove.""" + self.notifier.remove_channel(Mock()) + self.notifier_stop_mock.assert_not_called() + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: -- cgit v1.2.3 From 2bcadd209e14e3a119806e069b8ae7150a73c1ba Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 19:12:34 +0100 Subject: Change various logging levels. --- bot/cogs/moderation/silence.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 4153b3439..1c751a4b1 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -41,7 +41,7 @@ class SilenceNotifier(tasks.Loop): """Add channel to `_silenced_channels` and start loop if not launched.""" if not self._silenced_channels: self.start() - log.trace("Starting notifier loop.") + log.info("Starting notifier loop.") self._silenced_channels.add(FirstHash(channel, self._current_loop)) def remove_channel(self, channel: TextChannel) -> None: @@ -50,7 +50,7 @@ class SilenceNotifier(tasks.Loop): self._silenced_channels.remove(FirstHash(channel)) if not self._silenced_channels: self.stop() - log.trace("Stopping notifier loop.") + log.info("Stopping notifier loop.") async def _notifier(self) -> None: """Post notice of `_silenced_channels` with their silenced duration to `_alert_channel` periodically.""" @@ -121,7 +121,7 @@ class Silence(commands.Cog): """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: - log.debug(f"Tried to silence channel #{channel} ({channel.id}) but the channel was already silenced.") + log.info(f"Tried to silence channel #{channel} ({channel.id}) but the channel was already silenced.") return False await channel.set_permissions( self._verified_role, @@ -129,11 +129,11 @@ class Silence(commands.Cog): ) self.muted_channels.add(channel) if persistent: - log.debug(f"Silenced #{channel} ({channel.id}) indefinitely.") + log.info(f"Silenced #{channel} ({channel.id}) indefinitely.") self.notifier.add_channel(channel) return True - log.debug(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") + log.info(f"Silenced #{channel} ({channel.id}) for {duration} minute(s).") return True async def _unsilence(self, channel: TextChannel) -> bool: @@ -149,12 +149,12 @@ class Silence(commands.Cog): self._verified_role, overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=True)) ) - log.debug(f"Unsilenced channel #{channel} ({channel.id}).") + log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) with suppress(KeyError): self.muted_channels.remove(channel) return True - log.debug(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") + log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False def cog_unload(self) -> None: -- cgit v1.2.3 From d40a8688bb1e9cf5aa33d7b9fbc5272417eb1c81 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 21:13:43 +0100 Subject: Add logging to commands. --- bot/cogs/moderation/silence.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 1c751a4b1..e1b0b703f 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -91,6 +91,7 @@ class Silence(commands.Cog): If duration is forever, start a notifier loop that triggers every 15 minutes. """ + log.debug(f"{ctx.author} is silencing channel #{ctx.channel}.") if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): await ctx.send(f"{Emojis.cross_mark} current channel is already silenced.") return @@ -100,6 +101,7 @@ class Silence(commands.Cog): await ctx.send(f"{Emojis.check_mark} silenced current channel for {duration} minute(s).") await asyncio.sleep(duration*60) + log.info(f"Unsilencing channel after set delay.") await ctx.invoke(self.unsilence) @commands.command(aliases=("unhush",)) @@ -109,6 +111,7 @@ class Silence(commands.Cog): Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. """ + log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") if await self._unsilence(ctx.channel): await ctx.send(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From 9bb6f4ae560df90bc45ebe2af449fbb100c5970b Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 21:49:14 +0100 Subject: Improve commands help. --- bot/cogs/moderation/silence.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index e1b0b703f..8ed1cb28b 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -87,9 +87,10 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) async def silence(self, ctx: Context, duration: HushDurationConverter = 10) -> None: """ - Silence `channel` for `duration` minutes or `"forever"`. + Silence `channel` for `duration` minutes or `forever`. - If duration is forever, start a notifier loop that triggers every 15 minutes. + Duration is capped at 15 minutes, passing forever makes the silence indefinite. + Indefinitely silenced channels get added to a notifier which posts notices every 15 minutes from the start. """ log.debug(f"{ctx.author} is silencing channel #{ctx.channel}.") if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): @@ -109,7 +110,8 @@ class Silence(commands.Cog): """ Unsilence `channel`. - Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. + Unsilence a previously silenced `channel`, + remove it from notifier of indefinitely silenced channels and cancel the notifier if empty. """ log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") if await self._unsilence(ctx.channel): -- cgit v1.2.3 From 28cf22bcd98d94fa27e80dde4c86c9054b33c538 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 21:55:09 +0100 Subject: Add tests for `_notifier`. --- tests/bot/cogs/moderation/test_silence.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c52ca2a2a..d4719159e 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -72,6 +72,25 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier.remove_channel(Mock()) self.notifier_stop_mock.assert_not_called() + async def test_notifier_private_sends_alert(self): + """Alert is sent on 15 min intervals.""" + test_cases = (900, 1800, 2700) + for current_loop in test_cases: + with self.subTest(current_loop=current_loop): + with mock.patch.object(self.notifier, "_current_loop", new=current_loop): + await self.notifier._notifier() + self.alert_channel.send.assert_called_once_with(f"<@&{Roles.moderators}> currently silenced channels: ") + self.alert_channel.send.reset_mock() + + async def test_notifier_skips_alert(self): + """Alert is skipped on first loop or not an increment of 900.""" + test_cases = (0, 15, 5000) + for current_loop in test_cases: + with self.subTest(current_loop=current_loop): + with mock.patch.object(self.notifier, "_current_loop", new=current_loop): + await self.notifier._notifier() + self.alert_channel.send.assert_not_called() + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: -- cgit v1.2.3 From 252b385e46ef542203e69f4f6d147dadbcec8f0f Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:11:35 +0100 Subject: Use dict instead of a set and custom class. The FirstHash class is no longer necessary with only channels and the current loop in tuples. FirstHash was removed, along with its tests and tests were adjusted for new dict behaviour. --- bot/cogs/moderation/silence.py | 24 +++++------------------- tests/bot/cogs/moderation/test_silence.py | 28 +++------------------------- 2 files changed, 8 insertions(+), 44 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 8ed1cb28b..5df1fbbc0 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -15,26 +15,12 @@ from bot.utils.checks import with_role_check log = logging.getLogger(__name__) -class FirstHash(tuple): - """Tuple with only first item used for hash and eq.""" - - def __new__(cls, *args): - """Construct tuple from `args`.""" - return super().__new__(cls, args) - - def __hash__(self): - return hash((self[0],)) - - def __eq__(self, other: "FirstHash"): - return self[0] == other[0] - - class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" def __init__(self, alert_channel: TextChannel): super().__init__(self._notifier, seconds=1, minutes=0, hours=0, count=None, reconnect=True, loop=None) - self._silenced_channels = set() + self._silenced_channels = {} self._alert_channel = alert_channel def add_channel(self, channel: TextChannel) -> None: @@ -42,12 +28,12 @@ class SilenceNotifier(tasks.Loop): if not self._silenced_channels: self.start() log.info("Starting notifier loop.") - self._silenced_channels.add(FirstHash(channel, self._current_loop)) + self._silenced_channels[channel] = self._current_loop def remove_channel(self, channel: TextChannel) -> None: """Remove channel from `_silenced_channels` and stop loop if no channels remain.""" with suppress(KeyError): - self._silenced_channels.remove(FirstHash(channel)) + del self._silenced_channels[channel] if not self._silenced_channels: self.stop() log.info("Stopping notifier loop.") @@ -58,11 +44,11 @@ class SilenceNotifier(tasks.Loop): if self._current_loop and not self._current_loop/60 % 15: log.debug( f"Sending notice with channels: " - f"{', '.join(f'#{channel} ({channel.id})' for channel, _ in self._silenced_channels)}." + f"{', '.join(f'#{channel} ({channel.id})' for channel in self._silenced_channels)}." ) channels_text = ', '.join( f"{channel.mention} for {(self._current_loop-start)//60} min" - for channel, start in self._silenced_channels + for channel, start in self._silenced_channels.items() ) await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index d4719159e..6114fee21 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,33 +2,11 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock -from bot.cogs.moderation.silence import FirstHash, Silence, SilenceNotifier +from bot.cogs.moderation.silence import Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel -class FirstHashTests(unittest.TestCase): - def setUp(self) -> None: - self.test_cases = ( - (FirstHash(0, 4), FirstHash(0, 5)), - (FirstHash("string", None), FirstHash("string", True)) - ) - - def test_hashes_equal(self): - """Check hashes equal with same first item.""" - - for tuple1, tuple2 in self.test_cases: - with self.subTest(tuple1=tuple1, tuple2=tuple2): - self.assertEqual(hash(tuple1), hash(tuple2)) - - def test_eq(self): - """Check objects are equal with same first item.""" - - for tuple1, tuple2 in self.test_cases: - with self.subTest(tuple1=tuple1, tuple2=tuple2): - self.assertTrue(tuple1 == tuple2) - - class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.alert_channel = MockTextChannel() @@ -41,7 +19,7 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.add_channel(channel) - silenced_channels.add.assert_called_with(FirstHash(channel, self.notifier._current_loop)) + silenced_channels.__setitem__.assert_called_with(channel, self.notifier._current_loop) def test_add_channel_starts_loop(self): """Loop is started if `_silenced_channels` was empty.""" @@ -59,7 +37,7 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.remove_channel(channel) - silenced_channels.remove.assert_called_with(FirstHash(channel)) + silenced_channels.__delitem__.assert_called_with(channel) def test_remove_channel_stops_loop(self): """Notifier loop is stopped if `_silenced_channels` is empty after remove.""" -- cgit v1.2.3 From 39e9fc2dced2a4ce5e210f671bb03036ee91c2c6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:27:53 +0100 Subject: Adjust docstring styling. Co-authored-by: MarkKoz --- 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 45ab1f326..7989acde7 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -32,8 +32,8 @@ class ErrorHandler(Cog): prioritised as follows: 1. If the name fails to match a command: - If it matches shh+ or unshh+, the channel is silenced or unsilenced respectively. - otherwise if it matches a tag, the tag is invoked + * If it matches shh+ or unshh+, the channel is silenced or unsilenced respectively. + Otherwise if it 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 -- cgit v1.2.3 From dc534b72fcb561c057b1584311ca9e27244f08ae Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:47:03 +0100 Subject: Move coro execution outside of if condition. This gives us a clearer look at the general flow control and what's getting executed. Comment was also moved to its relevant line. Co-authored-by: MarkKoz --- bot/cogs/error_handler.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 7989acde7..73757b7b7 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -50,15 +50,13 @@ class ErrorHandler(Cog): log.trace(f"Command {command} had its error already handled locally; ignoring.") return - # Try to look for a tag with the command's name if the command isn't found. - if isinstance(e, errors.CommandNotFound): - if ( - not await self.try_silence(ctx) - and not hasattr(ctx, "invoked_from_error_handler") - and ctx.channel.id != Channels.verification - ): + if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): + if await self.try_silence(ctx): + return + if ctx.channel.id != Channels.verification: + # Try to look for a tag with the command's name await self.try_get_tag(ctx) - return # Exit early to avoid logging. + return # Exit early to avoid logging. elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): -- cgit v1.2.3 From d2fad8a0e21a1cb074244bc371c88cf488229774 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:47:45 +0100 Subject: Add Silence cog load to docstring. --- bot/cogs/moderation/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/__init__.py b/bot/cogs/moderation/__init__.py index 0349fe4b1..6880ca1bd 100644 --- a/bot/cogs/moderation/__init__.py +++ b/bot/cogs/moderation/__init__.py @@ -7,7 +7,7 @@ from .superstarify import Superstarify def setup(bot: Bot) -> None: - """Load the Infractions, ModManagement, ModLog, and Superstarify cogs.""" + """Load the Infractions, ModManagement, ModLog, Silence, and Superstarify cogs.""" bot.add_cog(Infractions(bot)) bot.add_cog(ModLog(bot)) bot.add_cog(ModManagement(bot)) -- cgit v1.2.3 From 4c2b8b715abd32b6818ef0cf4cdd96369eec192e Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:49:37 +0100 Subject: Change BadArgument error wording. Co-authored-by: MarkKoz --- bot/converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/converters.py b/bot/converters.py index 976376fce..635fef1c7 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -286,7 +286,7 @@ class HushDurationConverter(Converter): duration = int(match.group(1)) if duration > 15: - raise BadArgument("Duration must be below 15 minutes.") + raise BadArgument("Duration must be at most 15 minutes.") return duration -- cgit v1.2.3 From 678f8552fddcaca65289f6da0bcaa0c7b5ac14d1 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 17:56:24 +0100 Subject: Pass kwargs directly instead of a PermissionOverwrite. The `set_permissions` method creates a `PermissionOverwrite` from kwargs internally, so we can skip creating it ourselves and unpack the dict directly into kwargs. --- bot/cogs/moderation/silence.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 5df1fbbc0..3d6ca8867 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -3,7 +3,7 @@ import logging from contextlib import suppress from typing import Optional -from discord import PermissionOverwrite, TextChannel +from discord import TextChannel from discord.ext import commands, tasks from discord.ext.commands import Context @@ -114,10 +114,7 @@ class Silence(commands.Cog): if current_overwrite.send_messages is False: log.info(f"Tried to silence channel #{channel} ({channel.id}) but the channel was already silenced.") return False - await channel.set_permissions( - self._verified_role, - overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=False)) - ) + await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=False)) self.muted_channels.add(channel) if persistent: log.info(f"Silenced #{channel} ({channel.id}) indefinitely.") @@ -136,10 +133,7 @@ class Silence(commands.Cog): """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: - await channel.set_permissions( - self._verified_role, - overwrite=PermissionOverwrite(**dict(current_overwrite, send_messages=True)) - ) + await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=True)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) with suppress(KeyError): -- cgit v1.2.3 From a52bff17234788f40e8a349cf78e72579621e8db Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:02:54 +0100 Subject: Assign created task to a var. --- bot/cogs/moderation/silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 3d6ca8867..3a3acf216 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -59,7 +59,7 @@ class Silence(commands.Cog): def __init__(self, bot: Bot): self.bot = bot self.muted_channels = set() - self.bot.loop.create_task(self._get_instance_vars()) + self._get_instance_vars_task = self.bot.loop.create_task(self._get_instance_vars()) async def _get_instance_vars(self) -> None: """Get instance variables after they're available to get from the guild.""" -- cgit v1.2.3 From a17ccdb3d22d10070dfdc79077555fa840f93e96 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:06:20 +0100 Subject: Block commands until all instance vars are loaded. --- bot/cogs/moderation/silence.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 3a3acf216..42047d0f7 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -60,6 +60,7 @@ class Silence(commands.Cog): self.bot = bot self.muted_channels = set() self._get_instance_vars_task = self.bot.loop.create_task(self._get_instance_vars()) + self._get_instance_vars_event = asyncio.Event() async def _get_instance_vars(self) -> None: """Get instance variables after they're available to get from the guild.""" @@ -69,6 +70,7 @@ class Silence(commands.Cog): self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) self._mod_log_channel = self.bot.get_channel(Channels.mod_log) self.notifier = SilenceNotifier(self._mod_log_channel) + self._get_instance_vars_event.set() @commands.command(aliases=("hush",)) async def silence(self, ctx: Context, duration: HushDurationConverter = 10) -> None: @@ -78,6 +80,7 @@ class Silence(commands.Cog): Duration is capped at 15 minutes, passing forever makes the silence indefinite. Indefinitely silenced channels get added to a notifier which posts notices every 15 minutes from the start. """ + await self._get_instance_vars_event.wait() log.debug(f"{ctx.author} is silencing channel #{ctx.channel}.") if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): await ctx.send(f"{Emojis.cross_mark} current channel is already silenced.") @@ -99,6 +102,7 @@ class Silence(commands.Cog): Unsilence a previously silenced `channel`, remove it from notifier of indefinitely silenced channels and cancel the notifier if empty. """ + await self._get_instance_vars_event.wait() log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") if await self._unsilence(ctx.channel): await ctx.send(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From a6c9b4991dbb4dcdbc1e2abf97d1a167e8cd983c Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:10:06 +0100 Subject: Document returns values of private methods. --- bot/cogs/moderation/silence.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 42047d0f7..f532260ca 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -113,6 +113,7 @@ class Silence(commands.Cog): If `persistent` is `True` add `channel` to notifier. `duration` is only used for logging; if None is passed `persistent` should be True to not log None. + Return `True` if channel permissions were changed, `False` otherwise. """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: @@ -134,6 +135,7 @@ class Silence(commands.Cog): Check if `channel` is silenced through a `PermissionOverwrite`, if it is unsilence it and remove it from the notifier. + Return `True` if channel permissions were changed, `False` otherwise. """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: -- cgit v1.2.3 From 36c57c6f89a070fbb77a641182e37c788b6de7a0 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:21:49 +0100 Subject: Adjust tests for new calling behaviour. `.set_permissions` calls were changed to use kwargs directly instead of an overwrite, this reflects the changes in tests. --- tests/bot/cogs/moderation/test_silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6114fee21..b09426fde 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -141,7 +141,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel() self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() - self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + self.assertFalse(channel.set_permissions.call_args.kwargs['send_messages']) async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" @@ -175,7 +175,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() - self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + self.assertTrue(channel.set_permissions.call_args.kwargs['send_messages']) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_notifier(self, notifier): -- cgit v1.2.3 From 0a2774fadddd18a86822a47599ebc4b76f1e5a7e Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:23:11 +0100 Subject: Set `_get_instance_vars_event` in test's `setUp`. --- tests/bot/cogs/moderation/test_silence.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index b09426fde..c6f1fc1da 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -76,6 +76,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog = Silence(self.bot) self.ctx = MockContext() self.cog._verified_role = None + # Set event so command callbacks can continue. + self.cog._get_instance_vars_event.set() async def test_instance_vars_got_guild(self): """Bot got guild after it became available.""" -- cgit v1.2.3 From 166a4368a786c7aa1446a0348cec8c19307f1e55 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 23:20:03 +0100 Subject: Remove long indentation from docstrings. --- bot/cogs/error_handler.py | 4 ++-- bot/converters.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 73757b7b7..bad6e51a3 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -98,8 +98,8 @@ class ErrorHandler(Cog): Attempt to invoke the silence or unsilence command if invoke with matches a pattern. Respecting the checks if: - invoked with `shh+` silence channel for amount of h's*2 with max of 15. - invoked with `unshh+` unsilence channel + * invoked with `shh+` silence channel for amount of h's*2 with max of 15. + * invoked with `unshh+` unsilence channel Return bool depending on success of command. """ command = ctx.invoked_with.lower() diff --git a/bot/converters.py b/bot/converters.py index 635fef1c7..2b413f039 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -273,10 +273,10 @@ class HushDurationConverter(Converter): If `"forever"` is passed, None is returned; otherwise an int of the extracted time. Accepted formats are: - , - m, - M, - forever. + * , + * m, + * M, + * forever. """ if argument == "forever": return None -- cgit v1.2.3 From 65057491b31f798aa82cb1e907fda6685d42eb1d Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 16 Mar 2020 17:11:08 +0100 Subject: Handle and log `CommandErrors` on `.can_run`. --- bot/cogs/error_handler.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index bad6e51a3..6a622d2ce 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -104,7 +104,12 @@ class ErrorHandler(Cog): """ command = ctx.invoked_with.lower() silence_command = self.bot.get_command("silence") - if not await silence_command.can_run(ctx): + ctx.invoked_from_error_handler = True + try: + if not await silence_command.can_run(ctx): + log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") + return False + except errors.CommandError: log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") return False if command.startswith("shh"): -- cgit v1.2.3 From a184b304a136d6f2e3373e475433caf7665fde6d Mon Sep 17 00:00:00 2001 From: ks123 Date: Tue, 17 Mar 2020 09:09:20 +0200 Subject: (!zen Command): Added exact word check before `difflib`'s matching, due matching may not count exact word as best choice. --- bot/cogs/utils.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 024141d62..2ca2c028e 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -230,7 +230,16 @@ class Utils(Cog): await ctx.send(embed=embed) return - # handle if it's a search string + # Try to handle first exact word due difflib.SequenceMatched may use some other similar word instead + # exact word. + for i, line in enumerate(zen_lines): + if search_value.lower() in line.lower(): + embed.title += f" (line {i}):" + embed.description = line + await ctx.send(embed=embed) + return + + # handle if it's a search string and not exact word matcher = difflib.SequenceMatcher(None, search_value.lower()) best_match = "" -- cgit v1.2.3 From 039a04462be58e9d345e32efcae13c8c999776db Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 13:23:44 +0100 Subject: Fix `test_cog_unload` passing tests with invalid values. The first assert - `asyncio_mock.create_task.assert_called_once_with` called `alert_channel`'s send resulting in an extra call. `send` on `alert_channel` was not tested properly because of a typo and a missing assert in the method call. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c6f1fc1da..febfd584b 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -202,8 +202,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() + alert_channel.send.assert_called_once_with(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) - alert_channel.send.called_once_with(f"<@&{Roles.moderators}> chandnels left silenced on cog unload: ") @mock.patch("bot.cogs.moderation.silence.asyncio") def test_cog_unload1(self, asyncio_mock): -- cgit v1.2.3 From 2803c13c477634ceefe3501ad9cb7c76cfecf450 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:10:16 +0100 Subject: Rename `cog_unload` tests. Previous names were undescriptive from testing phases. --- tests/bot/cogs/moderation/test_silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index febfd584b..07a70e7dc 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -198,7 +198,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) - def test_cog_unload(self, alert_channel, asyncio_mock): + def test_cog_unload_starts_task(self, alert_channel, asyncio_mock): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() @@ -206,7 +206,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) @mock.patch("bot.cogs.moderation.silence.asyncio") - def test_cog_unload1(self, asyncio_mock): + def test_cog_unload_skips_task_start(self, asyncio_mock): """No task created with no channels.""" self.cog.cog_unload() asyncio_mock.create_task.assert_not_called() -- cgit v1.2.3 From 331cd64c4d874937fad052ff83388e73db3441ee Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:13:17 +0100 Subject: Remove `channel` mentions from command docstrings. With the new behaviour of not accepting channels and muting the current one, it's no longer neccessary to keep the channel param in the docstring. Co-authored-by: MarkKoz --- bot/cogs/moderation/silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index f532260ca..552914ae8 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -75,7 +75,7 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) async def silence(self, ctx: Context, duration: HushDurationConverter = 10) -> None: """ - Silence `channel` for `duration` minutes or `forever`. + Silence the current channel for `duration` minutes or `forever`. Duration is capped at 15 minutes, passing forever makes the silence indefinite. Indefinitely silenced channels get added to a notifier which posts notices every 15 minutes from the start. @@ -97,7 +97,7 @@ class Silence(commands.Cog): @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context) -> None: """ - Unsilence `channel`. + Unsilence the current channel. Unsilence a previously silenced `channel`, remove it from notifier of indefinitely silenced channels and cancel the notifier if empty. -- cgit v1.2.3 From 55de2566581fbc2696c932ed57c5600e9b19f1c9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:14:00 +0100 Subject: Reword `unsilence` docstring. Co-authored-by: MarkKoz --- bot/cogs/moderation/silence.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 552914ae8..1523baf11 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -99,8 +99,7 @@ class Silence(commands.Cog): """ Unsilence the current channel. - Unsilence a previously silenced `channel`, - remove it from notifier of indefinitely silenced channels and cancel the notifier if empty. + If the channel was silenced indefinitely, notifications for the channel will stop. """ await self._get_instance_vars_event.wait() log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") -- cgit v1.2.3 From 20c41f2c5af6fd716c3e7f15de412f7f16f5ff1e Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:15:10 +0100 Subject: Remove one indentation level. Co-authored-by: MarkKoz --- tests/bot/cogs/moderation/test_silence.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 07a70e7dc..8b9e30cfe 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -115,9 +115,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for duration, result_message, _silence_patch_return in test_cases: with self.subTest( - silence_duration=duration, - result_message=result_message, - starting_unsilenced_state=_silence_patch_return + silence_duration=duration, + result_message=result_message, + starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) -- cgit v1.2.3 From d456e40ac97a38ee99561546bcafb6aa94117cb7 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:16:31 +0100 Subject: Remove `alert_channel` mention from docstring. After removing the optional channel arg and changing output message channels we're only testing `ctx`'s `send`. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 8b9e30cfe..b4a34bbc7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -125,7 +125,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.ctx.reset_mock() async def test_unsilence_sent_correct_discord_message(self): - """Check if proper message was sent to `alert_chanel`.""" + """Proper reply after a successful unsilence.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): await self.cog.unsilence.callback(self.cog, self.ctx) self.ctx.send.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From 95dae9bc7a7519c723539382848c02b9748d067f Mon Sep 17 00:00:00 2001 From: ks123 Date: Tue, 17 Mar 2020 19:32:48 +0200 Subject: (!zen Command): Under exact word match, change matching way from substring to sentence split iterate and equality check. --- bot/cogs/utils.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 2ca2c028e..0619296ad 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -233,11 +233,12 @@ class Utils(Cog): # Try to handle first exact word due difflib.SequenceMatched may use some other similar word instead # exact word. for i, line in enumerate(zen_lines): - if search_value.lower() in line.lower(): - embed.title += f" (line {i}):" - embed.description = line - await ctx.send(embed=embed) - return + for word in line.split(): + if word.lower() == search_value.lower(): + embed.title += f" (line {i}):" + embed.description = line + await ctx.send(embed=embed) + return # handle if it's a search string and not exact word matcher = difflib.SequenceMatcher(None, search_value.lower()) -- cgit v1.2.3 From 386c93a6f18adbf84691f17b13f5113800a353ae Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 19:40:27 +0100 Subject: Fix test name. `removed` was describing the opposite behaviour. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index b4a34bbc7..55193e2f8 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -158,7 +158,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() - async def test_silence_private_removed_muted_channel(self): + async def test_silence_private_added_muted_channel(self): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(channel, False, None) -- cgit v1.2.3 From dced6fdf5f571b82bc975dd3159af57c6f9a12b3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 19:41:13 +0100 Subject: Add docstring to test. --- tests/bot/cogs/moderation/test_silence.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 55193e2f8..71541086d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -159,6 +159,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.notifier.add_channel.assert_not_called() async def test_silence_private_added_muted_channel(self): + """Channel was added to `muted_channels` on silence.""" channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(channel, False, None) -- cgit v1.2.3 From c68b943708eaca110ddfa6121872513a422bbef4 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 20:10:50 +0100 Subject: Use set `discard` instead of `remove`. Discard ignores non present values, allowing us to skip the KeyError suppress. --- bot/cogs/moderation/silence.py | 3 +-- tests/bot/cogs/moderation/test_silence.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 1523baf11..a1446089e 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -141,8 +141,7 @@ class Silence(commands.Cog): await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=True)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) - with suppress(KeyError): - self.muted_channels.remove(channel) + self.muted_channels.discard(channel) return True log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 71541086d..eee020455 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -195,7 +195,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - muted_channels.remove.assert_called_once_with(channel) + muted_channels.discard.assert_called_once_with(channel) @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) -- cgit v1.2.3 From cd429230fcb18c7101afd931317d37ad142bfe4b Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 20:11:44 +0100 Subject: Add tests ensuring permissions get preserved. --- tests/bot/cogs/moderation/test_silence.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index eee020455..44682a1bd 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,6 +2,8 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock +from discord import PermissionOverwrite + from bot.cogs.moderation.silence import Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -145,6 +147,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['send_messages']) + async def test_silence_private_preserves_permissions(self): + """Previous permissions were preserved when channel was silenced.""" + channel = MockTextChannel() + # Set up mock channel permission state. + mock_permissions = PermissionOverwrite() + mock_permissions_dict = dict(mock_permissions) + channel.overwrites_for.return_value = mock_permissions + await self.cog._silence(channel, False, None) + new_permissions = channel.set_permissions.call_args.kwargs + # Remove 'send_messages' key because it got changed in the method. + del new_permissions['send_messages'] + del mock_permissions_dict['send_messages'] + self.assertDictEqual(mock_permissions_dict, new_permissions) + async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" channel = MockTextChannel() @@ -197,6 +213,21 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(channel) muted_channels.discard.assert_called_once_with(channel) + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_preserves_permissions(self, _): + """Previous permissions were preserved when channel was unsilenced.""" + channel = MockTextChannel() + # Set up mock channel permission state. + mock_permissions = PermissionOverwrite(send_messages=False) + mock_permissions_dict = dict(mock_permissions) + channel.overwrites_for.return_value = mock_permissions + await self.cog._unsilence(channel) + new_permissions = channel.set_permissions.call_args.kwargs + # Remove 'send_messages' key because it got changed in the method. + del new_permissions['send_messages'] + del mock_permissions_dict['send_messages'] + self.assertDictEqual(mock_permissions_dict, new_permissions) + @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) def test_cog_unload_starts_task(self, alert_channel, asyncio_mock): -- cgit v1.2.3 From cefcc575b6faa94fb18f1985f039125d023b2580 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 22:18:58 +0100 Subject: Add tests for `HushDurationConverter`. --- tests/bot/test_converters.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 1e5ca62ae..ca8cb6825 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -8,6 +8,7 @@ from discord.ext.commands import BadArgument from bot.converters import ( Duration, + HushDurationConverter, ISODateTime, TagContentConverter, TagNameConverter, @@ -271,3 +272,32 @@ class ConverterTests(unittest.TestCase): exception_message = f"`{datetime_string}` is not a valid ISO-8601 datetime string" with self.assertRaises(BadArgument, msg=exception_message): asyncio.run(converter.convert(self.context, datetime_string)) + + def test_hush_duration_converter_for_valid(self): + """HushDurationConverter returns correct value for minutes duration or `"forever"` strings.""" + test_values = ( + ("0", 0), + ("15", 15), + ("10", 10), + ("5m", 5), + ("5M", 5), + ("forever", None), + ) + converter = HushDurationConverter() + for minutes_string, expected_minutes in test_values: + with self.subTest(minutes_string=minutes_string, expected_minutes=expected_minutes): + converted = asyncio.run(converter.convert(self.context, minutes_string)) + self.assertEqual(expected_minutes, converted) + + def test_hush_duration_converter_for_invalid(self): + """HushDurationConverter raises correct exception for invalid minutes duration strings.""" + test_values = ( + ("16", "Duration must be at most 15 minutes."), + ("10d", "10d is not a valid minutes duration."), + ("-1", "-1 is not a valid minutes duration."), + ) + converter = HushDurationConverter() + for invalid_minutes_string, exception_message in test_values: + with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): + with self.assertRaisesRegex(BadArgument, exception_message): + asyncio.run(converter.convert(self.context, invalid_minutes_string)) -- cgit v1.2.3 From 4d33b9f863bb54e69b9530a1ee05e4068cafa9a6 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Thu, 26 Mar 2020 10:56:01 -0400 Subject: Initial pass on log severity reduction With the updated definition on logging levels, there are a few events that were issuing logs at too high of a level. This also includes some kaizening of existing log messages. --- bot/bot.py | 4 ++-- bot/cogs/alias.py | 2 +- bot/cogs/bot.py | 1 - bot/cogs/moderation/scheduler.py | 2 +- bot/cogs/moderation/superstarify.py | 4 ++-- bot/cogs/snekbox.py | 2 +- bot/converters.py | 2 +- 7 files changed, 8 insertions(+), 9 deletions(-) diff --git a/bot/bot.py b/bot/bot.py index 950ac6751..3e1b31342 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -77,7 +77,7 @@ class Bot(commands.Bot): # Its __del__ does send a warning but it doesn't always show up for some reason. if self._connector and not self._connector._closed: - log.warning( + log.info( "The previous connector was not closed; it will remain open and be overwritten" ) @@ -94,7 +94,7 @@ class Bot(commands.Bot): # Its __del__ does send a warning but it doesn't always show up for some reason. if self.http_session and not self.http_session.closed: - log.warning( + log.info( "The previous session was not closed; it will remain open and be overwritten" ) diff --git a/bot/cogs/alias.py b/bot/cogs/alias.py index 9001e18f0..55c7efe65 100644 --- a/bot/cogs/alias.py +++ b/bot/cogs/alias.py @@ -29,7 +29,7 @@ class Alias (Cog): return log.info(f'Did not find command "{cmd_name}" to invoke.') elif not await cmd.can_run(ctx): return log.info( - f'{str(ctx.author)} tried to run the command "{cmd_name}"' + f'{str(ctx.author)} tried to run the command "{cmd_name}" but lacks permission.' ) await ctx.invoke(cmd, *args, **kwargs) diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index f17135877..e897b30ff 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -67,7 +67,6 @@ class BotCog(Cog, name="Bot"): icon_url=URLs.bot_avatar ) - log.info(f"{ctx.author} called !about. Returning information about the bot.") await ctx.send(embed=embed) @command(name='echo', aliases=('print',)) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index f0b6b2c48..917697be9 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -222,7 +222,7 @@ class InfractionScheduler(Scheduler): # If multiple active infractions were found, mark them as inactive in the database # and cancel their expiration tasks. if len(response) > 1: - log.warning( + log.info( f"Found more than one active {infr_type} infraction for user {user.id}; " "deactivating the extra active infractions too." ) diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 893cb7f13..ca3dc4202 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -59,7 +59,7 @@ class Superstarify(InfractionScheduler, Cog): return # Nick change was triggered by this event. Ignore. log.info( - f"{after.display_name} is currently in superstar-prison. " + f"{after.display_name} ({after.id}) tried to escape superstar prison. " f"Changing the nick back to {before.display_name}." ) await after.edit( @@ -80,7 +80,7 @@ class Superstarify(InfractionScheduler, Cog): ) if not notified: - log.warning("Failed to DM user about why they cannot change their nickname.") + log.info("Failed to DM user about why they cannot change their nickname.") @Cog.listener() async def on_member_join(self, member: Member) -> None: diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index cff7c5786..b65b146ea 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -281,7 +281,7 @@ class Snekbox(Cog): code = await self.continue_eval(ctx, response) if not code: break - log.info(f"Re-evaluating message {ctx.message.id}") + log.info(f"Re-evaluating code from message {ctx.message.id}:\n{code}") def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message) -> bool: diff --git a/bot/converters.py b/bot/converters.py index 1945e1da3..98f4e33c8 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -323,7 +323,7 @@ class FetchedUser(UserConverter): except discord.HTTPException as e: # If the Discord error isn't `Unknown user`, return a proxy instead if e.code != 10013: - log.warning(f"Failed to fetch user, returning a proxy instead: status {e.status}") + log.info(f"Failed to fetch user, returning a proxy instead: status {e.status}") return proxy_user(arg) log.debug(f"Failed to fetch user {arg}: user does not exist.") -- cgit v1.2.3 From e88bb946fea8c4bc861f17772f8aca28f99be512 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 27 Mar 2020 11:22:34 -0700 Subject: Filtering: merge the word and token watch filters The only difference was the automatic addition of word boundaries. Otherwise, they shared a lot of code. The regex lists were kept separate in the config to retain the convenience of word boundaries automatically being added. * Rename filter to `watch_regex` * Expand spoilers for both words and tokens * Ignore URLs for both words and tokens --- bot/cogs/filtering.py | 56 +++++++++++++++++---------------------------------- bot/constants.py | 3 +-- config-default.yml | 3 +-- 3 files changed, 21 insertions(+), 41 deletions(-) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 6651d38e4..3f3dbb853 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -38,6 +38,7 @@ WORD_WATCHLIST_PATTERNS = [ TOKEN_WATCHLIST_PATTERNS = [ re.compile(fr'{expression}', flags=re.IGNORECASE) for expression in Filter.token_watchlist ] +WATCHLIST_PATTERNS = WORD_WATCHLIST_PATTERNS + TOKEN_WATCHLIST_PATTERNS def expand_spoilers(text: str) -> str: @@ -88,24 +89,18 @@ class Filtering(Cog): f"Your URL has been removed because it matched a blacklisted domain. {staff_mistake_str}" ) }, + "watch_regex": { + "enabled": Filter.watch_regex, + "function": self._has_watch_regex_match, + "type": "watchlist", + "content_only": True, + }, "watch_rich_embeds": { "enabled": Filter.watch_rich_embeds, "function": self._has_rich_embed, "type": "watchlist", "content_only": False, }, - "watch_words": { - "enabled": Filter.watch_words, - "function": self._has_watchlist_words, - "type": "watchlist", - "content_only": True, - }, - "watch_tokens": { - "enabled": Filter.watch_tokens, - "function": self._has_watchlist_tokens, - "type": "watchlist", - "content_only": True, - }, } @property @@ -191,8 +186,8 @@ class Filtering(Cog): else: channel_str = f"in {msg.channel.mention}" - # Word and match stats for watch_words and watch_tokens - if filter_name in ("watch_words", "watch_tokens"): + # Word and match stats for watch_regex + if filter_name == "watch_regex": surroundings = match.string[max(match.start() - 10, 0): match.end() + 10] message_content = ( f"**Match:** '{match[0]}'\n" @@ -248,37 +243,24 @@ class Filtering(Cog): break # We don't want multiple filters to trigger @staticmethod - async def _has_watchlist_words(text: str) -> Union[bool, re.Match]: + async def _has_watch_regex_match(text: str) -> Union[bool, re.Match]: """ - Returns True if the text contains one of the regular expressions from the word_watchlist in our filter config. + Return True if `text` matches any regex from `word_watchlist` or `token_watchlist` configs. - Only matches words with boundaries before and after the expression. + `word_watchlist`'s patterns are placed between word boundaries while `token_watchlist` is + matched as-is. Spoilers are expanded, if any, and URLs are ignored. """ if SPOILER_RE.search(text): text = expand_spoilers(text) - for regex_pattern in WORD_WATCHLIST_PATTERNS: - match = regex_pattern.search(text) - if match: - return match # match objects always have a boolean value of True - return False - - @staticmethod - async def _has_watchlist_tokens(text: str) -> Union[bool, re.Match]: - """ - Returns True if the text contains one of the regular expressions from the token_watchlist in our filter config. + # Make sure it's not a URL + if URL_RE.search(text): + return False - This will match the expression even if it does not have boundaries before and after. - """ - for regex_pattern in TOKEN_WATCHLIST_PATTERNS: - match = regex_pattern.search(text) + for pattern in WATCHLIST_PATTERNS: + match = pattern.search(text) if match: - - # Make sure it's not a URL - if not URL_RE.search(text): - return match # match objects always have a boolean value of True - - return False + return match @staticmethod async def _has_urls(text: str) -> bool: diff --git a/bot/constants.py b/bot/constants.py index 14f8dc094..549e69c8f 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -206,9 +206,8 @@ class Filter(metaclass=YAMLGetter): filter_zalgo: bool filter_invites: bool filter_domains: bool + watch_regex: bool watch_rich_embeds: bool - watch_words: bool - watch_tokens: bool # Notifications are not expected for "watchlist" type filters notify_user_zalgo: bool diff --git a/config-default.yml b/config-default.yml index 5788d1e12..ef0ed970f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -248,9 +248,8 @@ filter: filter_zalgo: false filter_invites: true filter_domains: true + watch_regex: true watch_rich_embeds: true - watch_words: true - watch_tokens: true # Notify user on filter? # Notifications are not expected for "watchlist" type filters -- cgit v1.2.3 From 81d2cdd39316b834b6b1de36b81260c4ab8489f5 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Sat, 28 Mar 2020 13:36:28 -0400 Subject: Logging severity pass from review --- bot/bot.py | 4 ++-- bot/cogs/sync/syncers.py | 2 +- bot/utils/messages.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/bot.py b/bot/bot.py index 3e1b31342..950ac6751 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -77,7 +77,7 @@ class Bot(commands.Bot): # Its __del__ does send a warning but it doesn't always show up for some reason. if self._connector and not self._connector._closed: - log.info( + log.warning( "The previous connector was not closed; it will remain open and be overwritten" ) @@ -94,7 +94,7 @@ class Bot(commands.Bot): # Its __del__ does send a warning but it doesn't always show up for some reason. if self.http_session and not self.http_session.closed: - log.info( + log.warning( "The previous session was not closed; it will remain open and be overwritten" ) diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py index c7ce54d65..c9b3f0d40 100644 --- a/bot/cogs/sync/syncers.py +++ b/bot/cogs/sync/syncers.py @@ -131,7 +131,7 @@ class Syncer(abc.ABC): await message.edit(content=f':ok_hand: {mention}{self.name} sync will proceed.') return True else: - log.warning(f"The {self.name} syncer was aborted or timed out!") + log.trace(f"The {self.name} syncer was aborted or timed out!") await message.edit( content=f':warning: {mention}{self.name} sync aborted or timed out!' ) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index a36edc774..e969ee590 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -92,7 +92,7 @@ async def send_attachments( elif link_large: large.append(attachment) else: - log.warning(f"{failure_msg} because it's too large.") + log.info(f"{failure_msg} because it's too large.") except HTTPException as e: if link_large and e.status == 413: large.append(attachment) -- cgit v1.2.3 From 608f5c5edec5804ba6dd546d25f63ce13b34b948 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 28 Mar 2020 12:54:26 -0700 Subject: Use debug log level instead of warning in `post_user` --- bot/cogs/moderation/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/utils.py b/bot/cogs/moderation/utils.py index 5052b9048..3598f3b1f 100644 --- a/bot/cogs/moderation/utils.py +++ b/bot/cogs/moderation/utils.py @@ -38,7 +38,7 @@ async def post_user(ctx: Context, user: UserSnowflake) -> t.Optional[dict]: log.trace(f"Attempting to add user {user.id} to the database.") if not isinstance(user, (discord.Member, discord.User)): - log.warning("The user being added to the DB is not a Member or User object.") + log.debug("The user being added to the DB is not a Member or User object.") payload = { 'avatar_hash': getattr(user, 'avatar', 0), -- cgit v1.2.3 From cc153e052b765ddd8ee1494ad3eea2a552d9459c Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Sat, 28 Mar 2020 16:26:01 -0400 Subject: Increase syncer logging level --- bot/cogs/sync/syncers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py index c9b3f0d40..003bf3727 100644 --- a/bot/cogs/sync/syncers.py +++ b/bot/cogs/sync/syncers.py @@ -131,7 +131,7 @@ class Syncer(abc.ABC): await message.edit(content=f':ok_hand: {mention}{self.name} sync will proceed.') return True else: - log.trace(f"The {self.name} syncer was aborted or timed out!") + log.info(f"The {self.name} syncer was aborted or timed out!") await message.edit( content=f':warning: {mention}{self.name} sync aborted or timed out!' ) -- cgit v1.2.3 From 582ddbb1ca8bab2cb883781911f5f35962330995 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 30 Mar 2020 13:36:34 +0200 Subject: Set unsilence permissions to inherit instead of true The "unsilence" action of the silence/hush command used `send_messages=True` when unsilencing a hushed channel. This had the side effect of also enabling send messages permissions for those with the Muted rule, as an explicit True permission apparently overwrites an explicit False permission, even if the latter was set for a higher top-role. The solution is to revert back to the `Inherit` permission by assigning `None`. This is what we normally use when Developers are allowed to send messages to a channel. --- bot/cogs/moderation/silence.py | 2 +- tests/bot/cogs/moderation/test_silence.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index a1446089e..1ef3967a9 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -138,7 +138,7 @@ class Silence(commands.Cog): """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: - await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=True)) + await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=None)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) self.muted_channels.discard(channel) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 44682a1bd..3fd149f04 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -194,7 +194,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() - self.assertTrue(channel.set_permissions.call_args.kwargs['send_messages']) + self.assertIsNone(channel.set_permissions.call_args.kwargs['send_messages']) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_notifier(self, notifier): -- cgit v1.2.3